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

Support multiple queues at the IBMMQ scaler #6182

Merged
merged 8 commits into from
Nov 5, 2024

Conversation

rickbrouwer
Copy link
Contributor

@rickbrouwer rickbrouwer commented Sep 23, 2024

Changed the queueName parameter so that it accepts multiple queues (comma (,) separated). The code is is changed so it's request the current depth per queue and only use the current depth from the queue with the most messages. Because it's accepts multiple queues, queueNames will be also supported.

Also added extra check for authentication error. When an incorrect password is given the error message is error inspecting IBM MQ queue depth: failed to parse response from REST call. The reason is a bit unclear. That's why I added a better error message error inspecting IBM MQ queue depth: authentication failed: incorrect username or password.

Checklist

Fixes #6181
Docs: kedacore/keda-docs#1473

@rickbrouwer rickbrouwer marked this pull request as ready for review September 23, 2024 15:22
@rickbrouwer rickbrouwer force-pushed the issue-6181 branch 5 times, most recently from a31b5ed to dc66dcc Compare October 2, 2024 12:26
Signed-off-by: Rick Brouwer <[email protected]>
@JorTurFer
Copy link
Member

JorTurFer commented Oct 6, 2024

I see the use case of this feature, but IMHO we should expose the aggregation algorithm if we allow multiple queries in the same trigger. If you check the RabbitMQ trigger, it exposes the aggregation as parameter (min, max, sum).
WDTY @zroubalik @wozniakjan

@rickbrouwer
Copy link
Contributor Author

I see the use case of this feature, but IMHO we should expose the aggregation algorithm if we allow multiple queries in the same trigger. If you check the RabbitMQ trigger, it exposes the aggregation as parameter (min, max, sum). WDTY @zroubalik @wozniakjan

I totally agree with you. That is a nicer setup. I will make this adjustment and add it.

@rickbrouwer rickbrouwer force-pushed the issue-6181 branch 6 times, most recently from 22d4b75 to 77c8ac7 Compare October 8, 2024 08:53
@rickbrouwer rickbrouwer force-pushed the issue-6181 branch 6 times, most recently from d97486c to 28b5841 Compare October 8, 2024 13:24
Signed-off-by: rickbrouwer <[email protected]>
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! PTAL @zbynek @wozniakjan

@JorTurFer
Copy link
Member

JorTurFer commented Oct 16, 2024

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

@zbynek
Copy link

zbynek commented Oct 16, 2024

PTAL @zbynek

I guess you meant @zroubalik

@JorTurFer
Copy link
Member

PTAL @zbynek

I guess you meant @zroubalik

lol, sorry :( yes... I meant @zroubalik

@JorTurFer JorTurFer mentioned this pull request Nov 3, 2024
28 tasks
@zroubalik
Copy link
Member

PTAL @zbynek

I guess you meant @zroubalik

lol, sorry :( yes... I meant @zroubalik

I wish I had that GH handle :))

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, thanks!

Signed-off-by: Jorge Turrado Ferrero <[email protected]>
@zroubalik zroubalik enabled auto-merge (squash) November 5, 2024 20:38
@JorTurFer
Copy link
Member

JorTurFer commented Nov 5, 2024

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

@zroubalik zroubalik merged commit 4eb7149 into kedacore:main Nov 5, 2024
18 checks passed
@rickbrouwer rickbrouwer deleted the issue-6181 branch November 6, 2024 06:14
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.

Support multiple queues at the IBMMQ scaler
4 participants