ci: fix publish-release workflow indentation and switch to GCP_ARTIFA… #760
Conversation
davidagustin
requested review from
Anchellon,
jjfreund,
lgarofalo and
richardxia
|
So while building the container, the step: |
…server.sh to fix buildx error 100
…icates, add signed-by for Yarn and Postgres
davidagustin
force-pushed
the
migrate-to-gcp
branch
from
00c3159 to
2c9bd8e
Compare
…to fix build error
…s, add HTTPS support and error handling
…e adding PostgreSQL repo
…at build is working
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| touch /var/lib/dpkg/status && \ | ||
| curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | apt-key add - && \ | ||
| wget -q -O - https://dl.google.com/linux/linux_signing_key.pub | apt-key add - && \ | ||
| mv /home/app/webapp/config/appserver.sh /etc/service/appserver/run && \ | ||
| chmod 777 /etc/service/appserver/run && \ | ||
| echo 'deb http://apt.postgresql.org/pub/repos/apt/ focal-pgdg main' > /etc/apt/sources.list.d/pgdg.list && \ | ||
| curl --silent https://www.postgresql.org/media/keys/ACCC4CF8.asc | apt-key add - && \ | ||
| apt-get update && \ |
There was a problem hiding this comment.
It's kind of hard for me to review any of the changes in this file, since there are a lot of changes, and it's not exactly clear what issues you were working around. I'm assuming that you were hitting some issues with the Docker image build, where some of the packages were out of date, since that's a common issue we hit every now and then. I'd like to ask you to do the following things:
- Could you split this out into its own PR? That way, if we need to revert the GCP changes, we don't have to also revert the Dockerfile changes, and vice versa
- Could you try making the minimum number of changes necessary to this file? It's a bit of a code smell to me that your changes add 5x the number of lines that there were previously, and even if many of those lines are comments, it still feels like there's probably more happening here than necessary.
There was a problem hiding this comment.
Since #763 has merged with an alternative fix, I think you can revert all the changes you made to the Dockerfile now. Once you do that, then I think this PR can probably merge. Thanks again for working on it!
davidagustin
force-pushed
the
migrate-to-gcp
branch
from
5918a37 to
8e24a14
Compare
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
.github/workflows/ci.yml
Outdated
| on: | ||
| push: | ||
| branches: [ master ] | ||
| branches: [ master, migrate-to-gcp ] |
There was a problem hiding this comment.
Oh, and this is a reminder to go revert this change and all other changes for allowing you to test the actual deploy flow on this branch.
2 participants
…CT_REGISTRY_KEY
Note
Publishes images to both Docker Hub and Google Artifact Registry in CI and release workflows.
GCP_REPOand configuresgoogle-github-actions/auth+docker/login-actionforus-west2-docker.pkg.devci.ymlpublish-latestto tag/push${DOCKER_REPO}:latestand${GCP_REPO}:latestpublish-release.ymlto generate tags for both${GCP_REPO}and${DOCKER_REPO}and push to both registries.dockerignorerule to excludegha-creds-*.jsonWritten by Cursor Bugbot for commit fad8666. This will update automatically on new commits. Configure here.