-
Notifications
You must be signed in to change notification settings - Fork 2
deploy-queue v0.7.0 #70
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
d055fc3 to
4495659
Compare
7c356e1 to
392fe81
Compare
e23be37 to
56057da
Compare
An issue that only surfaced in tests is that concurrent writes to `GITHUB_OUTPUT` sometimes mess up outputs. Using advisory locks prevents this effectively.
As the deploy-queue runs in a bunch of different places and in some cases with git repos that are not up-to-date, we'll regularly have older versions of deploy-queue still in service. Requiring all migrations to be present in the source code means that as soon as a new migration is deployed, all older versions will break. That is unacceptable, so we'll have to allow running with missing migrations.
56057da to
44b0c6d
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.
Reviewed the workflow part — nice!
And added a question, if everything is fine — LGTM
44b0c6d to
4e51c78
Compare
|
Only new changes are the newly introduced compatibility tests and the matrix strategy for the integration tests, the rest remained unchanged: |
4e51c78 to
c6a0fa8
Compare
|
And as expected, testing against 0.5.1 failed because 0.5.1 refuses to work when there are unknown migrations applied to the database. See https://github.com/neondatabase/dev-actions/actions/runs/19466750059/job/55703743925. I've removed the test for 0.5.1, but we'll have to make sure nothing is referencing 0.5.1 anymore before we can update to >=0.6.0. We've got 0.5.2 as an intermediate release to help with that. |
Shugenya
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.
Looks good!
I guess this means we can finally get rid of the "Get URL for current job" step
GHA job URL as fallback
We have a job like this everywhere where we use the deploy queue:
For job steps, is definitely not necessary, we get the same URL like this:
Hoping that composite actions also get access to
job.check_run_id, I'm trying to fully inline this.Small follow-up fixes for #67
list cellswith slack messagelist-cellsthrough gh actionlist cellsconsistentlyAdjustments to github output handling
Added after flakyness like seen in https://github.com/neondatabase/dev-actions/actions/runs/19367137928/job/55414533340?pr=70 printing things like this:
Not knowing that this is a concurrency problem and just trying a random thing that came to mind I implemented multi-line output support. This turned it from flaky to always failing, but gave very clear output showing that this is a concurrency issue:
Now with the concurrency problem fixed, it's stable again.
Test updates:
I'm adding both because the compatibility problem between 0.5.1 and 0.6.0 with missing migrations being treated as a critical problem would not have been caught by the latter, while most more nuanced query-level problems would not be caught by the former.