fixed .loads error for non decoded json in Python 3 #32065

andy-pi · 2017-10-24T09:41:49Z

SUMMARY

Using Python 3 this module throws an error when parsing the JSON repsonse, as it has not been decoded from its native utf-8.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

Cloudflare DNS

ANSIBLE VERSION

ansible 2.4.0.0
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/usr/share/ansible']
  ansible python module location = /home/ubuntu/workspace/ansible-roles/env/lib/python3.5/site-packages/ansible
  executable location = /home/ubuntu/workspace/ansible-roles/env/bin/ansible
  python version = 3.5.1 (default, Dec 18 2015, 00:00:00) [GCC 4.8.4]

ADDITIONAL INFORMATION

Ansible command used:

 name: Set A record to point to this server in cloudflare
 cloudflare_dns:
     zone: "{{ domain }}"
     record: "@"
     type: A
     value: "{{ ansible_default_ipv4.address }}"
     account_email: "{{ cloudflare_email }}"
     account_api_token: "{{ cloudflare_api_key }}"

Before:

TASK [config-vm : Set A record to point to this server in cloudflare] 
****************************************************
--TRUNCATED--
line 372, in _cf_simple_api_call\r\n    
result = json.loads(content)\r\n  File \"/usr/lib/python3.5/json/__init__.py\", 
line 312, in loads\r\n    s.__class__.__name__))\r\n
TypeError: the JSON object must be str, not 'bytes'\r\n", "msg": "MODULE FAILURE", "rc": 0}

After:

TASK [config-vm : Set A record to point to this server in cloudflare] ****************************************************
changed:

ansibot · 2017-10-24T09:45:44Z

cc @mgruener
click here for bot help

ansibot · 2017-10-27T10:09:41Z

The test ansible-test compile --python 2.6 [?] failed with the following error:

lib/ansible/modules/net_tools/fly.py:232:6718: SyntaxError: RETURN = '''

The test ansible-test compile --python 2.7 [?] failed with the following error:

lib/ansible/modules/net_tools/fly.py:232:6719: SyntaxError: RETURN = '''

The test ansible-test compile --python 3.5 [?] failed with the following error:

lib/ansible/modules/net_tools/fly.py:232:6719: SyntaxError: RETURN = '''

The test ansible-test compile --python 3.6 [?] failed with the following error:

lib/ansible/modules/net_tools/fly.py:232:6719: SyntaxError: RETURN = '''

The test ansible-test sanity --test ansible-doc --python 2.6 [?] failed with the following error:

Command "ansible-doc cloudflare_dns fly" returned exit status 1.
>>> Standard Error
ERROR! module fly missing documentation (or could not parse documentation): Parsing produced an empty object.

The test ansible-test sanity --test ansible-doc --python 2.7 [?] failed with the following error:

Command "ansible-doc cloudflare_dns fly" returned exit status 1.
>>> Standard Error
ERROR! module fly missing documentation (or could not parse documentation): Parsing produced an empty object.

The test ansible-test sanity --test ansible-doc --python 3.5 [?] failed with the following error:

Command "ansible-doc cloudflare_dns fly" returned exit status 1.
>>> Standard Error
ERROR! module fly missing documentation (or could not parse documentation): Parsing produced an empty object.

The test ansible-test sanity --test ansible-doc --python 3.6 [?] failed with the following error:

Command "ansible-doc cloudflare_dns fly" returned exit status 1.
>>> Standard Error
ERROR! module fly missing documentation (or could not parse documentation): Parsing produced an empty object.

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

Command "test/sanity/code-smell/boilerplate.sh" returned exit status 2.
>>> Standard Output
== Missing __metaclass__ = type ==
./lib/ansible/modules/net_tools/fly.py

== Missing from __future__ import (absolute_import, division, print_function) ==
./lib/ansible/modules/net_tools/fly.py

The test ansible-test sanity --test import --python 2.6 [?] failed with the following error:

lib/ansible/modules/net_tools/fly.py:233:6719: SyntaxError: EOF while scanning triple-quoted string literal

The test ansible-test sanity --test import --python 2.7 [?] failed with the following error:

lib/ansible/modules/net_tools/fly.py:233:6719: SyntaxError: EOF while scanning triple-quoted string literal

The test ansible-test sanity --test import --python 3.5 [?] failed with the following error:

lib/ansible/modules/net_tools/fly.py:232:6719: SyntaxError: EOF while scanning triple-quoted string literal

The test ansible-test sanity --test import --python 3.6 [?] failed with the following error:

lib/ansible/modules/net_tools/fly.py:232:6719: SyntaxError: EOF while scanning triple-quoted string literal

The test ansible-test sanity --test no-wildcard-import [?] failed with the following error:

Command "test/sanity/code-smell/no-wildcard-import.sh" returned exit status 3.
>>> Standard Output
== Wildcard imports detected ==
./lib/ansible/modules/net_tools/fly.py:from ansible.module_utils.basic import *
./lib/ansible/modules/net_tools/fly.py:from ansible.module_utils.urls import *

The test ansible-test sanity --test pep8 [?] failed with the following errors:

lib/ansible/modules/net_tools/fly.py:17:1: W293 blank line contains whitespace
lib/ansible/modules/net_tools/fly.py:51:24: W291 trailing whitespace
lib/ansible/modules/net_tools/fly.py:73:10: E901 TokenError: EOF in multi-line string
lib/ansible/modules/net_tools/fly.py:232:11: W292 no newline at end of file

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

lib/ansible/modules/net_tools/fly.py:232:0: syntax-error EOF while scanning triple-quoted string literal (&lt;string&gt;, line 232)

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

lib/ansible/modules/net_tools/fly.py:0:0: E401 Python SyntaxError while parsing module

click here for bot help

andy-pi · 2017-10-27T10:21:16Z

Tried to submit a new module, but rolled back to previous commit. Will wait until this pull request accepted before adding another.

andy-pi · 2017-11-14T08:47:37Z

bot_status

andy-pi · 2017-11-14T08:51:33Z

@samdoran thanks for taking on the review for this. It's my first open source contribution, so please let me know if there are any mistakes - hoping to submit a new module after this.
This fix is important for python 3.5 (json loads cannot accept bytes as input) but is not required for 3.6+ see https://docs.python.org/3.6/library/json.html#json.loads

ansibot · 2017-11-14T08:53:16Z

waiting_on: ansible
module: cloudflare_dns
supported_by: community
maintainers: mgruener
changes_requested_by: null
needs_info: False
needs_revision: False
needs_rebase: False
merge_commits: []
mergeable_state: clean
shippable_status: success
maintainer_shipits (module maintainers): 0
community_shipits (namespace maintainers): 0
ansible_shipits (core team members): 0
shipit_actors (maintainer or core team member): []
shipit_actors_other: []

click here for bot help

samdoran · 2017-11-15T14:49:14Z

lib/ansible/modules/net_tools/cloudflare_dns.py

@@ -369,7 +369,8 @@ def _cf_simple_api_call(self,api_call,method='GET',payload=None):

        if content:
            try:
-                result = json.loads(content)
+                resp_json = content.decode('utf-8')


Rather than using separate variables, you can accomplish this in one line:

result = json.loads(content.decode('utf-8'))

samdoran · 2017-11-15T14:50:19Z

Thanks for the contribution! You did everything correctly as far as contributions go. Please see my comment on your code.

andy-pi · 2017-11-16T04:25:24Z

@samdoran thankyou for your review - code refactored now as per your suggestion!

abadger · 2017-11-17T01:21:01Z

lib/ansible/modules/net_tools/cloudflare_dns.py

@@ -369,7 +369,7 @@ def _cf_simple_api_call(self,api_call,method='GET',payload=None):

        if content:
            try:
-                result = json.loads(content)
+                result = json.loads(content.decode('utf-8'))


We should use to_text here instead of .decode. to_text is resistant to unicode errors in a way that .decode is not.

from ansible.module_utils._text import to_native, to_text [...] try: result = json.loads(to_text(content, errors='surrogate_then_strict')) except (json.JSONDecodeError, UnicodeError) as e: error_msg += "; Failed to parse API response with error {0}: {1}".format(to_native(e), content)

samdoran · 2017-11-17T02:54:14Z

@andy-pi Sorry to give you bad advice. I asked for a wiser opinion and got it. Toshio's recommendation is the most resilient way to fix this issue.

andy-pi · 2017-11-17T04:31:17Z

@samdolan no probs @abadger thanks for the advice. I don't understand the detail of how to_text works exactly but I do prefer giving a user friendly error - which I did not get when fixing this bug.
Anyway I will update again with your suggestion shortly.
(EDITED - 1st draft not relevant)

… as per reviewer comment

samdoran · 2017-11-17T19:57:10Z

@andy-pi to_text and to_native encode/decode text and byte strings properly depending on the Python version. That's the high level overview.

* fixed .loads error for non decoded json in Python 3 * fixed .loads error Python 3.5 - refactor code to one line * fixed .loads error python 3.5 - mod to use to_text instead of .decode as per reviewer comment (cherry picked from commit 67d5e1d)

samdoran · 2017-11-17T20:07:19Z

Cherry-picked to stable-2.4 for inclusion in Ansible 2.4.2

andy-pi · 2017-11-18T06:46:57Z

@samdoran thanks!!!

fixed .loads error for non decoded json in Python 3

d1493a6

jborean93 removed the needs_triage Needs a first human triage before being processed. label Oct 25, 2017

ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Oct 27, 2017

ansibot added stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. and removed new_contributor This PR is the first contribution by a new community member. labels Nov 4, 2017

samdoran self-assigned this Nov 13, 2017

samdoran reviewed Nov 15, 2017

View reviewed changes

fixed .loads error Python 3.5 - refactor code to one line

eb80c9c

ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Nov 16, 2017

abadger reviewed Nov 17, 2017

View reviewed changes

fixed .loads error python 3.5 - mod to use to_text instead of .decode…

60df4e1

… as per reviewer comment

samdoran merged commit 67d5e1d into ansible:devel Nov 17, 2017

pyup-bot mentioned this pull request Jan 1, 2018

Scheduled monthly dependency update for January napalm255/ansible-vmware_inventory#11

Closed

ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 6, 2018

ansible locked and limited conversation to collaborators Apr 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fixed .loads error for non decoded json in Python 3 #32065

fixed .loads error for non decoded json in Python 3 #32065

andy-pi commented Oct 24, 2017

ansibot commented Oct 24, 2017

ansibot commented Oct 27, 2017

andy-pi commented Oct 27, 2017

andy-pi commented Nov 14, 2017

andy-pi commented Nov 14, 2017

ansibot commented Nov 14, 2017

samdoran Nov 15, 2017

samdoran commented Nov 15, 2017

andy-pi commented Nov 16, 2017

abadger Nov 17, 2017

samdoran commented Nov 17, 2017

andy-pi commented Nov 17, 2017 •

edited

Loading

samdoran commented Nov 17, 2017

samdoran commented Nov 17, 2017

andy-pi commented Nov 18, 2017

fixed .loads error for non decoded json in Python 3 #32065

fixed .loads error for non decoded json in Python 3 #32065

Conversation

andy-pi commented Oct 24, 2017

SUMMARY

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

ADDITIONAL INFORMATION

ansibot commented Oct 24, 2017

ansibot commented Oct 27, 2017

andy-pi commented Oct 27, 2017

andy-pi commented Nov 14, 2017

andy-pi commented Nov 14, 2017

ansibot commented Nov 14, 2017

samdoran Nov 15, 2017

Choose a reason for hiding this comment

samdoran commented Nov 15, 2017

andy-pi commented Nov 16, 2017

abadger Nov 17, 2017

Choose a reason for hiding this comment

samdoran commented Nov 17, 2017

andy-pi commented Nov 17, 2017 • edited Loading

samdoran commented Nov 17, 2017

samdoran commented Nov 17, 2017

andy-pi commented Nov 18, 2017

andy-pi commented Nov 17, 2017 •

edited

Loading