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

Bulk "index" action and HEAD support #78

Merged

Conversation

tcrossland
Copy link
Contributor

This PR includes support for the bulk "index" action (create or replace), while leaving "create" as the default.

I've also included support for HTTP HEAD requests, which Elasticsearch uses in various places (index exists, type exists, etc.)

@tcrossland
Copy link
Contributor Author

Another small improvement to allow index settings to be specified as a map instead of a file path (to work around issues like #51 and #76)

Copy link
Owner

@danielberkompas danielberkompas left a comment

Choose a reason for hiding this comment

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

Approved when tests pass!

@tcrossland
Copy link
Contributor Author

tcrossland commented Oct 7, 2019

@danielberkompas thanks, I've changed the validation for settings to ensure that it's either a string (is_binary/1) or a map (is_map/1). I've removed the presence validation as it fails for empty maps (an empty map is now a valid value for settings)

@erikreedstrom
Copy link

It seems at least part of this functionality is covered under #68

We have a need to support delete actions as well in bulk, and would like to know which of the two PRs will be moving forward.

@erikreedstrom
Copy link

We've added some onto this in the expectation this is merged. 😄 We'll PR once this is in.

For reference, here's our changes compared to this branch: Bluetab/elasticsearch-elixir@feature/bulk-index-action...carsdotcom:decouple_bulk_action

@tcrossland
Copy link
Contributor Author

@danielberkompas is there anything preventing this being merged? thanks

@michelboaventura
Copy link

Hey @danielberkompas thank you for the great project, it's been really useful for us. Unfortunately I'm hitting the issue this changes seem to address. Does this PR need anything else to be merged?

Thanks!

@danielberkompas danielberkompas merged commit 3332e6e into danielberkompas:master Jun 12, 2021
@danielberkompas
Copy link
Owner

@michelboaventura nothing else was needed, I just lost track of it!

@michelboaventura
Copy link

Thank you @danielberkompas!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants