Add parameter merging functionality with inline precedence #1215
Conversation
Gijsreyn
force-pushed
the
gh-1213/main/add-inline-parameter-support
branch
from
357c720 to
4fca3dd
Compare
Gijsreyn
force-pushed
the
gh-1213/main/add-inline-parameter-support
branch
from
4fca3dd to
d98b54c
Compare
…ttps://github.com/Gijsreyn/operation-methods into PowerShellgh-1213/main/add-inline-parameter-support
| if file_name == "-" { | ||
| info!("{}", t!("main.readingParametersFromStdin")); | ||
| let mut stdin = Vec::<u8>::new(); | ||
| match io::stdin().read_to_end(&mut stdin) { | ||
| Ok(_) => { | ||
| match String::from_utf8(stdin) { | ||
| Ok(input) => Some(input), | ||
| Err(err) => { | ||
| error!("{}: {err}", t!("util.invalidUtf8")); | ||
| exit(EXIT_INVALID_INPUT); | ||
| } | ||
| } | ||
| }, | ||
| Err(err) => { | ||
| error!("{}: {err}", t!("util.failedToReadStdin")); | ||
| exit(EXIT_INVALID_INPUT); | ||
| } | ||
| } |
There was a problem hiding this comment.
Reading from STDIN is duplicated here and lines below. I think instead of this approach, you can always assume to merge and sometimes file will be empty or inline will be empty.
| return Err(DscError::Parser("File parameters must be a JSON object".to_string())); | ||
| }; | ||
|
|
||
| let Some(inline_obj) = inline_value.as_object() else { | ||
| return Err(DscError::Parser("Inline parameters must be a JSON object".to_string())); |
There was a problem hiding this comment.
Error messages should be in the locale file, but also this error should just say must be an object since it could be YAML
| Ok(json) => json, | ||
| Err(_) => { | ||
| // YAML | ||
| match serde_yaml::from_str::<serde_yaml::Value>(inline_params) { | ||
| Ok(yaml) => serde_json::to_value(yaml)?, | ||
| Err(err) => { | ||
| return Err(DscError::Parser(format!("Failed to parse inline parameters: {err}"))); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This sequence is often repeated, in dsc_lib we have parse_input_to_json() although it returns a String and not a Value. You could add the same util function in this file and have it return a Value.
| if let (Some(file_params_value), Some(inline_params_value)) = (file_obj.get("parameters"), inline_obj.get("parameters")) { | ||
| if let (Some(mut file_params_obj), Some(inline_params_obj)) = (file_params_value.as_object().cloned(), inline_params_value.as_object()) { | ||
| // Merge the nested parameters objects | ||
| for (key, value) in inline_params_obj { | ||
| file_params_obj.insert(key.clone(), value.clone()); | ||
| } | ||
| file_obj.insert("parameters".to_string(), Value::Object(file_params_obj)); | ||
| } else { | ||
| // If one is not an object, inline takes precedence | ||
| file_obj.insert("parameters".to_string(), inline_params_value.clone()); | ||
| } | ||
| } else if let Some(inline_params_value) = inline_obj.get("parameters") { | ||
| // Only inline has parameters | ||
| file_obj.insert("parameters".to_string(), inline_params_value.clone()); | ||
| } |
There was a problem hiding this comment.
A bit worried how this will work when we fix the other issue to support ARM style parameters files. I think it would be better to simply convert the file_params to a hashtable, the inline_params to a hashtable and layer the inline over the file one. Then if we support different formats it's simply a function to convert it to a hashtable.
…into PowerShellgh-1213/main/add-inline-parameter-support
2 participants
PR Summary
This PR implements support for using
--parametersand--parameters-filetogether, with inline parameters taking precedence over file parameters, matching Azure ARM template behavior.PR Context
Fix #1213