-
Notifications
You must be signed in to change notification settings - Fork 60
Implement ListWorkers and DescribeWorker #867
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
| Elapsed string | ||
| } | ||
|
|
||
| type workerDeploymentVersionRef struct { |
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 see the JSON tags were removed for the above struct but remain for all of these others. Are they still needed?
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.
oops! Didn't realize we were no longer json printing any of these structs. Removed
cretz
left a comment
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 LGTM, but wanting to let the CLI team review
| short: q | ||
| type: string | ||
| description: Content for an SQL-like `QUERY` List Filter. | ||
| - name: limit |
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 there a skip parameter for this endpoint? Wondering how/if we support pagination
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.
limit and query are both optional parameters, so I don't think we need a skip param. Users can just omit if they just want the default return value
Pagination is built into the API def, but not currently supported in server (@rkannan82 correct me if I'm wrong). But we do intend to support pagination down the line
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.
🚢
What was changed
Implemented the ListWorkers and DescribeWorker gRPC requests
note: list workers doesn't display all heartbeat information. Open to feedback on if we should display more/all/less fields. See screenshots in testing section for an example.
PageSize is also not supported in server for the ListWorkers call, so that was left out. Added a
limitfield to allow users to limit how many results are displayed, similar to other CLI commands.Sample output of the two new commands

Why?
New feature.
Checklist
Closes [Feature Request] Support ListWorkers and DescribeWorker #868
How was this tested:
Added tests, because Go SDK doesn't support worker heartbeating yet, we manually send a
RecordWorkerHeartbeatRequestgRPC request to mimic the behavior of a worker.