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

Support _routing meta-field #37

Merged
merged 5 commits into from Aug 3, 2018
Merged

Conversation

cdunn
Copy link
Contributor

@cdunn cdunn commented Jul 31, 2018

Add a new optional protocol DocumentMeta that adds the ability to specify a _routing key. This allows specific shard targeting which is required for join fields.

@danielberkompas let me know if you think this is the right approach and I can add a few more tests.

@danielberkompas
Copy link
Owner

@cdunn thanks for this! I'm wondering if we could just incorporate a new routing function into the existing Document protocol, rather than creating a new protocol.

defimpl Elasticsearch.Document, for: MyApp.Type do
  # if you don't want to use routing, just return false
  def routing(_document), do: false

  # if you want to use routing, do this:
  def routing(document) do
    # ...
  end
end

@cdunn
Copy link
Contributor Author

cdunn commented Aug 1, 2018

@danielberkompas sounds good. I agree it's cleaner as a single protocol...I originally tried that but couldn't figure out if there was a way to make just the routing function fallback_to_any and false it there by default so it wouldn't be a breaking change given that most folks probably don't use it to start. Will consolidate and document later but let me know if you can think of any way to make this non-breaking.

@danielberkompas
Copy link
Owner

I don't think it matters too much if it's a breaking change. The library is still <1.0.0 at this point, so breaking changes are not a really big problem. As long as we write a good upgrade guide, I think it will be fine.

@coveralls
Copy link

coveralls commented Aug 2, 2018

Coverage Status

Coverage increased (+0.08%) to 97.949% when pulling 504c926 on cdunn:routing into a8d5d34 on infinitered:master.

@danielberkompas
Copy link
Owner

This looks great, thanks! I'll try and get a new version released over the weekend.

@danielberkompas danielberkompas merged commit d7a44df into danielberkompas:master Aug 3, 2018
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

3 participants