- Notifications
You must be signed in to change notification settings - Fork919
[KIP-848] Added online upgrade and downgrade test#2012
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
🎉 All Contributor License Agreements have been signed. Ready to merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
This PR adds comprehensive integration tests for Kafka consumer upgrade and downgrade scenarios, specifically testing the transition between classic and consumer group protocols. The tests verify that consumers can successfully upgrade from classic protocol to consumer protocol and downgrade back, while maintaining proper partition assignment and message consumption throughout the process.
Key changes:
- Added new integration test file with upgrade/downgrade test logic for multiple partition assignment strategies
- Enhanced cluster fixture with topic deletion capability and improved message seeding
- Refactored common utility functions for better group protocol configuration handling
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tests/integration/consumer/test_consumer_upgrade_downgrade.py | New test file implementing consumer protocol upgrade/downgrade tests with partition assignment validation |
| tests/integration/cluster_fixture.py | Added delete_topic method and improved seed_topic to generate proper test keys |
| tests/common/init.py | Refactored group protocol configuration utilities with new helper method and improved conditional logic |
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The comment appears to be truncated. It should likely read '# limitations under the License.' to complete the Apache License header.
| #limit | |
| #limitations under the License. |
Copilot uses AI. Check for mistakes.
| # def produce_messages(producer, topic, partitions, num_messages): | ||
| # for i in range(num_messages): | ||
| # key = "key-{}".format(i) | ||
| # value = "value-{}".format(i) | ||
| # partition = i % partitions | ||
| # producer.produce(topic, key=key, value=value, partition=partition) | ||
| # producer.flush() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This commented-out function should be removed as it's not being used and adds clutter to the codebase.
| # def produce_messages(producer, topic, partitions, num_messages): | |
| # for i in range(num_messages): | |
| # key = "key-{}".format(i) | |
| # value = "value-{}".format(i) | |
| # partition = i % partitions | |
| # producer.produce(topic, key=key, value=value, partition=partition) | |
| # producer.flush() | |
| # Removed the commented-out `produce_messages` function to reduce clutter. |
Copilot uses AI. Check for mistakes.
| """ | ||
| Test consumer upgrade and downgrade. | ||
| """ | ||
| topic = kafka_cluster.create_topic_and_wait_propogation(topic_prefix, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The method name contains a typo: 'propogation' should be 'propagation'.
| topic=kafka_cluster.create_topic_and_wait_propogation(topic_prefix, | |
| topic=kafka_cluster.create_topic_and_wait_propagation(topic_prefix, |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Analysis Details4 Issues
Coverage and Duplications
Project ID: confluent-kafka-python |
No description provided.