Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Issues with cooperative-sticky strategy #593

Merged
merged 2 commits into from
Feb 13, 2025

Conversation

golanbz
Copy link
Contributor

@golanbz golanbz commented Sep 2, 2024

Description

How Has This Been Tested?

  • Introduced a new sample project specifically designed to evaluate and test the support for the cooperative sticky strategy, allowing for more thorough analysis.
  • Unit tests were added for the updated classes to address the new behavior.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have added tests to cover my changes
  • I have made corresponding changes to the documentation

Disclaimer

By sending us your contributions, you are agreeing that your contribution is made subject to the terms of our Contributor Ownership Statement.

@dosper7
Copy link

dosper7 commented Sep 11, 2024

Great work 🚀 with this PR. Can you also add some tests as well to validate this use case? 🙇

@BasJ93
Copy link

BasJ93 commented Nov 14, 2024

We'd love to have this feature. Any updates on the implementation of the tests?

@golanbz
Copy link
Contributor Author

golanbz commented Nov 14, 2024

We'd love to have this feature. Any updates on the implementation of the tests?

Unit tests were added 2 months ago.

@BasJ93
Copy link

BasJ93 commented Nov 15, 2024

Great. Any updates on getting the feature reviewed @dosper7?

@AlexeyRaga
Copy link

Great to see it happening! Thanks @golanbz for working on it!

Copy link
Contributor

@MiguelCosta MiguelCosta left a comment

Choose a reason for hiding this comment

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

thanks @golanbz !

@brmagadutra
Copy link
Contributor

@golanbz thanks for the contribution! Can you please rebase the PR with master.

@AlexeyRaga
Copy link

Hooray! Things are moving! I am so excited! :)

@brmagadutra brmagadutra force-pushed the fix/sticky-cooperative-strategy branch from 0816113 to 0683df5 Compare February 4, 2025 11:29
@brmagadutra
Copy link
Contributor

brmagadutra commented Feb 6, 2025

@golanbz can you please check the errors on the lint step? Also, rebase with the latest changes from master.

@golanbz golanbz force-pushed the fix/sticky-cooperative-strategy branch 2 times, most recently from e37fb6f to 548cd18 Compare February 11, 2025 13:30
- Added support for consumer strategies that do not act as "stop the world" scenarios (e.g., cooperative sticky).
- Enabled automatic committing with `confluent auto commit: true` when the consumer strategy is cooperative sticky.
- Referenced open librdkafka issue: confluentinc/librdkafka#4059.
test: add unit tests and fix bug in ClusterConfiguration

- Added 3 new test files to improve coverage:
  - ConsumerConfigurationBuilderTests.cs
  - KafkaConfigTests.cs
  - PartitionAssignmentStrategyTests.cs
- Fixed a bug in ClusterConfiguration related to AutoCommitInterval initialization

chore: update AutoCommitInterval to 100ms in Cooperative-sticky sample

- Updated AutoCommitInterval in Cooperative-sticky sample Program.cs to 100ms.

fix: resolve Codacy issues

- Fixed various code quality issues flagged by Codacy.
@golanbz golanbz force-pushed the fix/sticky-cooperative-strategy branch 2 times, most recently from 35f0650 to d970b5d Compare February 11, 2025 14:32
@brmagadutra brmagadutra merged commit 83bce82 into Farfetch:master Feb 13, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants