Skip to content

Fail on http errors #56

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

nzlosh
Copy link
Contributor

@nzlosh nzlosh commented Jun 15, 2025

This PR updates the following:

  • actions regenerated against netbox api 4.3.
  • removed masking netbox http return codes and messages
  • added parameter fail_non_2xx so actions will return failed instead of succeeded on non 2xx http return codes.

@nzlosh nzlosh requested review from lampwins, abhi1693 and a team as code owners June 15, 2025 10:28
@nzlosh nzlosh requested review from mickmcgrath13 and removed request for a team June 15, 2025 10:28
@nzlosh nzlosh force-pushed the fail_on_http_errors branch from ab70c2c to 9c47a9f Compare June 16, 2025 05:31
Copy link
Collaborator

@abhi1693 abhi1693 left a comment

Choose a reason for hiding this comment

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

I'm not in favour of hard coded 200 and 300 status code checks when the parameter is for 2xx checks. It's a bit misleading for the user. Moreover, due to different configurations or - actions, an http code of a different value may be received. Assuming it's 201, which must still be accepted, the code will raise an error leading to more issues.

@nzlosh
Copy link
Contributor Author

nzlosh commented Jun 21, 2025

The range function generates a list from 200 to 299.

        if fail_non_2xx:
            action_succeeded = result.get("status") in range(200, 300)

For an HTTP status code of 201 Created the action will be successful while a 403 Forbidden will be failure. In the previous version of the code, the action was always successful even when 403 Forbidden was returned.

I understand in some use cases this behaviour is desirable because the action did succeed in communicating with the netbox API despite access refused. In other use cases the action success is based on fetching the data to proceed with processing the workflow and a failure to fetch the data is considered an action failure.

The PR is written to default to the same behaviour as the previous version while allowing users to choose which behaviour fits their use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants