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

add msg if API response can't be converted #486

Merged
merged 7 commits into from
Sep 30, 2024
Merged

add msg if API response can't be converted #486

Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f84fe92
add msg if API response cant be converted
konstruktoid Jul 2, 2024
2c7c91a
add default response
konstruktoid Aug 1, 2024
9bd5c1b
catch ValueError and increase msg length check
konstruktoid Aug 2, 2024
4588d26
revert msg length check
konstruktoid Aug 2, 2024
7e439e5
align with code standard
konstruktoid Sep 30, 2024
26ef1c4
fix syntax
konstruktoid Sep 30, 2024
6777820
revert back to the beginning
konstruktoid Sep 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion plugins/module_utils/entity.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,8 +411,16 @@ def _fetch_url(
return {"status_code": status_code}

if resp_json is None:
try:
if len(info.get("msg")) > 2:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thats fine, let it be 0 so that we can back trace any error message in our product code base.

Copy link
Author

Choose a reason for hiding this comment

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

done

Copy link
Collaborator

@bhati-pradeep bhati-pradeep Sep 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@konstruktoid What happens if msg key is not present in info ? I am just thinking it can cause TypeError ? As len(None) can cause that. Does ansible's fetch_url confirms that msg will be always a string ?

Copy link
Collaborator

@bhati-pradeep bhati-pradeep Sep 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Since you are only checking ValueError.

Copy link
Author

Choose a reason for hiding this comment

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

yeah, perhaps i should just simplify it like https://github.com/nutanix/nutanix.ansible/pull/486/files#diff-be0a07cfc89126d4e28a8fbd2d2716635f9ab383eb08f54b504429dd647853efL399-L400

Copy link
Collaborator

Choose a reason for hiding this comment

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

@konstruktoid there is a small syntax issue. Please check latest comment

resp_json_msg = "{}".format(info.get("msg"))
else:
resp_json_msg = "Failed to convert API response to json"
except ValueError:
resp_json_msg = "Failed to convert API response to json"

self.module.fail_json(
msg="Failed to convert API response to json",
msg=resp_json_msg,
status_code=status_code,
error=body,
response=resp_json,
Expand Down