Remove deepmerge dependency #56
Conversation
This patch removes the deepmerge module as a required dependency of this cloud-init datasource. There is still an option to use the deepmerge module if the environment variable CLOUD_INIT_VMWARE_GUEST_INFO_MERGE_STRATEGY is set to 'deepmerge'. Otherwise a deep merge is performed with a recursive strategy using Python built-ins.
|
@akutz it would be good to write a unit test that covers the cases here: https://github.com/vmware/cloud-init-vmware-guestinfo/pull/56/files#diff-887b9ab215181ea8c8772e608c0408fdL165 |
|
I tested it with Ubuntu 20.04 and everything seems to be working correctly. |
| return always_merger.merge(a, b) | ||
|
|
||
|
|
||
| def merge_dicts_with_stdlib(a, b): |
There was a problem hiding this comment.
Please use meaningful names instead of a, b. (applies throughout)
There was a problem hiding this comment.
Hi @sshedi,
Sure, with a small function though, a and b are often used as the variables for two operands, such as src and dst.
| return merge_dicts_with_deep_merge(a, b) | ||
| except Exception as err: | ||
| LOG.error("deep merge failed: %s" % err) | ||
| LOG.info('merging dictionaries with stdlib strategy') |
There was a problem hiding this comment.
why is this stdlib strategy? IMO it should be recursive strategy.
There was a problem hiding this comment.
Because it has no dependencies other than Python stdlib.
|
@akutz I tried this in my dev environment, LGTM. |
Overview
This patch removes the deepmerge module as a required dependency of this cloud-init datasource. There is still an option to use the
deepmergemodule if the environment variableCLOUD_INIT_VMWARE_GUEST_INFO_MERGE_STRATEGYis set todeepmerge. Otherwise a deep merge is performed with a recursive strategy using Python built-ins.This does not remove the dependency on the
netifacesmodule.Testing
The following test cases were executed to assert this works as designed:
deepmergepython2 DataSourceVMwareGuestInfo.py 1>p2-deepmerge.jsonstdlibpython2 DataSourceVMwareGuestInfo.py 1>p2-stdlib.jsondeepmergepython3 DataSourceVMwareGuestInfo.py 1>p3-deepmerge.jsonstdlibpython3 DataSourceVMwareGuestInfo.py 1>p3-stdlib.jsonNote: The above commands will redirect the merged JSON into the specified file. However, the logger will emit a message to
stderron the console to indicate the selected merge strategy. This log entry will not be part of the contents written to the file. For example:The following command will
diffall permutations (I'm not inclined to figure out how to eliminate the repetition), and the result is no differences between any of the above files:Does anyone care to do some additional testing on this? Thank you!
cc @yvespp @powersj @sshedi