fix: Refactor Nautobot api client and update logging / exception handling #666
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
stevekeay
commented
Feb 11, 2025
•
stevekeay
commented
- make sure exceptions have useful "message"
- consider HTTP redirect responses to be errors
- include URL and payload in exceptions on failed API request
- remove default params (all our callers pass all the args)
- don't log errors, just raise the exception
- re-order args to make_api_request to match the requests library
- combine api request and response into single log message
- pretty-print payload data when logging Nautobot API requests
stevekeay
force-pushed
the
refactor-nautobot-logging-exceptions
branch
from
c2c7f21 to
dcd16f6
Compare
skrobul
approved these changes
Feb 11, 2025
| response = self.make_api_request(url, "get", params=params) | ||
| if not response: | ||
| return False | ||
| params = {"interface_id": interface_id, "vlan_tag": str(vlan_tag)} |
There was a problem hiding this comment.
the conversion to integer was intentional here - we want to fail early if the user calls this method with a non-numeric vlan_tag
There was a problem hiding this comment.
OK I will put it back - I naively assumed that a parameter of type "int" must contain an int value :)
There was a problem hiding this comment.
Python's type whispering in full force. It's more like an ask instead of an insistence.
added 2 commits
February 11, 2025 17:13
- make sure exceptions have useful "message" - consider HTTP redirect responses to be errors - include URL and payload in exceptions on failed API request - remove default params (all our callers pass all the args) - don't log errors, just raise the exception - re-order args to make_api_request to match the requests library - combine api request and response into single log message
stevekeay
force-pushed
the
refactor-nautobot-logging-exceptions
branch
from
dcd16f6 to
9456ad4
Compare
cardoe
reviewed
Feb 11, 2025
|
|
||
| if response.status_code >= 300: | ||
| raise NautobotRequestError( | ||
| code=response.status_code, url=full_url, body=response.content |
There was a problem hiding this comment.
Suggested change
| code=response.status_code, url=full_url, body=response.content | |
| code=response.status_code, url=full_url, body=response.content |
long term you'll probably want the response.json()["error"] by default.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.