Repository navigation
Conversation
…create client only once
sbernauer
left a comment
There was a problem hiding this comment.
I very much like the extensive kuttl topic test! I only reviewed that.
One problem I noticed: I often times do --skip-delete to end up with a working example I can play around with. This PR always deletes everything at the end. Maybe you can ask AI if we can easily add something like stackabletech/airflow-operator#841 here as well. If not possible, I think this case is ok, as there is much deletion testing going on.
| spec: | ||
| clusterRef: | ||
| name: test-kafka | ||
| topicName: orders |
There was a problem hiding this comment.
strimzi recommends to only set this when topicName != .metadata.name, which I agree with
| topicName: orders |
| spec: | ||
| clusterRef: | ||
| name: test-kafka | ||
| topicName: orders |
There was a problem hiding this comment.
strimzi recommends to only set this when topicName != .metadata.name, which I agree with
| topicName: orders |
| clusterRef: | ||
| name: test-kafka | ||
| topicName: orders | ||
| partitions: 2 | ||
| replicationFactor: 3 | ||
| configOverrides: | ||
| retention.ms: "7200000" |
There was a problem hiding this comment.
I find the tests easier to understand when we only patch what we want to patch
| clusterRef: | |
| name: test-kafka | |
| topicName: orders | |
| partitions: 2 | |
| replicationFactor: 3 | |
| configOverrides: | |
| retention.ms: "7200000" | |
| partitions: 2 | |
| replicationFactor: 3 |
| spec: | ||
| clusterRef: | ||
| name: test-kafka | ||
| topicName: throttled |
There was a problem hiding this comment.
strimzi recommends to only set this when topicName != .metadata.name, which I agree with
| topicName: throttled |
| spec: | ||
| clusterRef: | ||
| name: test-kafka | ||
| topicName: legacy |
There was a problem hiding this comment.
strimzi recommends to only set this when topicName != .metadata.name, which I agree with
| topicName: legacy |
| clusterRef: | ||
| name: test-kafka | ||
| topicName: orders | ||
| partitions: 5 | ||
| replicationFactor: 1 | ||
| configOverrides: | ||
| retention.ms: "10800000" |
There was a problem hiding this comment.
I would contain the patch to what changed
| clusterRef: | |
| name: test-kafka | |
| topicName: orders | |
| partitions: 5 | |
| replicationFactor: 1 | |
| configOverrides: | |
| retention.ms: "10800000" |
| clusterRef: | ||
| name: test-kafka | ||
| topicName: orders | ||
| partitions: 5 | ||
| replicationFactor: 1 | ||
| configOverrides: | ||
| retention.ms: "10800000" |
There was a problem hiding this comment.
I would contain the patch to what changed
| clusterRef: | |
| name: test-kafka | |
| topicName: orders | |
| partitions: 5 | |
| replicationFactor: 1 | |
| configOverrides: | |
| retention.ms: "10800000" |
| clusterRef: | ||
| name: test-kafka |
There was a problem hiding this comment.
I would contain the patch to what changed
| clusterRef: | |
| name: test-kafka |
There was a problem hiding this comment.
Can we please reduce the patch to what we actually want to change, I guess managementAccess.enabled; false?
Part of stackabletech/issues#888.
Description
Definition of Done Checklist
Author
Reviewer
Acceptance
type/deprecationlabel & add to the deprecation scheduletype/experimentallabel & add to the experimental features tracker