Enhancement : Refactor Settings #106
Conversation
ocdbytes
commented
Sep 3, 2024
ocdbytes
commented
- Issue : feat: refactor settings #17
ocdbytes
changed the title
ocdbytes
changed the title
| pub struct EthereumDaConfig { | ||
| pub rpc_url: String, | ||
| pub memory_pages_contract: String, | ||
| pub private_key: String, | ||
| } | ||
|
|
||
| #[async_trait] |
There was a problem hiding this comment.
so we actually did this before but then later in the celestia PR we realised that we wanted it to be async and this was the better way to do it, over here #46
There was a problem hiding this comment.
What is the reason for this to be async ? this is config which should not be an async operation.
There was a problem hiding this comment.
in celestia it wasn't possible to do it sync, can you check with @heemankv once?
| @@ -37,3 +45,14 @@ impl DaClient for EthereumDaClient { | |||
| 131072 | |||
| } | |||
| } | |||
|
|
|||
| impl EthereumDaClient { | |||
| pub fn new_with_settings(settings: &impl Settings) -> Self { | |||
There was a problem hiding this comment.
we should restore the build_client one as mentioned above
crates/orchestrator/src/config.rs
Outdated
| /// using the settings provider. More providers can be added eg : GCP, AZURE etc. | ||
| pub enum ProviderConfig { | ||
| AWS(Box<SdkConfig>), | ||
| INVALID, |
There was a problem hiding this comment.
abhi code me jha pr me match kr rha hu providerconfig ko vha pr I need to handle the other cases with a panic. so when it was alone AWS it was giving code unreachable error. So that's why added this enum temporarily.
There was a problem hiding this comment.
that's not the correct design. you don't need add another enum so that you can panic. if there was only AWS over here, the user couldn't pass anything else, the code won't compile so there's no issue
crates/orchestrator/src/config.rs
Outdated
| /// `ProviderConfig` is an enum used to represent the global config built | ||
| /// using the settings provider. More providers can be added eg : GCP, AZURE etc. | ||
| pub enum ProviderConfig { | ||
| AWS(Box<SdkConfig>), |
There was a problem hiding this comment.
clippy. error aa rha tha
There was a problem hiding this comment.
Sdk config ka size kaafi bda h.
There was a problem hiding this comment.
intersting, can you comment that?
crates/orchestrator/src/config.rs
Outdated
| let settlement_client = build_settlement_client(&settings_provider).await; | ||
| let prover_client = build_prover_service(&settings_provider); | ||
| let storage_client = | ||
| build_storage_client(&settings_provider, ProviderConfig::AWS(Box::new(aws_config.clone()))).await; |
* feat : added settings * changelog * feat : added env settings fucntion * feat : updated settings module implementation * feat : updated provider config * feat : added arc for aws config * chore : refactoring
