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

implement sync producing with a strategy different from the default one #71

Closed
wants to merge 1 commit into from

Conversation

dams
Copy link
Contributor

@dams dams commented Jun 5, 2018

when you have defined a producer in the config, but you want to produce_sync to it with a different strategy (say you tried with md5 but it failed and you want to try it with :random)

@objectuser
Copy link
Contributor

objectuser commented Jun 5, 2018

@dams Would this signature also work?

def produce_sync(topic, message_list, strategy) when is_list(message_list) ...

Seems like there's a lot of overloaded names in there, but I don't know what the Elixir best practice is here. Maybe a keyword list?

@dams
Copy link
Contributor Author

dams commented Jun 5, 2018

no, there is already:

def produce_sync(topic, partition, message_list) when is_list(message_list)

I have to say that the public API of this file is ... messy :) Or at leat, not the style I like the best

@objectuser
Copy link
Contributor

Those two wouldn't conflict would they? The list is the third parameter vs. the second.

But I agree, I find it confusing. I'm thinking an options list would be better.

@objectuser
Copy link
Contributor

What about something like this:

def produce(topic, message_list, opts \\ [])

Then if a single message was being produced, it would be:

Kaffe.Producer.produce(topic, [{key, value}], partition_strategy: :md5)

It looks simple from the signature perspective, but I'm not sure how complicated it would be to implement.

@dams
Copy link
Contributor Author

dams commented Jun 5, 2018

Oh yes, you're right, technically it doesn't conflict, but it'd be soooo confusing :)

In my humble opinion you should keep it simple indeed, maybe even simler than what you propose. Only support message_list, and have "sync_mode" or "strategy" be specified via options

def produce(topic, messages, opt \\ [])

@objectuser objectuser closed this in 4d33e3c Jul 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

2 participants