Closed gvegayon closed 1 month ago
All modified and coverable lines are covered by tests :white_check_mark:
:loudspeaker: Thoughts on this report? Let us know!
I think the docker file needs to point to the tag dynamically. Will figure it out later today (about an hour). Unless you want to give it a try!
I won't have time to get to it today, but I can try to look tomorrow if needed!
Did we ever figure out if the pool was actually linked to the pushed image in ACR? If no, I'd like to keep it out of scope here but I want to know to make it an issue and tag it for Jon.
I don't think it is linked, as I don't see any line passing that information to the pool. I could create an issue out of it.
I flagged the scheduling inline, but does the scheduling still make sense here? I think no, but would like a second pair of eyes.
Just commented on that. We could comment that out for the moment and put it back as soon as we have a few runs in production.
Do we want to renumber the workflows? I think it's fine if the answer is no and we'll plan to slot the job creation in (2)
Probably, I would renumber them (a separate PR).
What are the other issues to create? ACR image deletion on PR close? Pool deletion on PR close? Polling for pool status before rebuild (and figuring out the strategy)?
From the top of my mind:
For the future, I suggest having a workflow (or a job within existing workflows) that creates a GitHub issue for the run (e.g., weekly runs) and uses that for posting information. That could be the way in which Azure batch communicates it is done or whatnot. That could be an action for https://github.com/CDCgov/cfa-actions!
Also, important: pool creation fails when there's a pool with that id/name. This could be either addressed here or somewhere else. I suggest opening an issue for doing this, as you can still stop/delete pools manually.
branch name
if it is a PR orlatest
if it is merging to main.