Sync Producer: Don't change config in constructor #790
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It's surprising, for one thing, and as Alexander Zhuravlev discovered it can actually lead to data races if you already have async producers active using the same config object. Instead, validate that it is pre-set correctly and return a config error if it is not, so it is on the user to set things up properly (it's only one flag away from default so it's not that big a deal).
|
There's no easy way to clone a config, and set the properties to |
|
We considered that in #789; there are a bunch of config values where it doesn't make any sense to have different instances of the config. |
|
@eapache Looks great, thanks. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It's surprising, for one thing, and as Alexander Zhuravlev discovered it can
actually lead to data races if you already have async producers active using the
same config object.
Instead, validate that it is pre-set correctly and return a config error if it
is not, so it is on the user to set things up properly (it's only one flag away
from default so it's not that big a deal).
Fixes #789.
cc @zaa