add create/create_unless_exists actions support to node client (from pickypg) #39
Conversation
| subject.register | ||
| subject.receive(LogStash::Event.new("message" => "sample message here")) | ||
| subject.buffer_flush(:final => true) | ||
| es.indices.refresh |
There was a problem hiding this comment.
we can do a get here immediately to assert the existence of the doc (without a refresh)
There was a problem hiding this comment.
this is needed, otherwise does not get indexed in time
There was a problem hiding this comment.
aye, for search we need a refresh..get is real time :) anyhow, can leave this as is
|
+1, one new test needed to verify behavior |
| @@ -178,6 +178,8 @@ class LogStash::Outputs::ElasticSearch < LogStash::Outputs::Base | |||
| # | |||
| # - index: indexes a document (an event from Logstash). | |||
| # - delete: deletes a document by id | |||
| # - create_unless_exists: creates a document, raises exception if no id is provided | |||
There was a problem hiding this comment.
Also fails if a document by that id already exists in the index.
There was a problem hiding this comment.
We should explicitly document that this is not supported by the HTTP client unless it does. I'm torn on that difference too, but it makes sense for the HTTP client to be a bit truer to the REST spec.
There was a problem hiding this comment.
this isn't supported by the http client?
There was a problem hiding this comment.
wording - "raises exception" <-- exceptions are something developers care about. Maybe instead say "will fail if no ID is provided or if the document already exists?"
The important phrase to change, for me, is "raises exception" (dev focused) to be "fails" (user focused)
There was a problem hiding this comment.
@jordansissel No, that's why we changed it from create originally to create_unless_exists to drive home that it's different.
The functionality implied by create_unless_exists is still true in the HTTP client as long as you supply an ID with create. The only difference is that create_unless_exists requires you to specify the ID (as opposed to create, which will generate one if not supplied).
talevy
force-pushed
the
feature/create
branch
from
9998916 to
a46a9a7
Compare
|
LGTM |
talevy
force-pushed
the
feature/create
branch
from
a46a9a7 to
234e2c6
Compare
talevy
force-pushed
the
feature/create
branch
from
234e2c6 to
1c8f6c1
Compare
|
Merged sucessfully into master! |
original PR: #29
@pickypg