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

WEBServer / query.execute / Result as string (OK/WARNING/...) #560

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Samuel-BF
Copy link
Contributor

With current version, /api/v1/queries/check_xxx/commands/execute returns "result" field as an int (0/1/2/...), whereas /api/v1/queries/check_xxx/commands/execute_nagios returns "result" as a string (OK/WARNING/...), like in legacy API.
This modification makes the behaviour consistent by always returning the string.

Note that I did not test the proposed modification.

With current version, /api/v1/queries/check_xxx/commands/execute returns "result" field as an int (0/1/2/...), whereas /api/v1/queries/check_xxx/commands/execute_nagios returns "result" as a string (OK/WARNING/...), like in legacy API.
This modification makes the behaviour consistent by always returning the string.

Note that I did not test the proposed modification.
Samuel-BF added a commit to Samuel-BF/check_nsc_web that referenced this pull request Apr 6, 2018
Previous version fails to exit with the correct return code (1 for WARNING, 2 for CRITICAL ...).
The new version fixes that point. Note that i've proposed a patch to the author of NSClient++ which also corrects the problem. (see : mickem/nscp#560 ). If the patch to NSClient++ is accepted, we should roll back to previous version.
@mickem
Copy link
Owner

mickem commented Apr 12, 2018

As this is a breaking change to the API i think it would be prudent to create a new version right?

@Samuel-BF
Copy link
Contributor Author

Samuel-BF commented Apr 16, 2018

As you like. There are no known open-source API clients relying on APIv1 yet. So it should not annoy lot of people.

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.

2 participants