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

Wrap isSecure config in config map for kafka topic #5474

Merged
merged 5 commits into from
Dec 11, 2023

Conversation

neil-xie
Copy link
Contributor

What changed?
Unexpose isSecure config, replace with config map

Why?
IsSecure is internal config for kafka topic, we don't want it to be exposed for public users

How did you test it?

Potential risks

Release notes

Documentation Changes

common/config/kafkaConfig.go Outdated Show resolved Hide resolved
common/config/kafkaConfig.go Show resolved Hide resolved
Copy link
Contributor

@Groxx Groxx left a comment

Choose a reason for hiding this comment

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

yea, probably worth the get(topic, prop) API for unpacking that structure, which seems reasonable.

maybe one day we'll have a more standardized "externally-extendable config" thing. but this seems fine for now, and it's constrained enough that changing it later should be fairly safe / straight-forward for anyone using it.

@neil-xie neil-xie merged commit 0c066ec into uber:master Dec 11, 2023
16 checks passed
@neil-xie neil-xie deleted the CDNC_6374 branch December 11, 2023 22:42
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