-
Notifications
You must be signed in to change notification settings - Fork 93
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
ci: switch to using GitHub artefacts instead of ghcr for PR testing #1918
Conversation
cef81e9
to
85463bc
Compare
This change skips uploading to GCR and the self-hosted e2e tests for forks.
62a2e4b
to
9c62dfe
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.
Looks good to me, will leave final review to @olksdr though. |
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.
Thank you for doing this! I'll also leave the approval to @olksdr.
retention-days: 1 | ||
name: relay-docker-image | ||
path: relay-docker-image.tgz | ||
|
||
- name: Push to ghcr.io |
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.
If we are moving to use artifacts, can we get rid of this step to push to GHCR? There are no docker pull
s left in the file.
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 believe it is still needed for dev environments and local dev. This all build on GitHub
kind of pushed us to migrate our images build processes in #1658
We might want to restrict this now to run only on merges to master
though.
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.
ghcr is still used for self-hosted e2e testing, but that is only run on the default branch and PRs on this repo (i.e. PRs from sentry folks vs external)
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.
lgtm
Just left few comments / questions.
retention-days: 1 | ||
name: relay-docker-image | ||
path: relay-docker-image.tgz | ||
|
||
- name: Push to ghcr.io |
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 believe it is still needed for dev environments and local dev. This all build on GitHub
kind of pushed us to migrate our images build processes in #1658
We might want to restrict this now to run only on merges to master
though.
…or-slug-parameter * master: (21 commits) ci: switch to using GitHub artefacts instead of ghcr for PR testing (#1918) ref: Add new mobile vitals (#1943) release: 23.3.0 ref(profiling): Remove platform validation (#1933) ref(relay_cache): Inject upstream realy into the service (#1932) feat(csp): Add the request's origin to CSP events (#1934) chore(common): Remove unused tryf macro (#1931) ref(test): Remove actix test utilities (#1930) ref(actix): Refactor shutdown without actix (#1912) ref(outcomes): Do not use registry in outcome producer (#1927) ref: Inject services in project cache and project upstream (#1926) feat(config): Add links to docs in YAML config file (#1923) feat(mobile): Move device.class from contexts to tags (#1911) fix(ci): Use correct Rust toolchain in Beta CI (#1925) ci(gha): Remove unmaintained actions-rs actions (#1922) feat(normalization): Optionally mark scrubbed transactions as sanitized (#1917) chore: Apply import grouping from coding style (#1921) feat(general): Fix leaked ip address (#1892) fix(normalize): No original_value for untouched transactions (#1916) chore(build): Update rust toolchain to 1.68.0 (#1914) ...
This skips gcr uploads and e2e tests for forks.
Tested this on a forked PR here: #1942
#skip-changelog