Skip to content

Conversation

crepererum
Copy link
Collaborator

No description provided.

Copy link
Contributor

@mkmik mkmik left a comment

Choose a reason for hiding this comment

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

ideally we should just get config via the standard kafka map<string,string>; client.id would be one of the many kafka config options conveyed with that method

@crepererum
Copy link
Collaborator Author

ideally we should just get config via the standard kafka map<string,string>; client.id would be one of the many kafka config options conveyed with that method

image

You mean the "replicate whatever the default Java client in some version X does"? 😉

@crepererum crepererum added the automerge Instruct kodiak to merge the PR label Oct 26, 2022
@kodiakhq kodiakhq bot merged commit 55755ba into main Oct 26, 2022
@kodiakhq kodiakhq bot deleted the crepererum/custom_client_id branch October 26, 2022 15:38
@mkmik
Copy link
Contributor

mkmik commented Oct 26, 2022

ideally we should just get config via the standard kafka map<string,string>; client.id would be one of the many kafka config options conveyed with that method

image

You mean the "replicate whatever the default Java client in some version X does"? 😉

Yeah; don't know how common that is across other client libraries. But the core idea is that every time we support a new parameter in rskafka we shouldn't also have to add some code in the caller (e.g. iox) just to make use of the parameter when ultimately it's likely to come from a config env string.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Instruct kodiak to merge the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants