Skip to content

KAFKA-9494: Include data type of the config in ConfigEntry#8034

Closed
srpanwar-confluent wants to merge 1 commit intoapache:trunkfrom
srpanwar-confluent:KAFKA-9494
Closed

KAFKA-9494: Include data type of the config in ConfigEntry#8034
srpanwar-confluent wants to merge 1 commit intoapache:trunkfrom
srpanwar-confluent:KAFKA-9494

Conversation

@srpanwar-confluent
Copy link
Copy Markdown
Contributor

Why this change?
To enable better validation. Including the data type can significantly improve client's(web or cli or any other client) ability to validate the values before even making a request to Kafka. In the absence of type the only way to know if the user specified value is correct is to make an alter call and check if there is no error.

Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.

Updated and added new unit tests

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

val readOnly = !DynamicBrokerConfig.AllDynamicConfigs.contains(name)
new DescribeConfigsResponse.ConfigEntry(name, valueAsString, source, isSensitive, readOnly, synonyms.asJava)
new DescribeConfigsResponse.ConfigEntry(
name, valueAsString, source, isSensitive, readOnly, synonyms.asJava, configEntryType.toString())
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Check for null

@rondagostino
Copy link
Copy Markdown
Contributor

Is there a KIP for this? Unless I am mistaken, the org.apache.kafka.clients.admin package is Javadoc'ed , which means anything in this package is part of the public API, and adding a datatype would require a KIP.

@srpanwar-confluent
Copy link
Copy Markdown
Contributor Author

@rondagostino I am in the process of writing the KIP. I'll update the PR and move it out of draft stage once its ready.

@tombentley
Copy link
Copy Markdown
Member

@srpanwar-confluent you should probably we aware of #8312, which converts the DescribeConfigs RPC to use the generated protocols.

@srpanwar-confluent
Copy link
Copy Markdown
Contributor Author

thanks @tombentley for the heads up. I'll wait for your changes to go merge before resuming work on this.

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.

3 participants