-
Notifications
You must be signed in to change notification settings - Fork 151
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
feature: add queued status type and build step field #3351
Conversation
@CGoodwin90 just want to get you to 👀 the migration part to make sure it looks ok |
Looks good to me! |
I buggered something up, also turns out you can't do enum the way I thought, so basically now just dropping the column and recreating it seems to do the trick. Also, I'm not 100% sure about the rollback, do I need to do the column check on rollback? |
75c43ca
to
c000564
Compare
Ah I see, turns out Knex can't alter enums after creation. An alternate approach would be to use Knex.raw - https://knexjs.org/guide/raw.html. Don't necessarily need a column check for the rollback, but I've been adding them just incase somethng goes awry. |
services/api/database/migrations/20221129221152_queuedstatus.js
Outdated
Show resolved
Hide resolved
services/api/database/migrations/20221129221152_queuedstatus.js
Outdated
Show resolved
Hide resolved
Would be good to get this in soon so that clusters with QoS enabled get some visibility of when builds are queued. |
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've reviewed the migration, and now with a safe rollback, I'm happy. I would like @rocketeerbkw to re-review the code logic
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.
Everything api/api-db wise LGTM.
One question about the actions-handler dependencies, but feel free to ignore if it doesn't make sense.
Checklist
To make it clear when a build or task is queued, add a queued status. This will make it so that when a build or task has been received by the remote-controller and marked as in a queue for processing, Lagoon will be able to display it as
queued
rather thannew
orpending
This also updates the enums to be the same for tasks and builds now that the remote-controller has supported using the same status types for both builds and tasks for some time.
Also adds support for
buildStep
to the deployment, currently Lagoon sends this data in the payload back to core, but it is not consumed by the API. This means we will be able to expose the current build step in the UI, as this build step contains useful information about which step the build is currently progressing through, or other information that could be useful at a quick glance in an overview page.closes #3352