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

Add queueLengthStrategy option to azure-storage-queue scaler. #1406

Merged
merged 1 commit into from
Jul 30, 2024

Conversation

leodip
Copy link
Contributor

@leodip leodip commented Jun 9, 2024

This adds documentation about the new queueLengthStrategy configuration option for the Azure Storage Queue scaler.

Related to kedacore/keda#4478

@leodip leodip requested a review from a team as a code owner June 9, 2024 12:47
Copy link

github-actions bot commented Jun 9, 2024

Thank you for your contribution! 🙏 We will review your PR as soon as possible.

While you are waiting, make sure to:

  • Add your contribution to all applicable KEDA versions
  • GitHub checks are passing
  • Is the DCO check failing? Here is how you can fix DCO issues

Learn more about:

Copy link

netlify bot commented Jun 9, 2024

Deploy Preview for keda ready!

Name Link
🔨 Latest commit 316d648
🔍 Latest deploy log https://app.netlify.com/sites/keda/deploys/6669f0f6e9550400084cb5a2
😎 Deploy Preview https://deploy-preview-1406--keda.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@@ -26,6 +27,7 @@ triggers:

- `queueName` - Name of the queue.
- `queueLength` - Target value for queue length passed to the scaler. Example: if one pod can handle 10 messages, set the queue length target to 10. If the actual number of messages in the queue is 30, the scaler scales to 3 pods. (Default: `5`, Optional)
- `queueLengthStrategy` - `default` considers both visible and invisible messages, while `visibleonly` uses Peek to count only visible messages. In `visibleonly`, if the count of messages is 32 or higher, it falls back to the default strategy, counting both visible and invisible messages.
Copy link
Member

Choose a reason for hiding this comment

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

Default is not really clarifying what it includes. Instead I would propose to call it all

Copy link
Member

Choose a reason for hiding this comment

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

Also, please align with the "(Default: 5, Optional)" convention as per https://github.com/kedacore/keda-docs#writing-documentation-for-a-scaler

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback. Hope this version is better?

@tomkerkhove
Copy link
Member

Can you fix the DCO issues please? You can learn more in our contribution guide.

@leodip leodip force-pushed the 4478-add-queueLengthStrategy branch from ab99319 to 5ffdeb4 Compare June 12, 2024 19:00
Signed-off-by: Leonardo D'Ippolito <contact@leodip.com>
@leodip leodip force-pushed the 4478-add-queueLengthStrategy branch from 5ffdeb4 to 316d648 Compare June 12, 2024 19:03
@leodip
Copy link
Contributor Author

leodip commented Jun 12, 2024

DCO fixed.

Copy link
Member

@tomkerkhove tomkerkhove left a comment

Choose a reason for hiding this comment

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

LGTM, waiting for feature to merge first

@zroubalik zroubalik merged commit 16b6f65 into kedacore:main Jul 30, 2024
8 checks passed
shubhusion pushed a commit to shubhusion/keda-docs that referenced this pull request Aug 5, 2024
…re#1406)

Signed-off-by: Leonardo D'Ippolito <contact@leodip.com>
Signed-off-by: shubhusion <shubham27.sharma03@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.

None yet

3 participants