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

Rekey on member #33836

Merged
merged 4 commits into from
Dec 13, 2017
Merged

Rekey on member #33836

merged 4 commits into from
Dec 13, 2017

Conversation

abadger
Copy link
Contributor

@abadger abadger commented Dec 12, 2017

SUMMARY

Objections have been raised to the new filters, cast_list_to_dict() and cast_dict_to_list() which take an arbitrary data structure and transform it into another data structure. Renamed and reworked the filters and code so that the input and output are much less arbitrary.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

lib/ansible/plugins/filters/mathstuff.py
lib/ansible/plugins/filters/cast_type.py

ANSIBLE VERSION
devel
ADDITIONAL INFORMATION

I was only given one use case. Here's how that one case would compare to the new code as well as how to use the new code in other ways: https://gist.github.com/abadger/af5e5b03b8d8a121f983cc98743062cc

cast_list_to_dict was taking an arbitrary data format in and returning
an arbitrary data format out.  Rework this to be a more generic function
which creates a dict of dicts based on a member of the dict.

Remove cast_dict_to_list since rekey_on_member handles the use cases we
know about and cast_dict_to_list suffers from the same problems as
cast_list_to_dict.  If this is still needed we could think about filters
we could add to do this in a short jinja2 pipeline.
@ansibot ansibot added affects_2.5 This issue/PR affects Ansible v2.5 feature_pull_request needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests. labels Dec 12, 2017
@abadger
Copy link
Contributor Author

abadger commented Dec 12, 2017

raise errors.AnsibleFilterError("human_readable() can't interpret following string: %s" % size)


def human_to_bytes(size, default_unit=None, isbits=False):
''' Return bytes count from a human readable string '''
try:
return basic.human_to_bytes(size, default_unit, isbits)
except:
except Exception as e:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e is unsued, just except Exception: to avoid 'bare exceptions'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

raise errors.AnsibleFilterError(to_native(e))

# TODO: We may want to add parameters to allow overwrite or constructing lists instead of erroring
if new_obj.get(key_elem, None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add an allow_overwrite or similar param.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@abadger abadger mentioned this pull request Dec 12, 2017
@itdependsnetworks
Copy link
Contributor

Lol, good comments. I have come to understand that ansible core likes what ansible core likes. I do feel at times you have lost understanding of how users use products, but this does cover the main use case so let’s do it!!!!

@abadger abadger merged commit 155f36b into ansible:devel Dec 13, 2017
@abadger abadger deleted the rekey_on_member branch December 13, 2017 03:02
@abadger
Copy link
Contributor Author

abadger commented Dec 13, 2017

Merged to devel for the 2.5.0 release.

@mkrizek mkrizek removed the needs_triage Needs a first human triage before being processed. label Dec 15, 2017
@ansibot ansibot added feature This issue/PR relates to a feature request. and removed feature_pull_request labels Mar 5, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.5 This issue/PR affects Ansible v2.5 feature This issue/PR relates to a feature request. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants