docs(api): updates description of config exclusions #11139
Conversation
Signed-off-by: prmellor <pmellor@redhat.com>
There was a problem hiding this comment.
Please don't change this. This can have all kind of consequences. I would changeonly the handwritten list in documentation/api/io.strimzi.api.kafka.model.kafka.KafkaClusterSpec.adoc.
There was a problem hiding this comment.
What are the consequences you are referring to? ZooKeeper is not supported anymore why should we keep those ZK related configurations as forbidden? The only reason I can think of is that with the coming Strimzi we don't support ZooKeeper anymore but the Kafka 3.9 version has still such support and not forbidding zookeeper. stuff it would allow the user to set them in the custom resource and messing up with the cluster?
There was a problem hiding this comment.
I thought it would be okay to take them out, but I've just reverted.
There was a problem hiding this comment.
What are the consequences you are referring to? ZooKeeper is not supported anymore why should we keep those ZK related configurations as forbidden? The only reason I can think of is that with the coming Strimzi we don't support ZooKeeper anymore but the Kafka 3.9 version has still such support and not forbidding
zookeeper.stuff it would allow the user to set them in the custom resource and messing up with the cluster?
There might be all kind of validations, warnings, etc.
Also, TBH, we do not really want these to just make the config files more confusing either. You do not want to spend the time on some log showing you that it has ZooKeeper configured just to find out few days later that it is a Kraft cluster where a user for some reason put these values into .spec.kafka.config. (and yes, there are users who have these things configured in the Kafka CR fro years and do not really understand that they are completely ignored 🤷 )
There was a problem hiding this comment.
Maybe we could at least adding a code comment on the top of the FORBIDDEN_PREFIXES declaration mentioning why the zookeeper. stuff is there. I could expect someone coming on the same code in the next months/years and trying to open a PR to remove them. @PaulRMellor can you do that?
There was a problem hiding this comment.
Yeah. We could add something to the property description.
There was a problem hiding this comment.
I would leave it as is TBH. If someone removes it next year, nobody might care anymore maybe?
There was a problem hiding this comment.
Next year, for sure, but what if someone does beforehand in the next coming months ;-)
There was a problem hiding this comment.
I've updated the description in the code to make it clear that the zookeeper options are no longer relevant
Signed-off-by: prmellor <pmellor@redhat.com>
Signed-off-by: prmellor <pmellor@redhat.com>
Documentation
Updates description of config exclusions for KafkaClusterSpec considering support ending for ZooKeeper
Checklist
Please go through this checklist and make sure all applicable tasks have been done