-
-
Notifications
You must be signed in to change notification settings - Fork 119
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
Improve nightly build and deployment logic #3164
Conversation
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.
Stoked for this to get in, I think probably making a limited bot token makes the most sense for auth!
74e3ecf
to
b033044
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.
I've got something here in all the places I think we'd need to update to get the build and release tagging working:
vYYYY.MM.DD
tags are created manually.nightly-YYYY-MM-DD
tags are created by the workflow, before the build is kicked off.- The
nightly
branch (which is currently a tag) has to be updated from within the Docker container, but we don't currently have a git repo inside the Docker container. - The
stable
branch (also currently a tag) will be updated by therelease
workflow.
Questions in review comments.
- name: Tag nightly build | ||
if: ${{ (github.event_name == 'schedule') }} | ||
run: | | ||
git config user.email "[email protected]" | ||
git config user.name "PudlBot" | ||
git tag -a -m "$NIGHTLY_TAG" $NIGHTLY_TAG $GITHUB_REF | ||
git push origin $NIGHTLY_TAG | ||
|
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 think this is doing what we want it to now. It's a tag, it's applied to the branch that we're going to build (not main
which is checked out here).
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.
Sweet! Just to get this straight: we are tagging whatever this build gets kicked off on. Since this only triggers on schedule
, and GITHUB_REF
gets munged to dev
in that case, we will always be tagging whatever the latest dev
is with nightly-build-202x-xx-xx
.
And then, when the nightly build succeeds, we can tell the nightly
branch to point at nightly-build-202x-xx-xx
and everyone who downloads the most recent nightly build off AWS will know that that corresponds to the nightly
branch. But if they want to download nightly-build-202x-xx-xx
from AWS they will also be able to associate that with the nightly-build-202x-xx-xx
tag.
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 think that's close, except that we only distribute the most recent nightly build and tagged releases to AWS. I imagine we would also have a copy of the most recent release in a stable/
directory. So if you want to refer to nightly-202x-xx-xx
you would need to look at the gs://builds.catalyst.coop/nightly-202x-xx-xx
path where the last 30 days of nightly build outputs are stashed, regardless of whether they were successful or not. So if you want to look at nightly build outputs + the code that generated them, you can look at the GCS output, and the tagged commit that matches its name.
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.
Ah, right - that makes sense! Are there any barriers to distributing a few recent nightly builds on AWS as well as GCS?
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.
We have a limited storage quota on AWS, and I don't think we actually want the public looking at our nightly build outputs, which may or or may not correspond to successful runs.
a946e01
to
7319bab
Compare
…ease notes config.
7319bab
to
b457942
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.
@jdangerx Tried to implement the merge-tag-into-branch thing here and cleaned up the Dockerfile a bit. My attempt to test the git authentication from within the container failed though.
I went ahead and created |
- Remove ad-hoc test of nightly build tagging - Use "$DOUBPLE_QUOTES" around envvars to prevent globbing/split words - Use $BUILD_ID consistently when referring to nightly build outputs. - Update name of epacems directory that we remove before deployment.
@jdangerx Cleaned up the build script to remove ad-hoc tests. Integrated some of the bugfixes from #3171 here and extended them into the build script -- we construct a single |
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.
Just one blocking question about the datasette build trigger. I think we can knock out like half of #3140 in one PR though once this gets merged!
Overview
stable
branch to point at newly tagged releasenightly
branch to point at newly successful nightly build tag.Issues / Questions:
Part of #3140