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

Limit Kafka Partitions KEDA operates on #3879

Merged
merged 5 commits into from
Dec 7, 2022
Merged

Limit Kafka Partitions KEDA operates on #3879

merged 5 commits into from
Dec 7, 2022

Conversation

tobiaskrause
Copy link
Contributor

@tobiaskrause tobiaskrause commented Nov 18, 2022

Signed-off-by: Tobias Krause tobias.krause@otto.de

Limit Kafka Partitions KEDA operates on. #3830

Checklist

  • Commits are signed with Developer Certificate of Origin
  • Tests have been added
  • A PR is opened to update the documentation on (repo) (if applicable)
  • Changelog has been updated and is aligned with our changelog requirements

Relates to #3830

Relates to kedacore/keda-docs#981

Signed-off-by: Tobias Krause <tobias.krause@otto.de>
tobiaskrause and others added 3 commits November 18, 2022 15:24
Signed-off-by: Tobias Krause <tobias.krause@otto.de>
Signed-off-by: Tobias Krause <tobias.krause@otto.de>
@tobiaskrause tobiaskrause marked this pull request as ready for review November 18, 2022 14:28
@tobiaskrause tobiaskrause requested a review from a team as a code owner November 18, 2022 14:28
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 but I'm not kafka expert, WDYT about this feature @zroubalik ?

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.

Looking good, @tobiaskrause do you think you can add e2e test for this feature? Then we can go ahead and merge this. Thanks

https://github.com/kedacore/keda/tree/main/tests

@tomkerkhove
Copy link
Member

We are going to release KEDA v2.9 on Thursday. Do you think you can add e2e tests by Wednesday @tobiaskrause?

@tobiaskrause
Copy link
Contributor Author

tobiaskrause commented Dec 2, 2022

We are going to release KEDA v2.9 on Thursday. Do you think you can add e2e tests by Wednesday @tobiaskrause?

Not sure, to be honest. Till now I wasn't able to find a reliable way to send a message to a specific partition with the kafka-console-producer script. I would need a custom producer on a pod. Do you have any idea to solve this @tomkerkhove?

@zroubalik
Copy link
Member

zroubalik commented Dec 5, 2022

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

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,

I think we can merge it without e2e test, as it is something not very easy testable. WDYT @tomkerkhove @JorTurFer ?

@JorTurFer
Copy link
Member

LGTM,

I think we can merge it without e2e test, as it is something not very easy testable. WDYT @tomkerkhove @JorTurFer ?

You are the kafka expert xD

@zroubalik
Copy link
Member

@JosephABC would you mind looking at this as well, since you are doing a Kafka PR in parallel?

@zroubalik
Copy link
Member

zroubalik commented Dec 7, 2022

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

@zroubalik zroubalik merged commit ccd6705 into kedacore:main Dec 7, 2022
zroubalik added a commit to zroubalik/keda that referenced this pull request Dec 8, 2022
Signed-off-by: Tobias Krause <tobias.krause@otto.de>
Co-authored-by: Zbynek Roubalik <zroubalik@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants