Skip to content
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

feat: preview label deployments #1413

Merged
merged 2 commits into from
Jan 2, 2025
Merged

Conversation

codemonkey800
Copy link
Contributor

Adds the ability to create preview deployments from an existing PR by adding a preview label to the PR. This works by updating the dev-deploy.yml so that:

  1. Adding the preview label creates a new preview deployment and posts a comment on the PR with the URL and last updated date
  2. Removing the preview label takes down preview deployment and deletes the PR comment
  3. Merging or closing the PR will take down the preview deployment and delete the PR comment

Changes to existing workflow

  • Pushes to dev-* should still work as expected
  • Workflow will no longer throw an error if the title is > 25 characters, instead it'll trim the rest of the characters

Demo

PR comment

image

Create preview on label

https://github.com/chanzuckerberg/cryoet-data-portal/actions/runs/12418548047

Delete preview on label

https://github.com/chanzuckerberg/cryoet-data-portal/actions/runs/12405421842

Delete preview on close

https://github.com/chanzuckerberg/cryoet-data-portal/actions/runs/12421062766

@bchu1
Copy link
Contributor

bchu1 commented Dec 19, 2024

Yes! Thank you!

const event = ${{ toJson(github.event) }}
const eventName = '${{ github.event_name }}'

let name = ''
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this necessary? does using the branch name not work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let me check, I think it's because the ref is different depending on the event but maybe we can use event.ref for both 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually yeah this is required, we can only get the ref name for the PR from the pull request event object. using something like github.ref will only return the PR number:

https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/accessing-contextual-information-about-workflow-runs#github-context

image

I did a test run here and you can see it doesn't include the ref name in the last value. instead, it shows up as 1428/merge:

https://github.com/chanzuckerberg/cryoet-data-portal/actions/runs/12481761093/job/34834780913?pr=1428

github.event_name == 'pull_request' && (
github.event.action == 'labeled' || (
github.event.action == 'synchronize' &&
contains(github.event.pull_request.labels.*.name, 'preview')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this the only thing you need to check? this whole condition only has to be a subset of the condition at the job level right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the main thing is we only want this to run when:

  1. it's a push to dev-*
  2. the PR is labeled
  3. the PR is updated

we actually do check if preview exists before executing the job here:

https://github.com/chanzuckerberg/cryoet-data-portal/pull/1413/files/71b1d537f6ebcb5b719ba03e31df4580116f8d82#diff-e7d952112309f847816123cb3297f8db4d225d21505ac9a474488114ce6bb83dR28

so maybe we can remove this since it's already checked. if the event is synchronize, then it should have the preview label since the above condition passed 🤣

let me check and see if it works as expected without this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just checked, yeah this can be removed. thank you for catching this!! 🔥


https://${{ steps.deploy-data.outputs.result }}.cryoet.dev.si.czi.technology

Updated: ${{ steps.get-comment-date.outputs.result }} PST
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fancy!

@codemonkey800 codemonkey800 merged commit 79badba into main Jan 2, 2025
7 checks passed
@codemonkey800 codemonkey800 deleted the jeremy/label-preview-deployment branch January 2, 2025 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants