feat: ✨ Additional features to Vercel transition#751
Open
Conversation
|
Thank you for submitting this pull request! We appreciate your contribution to the project. Before we can merge it, we need to review the changes you've made to ensure they align with our code standards and meet the requirements of the project. We'll get back to you as soon as we can with feedback. Thanks again! |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Reviewer's GuideThis PR enhances the preview and production deployment workflows, introduces automated cleanup of stale preview environments, and switches the Next.js app to a standalone Docker deployment model with tighter integration to GitHub Deployments and richer PR commenting behavior. Sequence diagram for preview deployment workflow and PR commentssequenceDiagram
actor Developer
participant GitHub
participant Workflow_deploy_preview
participant Kamal
participant Preview_Server
participant Registry
participant GitHub_Deployments
participant PR_Comments
Developer->>GitHub: Push to branch / open PR
GitHub-->>Workflow_deploy_preview: Trigger deploy-preview-app workflow
activate Workflow_deploy_preview
Workflow_deploy_preview->>PR_Comments: Post "Preview deployment in progress" (or update existing)
Workflow_deploy_preview->>Kamal: kamal deploy -d preview
Kamal->>Preview_Server: Build and deploy preview app
Kamal->>Registry: Push preview image
Preview_Server-->>Workflow_deploy_preview: Deployment result
alt Deployment succeeded
Workflow_deploy_preview->>GitHub_Deployments: createDeployment(environment = KAMAL_APP_NAME)
GitHub_Deployments-->>Workflow_deploy_preview: deployment id
Workflow_deploy_preview->>GitHub_Deployments: createDeploymentStatus(state = success, environment_url = preview URL)
Workflow_deploy_preview->>GitHub_Deployments: Mark older deployments inactive
Workflow_deploy_preview->>PR_Comments: Update comment to "Preview deployment ready" with URL and timestamp
else Deployment failed
Workflow_deploy_preview->>GitHub_Deployments: createDeployment(environment = KAMAL_APP_NAME)
GitHub_Deployments-->>Workflow_deploy_preview: deployment id
Workflow_deploy_preview->>GitHub_Deployments: createDeploymentStatus(state = failure)
Workflow_deploy_preview->>PR_Comments: Update comment to "Preview deployment failed" with link to workflow run
end
deactivate Workflow_deploy_preview
Developer->>GitHub: Reopen PR / check status
GitHub-->>Workflow_deploy_preview: Trigger notify-pr-on-open job
activate Workflow_deploy_preview
Workflow_deploy_preview->>GitHub_Deployments: listDeployments(environment = KAMAL_APP_NAME)
alt Deployment record found
Workflow_deploy_preview->>GitHub_Deployments: listDeploymentStatuses(deployment_id)
GitHub_Deployments-->>Workflow_deploy_preview: latest state
Workflow_deploy_preview->>PR_Comments: Upsert comment to match success / in_progress / pending state
else No deployment record yet
Workflow_deploy_preview->>GitHub: listWorkflowRunsForRepo(branch)
GitHub-->>Workflow_deploy_preview: running workflows
Workflow_deploy_preview->>PR_Comments: Upsert comment reflecting in_progress or waiting for next push
end
deactivate Workflow_deploy_preview
Sequence diagram for preview teardown and stale cleanupsequenceDiagram
actor Developer
participant GitHub
participant Workflow_close_PR as Workflow_close_PR_remove_preview
participant Workflow_cleanup as Workflow_cleanup_stale_preview_envs
participant Kamal
participant Registry
participant GitHub_Deployments
participant GitHub_Environments
participant PR_Comments
participant Scheduler
Developer->>GitHub: Close PR / delete branch
GitHub-->>Workflow_close_PR: Trigger remove-preview job
activate Workflow_close_PR
Workflow_close_PR->>Kamal: kamal remove -y -d preview
Kamal-->>Workflow_close_PR: Preview removed from server
Workflow_close_PR->>Registry: Delete preview images for KAMAL_APP_NAME
Workflow_close_PR->>GitHub_Deployments: listDeployments(environment = KAMAL_APP_NAME)
loop For each deployment
Workflow_close_PR->>GitHub_Deployments: createDeploymentStatus(state = inactive, description = Preview environment removed)
Workflow_close_PR->>GitHub_Deployments: deleteDeployment
end
Workflow_close_PR->>GitHub_Environments: deleteAnEnvironment(environment_name = KAMAL_APP_NAME)
Workflow_close_PR->>PR_Comments: Upsert comment to "Preview deployment removed"
deactivate Workflow_close_PR
Scheduler->>GitHub: Cron schedule (weekly)
GitHub-->>Workflow_cleanup: Trigger cleanup-preview-envs workflow
activate Workflow_cleanup
Workflow_cleanup->>GitHub: listBranches
Workflow_cleanup->>GitHub_Environments: getAllEnvironments
Workflow_cleanup->>GitHub: getCommit for each branch
Workflow_cleanup-->>Workflow_cleanup: Determine stale_envs based on missing branch or age > 90 days
loop For each stale_env
Workflow_cleanup->>Kamal: kamal remove -y -d preview (KAMAL_APP_NAME = env_name)
Workflow_cleanup->>Registry: Delete registry images for env_name
Workflow_cleanup->>GitHub_Deployments: listDeployments(environment = env_name)
loop For each deployment of env_name
Workflow_cleanup->>GitHub_Deployments: createDeploymentStatus(state = inactive)
Workflow_cleanup->>GitHub_Deployments: deleteDeployment
end
Workflow_cleanup->>GitHub_Environments: deleteAnEnvironment(environment_name = env_name)
end
deactivate Workflow_cleanup
Sequence diagram for production deployment with GitHub DeploymentssequenceDiagram
actor Developer
participant GitHub
participant Workflow_deploy_ui
participant Kamal
participant Production_Server
participant GitHub_Deployments
Developer->>GitHub: Push to production branch (e.g. main)
GitHub-->>Workflow_deploy_ui: Trigger deploy-app workflow
activate Workflow_deploy_ui
Workflow_deploy_ui->>GitHub_Deployments: createDeployment(environment = production, ref = context.ref)
GitHub_Deployments-->>Workflow_deploy_ui: deployment id
Workflow_deploy_ui->>GitHub_Deployments: createDeploymentStatus(state = in_progress)
Workflow_deploy_ui->>Kamal: kamal setup (full production deploy)
Kamal->>Production_Server: Build and deploy production app
Production_Server-->>Workflow_deploy_ui: Deployment result
alt Deployment succeeded
Workflow_deploy_ui->>GitHub_Deployments: createDeploymentStatus(state = success, environment_url = production URL)
else Deployment failed
Workflow_deploy_ui->>GitHub_Deployments: createDeploymentStatus(state = failure)
end
deactivate Workflow_deploy_ui
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
Preview deployment readyPreview URL: https://website-next-preview.fairdataihub.org
|
|
Thanks for making updates to your pull request. Our team will take a look and provide feedback as soon as possible. Please wait for any GitHub Actions to complete before editing your pull request. If you have any additional questions or concerns, feel free to let us know. Thank you for your contributions! |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- There’s a fair amount of duplicated GitHub Script logic for finding the matching PR and creating/updating the
## Preview deployment...comment; consider extracting this into a composite action or a shared script to reduce drift between the different workflows. - The cleanup workflow hardcodes the
ENV_PREFIX(fairdataihub-website-) and the preview domain pattern (website-${branch_slug}.fairdataihub.org), which are separate from howKAMAL_APP_NAME/KAMAL_APP_DOMAINare derived elsewhere; aligning these through a single shared naming convention or constants will make future changes less error‑prone.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There’s a fair amount of duplicated GitHub Script logic for finding the matching PR and creating/updating the `## Preview deployment...` comment; consider extracting this into a composite action or a shared script to reduce drift between the different workflows.
- The cleanup workflow hardcodes the `ENV_PREFIX` (`fairdataihub-website-`) and the preview domain pattern (`website-${branch_slug}.fairdataihub.org`), which are separate from how `KAMAL_APP_NAME`/`KAMAL_APP_DOMAIN` are derived elsewhere; aligning these through a single shared naming convention or constants will make future changes less error‑prone.
## Individual Comments
### Comment 1
<location path=".github/workflows/cleanup-preview-envs.yml" line_range="51-40" />
<code_context>
+ }
+
+ // Get all GitHub environments
+ const { data: envData } = await github.rest.repos.getAllEnvironments({
+ owner: context.repo.owner,
+ repo: context.repo.repo,
+ });
+
</code_context>
<issue_to_address>
**suggestion:** Environment list is not paginated, which can miss stale environments in larger repos
`github.rest.repos.getAllEnvironments` only returns the first page of results. In repos with many environments, some preview environments may never be considered for cleanup. Use `github.paginate` here (as with branches) so all environments are included when determining staleness.
Suggested implementation:
```
// Get all GitHub environments
const environments = await github.paginate(
github.rest.repos.getAllEnvironments,
{
owner: context.repo.owner,
repo: context.repo.repo,
},
(response) => response.data.environments
);
on:
```
Because the original change used:
```js
const { data: envData } = await github.rest.repos.getAllEnvironments({ ... });
```
you should also:
1. Remove that old `const { data: envData } = ...` line (if it still exists elsewhere in the script).
2. Update any subsequent references:
* If the code currently uses `envData.environments`, change those references to just `environments`.
* If the code currently uses `envData` directly as the list, change those references to `environments` as well.
3. Ensure this matches how branches are paginated in the same script (likely also via `github.paginate`).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| // List all branches | ||
| const branches = await github.paginate(github.rest.repos.listBranches, { | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, |
Contributor
There was a problem hiding this comment.
suggestion: Environment list is not paginated, which can miss stale environments in larger repos
github.rest.repos.getAllEnvironments only returns the first page of results. In repos with many environments, some preview environments may never be considered for cleanup. Use github.paginate here (as with branches) so all environments are included when determining staleness.
Suggested implementation:
// Get all GitHub environments
const environments = await github.paginate(
github.rest.repos.getAllEnvironments,
{
owner: context.repo.owner,
repo: context.repo.repo,
},
(response) => response.data.environments
);
on:
Because the original change used:
const { data: envData } = await github.rest.repos.getAllEnvironments({ ... });you should also:
- Remove that old
const { data: envData } = ...line (if it still exists elsewhere in the script). - Update any subsequent references:
- If the code currently uses
envData.environments, change those references to justenvironments. - If the code currently uses
envDatadirectly as the list, change those references toenvironmentsas well.
- If the code currently uses
- Ensure this matches how branches are paginated in the same script (likely also via
github.paginate).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The GH_TOKEN or GH_PAT will need a scope of repo or Environments: Read and write to allow the removal of deployment environments. This will help prevent the deployments page being cluttered with so many preview deployments. It should remove environments that are no longer needed/stale.
Summary by Sourcery
Enhance preview and production deployment workflows with richer GitHub deployment tracking, automated cleanup, and improved Next.js standalone Docker packaging.
New Features:
Enhancements:
Build: