Resolve "durs-core: load conf from environment variables as a priority"
Ticket: #141 (closed)
Première MR minimal répondant au ticket donc:
- Charge une config optionnel/partielle par variable d’environnement qui est prioritaire par rapport à la config chargé depuis le fichier de conf.
Par contre la lecture et écriture du fichier de conf n'a pas évolué.
- Il n'est donc pas encore optionnel.
- Le trait
Merge
n'est donc pas utilisé.
Je compte tenter un refacto avant de créer une version DursUserConfV3
qui rendras les configurations depuis le fichier optionnelles.
Note sur le dev:
- Pour pouvoir deserializer l'
enum ResourceUsage
depuis les variables d'env il faut ajouter la macro#[serde(field_identifier)]
, par contre cela empêche la résolution de la macro#[derive(Serialize)]
. La solution que j'ai trouvé est d'implémenter le traitSerialize
sans passer par la macro, donc:impl Serialize for ResourceUsage
.
Merge request reports
Activity
changed milestone to %v1.0 full member node
added 1 commit
- 3450dc76 - [feat] conf: add configurable environment variable prefix
Pour revue: @librelois
@JonasSprenger je n'ai pas encore lu le code, j'ai tout d'abord des questions sur ta description :
Par contre la lecture et écriture du fichier de conf n'a pas évolué.
- Il n'est donc pas encore optionnel.
- Le trait
Merge
n'est donc pas utilisé.
Qu'entend tu part "Il n'est donc pas encore optionnel." ?? je en comprend pas. Le fichier de conf était déjà optionnel avant ton dev, s'il n'existe pas ont le créer avec la conf par défaut. Tu n'a pas besoin de changer ce comportement :)
Quel lien avec le trait "merge" ? Tu peut d'ores et déjà l'utiliser je ne comprend pas ce qui te bloque ?
Je compte tenter un refacto avant de créer une version
DursUserConfV3
qui rendras les configurations depuis le fichier optionnelles."je crois qu'on ne s'est pas compris sur les spec, les configurations depuis les fichiers sont déjà optionnelles.
666 740 Ok((conf, keypairs)) 667 741 } 668 742 743 fn merge_env_conf(file_conf: DuRsConf, env_conf: EnvConf) -> DuRsConf { 744 match file_conf { 745 DuRsConf::V2 { 746 global_conf, 747 modules_conf, 748 } => DuRsConf::V2 { 749 modules_conf: env_conf.modules_conf.clone().unwrap_or(modules_conf), 750 global_conf: global_conf.apply_env(env_conf), 751 }, 752 _ => panic!("Should never happen : didn't generate DuRsConf::V2"), - Oui lorsque le fichier n'existe pas il est créé avec la conf par défaut. Mais lorsque un paramètre de la conf est absent (champs Json), on crash. J'avais compris que tu voulais utiliser la même
struc
pour désérialiser le conf venant des variables d'environnement comme ceux venant du fichier Json, ce qui n'est pas encore le cas, j'utilise une struct dédié pour les variables d'environnement (avec tout les champs optionnels). - Du coup je ne peux pas utiliser le trait
Merge
, car la conf vient de deux struct différentes.
- Oui lorsque le fichier n'existe pas il est créé avec la conf par défaut. Mais lorsque un paramètre de la conf est absent (champs Json), on crash. J'avais compris que tu voulais utiliser la même
132 133 } 134 135 impl Serialize for ResourceUsage { 136 fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error> 137 where 138 S: Serializer, 139 { 140 serializer.serialize_str(match *self { 141 ResourceUsage::Minimal => "Minimal", 142 ResourceUsage::Medium => "Medium", 143 ResourceUsage::Large => "Large", 144 ResourceUsage::Infinite => "Infinite", 145 }) 146 } 147 } 148 203 }; 204 205 DuRsConfV2 { 206 currency: conf.currency.unwrap_or(self.currency), 207 my_node_id: conf.my_node_id.unwrap_or(self.my_node_id), 208 default_sync_module: conf.default_sync_module.unwrap_or(self.default_sync_module), 209 ressources_usage, 210 disabled: conf.disabled.unwrap_or(self.disabled), 211 enabled: conf.enabled.unwrap_or(self.enabled), 212 } 213 } 214 } 215 216 #[derive(Debug, Clone, Deserialize, PartialEq)] 217 /// Optional configs loaded from environment variables 218 pub struct EnvConf { Il ne doit pas y avoir d'objet EnvConf, c'est une UserCOnf qui doit etre générée via envy. De plus tout dopit etre versionné. A renommer en
UserConfV3
du coup.Edited by Éloïs
171 187 pub enabled: HashSet<ModuleName>, 172 188 } 173 189 190 impl DuRsConfV2 { 191 fn apply_env(self, conf: EnvConf) -> DuRsConfV2 { Ce n'est pas ce qui était demandé.
Il faut a la place implémenter le trait
From<DursUserConfVx>
a la structDuRsConfVx
.De plus la env conf ne s'applique pas en V2, ça commence seulement a partir de la conf v3, sinon on ne passerai pas en v3 d'ailleurs ;)
Edited by Éloïs
631 700 keypairs 632 701 }; 633 702 703 let env_prefix: &str = if let Some(e) = env_prefix { 704 &e[..] 705 } else { 706 constants::DEFAULT_ENV_PREFIX 707 }; 708 709 let env_conf = envy::prefixed(env_prefix) 710 .from_env::<EnvConf>() 711 .unwrap_or_else(|e| { 712 let err_msg = format!("Fatal error : fail to parse environment variable : {}", e); 713 dbg!(&err_msg); 714 panic!(err_msg); 641 724 f.read_to_string(&mut contents) 642 725 .map_err(DursConfFileError::ReadError)?; 643 726 // Parse conf file 644 let conf: DuRsConf = 727 let file_conf: DuRsConf = 645 728 serde_json::from_str(&contents).map_err(DursConfFileError::ParseError)?; 646 729 // Upgrade conf to latest version 647 let (conf, upgraded) = conf.upgrade(); 730 let (conf, upgraded) = file_conf.upgrade(); 648 731 // If conf is upgraded, rewrite conf file 649 732 if upgraded { 650 733 write_conf_file(conf_path.as_path(), &conf) 651 734 .map_err(DursConfFileError::WriteError)?; 652 735 } 653 conf 736 merge_env_conf(conf, env_conf) added S-waiting author label