AndrewJSchofield commented on code in PR #15067:
URL: https://github.com/apache/kafka/pull/15067#discussion_r1693321580
##########
clients/src/main/java/org/apache/kafka/common/config/ConfigResource.java:
##########
@@ -33,7 +33,12 @@ public final class ConfigResource {
* Type of resource.
*/
public enum Type {
- CLIENT_METRICS((byte) 16), BROKER_LOGGER((byte) 8), BROKER((byte) 4),
TOPIC((byte) 2), UNKNOWN((byte) 0);
+ GROUP((byte) 32),
Review Comment:
It seems a little unfortunate that `o.a.k.common.resource.ResourceType` (as
used by ACLs) and `o.a.k.common.config.ConfigResource.Type` (as used by config
resources) are separate enums. They do not have perfect alignment. For example,
`ResourceType.CLUSTER` (the whole cluster) and `ConfigResource.Type.BROKER`(a
single broker) are both 4. There are concepts in one of the enums which do not
exist in the other, and I expect there's an element of luck to whether values
align between them.
I personally don't see any reason why `ConfigResource.Type.GROUP` could not
be 3 (which I think would need a tweak to the KIP) which would then align with
`ResourceType.GROUP`.
More broadly, they are not the same enums and if we want to keep them
aligned as much as possible moving forwards, we need to take steps in the code
(such as comments) to ensure that when updates are being made to one of the
enums, it's done with awareness of the values used in the other.
@pranavrth I think you're going to have to bite the bullet in librdkafka and
define a second enum, when you are able to change the public API.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
