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.

Sign up for GitHub

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

Jump to bottom

Add logic to scale to zero on invalid offset even with earliest offsetResetPolicy #5689

Merged
merged 5 commits into from
May 15, 2024
Merged

Add logic to scale to zero on invalid offset even with earliest offsetResetPolicy #5689

merged 5 commits into from
May 15, 2024

Conversation

dttung2905
Copy link
Contributor

@dttung2905 dttung2905 commented Apr 12, 2024 •
edited
Loading

Provide a description of what has been changed

Checklist

Fixes #4910

@dttung2905 dttung2905 requested a review from a team as a code owner April 12, 2024 21:20
zroubalik
zroubalik reviewed Apr 16, 2024
View reviewed changes
Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

@dttung2905 what is the status of this? This would be a really great addition to our next release.

@zroubalik
Copy link
Member

zroubalik commented Apr 16, 2024 •
edited by github-actions bot
Loading

/run-e2e kafka
Update: You can check the progress here

@dttung2905
Copy link
Contributor Author

@zroubalik Thanks for helping me running the e2e test. This is my first time running it so I was checking with @JorTurFer last night if I could run it myself 😆 . I have found out the root cause why the e2e test failed. I will try to make changes tomorrow

What's left of this PR are just 🤞

  • Changelog
  • Fix e2e test cases

@dttung2905 dttung2905 force-pushed the scaleToZeroOnInvalidOffset-and-earliest-offsetResetPolicy branch from 5627770 to d3e5520 Compare April 17, 2024 22:12
@dttung2905
Copy link
Contributor Author

I think most of the E2E test cases have passed except 1 or 2 - testOneOnInvalidOffsetWithEarliestOffsetResetPolicy is one of them . Let me take more time to look into that

@dttung2905
Copy link
Contributor Author

dttung2905 commented Apr 21, 2024 •
edited by github-actions bot
Loading

/run-e2e kafka
Update: You can check the progress here

dttung2905 added 4 commits May 6, 2024 22:38
@dttung2905
Add logic to scale to zero on invalid offset event with earliest offs…
8f9f33a 
…etResetPolicy

Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
@dttung2905
Fix failed CI tests and changes to apache_kafka_test.go as well
54c8059 
Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
@dttung2905
Fix test cases
6c34d74 
Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
@dttung2905
Fixed failed tests
3b22a35 
Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
@dttung2905 dttung2905 force-pushed the scaleToZeroOnInvalidOffset-and-earliest-offsetResetPolicy branch from 3b979f6 to 3b22a35 Compare May 6, 2024 21:38
@dttung2905 dttung2905 changed the title WIP - Add logic to scale to zero on invalid offset even with earliest offsetResetPolicy Add logic to scale to zero on invalid offset even with earliest offsetResetPolicy May 6, 2024
@dttung2905
Update CHANGELOG
95351b8 
Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
@dttung2905
Copy link
Contributor Author

dttung2905 commented May 7, 2024 •
edited by github-actions bot
Loading

/run-e2e kafka
Update: You can check the progress here

@dttung2905 dttung2905 requested a review from zroubalik May 7, 2024 10:04
JorTurFer
JorTurFer approved these changes May 7, 2024
View reviewed changes
Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the improvement ❤️
@zroubalik PTAL 🙏

zroubalik
zroubalik approved these changes May 15, 2024
View reviewed changes
Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

LGTM, great job!

@zroubalik zroubalik merged commit a375d17 into kedacore:main May 15, 2024
21 checks passed
rxg8255 pushed a commit to rxg8255/keda that referenced this pull request May 27, 2024
@dttung2905
Add logic to scale to zero on invalid offset even with earliest offse…
40a7c17 
…tResetPolicy (kedacore#5689)

Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
Signed-off-by: Ranjith Gopal <ranjith.gopalreddy@7-11.com>
@somabodonyi somabodonyi mentioned this pull request Jun 29, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JorTurFer JorTurFer JorTurFer approved these changes

@zroubalik zroubalik zroubalik approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Kafka Scaler scaleToZeroOnInvalidOffset flag is only working for 'latest' offsetresetpolicy
3 participants
@dttung2905 @zroubalik @JorTurFer