Skip to content

fix: make review app URLs predictable #806

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

Merged
merged 1 commit into from
Mar 13, 2025
Merged

Conversation

rowanmanning
Copy link
Member

@rowanmanning rowanmanning commented Mar 7, 2025

Description

We now generate a hash based on the branch name and use this as a suffix on review app names. This means a couple of things:

  1. Review app names are predictable
  2. Only one review app will be created per branch, with future commits to that branch updating the existing review app instead of deploying another one

This is good for a couple of reasons:

We can now hand the review app URL off to future jobs. This is important for the n-test and cyprus plugins which expect a URL in the review or staging state.

We will stop creating a new review app for every commit, we're already seeing way too many review apps get created which is a problem while we don't have automatic deletion of old review apps.

Changes to a PR will deploy faster because most of the review app CloudFormation stack will have been created already - we'll be doing updates instead of fully creating a new stack on each commit.

Here's an example of a review app deployed with this change. The suffix f81bbd is example-branch hashed. I tried running the deploy a second time and the stack was updated instead of created, super quick.

See-also: CPREL-1300

Checklist:

  • My branch has been rebased onto the latest commit on main (don't merge main into your branch)
  • My commit messages are conventional commits, for example: feat(circleci): add support for nightly workflows, fix: set Heroku app name for staging apps too

@rowanmanning rowanmanning requested a review from a team March 7, 2025 12:19
Copy link
Contributor

@ivomurrell ivomurrell left a comment

Choose a reason for hiding this comment

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

looks good so far! let's see how the hako implementation goes.

@rowanmanning rowanmanning force-pushed the predictable-review-apps branch 2 times, most recently from 0fa2947 to d0af161 Compare March 10, 2025 16:59
We now generate a hash based on the branch name and use this as a suffix
on review app names. This means a couple of things:

  1. Review app names are predictable
  2. Only one review app will be created per branch, with future commits
     to that branch updating the existing review app instead of
     deploying another one

This is good for a couple of reasons:

We can now hand the review app URL off to future jobs. This is important
for the n-test and cyprus plugins which expect a URL in the `review` or
`staging` state.

We will stop creating a new review app for every commit, we're already
seeing way too many review apps get created which is a problem while we
don't have automatic deletion of old review apps.

Changes to a PR will deploy faster because most of the review app
CloudFormation stack will have been created already - we'll be doing
updates instead of fully creating a new stack on each commit.

See-also: https://financialtimes.atlassian.net/browse/CPREL-1300
@rowanmanning rowanmanning force-pushed the predictable-review-apps branch from d0af161 to 8aa5b22 Compare March 13, 2025 14:15
@rowanmanning rowanmanning marked this pull request as ready for review March 13, 2025 14:28
@rowanmanning rowanmanning requested a review from a team as a code owner March 13, 2025 14:28
@rowanmanning rowanmanning requested a review from ivomurrell March 13, 2025 14:28
Copy link
Contributor

@ivomurrell ivomurrell left a comment

Choose a reason for hiding this comment

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

great! thanks for working with cloud enablement on this. we'll test this with next-static once it's released

@rowanmanning rowanmanning merged commit 569515b into main Mar 13, 2025
17 checks passed
@rowanmanning rowanmanning deleted the predictable-review-apps branch March 13, 2025 14:36
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