Skip to content

Add additional fields to pages deploy detailed artifact#7252

Merged
jahands merged 1 commit intocloudflare:mainfrom
Maximo-Guk:maximo/add-additional-fields-to-pages-deploy-detailed-artifact
Nov 19, 2024
Merged

Add additional fields to pages deploy detailed artifact#7252
jahands merged 1 commit intocloudflare:mainfrom
Maximo-Guk:maximo/add-additional-fields-to-pages-deploy-detailed-artifact

Conversation

@Maximo-Guk
Copy link
Member

@Maximo-Guk Maximo-Guk commented Nov 13, 2024 •
edited
Loading

Fixes #7262

Include the production_branch and deployment_trigger from wrangler pages deploy in Wrangler's pages deploy detailed artifact file.

  • Tests
    • TODO (before merge)
    • Tests included
    • Tests not necessary because:
  • E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
    • I don't know
    • Required
    • Not required because:
  • Public documentation
    • TODO (before merge)
    • Cloudflare docs PR(s):
    • Documentation not necessary because: Wrangler's output file is not currently documented

@changeset-bot
Copy link

changeset-bot bot commented Nov 13, 2024 •
edited
Loading

🦋 Changeset detected

Latest commit: d5c37f9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
wrangler Minor
@cloudflare/vitest-pool-workers Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Nov 13, 2024 •
edited
Loading

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11918634505/npm-package-wrangler-7252

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/7252/npm-package-wrangler-7252

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11918634505/npm-package-wrangler-7252 dev path/to/script.js
Additional artifacts: 
npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11918634505/npm-package-create-cloudflare-7252 --no-auto-update
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11918634505/npm-package-cloudflare-kv-asset-handler-7252
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11918634505/npm-package-miniflare-7252
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11918634505/npm-package-cloudflare-pages-shared-7252
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11918634505/npm-package-cloudflare-vitest-pool-workers-7252
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11918634505/npm-package-cloudflare-workers-editor-shared-7252
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11918634505/npm-package-cloudflare-workers-shared-7252
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11918634505/npm-package-cloudflare-workflows-shared-7252

Note that these links will no longer work once the GitHub Actions artifact expires.

wrangler@3.87.0 includes the following runtime dependencies:

Package Constraint Resolved
miniflare workspace:* 3.20241106.0
workerd 1.20241106.1 1.20241106.1
workerd --version 1.20241106.1 2024-11-06

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

@Maximo-Guk Maximo-Guk force-pushed the maximo/add-additional-fields-to-pages-deploy-detailed-artifact branch from 1ece459 to bc286bd Compare November 13, 2024 21:57
@Maximo-Guk Maximo-Guk mentioned this pull request Nov 13, 2024
Merged
@Maximo-Guk Maximo-Guk force-pushed the maximo/add-additional-fields-to-pages-deploy-detailed-artifact branch 2 times, most recently from 828e195 to 35ba429 Compare November 14, 2024 18:57
@Maximo-Guk Maximo-Guk changed the title [DRAFT] Add additional fields to pages deploy detailed artifact Add additional fields to pages deploy detailed artifact Nov 14, 2024
@Maximo-Guk Maximo-Guk marked this pull request as ready for review November 14, 2024 19:27
@Maximo-Guk Maximo-Guk requested review from a team as code owners November 14, 2024 19:27
@Maximo-Guk Maximo-Guk force-pushed the maximo/add-additional-fields-to-pages-deploy-detailed-artifact branch 3 times, most recently from 70fadc2 to 1cf78d5 Compare November 14, 2024 19:55
emily-shen
emily-shen requested changes Nov 15, 2024
View reviewed changes
Copy link
Contributor

@emily-shen emily-shen left a comment

Choose a reason for hiding this comment

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

oops sorry didn't realise tests were still failing 🫥

@emily-shen emily-shen added the e2e Run wrangler + vite-plugin e2e tests on a PR label Nov 15, 2024
@Maximo-Guk Maximo-Guk force-pushed the maximo/add-additional-fields-to-pages-deploy-detailed-artifact branch 2 times, most recently from a671190 to ca6a7d3 Compare November 18, 2024 12:31
emily-shen
emily-shen reviewed Nov 19, 2024
View reviewed changes
@Maximo-Guk Maximo-Guk force-pushed the maximo/add-additional-fields-to-pages-deploy-detailed-artifact branch 3 times, most recently from 1a55e30 to d5c37f9 Compare November 19, 2024 17:30
@Maximo-Guk Maximo-Guk force-pushed the maximo/add-additional-fields-to-pages-deploy-detailed-artifact branch from d5c37f9 to 36cb0ef Compare November 19, 2024 17:39
jrf0110
jrf0110 reviewed Nov 19, 2024
View reviewed changes
packages/wrangler/src/pages/deploy.ts
deployment_trigger: {
metadata: {
commit_hash:
deploymentResponse.deployment_trigger?.metadata?.commit_hash ?? "",
Copy link
Contributor

@jrf0110 jrf0110 Nov 19, 2024

Choose a reason for hiding this comment

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

Under what circumstances would deployment_trigger ever be undefined?

Copy link
Member Author

@Maximo-Guk Maximo-Guk Nov 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure, it's not a required field in our openapi schema https://developers.cloudflare.com/api/operations/pages-deployment-create-deployment

Also we handle undefined case here

Copy link
Contributor

@jahands jahands Nov 19, 2024

Choose a reason for hiding this comment

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

For Pages Direct Upload deployments, I believe the git ref is optional because you could deploy a Pages project that is not in a repo

@jahands jahands merged commit 97acf07 into cloudflare:main Nov 19, 2024
@workers-devprod workers-devprod mentioned this pull request Nov 19, 2024
Merged
@Maximo-Guk Maximo-Guk mentioned this pull request Jan 17, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jahands jahands jahands left review comments

@emily-shen emily-shen emily-shen approved these changes

+1 more reviewer

@jrf0110 jrf0110 jrf0110 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

e2e Run wrangler + vite-plugin e2e tests on a PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

🚀 Feature Request: Write production_branch, project_name and deployment_trigger to pages deploy detailed artifact file

4 participants

@Maximo-Guk @jrf0110 @jahands @emily-shen