ci: Added build-image step to deploy-staging workflow#52
ci: Added build-image step to deploy-staging workflow#52Arshadul-Monir wants to merge 5 commits into
Conversation
Graphite Automations"Request reviewers once CI passes" took an action on this PR • (07/10/26)1 reviewer was added to this PR based on Henry Chen's automation. |
Arshadul-Monir
force-pushed
the
build-deploy-image
branch
from
c1ceff6 to
0f6ae2d
Compare
Arshadul-Monir
force-pushed
the
build-deploy-image
branch
from
97eb645 to
3c34077
Compare
Arshadul-Monir
force-pushed
the
build-deploy-image
branch
from
40d7163 to
3c34077
Compare
Arshadul-Monir
force-pushed
the
build-deploy-image
branch
from
24737c7 to
a910482
Compare
Arshadul-Monir
force-pushed
the
build-deploy-image
branch
from
a910482 to
bc211d1
Compare
ci: removed backend and database instance from ci
Arshadul-Monir
force-pushed
the
build-deploy-image
branch
3 times, most recently
from
f008b7a to
62275ca
Compare
Arshadul-Monir
force-pushed
the
build-deploy-image
branch
from
62275ca to
6e4764a
Compare
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Arshadul-Monir
force-pushed
the
build-deploy-image
branch
5 times, most recently
from
331f01e to
29e230f
Compare
Arshadul-Monir
force-pushed
the
build-deploy-image
branch
from
29e230f to
306e325
Compare
Arshadul-Monir
force-pushed
the
build-deploy-image
branch
from
306e325 to
1717f52
Compare
Arshadul-Monir
force-pushed
the
build-deploy-image
branch
from
1717f52 to
f02613f
Compare
|
| DOCKER_UPLOAD: | ||
| description: "Boolean indicating whether the image should be uploaded to Docker registry or not." | ||
| required: false | ||
| default: "true" |
There was a problem hiding this comment.
is there a reason we'd want this to be false?
|
|
||
| if (environment === "production") { | ||
| await ghClient.updateK8sTagWithPR({ | ||
| manifestRepo: ["Patina-Network", "k8s-manifest"], |
There was a problem hiding this comment.
if it's the repo name, isn't it k8s-manifests (as in plural) unless i'm missing some other context
| kustomizationFilePath: "base/staging/patchats/kustomization.yaml", | ||
| imageName: "Patina-Network/patchats", | ||
| newTag: newTagVersion, | ||
| environment: "staging", |
There was a problem hiding this comment.
if the only two environments are prod and staging, couldn't we just have one updateK8STagWithPR block? and sub in the environment into environment and kustomizationFilePath?
| } | ||
|
|
||
| console.log(`Waiting for backend... (${i}/${MAX_ATTEMPTS})`); | ||
| await Bun.sleep(RETRY_DELAY_MS); |
There was a problem hiding this comment.
this is definitely not necessary for this case (it's probably too complicated/overengineered), but just for your own knowledge, most production retry systems use exponential backoff on subsequent attempts. Here's an example: https://bpaulino.com/entries/retrying-api-calls-with-exponential-backoff/
This is so we dont hammer the backend at regular intervals. Useful for if you ever implement this sort of thing at a larger scale :)
| })(); | ||
|
|
||
| const DATABASE_HOST = (() => { | ||
| const v = process.env["PG_HOST"]; |
There was a problem hiding this comment.
could we make these blocks into a helper function? the logic is repeated 4 times
| throw new Error("Missing DOCKER_HUB_PAT from platform-infra patchats.yaml"); | ||
| } | ||
|
|
||
| // copy old tz format from build-image.sh |
There was a problem hiding this comment.
Does this file exist in this repo?
There was a problem hiding this comment.
Keeping .github/scripts/src/utils/run-local-db.ts and .github/scripts/src/utils/run-backend-instance.ts around is probably good.
run-local-db.ts tests that the migrations work from fresh, before we migrate staging or prod. We probably should only run this if there are .sql files in the changes tho, since it's kinda expensive/long.
run-backend-instance.ts is good to have around as a check that the server actually starts up, since we don't have integration tests set up yet.
Potentially can run them as separate actions, and think about which ones we can parallelize. We can add them back in, in a follow up PR instead of this one though.
| with: | ||
| ENVIRONMENT: staging | ||
|
|
||
| buildImage: |
There was a problem hiding this comment.
As Ryan said, this should probably be before migrate.
3 participants
No description provided.