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.

Sign up for GitHub

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

Jump to bottom

jsonify inventory #32990

Merged
merged 7 commits into from
Nov 21, 2017
Merged

jsonify inventory #32990

merged 7 commits into from
Nov 21, 2017

Conversation

bcoca
Copy link
Member

@bcoca bcoca commented Nov 16, 2017

SUMMARY

Use same 'jsonifyier' that we use for modules in ansible-inventory to allow encoding 'problem cases'

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ansible-inventory

ANSIBLE VERSION
2.5

@bcoca bcoca added this to the 2.5.0 milestone Nov 16, 2017
@bcoca bcoca added this to In Progress in 2.5 Nov 16, 2017
@ansibot ansibot added affects_2.5 This issue/PR affects Ansible v2.5 cli/ feature_pull_request module_utils/basic needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Nov 16, 2017
@s-hertel
Copy link
Contributor

LGTM

@s-hertel
Copy link
Contributor

Actually eating my words.
I had to add this to get it to work right for datetime objects:

+from datetime import datetime

 from ansible.module_utils._text import to_bytes, to_text
 from ansible.module_utils.six import binary_type, iteritems, text_type
@@ -39,6 +40,8 @@ class _SetEncoder(json.JSONEncoder):
     def default(self, obj):
         if isinstance(obj, Set):
             return list(obj)
+        elif isinstance(obj, datetime):
+            return obj.isoformat()

@sivel
Copy link
Member

sivel commented Nov 16, 2017

I had to add this to get it to work right for datetime objects:

I think this is a good addition, but we shouldn't have any datetime objects coming from vars, unless someone uses a YAML date. If so, you are likely going to experience problems with that elsewhere.

@s-hertel
Copy link
Contributor

I'm just experiencing the datetime issues with the WIP aws_ec2 inventory plugin. If that should be handled in aws_ec2 that's fine too.

@samdoran samdoran removed the needs_triage Needs a first human triage before being processed. label Nov 16, 2017
@bcoca
Copy link
Member Author

bcoca commented Nov 16, 2017

i would add the 'datetime' as many modules hit this issue also (i thought we had fixed already, but clearly we did not)

@ansibot
Copy link
Contributor

ansibot commented Nov 17, 2017

The test ansible-test sanity --test pylint [?] failed with the following error:

lib/ansible/module_utils/basic.py:115:21: undefined-variable Undefined variable 'simplejson'

click here for bot help

@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Nov 17, 2017
@mattclay mattclay added the ci_verified Changes made in this PR are causing tests to fail. label Nov 18, 2017
@abadger
Move json functions back out of json_utils.
442433e 
* The plan would be for that to migrate to common/json_utils when we split
  basic.py so no need to get people using a new location now when they
  will just be expected to switch again later
* json_dict_bytes_to_unicode and json_dict_unicode_to_bytes will also
  change names and move to common/text.py at that time (not to json).
  Their purpose is to recursively change the elements of a container
  (dict, list, set, tuple) into text or bytes, not to json encode or
  decode (they could be a generic precursor to that but are not limited
  to that.)
* Reimplement the private _SetEncoder which changes sets and datetimes
  into objects that are json serializable into a private function
  instead.  Functions are more flexible, less overhead, and simpler than
  an object.
* Remove code that handled simplejson-1.5.x and earlier.  Raise an error
  if that's the case instead.
  * We require python-2.6 or better which has the json module builtin to
    the stdlib.  So this is only an issue if the stdlib json has been
    overridden by a third party module and the simplejson on the system
    is 1.5.x or less.  (1.5 was released on 2007-01-18)
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Nov 21, 2017
@ansibot ansibot added the test This PR relates to tests. label Nov 21, 2017
@abadger abadger merged commit ebd08d2 into ansible:devel Nov 21, 2017
abadger pushed a commit that referenced this pull request Nov 21, 2017
@bcoca @abadger
jsonify inventory (#32990)
db83d42 
* jsonify inventory
* smarter import, dont pass kwargs where not needed
* added datetime
* Eventual plan for json utilities to migrate to common/json_utils when we split
  basic.py no need to move jsonify to another file now as we'll do that later.
* json_dict_bytes_to_unicode and json_dict_unicode_to_bytes will also
  change names and move to common/text.py at that time (not to json).
  Their purpose is to recursively change the elements of a container
  (dict, list, set, tuple) into text or bytes, not to json encode or
  decode (they could be a generic precursor to that but are not limited
  to that.)
* Reimplement the private _SetEncoder which changes sets and datetimes
  into objects that are json serializable into a private function
  instead.  Functions are more flexible, less overhead, and simpler than
  an object.
* Remove code that handled simplejson-1.5.x and earlier.  Raise an error
  if that's the case instead.
  * We require python-2.6 or better which has the json module builtin to
    the stdlib.  So this is only an issue if the stdlib json has been
    overridden by a third party module and the simplejson on the system
    is 1.5.x or less.  (1.5 was released on 2007-01-18)
(cherry picked from commit ebd08d2)
@abadger
Copy link
Contributor

abadger commented Nov 21, 2017

Merged and cherrypicked to stable-2.4

@s-hertel s-hertel moved this from In Progress to Done in 2.5 Dec 8, 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.
Reviewers
No reviews
Assignees
No one assigned
Labels
affects_2.5 This issue/PR affects Ansible v2.5 cli/ feature This issue/PR relates to a feature request. module_utils/basic needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests.
Projects
No open projects
2.5
Done
Milestone
2.5.0
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@bcoca @s-hertel @sivel @ansibot @abadger @mattclay @samdoran