-
Notifications
You must be signed in to change notification settings - Fork 1.7k
remove_keys_from_values: new filter plugin, remove keys with certain values #10139
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
base: main
Are you sure you want to change the base?
remove_keys_from_values: new filter plugin, remove keys with certain values #10139
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Thanks for your contribution! If anyone is wondering how to do this with pure ansible-core + jinja: - hosts: localhost
gather_facts: false
tasks:
- ansible.builtin.debug:
msg: >-
{{
input | dict2items | rejectattr("value", "none") | items2dict
}}
vars:
input:
foo: bar
baz: bam
bam: ~
- ansible.builtin.debug:
msg: >-
{{
input | map("dict2items") | map("rejectattr", "value", "none") | map("items2dict") | list
}}
vars:
input:
- foo: bar
baz: bam
bam: ~
- foo:
bar: baz
baz: I'm wondering whether it wouldn't be better to have a filter that's more flexible, by allowing the user to specify a test that's applied to values (similar to Jinja's I'm not sure how simple that is to implement, though. You'd definitely need more Jinja2 internals. |
A 'drop' filter could work with map, does not need to iterate over objects itself |
Hi @felixfontein, For the key we have the filter and the map, but i don'se something more complex for value. Let me know if you want to track it with issue. Thanks |
will add to my collection, but here is a quick and dirty 'drop' filter
|
@bcoca we already have a filter for removing keys from dictionaries: |
Using this PR for discussions is also fine IMO. |
TIL |
Yes, I also added the support for other value like "exact" or "regex", as the same as In this implementation values are customizable, like list of match case or single match case, and with matching type Let me now What do you think, Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @tanganellilore thanks for your contribution! Some adjustments are needed - regardless of the ongoing discussion.
@tanganellilore this PR contains the following merge commits: Please rebase your branch to remove these commits. |
Hi @russoz , Thanks and sorry for delay. |
Regarding my question about why not allow arbitrary Jinja2 tests, similar as for the
Was this related to that question? If yes I'm afraid I don't understand what you mean. |
Sorry, not clear to me the request :-). |
The |
@felixfontein thanks for clarification, now should be much clear. I tried also usage of Some example tested as per ref:
My proposal is to remove a key of dict based on it's value, on first or all level of dict and with support of multiple test case, that seems to me different from remove whole element in a list (as rejecet) or remove element with test case starting frok him key. Yes, my proposal support list of dict, but also in this case I will remove the key of dict element that match value/options passed. Let me know if it's clear. Thanks |
That's not my use case, that was an example of what "uses an arbitrary test plugin" means. My suggestion is to create a filter plugin With that you'd do something like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still a couple of adjustments needed. Please keep in mind that new 9E88 plugins are required to come with tests. I would suggest you write integration tests and use the code from EXAMPLES as the base for it.
# Copyright (c) 2024 Vladimir Botka <vbotka@gmail.com> | ||
# Copyright (c) 2024 Felix Fontein <felix@fontein.de> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing something here ;-)
DOCUMENTATION = r""" | ||
name: remove_keys_from_values | ||
short_description: Remove keys from a list of dictionaries or a dictionary in base of value content | ||
version_added: "11.0.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That boat has sailed, please use:
version_added: "11.0.0" | |
version_added: "11.1.0" |
- A single value or value pattern to remove, or a list of values or values patterns to remove. | ||
- If O(matching_parameter=regex) there must be equally one pattern provided. | ||
type: raw | ||
required: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Err... the code below disagrees with that - if nothing is passed, it will assume the default value.
equal: Matches values of equally one of the O(values[]) items. | ||
regex: Matches values that match one of the regular expressions provided in O(values[]). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
equal: Matches values of equally one of the O(values[]) items. | |
regex: Matches values that match one of the regular expressions provided in O(values[]). | |
equal: Matches values of equally one of the O(values) items. | |
regex: Matches values that match one of the regular expressions provided in O(values). |
values: | ||
description: | ||
- A single value or value pattern to remove, or a list of values or values patterns to remove. | ||
- If O(matching_parameter=regex) there must be equally one pattern provided. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not clear to me what that means. Should it be only the pattern? Should it be at least on pattern? Will it accept more than one pattern?
description: | ||
- This filter removes keys from a list of dictionaries or a dictionary, | ||
recursively or not depending on the parameters. | ||
- The type of values to be removed is defined by the O(values) parameter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this line would be better place in the description of values, but there is already a similar description there, so I would argue this line is redundant.
- This filter removes keys from a list of dictionaries or a dictionary, | ||
recursively or not depending on the parameters. | ||
- The type of values to be removed is defined by the O(values) parameter. | ||
- The default is to remove keys with values like C(''), C([]), C({}), C(None), usefull for removing empty keys. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would argur that this line should go to the description of the values parameter, with the adjustments:
- The default is to remove keys with values like C( |
|
- The default is to remove keys with values like V(''), V([]), V({}), V(None), useful for removing empty keys. |
b: {} | ||
c: None | ||
- debug: | ||
msg: "{{ my_list | remove_empty_keys }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The examples need updating:
msg: "{{ my_list | remove_empty_keys }}" | |
msg: "{{ my_list | remove_keys_from_values }}" |
from ansible.errors import AnsibleFilterError | ||
|
||
|
||
def remove_keys_from_value(data, values=None, recursive=True): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
err... where is this function ever used?
if matching_parameter not in ("equal", "regex"): | ||
raise AnsibleFilterError("matching_parameter must be 'equal' or 'regex'") | ||
|
||
values = values if isinstance(values, list) else [values or '', [], {}, None] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is going to be trickier than that:
>>> values = "a"
>>> values if isinstance(values, list) else [values or '', [], {}, None]
['a', [], {}, None]
>>> values = [1,2,3]
>>> values if isinstance(values, list) else [values or '', [], {}, None]
[1, 2, 3]
>>> values = None
>>> values if isinstance(values, list) else [values or '', [], {}, None]
['', [], {}, None]
>>>
>>> values = "a"
>>> values if isinstance(values, list) else [values or ['', [], {}, None]]
['a']
>>> values = None
>>> values if isinstance(values, list) else [values or ['', [], {}, None]]
[['', [], {}, None]]
I think lines 139-143 solve the problem.
SUMMARY
Hi team,
i will propose a special filter that i think is missing in this community, a filter that remove some keys on a dict (or list of dict) that contains certain values.
In origin I search a way to remove empty key in a dict and I can do it with select, match and so on, but i think that a filter that can remove key ina object absed on valuse can be very usefull.
This is why i'm poroposing a filter that can do it (as default behaviourremove keys with empty values), similar to
remove_keys
v filter.ISSUE TYPE
COMPONENT NAME
remove_keys_from_values.py
ADDITIONAL INFORMATION