feat: add provisioner daemon name to provisioner jobs responses #17877
Conversation
8bb4c7f to
5fc39a0
Compare
There was a problem hiding this comment.
I think we're also missing some changes to cliui.ProvisionerJob and friends. If I'm understanding correctly, the provisioner name will now be available in these responses and therefore can be shown in the CLI after the "Queued" message.
EDIT: if we're just keeping the scope to showing the name in the provisioner jobs page, you can disregard what I said above.
site/src/pages/OrganizationSettingsPage/OrganizationProvisionerJobsPage/JobRow.tsx
Outdated
Show resolved
Hide resolved
…dStatusWithQueuePositionAndProvisioner query
…-provisioner-daemon-name
|
Related to the design, and the UI, we want to remove the ID and just use the name as @ssncferreira posted on the description screenshot. No need to keep using the ID. If at some point, we have customers asking for the ID, we can think on adding it. |
Why do we want to remove the ID? Keeping it on the page allows you to Ctrl+F quickly if you already know an ID. |
|
@johnstcn what is the use case where the user will know the ID and not the name? Eg. We don't show the workspace name because it is more likely to the user know the name than the ID. We had this conversation with Steven and Kayla, and the thing was, users were debugging using the name and not IDs. |
My thought was that an admin would have an ID copied from a log or error message. If your research shows that users debug using name and not ID, then you have better data than I do! |
| @@ -120,7 +120,7 @@ export const JobRow: FC<JobRowProps> = ({ job, defaultIsOpen = false }) => { | |||
| <> | |||
| <dt>Completed by provisioner:</dt> | |||
| <dd className="flex items-center gap-2"> | |||
| <span>{job.worker_id}</span> | |||
| <span>{job.worker_name}</span> | |||
There was a problem hiding this comment.
Worker name could be empty here if the job is old enough and the provisioner daemon has been cleaned from the DB. We could have some kind of fallback like [removed].
There was a problem hiding this comment.
Good point! I’ll update it to include a fallback. That said, this raises a question, we still show the “View provisioner” button, which links to the provisioner daemon’s page. If the daemon has been cleaned from the DB, we should avoid showing the button, correct?
There was a problem hiding this comment.
Oh yeah, good catch. Definitely hide/disable the button as well 👍🏻
There was a problem hiding this comment.
Handled in commit: caed396
@BrunoQuaresma let me know if this change is ok 🙂
…-provisioner-daemon-name
…er' button when daemon no longer exists
…-provisioner-daemon-name
Description
This PR adds the
worker_namefield to the provisioner jobs endpoint.To achieve this, the following SQL query was updated:
GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisionerAs a result, the
codersdk.ProvisionerJobtype, which represents the provisioner job API response, was modified to include the new field.Notes:
GetProvisionerJobsByIDsWithQueuePositionquery was not changed due to load concerns. This means that for template and template version endpoints,worker_idwill still be returned, butworker_namewill not.worker_id, theworker_nameis only present once a job is assigned to a provisioner daemon. For jobs in a pending state (not yet assigned), neitherworker_idnorworker_namewill be returned.Affected Endpoints
/organizations/{organization}/provisionerjobs/organizations/{organization}/provisionerjobs/{job}Testing
worker_idandworker_nameare returned once a provisioner job reaches the succeeded state.Front-end Changes
Admin provisioner jobs dashboard:
Fixes: #16982