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

fix: fix how the buffer limit check evaluates streams config #6876

Merged

Conversation

rodesai
Copy link
Contributor

@rodesai rodesai commented Jan 20, 2021

The buffer limit check first creates a dummy streams config to get the current
max bytes buffering value. To do this it creates a map of the streams props and
creates the config (with dummy values for all the required fields). However
sometimes those fields are already provided and ImmutableMap doesn't like
overwrites in the builder. So, use a plain old HashMap.

The buffer limit check first creates a dummy streams config to get the current
max bytes buffering value. To do this it creates a map of the streams props and
creates the config (with dummy values for all the required fields). However
sometimes those fields are already provided and ImmutableMap doesn't like
overwrites in the builder. So, use a plain old HashMap
@rodesai rodesai requested a review from a team as a code owner January 20, 2021 07:05
Copy link
Contributor

@vcrfxia vcrfxia left a comment

Choose a reason for hiding this comment

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

LGTM besides the comment inline. Thanks @rodesai !

@@ -20,7 +20,8 @@
# The default is any IPv4 interface on the machine.
# NOTE: If set to wildcard or loopback set 'advertised.listener' to enable pull queries across machines
listeners=http://0.0.0.0:8088

ksql.service.id=test
Copy link
Contributor

Choose a reason for hiding this comment

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

This does not look like it's meant to be checked in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😞 thanks

@rodesai rodesai merged commit ad1cc2a into confluentinc:master Jan 21, 2021
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.

2 participants