-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Batch rate limit #675
Batch rate limit #675
Conversation
2ec1be0
to
debbf70
Compare
temporalcli/commandsmd/commands.md
Outdated
@@ -2530,6 +2530,7 @@ Includes options set for [payload input](#options-set-for-payload-input). | |||
* `--yes`, `-y` (bool) - | |||
Don't prompt to confirm signaling. | |||
Only allowed when --query is present. | |||
* `--rps` (float) - Limit batch's requests per second. Only allowed if query is present. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will need to be rebased again since Andrew just merged an update that converted this file to YAML. :/ Hopefully will be straightforward, but let me know if it gets gnarly and/or you'd like me to do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@josh-berry okay, rebased! Should be good now. 🤞
debbf70
to
345a5d4
Compare
345a5d4
to
2ebed83
Compare
temporalcli/commands.workflow.go
Outdated
@@ -442,11 +444,17 @@ func (s *SingleWorkflowOrBatchOptions) workflowExecOrBatch( | |||
reason = defaultReason() | |||
} | |||
|
|||
// Check rps is used together with query | |||
if s.Rps != 0 && s.Query == "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This condition will never be true down here (Query
is always non-empty down here, it's checked above)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good callout 👍 fixed!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Thanks for forward-porting and sorry I missed this before!
What was changed
Rebased #551 that was never actually merged.