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

added option 'ignorable_codes' to also return failed REST calls #32

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

Conversation

rpasche
Copy link

@rpasche rpasche commented Feb 22, 2021

Setting this option to true will return the response headers and response body also for failed REST calls. This means, unless there is an exception, setting ignore_errors to true will always succeed.

The idea behind is to also be able to fetch the results for failed HTTP requests, parse them and - maybe - use that data within elasticsearch.

@cla-checker-service
Copy link

cla-checker-service bot commented Feb 22, 2021

💚 CLA has been signed

@MikeKemmerer
Copy link

Any chance you can roll in the "form" change I made, too? They haven't been too quick on accepting pull requests for this plugin.

@kares
Copy link

kares commented Mar 16, 2021

Nice addition, however I think a better approach would be to make the 200-299 "valid" response codes configurable.
Accepting intervals as well as individual response code (in an array), wdyt?

@rpasche
Copy link
Author

rpasche commented Mar 16, 2021

Hmm... Yes. Need to think about it. So you mean that you need to give the codes of "valid" codes in an array of this option is set to true.

@kares
Copy link

kares commented Mar 17, 2021

I mean not having ignore_errors but having a valid_responses => ... option instead (anything else is an error).
The idea would be to somehow support ranges => ['200-299', 302] as well as individual response codes => [200, 201, 204], given that LS does not support ranges this might need more work.

Not yet sure what the best approach would be to represent ranges, hopefully some other plugin is using smt similar already.

@rpasche
Copy link
Author

rpasche commented Apr 12, 2021

@kares Found a bit time to further work on this. For now, I'm just doing exactly the same as the ignorable_codes (https://www.elastic.co/guide/en/logstash/current/plugins-outputs-http.html#plugins-outputs-http-ignorable_codes) within the logstash http output plugin. That should make it a bit more consistent.

Also use the same logic. Only difference is that http codes between 200 and 299
are - as before - considered success. Other http codes are only considered
success, if they are listed within 'ignorable_codes'.
@rpasche rpasche changed the title added option 'ignore_errors' to also return failed REST calls added option 'ignorable_codes' to also return failed REST calls Apr 12, 2021
@rpasche
Copy link
Author

rpasche commented Apr 12, 2021

I am not able to run bundle exec spec anymore. I get some dependency error when I try this within my WSL. So hoping, that all tests pass...

@fylie fylie mentioned this pull request Aug 27, 2021
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants