-
Notifications
You must be signed in to change notification settings - Fork 75
Fairness #617
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
Conversation
temporal/api/common/v1/message.proto
Outdated
// amount of time (minutes). It may change, but it may take some time for | ||
// the change to be reflected. | ||
// | ||
// The recommended range of usable weights is [0.001, 1000]. |
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.
Why this recommendation? Might be good to elaborate.
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.
Let me see ... so the code won't allow anything smaller than 0.001 or larger than 1000.
So they are allowed to pass in something smaller or larger; it just won't have an effect. I don't know if we have plans to change that range in the future and don't want to hard-code that as a hard-requirement.
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.
I'd be fine with saying values are currently clamped to that range (more than fine - it seems key that that will happen vs. just "recommended")
Co-authored-by: David Reiss <david@temporal.io>
Co-authored-by: David Reiss <david@temporal.io>
READ BEFORE MERGING: All PRs require approval by both Server AND SDK teams before merging! This is why the number of required approvals is "2" and not "1"--two reviewers from the same team is NOT sufficient. If your PR is not approved by someone in BOTH teams, it may be summarily reverted.
What changed?
Already discussed and approved previously here.
Breaking changes
N/A
Server PR
https://github.com/temporalio/temporal/tree/feature/fairness