Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More dictupdate tools #52455

Merged
merged 16 commits into from Apr 25, 2019
Merged

Conversation

github-abcde
Copy link
Contributor

@github-abcde github-abcde commented Apr 9, 2019

What does this PR do?

This PR adds functions to write to nested dicts, like traverse_dict does for reading from nested dicts.
The four basic functionalities (set, update, append, extend) that can be applied to a nested dict are also made available as jinja filters.
The key/keys provided to these functions need not exist in the provided dict yet, they will be created.
Support has been included for configurable delimiters and the use of OrderedDicts instead of dicts (when new nested entries are created).

Added two pylint-inspired fixes (line 58-59, 69-77).

What issues does this PR fix or reference?

None that I know of.

Previous Behavior

Unable to update nested dictionaries easily (in salt and in Jinja)

New Behavior

Updating nested dictionaries in salt (for example to set the changes in a state):

salt.utils.dictupdate.set_dict_key_value(ret, 'changes:old:sub_item', old_value)
salt.utils.dictupdate.set_dict_key_value(ret, 'changes:new:sub_item', new_value)

Though the above could also be used if multiple changes need to be presented in a list, by using:

for item in list_of_items_to_process:
  ... code ...
  salt.utils.dictupdate.append_dict_key_value(ret, 'changes:new:sub_item', new_value)

Updating nested dictionaries in Jinja:

{%- set foo = {} %}
Test dictupdate:
  module.run:
  - test.outputter:
    - data: {{ foo | set_dict_key_value('bar:baz', 42) }}

Results in:

          ID: Test dictupdate
    Function: module.run
      Result: True
     Comment: test.outputter: Success
     Started: 12:10:25.985492
    Duration: 18.949 ms
     Changes:   
              ----------
              test.outputter:
                  ----------
                  bar:
                      ----------
                      baz:
                          42

Another Jinja example:

{%- set foo = {'bar': [1, 2]} %}
Test dictupdate:
  module.run:
  - test.outputter:
    - data: {{ foo | append_dict_key_value('bar', 42) }}

Results in:

          ID: Test dictupdate
    Function: module.run
      Result: True
     Comment: test.outputter: Success
     Started: 12:14:42.047802
    Duration: 21.165 ms
     Changes:   
              ----------
              test.outputter:
                  ----------
                  bar:
                      - 1
                      - 2
                      - 42

Tests written?

Yes

Commits signed with GPG?

Yes

github-abcde added 3 commits Apr 9, 2019
…raverse_dict does with reading nested dicts.

Add "ensure_dict_key" to ensure all keys provided exist (they will be created if they do not) in a dict.
Add "set_dict_key_value" to set a nested dict value to a value specified.
Add "update_dict_key_value" to update a nested dict value (that is a dict) with a dict specified.
Add "append_dict_key_value" and "extend_dict_key_value" to append or extend a nested dict value (that is a list) with values specified.
@twangboy
Copy link
Contributor

@twangboy twangboy commented Apr 9, 2019

@github-abcde This looks great. Could we get some Unit tests on these new functions please?

@github-abcde
Copy link
Contributor Author

@github-abcde github-abcde commented Apr 10, 2019

Working on the unit tests...There are a lot of things to test :)

github-abcde and others added 5 commits Apr 10, 2019
Added TypeError handling in extend_dict_key_value.
Fixed incorrect usage of OrderedDict.
Small beautification of exception message in update_dict_key_value.
Added some pylint-inspired layout fixes (pylint really does not like this file).
Copy link
Contributor

@twangboy twangboy left a comment

This looks great. Thanks for the tests!

@github-abcde
Copy link
Contributor Author

@github-abcde github-abcde commented Apr 10, 2019

Not sure if I fixed all the failing python3 tests, will monitor the test results and fix things tomorrow should any still fail.

github-abcde and others added 7 commits Apr 11, 2019
… in order to avoid random-ordered keys in a rendered string comparison. Also added check for jmespath library, and skip test_json_query test if not present (because "RuntimeError: json_query requires jmespath module installed").
@github-abcde
Copy link
Contributor Author

@github-abcde github-abcde commented Apr 25, 2019

@twangboy Is there something else that I can (or need to) do in order for this branch to be merged?

@twangboy twangboy merged commit 95354e5 into saltstack:develop Apr 25, 2019
9 of 11 checks passed
@github-abcde github-abcde deleted the more_dictupdate_tools branch Apr 25, 2019
github-abcde pushed a commit to ogd-software/salt that referenced this issue Jul 26, 2019
github-abcde pushed a commit to ogd-software/salt that referenced this issue Jul 26, 2019
@waynew waynew added this to PR needs port to master in PRs to port to master Oct 24, 2019
github-abcde pushed a commit to ogd-software/salt that referenced this issue Dec 9, 2019
github-abcde pushed a commit to ogd-software/salt that referenced this issue Dec 9, 2019
@garethgreenaway garethgreenaway moved this from PR needs port to master to Failed: Skips, War room, CI Changes, Change already there in PRs to port to master Mar 24, 2020
@garethgreenaway garethgreenaway moved this from Failed: Skips, War room, CI Changes, Change already there to PR has port to master in PRs to port to master Mar 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
PRs to port to master
  
PR has port to master
Development

Successfully merging this pull request may close these issues.

None yet

2 participants