-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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 index, document_id and empty query return #1017
Conversation
|
||
@fields.each do |old, new| | ||
event[new] = results['hits']['hits'][0]['_source'][old] | ||
end | ||
|
||
unless @document_id.nil? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
double negative ;)
We can just do: if @document_id
The double negative is fixed. :) |
I guess no one notice the plugin wouldn't work with >1 index = no real use case for this plugin = not worth caring :E |
Can one of the admins verify this patch? |
Well it needs to be rebased. |
done. |
f86287d
to
a536eef
Compare
@codyit sorry to bother you once more, could you review your proposal on top of existing https://github.com/logstash-plugins/logstash-filter-elasticsearch and if relevant move your PR there. Thanks |
From the look of it the code would still fail if there are more than one index unless I'm missing something. It's been a long time since I touched it, and there are active contributors over there, I'll create an issue there instead. |
For Logstash 1.5.0, we've moved all plugins (and grok patterns) to individual repositories. Can you move this pull request to https://github.com/logstash-plugins/logstash-filter-elasticsearch? This sequence of steps may help you do this migration:
|
Improved elasticsearch filter.
Added fields
Specify index to be logstash-* by default.
Capture document_id for use in output overwriting.
Return properly if nothing is found.