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

refactor(iroha_config): Immutable configuration #5257

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

aoyako
Copy link
Contributor

@aoyako aoyako commented Nov 28, 2024

Refactors config structures to be externally immutable, as suggested in #4282.

Now, instead of directly accessing the config struct field, two approaches are adopted:

  1. To read the config field, getters are used (currently auto-generated). It is possible to override with a custom logic in the future.
  2. To change the config field, the config must be rebuilt from a builder (currently auto-generated). It is possible to add validation when the config builder actually generates a config.

The exception for mutability is log_level for the logger. Since there are many level switching, I believe it must not impact other configurations.

Pros:

  • Configuration is immutable and keeps invariants when parameters are interdependent.

Cons

  • Configs are inconvenient to use, i.e., now require separate builders.
  • Deconstruction is not possible, requiring frequent clone()s.

At this moment no validation or custom logic for getters/setters is implemented.
Possible options include, but are not limited to:

Signed-off-by: Lohachov Mykhailo <[email protected]>
Signed-off-by: Lohachov Mykhailo <[email protected]>
Signed-off-by: Lohachov Mykhailo <[email protected]>
@aoyako aoyako marked this pull request as draft November 28, 2024 12:03
@github-actions github-actions bot added the api-changes Changes in the API for client libraries label Nov 28, 2024
@aoyako aoyako self-assigned this Nov 28, 2024
@aoyako aoyako added the Refactor Improvement to overall code quality label Nov 28, 2024
Signed-off-by: Lohachov Mykhailo <[email protected]>
Signed-off-by: Lohachov Mykhailo <[email protected]>
Signed-off-by: Lohachov Mykhailo <[email protected]>
Signed-off-by: Lohachov Mykhailo <[email protected]>
@aoyako aoyako marked this pull request as ready for review November 30, 2024 14:20
Signed-off-by: Lohachov Mykhailo <[email protected]>
@0x009922
Copy link
Contributor

0x009922 commented Dec 9, 2024

With all respect to the work being done, I am doubting whether this refactor is really that valuable...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-changes Changes in the API for client libraries Refactor Improvement to overall code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants