elastisch icon indicating copy to clipboard operation
elastisch copied to clipboard

_version_type ignored in native bulk

Open royaldark opened this issue 8 years ago • 2 comments

#227 added support for _version and _version_type in bulk requests. This works great over REST, but the version type is ignored in native.

This happens because common.bulk/index-operation uses keys of the form _version_type, but native.conversion/->action-requests wants the key version-type (which, if passed in, would be stripped by common.bulk).

(.versionType
  (first
    (clojurewerkz.elastisch.native.conversion/->action-requests
      [(clojurewerkz.elastisch.native.bulk/index-operation
         {:_index "an_index", :_type "a_type", :_id "123456", :_version 5, :_version_type "external"})])))
=> #<INTERNAL>

Not sure the best solution - maybe ->action-requests needs to be more permissive and allow the key version_type as well as version-type?

royaldark avatar Jul 15 '16 21:07 royaldark

Is it possible that this function should be slightly different?

It looks like it should ensure the compatibility of the keys that are passed to ->action-requests (see here), but it is only removing the leading underscore. So the :_version_type key is transformed to :version_type instead of :version-type. I tried looking for more insight in the commit messages but it didn't help.

I think it is also fair to argue that clojurewerkz.elastisch.common.bulk should not take care of these keys, since they are different in each client.

@michaelklishin what do you think? I am open to submit a PR with the required changes.

jqmtor avatar Sep 24 '16 15:09 jqmtor

@quimrstorres if you have a test case that demonstrates the issue to go with it, feel free to do it. Af first glance your findings seem reasonable. Thank you.

michaelklishin avatar Sep 25 '16 12:09 michaelklishin