-
Notifications
You must be signed in to change notification settings - Fork 69
Add Stats to DescribeWorkerDeploymentVersion #603
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 Stats to DescribeWorkerDeploymentVersion #603
Conversation
f2d2dde
to
412818a
Compare
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 from a syntax POV, but will defer to other SDK team members more familiar w/ versioning. I admit I don't understand VersionTaskQueueInfo
vs VersionTaskQueue
and such.
FWIW, |
Update: I just added |
94fe9e8
to
361bc62
Compare
361bc62
to
94bd8a9
Compare
Co-authored-by: Spencer Judge <sjudge@hey.com>
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?
(1) Deprecated
task_queue_infos
indeployment.WorkerDeploymentVersionInfo
.(2) Added
version_task_queues
toDescribeWorkerDeploymentVersionResponse
.Why?
We want to report task queue stats for each task queue that is part of a worker deployment version.
The challenge is that the
taskqueue
package depends on thedeployment
package. So addingTaskQueueStats
todeployment.WorkerDeploymentVersionInfo
causes a cycle import error.Weighing our options, we decided to effectively move the task queue-related data from within the deployment package into the response message.
Breaking changes
Not yet; but in subsequent releases the deprecated field
task_queue_infos
will be removed.Server PR
temporalio/temporal#7959 (draft)