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

[suggestion] Fallible setters for configuration parameters #3528

Open
3 tasks
appetrosyan opened this issue May 24, 2023 · 1 comment
Open
3 tasks

[suggestion] Fallible setters for configuration parameters #3528

appetrosyan opened this issue May 24, 2023 · 1 comment
Labels
Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST

Comments

@appetrosyan
Copy link
Contributor

Feature request

We should add fallible setters as configuration builder value sanity checks.

Example:

iroha_config::genesis::ConfigurationBuilder::default()
        .set_gossip_period_ms(1<<127)

is a questionable value. We should ask the user of the library to handle the insane value but not refuse them the ability to still set it. So an API would look like this

iroha_config::genesis::ConfigurationBuilder::default()
        .set_gossip_period_ms(1<<127)
        .map_err(|e| e.compromised_value())
        .unwrap()
  • Produces the same reference, so setters can be chained.
  • Produces the ConfigurationBuilder with the insane value set via compromised_value.
  • Is still an error, so we have the option to halt, forward and ignore (as here).

Motivation

The earlier we catch an error, the easier it is to handle. This approach forces the user to be aware that not all values are acceptable, and to handle the check in Rust-y way.

Who can help?

@appetrosyan

@appetrosyan appetrosyan added Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST labels May 24, 2023
@mversic
Copy link
Contributor

mversic commented Jun 14, 2024

related to #4282 . IMO configuration should be immutable, i.e. no setters. Setters should be defined on the configuration builder only

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

No branches or pull requests

3 participants