Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restructure configuration mechanism #89

Merged
merged 23 commits into from
Mar 20, 2024
Merged

Restructure configuration mechanism #89

merged 23 commits into from
Mar 20, 2024

Conversation

hkupty
Copy link
Owner

@hkupty hkupty commented Mar 16, 2024

This is a new take on the configuration mechanism. This is PR also changes the required JDK version to 21, effectively bumping Penna out of the 0.7.x branch.

It proposes a few fundamental changes:

  • The configuration manager is now part of the API and irreplaceable;
    • Extensions can be added to the configuration manager for updating the configuration;
  • Configuration updates can be either full or partial updates, but only on the level of the manager;
    • The storage will always treat the updates as full replaces;

Tasks needed on this PR before merging:

  • File watcher;
  • Alternative file (penna-test.yaml);
  • SnakeYAML implementation;
  • Replace old way and clean-up;

Desirable/Maybe in a follow-up PR:

  • More tests;
    • Property based tests for yaml?
  • Audit config (last updated by provider) on storage level;
    • Providers are expected to be named anyways;
  • Atomic binding between provider and manager:
    • i.e. with this implementation, if the provider fails mid-way, it partially updates the configs in the storage;
  • Initialization time log sink:
    • So providers and the manager itself can log errors, warning or debug during init;
    • It could be as simple as a queue that unloads after initialization is complete;
  • Meta-configuration:
    • Filtering providers during initialization;
  • Better control over the manager lifecycle:
    • Currently the initialize method is public;
    • Ensure tests are also possible;
    • Ensure runtime can't tinker with this instance;
    • Ensure runtime can still reference it for direct update (if wanted);

@hkupty
Copy link
Owner Author

hkupty commented Mar 20, 2024

Not necessarily "atomic binding", but by making a two-step process we can first verify the configuration provider (and perform some meta-configuration, i.e. the file watcher option in yaml) before returning to the manager that it should be initialized, so we only initialize those that are correctly instantiated.

@hkupty
Copy link
Owner Author

hkupty commented Mar 20, 2024

On the meta-configuration level: It is not the manager's responsibility to figure out which provider should be added. It's the provider's own responsibility. For example, if no yaml configuraiton file exists, the provider will report false back to the manager and should not be initialized.

@hkupty hkupty changed the base branch from dev/0.7 to dev/0.8 March 20, 2024 18:57
hkupty added 22 commits March 20, 2024 19:57
- Using alpine should yield faster runs as it's smaller;
- Using jdk 21 is the first step towards bumping the requirement,
This should allow for a simpler management and easier collaboration.
So we can verify that a project fully runs and all the lifecycle works.
With this commit the yaml config module now requires to have one of the
three dependencies so it can properly read the yaml configuration.

More libraries should come but those three are arguably the most popular
ones for now so it makes sense to allow for something that the users
might be already using.
This should allow for more granular control over the flags of each
module.

Additionally, we are officially bumping the dependency requirement to
JDK21, so this implies bumping to a new 0.8.x branch
The old configuration mechanism is effectively replaced by the new
one. This allows us to remove a lot of unnecessary methods, classes and
interfaces and the penna-dev project.

The latter had two different justifications for being developed in the
first place:
 - Allow for a development-runtime that shows up the logs in
   human-readable format. That can be much more effectively be replaced
   by fblog or other alternatives, so it doesn't stand on its own here;
 - Allow for runtime level tweaking, so tests can change the verbosity
   of the logs if necessary. This is effectively replaced by the new
   config interfaces, so that in itself also doesn't sustain a strong
   reason for maintaining this module.

Therefore, it is better to remove it not to bloat penna, so the surface
can remain small and the focus can be on the important features.
@hkupty
Copy link
Owner Author

hkupty commented Mar 20, 2024

Any further cleanups should happen in a subsequent PR as this one scope creeped a bit and is gigantic already.

@hkupty hkupty merged commit 73a12a6 into dev/0.8 Mar 20, 2024
2 checks passed
@hkupty hkupty deleted the improve-build branch March 20, 2024 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant