(PDK-1093) Replace null values and empty data structures in metadata when converting #584
Conversation
| begin | ||
| metadata = PDK::Module::Metadata.from_file(metadata_path) | ||
| new_values = PDK::Module::Metadata::DEFAULTS.select do |key, _| | ||
| !metadata.data.key?(key) || metadata.data[key].nil? || |
There was a problem hiding this comment.
Hmm. This PR makes me think about how we handle operatingsystem_support. Currently.. if the key doesn't exist, we put the defaults in, which may be inaccurate. But if the key does exist and is empty, we leave it alone. With this PR, in both of the cases, we will update the metadata with the default operating system support, which might be undesirable.
This is out of scope of this PR, but IMO, I think we need to remove operating system support from the defaults, and only check the key (and prompt user to select OS) if --full-interview is specified or if they are doing a pdk build and we detect the key is missing.
There was a problem hiding this comment.
Yeah, I'm 👍 to replacing values that are null but I think replacing empty arrays with the defaults could be more problematic.
There was a problem hiding this comment.
Fair enough, but we'll have to add an exception for requirements as PDK will fail to validate as it expects to find a puppet version requirement.
There was a problem hiding this comment.
Should we just have a pre-condition somewhere that bails out if requirements is an empty array?
There was a problem hiding this comment.
I don't think so. If the requirements array exists and is empty, it is a pretty harmless change to add a puppet requirement and saves the user from having to make a manual change (either removing the array which would then be re-added with the default value anyway during convert, or manually creating the puppet requirement).
There was a problem hiding this comment.
Yeah, that sounds reasonable!
…when converting Currently when we're updating a module's `metadata.json` file during convert, we only replace values if they are completely absent from the file. Unfortunately, this means if - for example - the existing metadata defines `dependencies` as `null` or an empty array then it will not be updated when the module is converted and it will subsequently fail to validate cleanly. This PR changes the metadata merging logic so that values defined as `null`, empty array, or empty hash are considered to be undefined and will be replaced with the default metadata value.
Currently when we're updating a module's
metadata.jsonfile duringconvert, we only replace values if they are completely absent from the
file. Unfortunately, this means if - for example - the existing metadata
defines
dependenciesasnullor an empty array then it will not beupdated when the module is converted and it will subsequently fail to
validate cleanly.
This PR changes the metadata merging logic so that values defined as
nullwill be replaced with the default metadata value. Additionally, if therequirementsarray exists but is empty, it will also be updated with the default value.