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

-Fixes JSON parsing(use JSON object instead of string) for facts module #31818

Merged
merged 1 commit into from
Oct 23, 2017
Merged

-Fixes JSON parsing(use JSON object instead of string) for facts module #31818

merged 1 commit into from
Oct 23, 2017

Conversation

kedarkekan
Copy link
Contributor

SUMMARY

Fixes output parsing for junos facts to use JSON object instead of a string

Fixes #31489

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

modules/network/junos

ANSIBLE VERSION
2.5.0 (devel)

@kedarkekan kedarkekan added affects_2.4 This issue/PR affects Ansible v2.4 affects_2.5 This issue/PR affects Ansible v2.5 bugfix_pull_request networking Network category support:network This issue/PR relates to code supported by the Ansible Network Team. labels Oct 17, 2017
@ansibot
Copy link
Contributor

ansibot commented Oct 17, 2017

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. module This issue/PR relates to a module. labels Oct 17, 2017
from ansible.module_utils.basic import AnsibleModule
from ansible.module_utils.junos import junos_argument_spec, check_args, get_param
from ansible.module_utils.junos import get_configuration
from ansible.module_utils.pycompat24 import get_exception
from ansible.module_utils.netconf import send_request
from ansible.module_utils.six import iteritems
from ansible.module_utils._text import to_text
Copy link
Member

Choose a reason for hiding this comment

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

Is to_text used anywhere in this module?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@@ -155,7 +158,7 @@ def populate(self):
config = self.get_text(reply, 'configuration-text')

elif config_format == 'json':
config = str(reply.text).strip()
config = json.loads(reply.text)
Copy link
Member

Choose a reason for hiding this comment

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

Also, I think this can be updated to use module.from_json()

Copy link
Member

@ganeshrn ganeshrn left a comment

Choose a reason for hiding this comment

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

Please do add/modify unit and integration test for this fix.

@ansibot ansibot added the test This PR relates to tests. label Oct 18, 2017
@gundalow gundalow added this to the 2.5.0 milestone Oct 18, 2017
Copy link
Member

@ganeshrn ganeshrn left a comment

Choose a reason for hiding this comment

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

Looks good.

@kedarkekan kedarkekan merged commit 465fe58 into ansible:devel Oct 23, 2017
@kedarkekan kedarkekan deleted the fix31489 branch October 23, 2017 12:19
@kedarkekan kedarkekan added this to TODO: Nice to have in 2.4.x Blocker List Oct 25, 2017
@abadger
Copy link
Contributor

abadger commented Oct 26, 2017

cherry-picked for 2.4.2beta1

@abadger abadger moved this from TODO: Nice to have to Done in 2.4.2 in 2.4.x Blocker List Oct 26, 2017
@ansibot ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 6, 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.4 This issue/PR affects Ansible v2.4 affects_2.5 This issue/PR affects Ansible v2.5 bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. module This issue/PR relates to a module. networking Network category support:network This issue/PR relates to code supported by the Ansible Network Team. test This PR relates to tests.
Projects
No open projects
2.4.x Blocker List
Done in 2.4.2
Development

Successfully merging this pull request may close these issues.

'ansible.utils.unsafe_proxy.AnsibleUnsafeText object' has no attribute 'configuration'
5 participants