-
Notifications
You must be signed in to change notification settings - Fork 252
Add Fairness Keys/Weights #2012
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
test/worker_versioning_test.go
Outdated
@@ -832,8 +832,9 @@ func (ts *WorkerVersioningTestSuite) TestTaskQueueStats() { | |||
ts.NoError(err) | |||
ts.Equal(1, len(taskQueueInfo.VersionsInfo)) | |||
|
|||
ts.validateTaskQueueStats(expectedWorkflowStats, taskQueueInfo.VersionsInfo[""].TypesInfo[client.TaskQueueTypeWorkflow].Stats) | |||
ts.validateTaskQueueStats(expectedActivityStats, taskQueueInfo.VersionsInfo[""].TypesInfo[client.TaskQueueTypeActivity].Stats) | |||
// TODO: Fix to work with newer response format |
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.
Are you going to address this in this PR
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.
No, I think it needs a more comprehensive change and I don't want to combine them together. I'll make an item for it though and reference 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.
LGTM once the thing Quinn mentioned gets added
if result.FairnessKey != "fair-activity" { | ||
return 0, fmt.Errorf("activity did not return expected fairness key %s != %s", "fair-activity", result.FairnessKey) | ||
} | ||
if result.FairnessWeight != 4.20 { |
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.
Is the extra 0 here needed for correctness? I see the test below doesn't have an extra 0 in the decimal,
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.
No, it's pointless. I just wrote it that way lol
Warning
DRAFT until we have a CLI RC with this in it that can be used for tests
What was changed
Added fairness keys/weights to priority
Why?
New feature
Checklist
Closes
How was this tested:
Added test
Any docs updates needed?