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

A first stab at Support bulk operations in the native client #24 #144

Merged

Conversation

mitchelkuijpers
Copy link
Contributor

So this is my first stab at native bulk operations. The responses are not exactly the same as the but come pretty close.

The API is exactly the same only the params handling is a bit messy and only supports {:refresh true} currently. I implemented the same tests as the rest bulk test and they all succeed :) But you can pretty much switch between the native and rest bulk calls, which I think is pretty neat.

I am happy to receive some feedback I am working on improving my clojure skills.

(defn bulk-delete
"generates the content for a bulk delete operation"
([documents]
(let [operations (map delete-operation documents)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This local can be inlined here.

@michaelklishin
Copy link
Member

I've left some comments, none of them are major. If this is one of your first attempts at working with Clojure, good job! :)

@mitchelkuijpers
Copy link
Contributor Author

Thank you for your feedback :) I am using Clojure for about 3 months now, but I currently work alone so I get no code reviews :( Think I'm going to do some more open source work good way to improve my clojure (and a lot of fun)

michaelklishin added a commit that referenced this pull request Mar 2, 2015
A first stab at Support bulk operations in the native client #24
@michaelklishin michaelklishin merged commit 7c8ccf8 into clojurewerkz:master Mar 2, 2015
@michaelklishin
Copy link
Member

Thank you!

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.

2 participants