feat(wrangler): Add support for placement hints #6625

maxwellpeterson · 2024-09-03T20:25:37Z

Add hint field to smart placement configuration

What this PR solves / how to test

Fixes WP-1224. API support for placement hints is currently experimental and gated to Cloudflare employees only.

Manual testing

Create a test Worker with a placement hint set in the wrangler.toml file:

[placement]
mode = "smart"
hint = "wnam"

Run wrangler deploy (using the prerelease) to deploy the test Worker with the placement hint.
Hit the script settings API to ensure that the placement hint was set correctly:

$ curl -X GET "https://api.cloudflare.com/client/v4/accounts/<account_tag>/workers/scripts/<script_name>/settings" -H "Authorization: Bearer $API_TOKEN" | jq .result.placement
{
  "mode": "smart",
  "hint": "wnam"
}

Testing error cases

Placement mode is "off" instead of "smart" with hint set:

[placement]
mode = "off"
hint = "wnam"

$ wrangler deploy

✘ [ERROR] Processing wrangler.toml configuration:

    - "placement.hint" cannot be set if "placement.mode" is not "smart"

Placement mode is missing with hint set:

[placement]
hint = "wnam"

$ wrangler deploy

✘ [ERROR] Processing wrangler.toml configuration:

    - "placement.mode" is a required field.
    - "placement.hint" cannot be set if "placement.mode" is not "smart"

Author has addressed the following

Tests
- TODO (before merge)
- Included
- Not necessary because:
E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
- I don't know
- Required / Maybe required
- Not required because: no e2e tests for placement hints
Changeset (Changeset guidelines)
- TODO (before merge)
- Included
- Not necessary because:
Public documentation
- TODO (before merge)
- Cloudflare docs PR(s):
- Not necessary because: This feature is not publicly available yet.

changeset-bot · 2024-09-03T20:25:41Z

🦋 Changeset detected

Latest commit: 810594e

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 · 2024-09-03T20:29:04Z

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/10777635786/npm-package-wrangler-6625

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

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

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10777635786/npm-package-wrangler-6625 dev path/to/script.js

Additional artifacts:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10777635786/npm-package-create-cloudflare-6625 --no-auto-update

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10777635786/npm-package-cloudflare-kv-asset-handler-6625

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10777635786/npm-package-miniflare-6625

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10777635786/npm-package-cloudflare-pages-shared-6625

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10777635786/npm-package-cloudflare-vitest-pool-workers-6625

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10777635786/npm-package-cloudflare-workers-editor-shared-6625

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/10777635786/npm-package-cloudflare-workers-shared-6625

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

wrangler@3.75.0 includes the following runtime dependencies:

Package	Constraint	Resolved
`miniflare`	workspace:*	3.20240821.1
`workerd`	1.20240821.1	1.20240821.1
`workerd --version`	1.20240821.1	2024-08-21

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

- Add hint field to smart placement configuration

petebacondarwin · 2024-09-09T19:50:34Z

packages/wrangler/src/api/pages/create-worker-bundle-contents.ts

@@ -73,7 +73,9 @@ function createWorkerBundleFormData(

 	// The upload API only accepts an empty string or no specified placement for the "off" mode.
 	const placement: CfPlacement | undefined =
-		config?.placement?.mode === "smart" ? { mode: "smart" } : undefined;
+		config?.placement?.mode === "smart"
+			? { mode: "smart", hint: config.placement.hint }


Could this just be:

Suggested change

? { mode: "smart", hint: config.placement.hint }

? config.placement

Just tried this but TypeScript can't narrow config.placement to CfPlacement here (the wrangler and API types are slightly different), so I think it's simpler to keep as-is

OK. That's a shame. But fine as it is. Thanks for trying.

petebacondarwin · 2024-09-09T19:52:09Z

packages/wrangler/src/deploy/deploy.ts

@@ -673,7 +673,9 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m

 		// The upload API only accepts an empty string or no specified placement for the "off" mode.
 		const placement: CfPlacement | undefined =
-			config.placement?.mode === "smart" ? { mode: "smart" } : undefined;
+			config.placement?.mode === "smart"
+				? { mode: "smart", hint: config.placement.hint }


Similarly

Suggested change

? { mode: "smart", hint: config.placement.hint }

? config.placement

and elsewhere

penalosa

~~It doesn't look like this includes support for init --from-dash—could you add that?~~

Leaving this for now as it's internal-only

* main: Add pipeline binding to wrangler.toml (#6674) Fix Pages duplicating hash in redirects (#6680) Bradley/r2 event notification get (#6652) feat(wrangler): Add support for placement hints (#6625) fix(wrangler): Validate `routes` for Workers with assets (#6621) chore(deps): bump the workerd-and-workers-types group across 1 directory with 2 updates (#6673) chore(workers-shared): Configure GitHub Actions to deploy Asset Worker (#6542) feat: experimental workers assets can be ignored by adding a .assetsignore file (#6640)

maxwellpeterson force-pushed the placement-hints branch from d7ec124 to 61dc0fb Compare September 4, 2024 17:13

maxwellpeterson marked this pull request as ready for review September 4, 2024 17:27

maxwellpeterson requested review from a team as code owners September 4, 2024 17:27

feat(wrangler): Add support for placement hints

810594e

- Add hint field to smart placement configuration

maxwellpeterson force-pushed the placement-hints branch from 61dc0fb to 810594e Compare September 9, 2024 16:49

petebacondarwin approved these changes Sep 9, 2024

View reviewed changes

penalosa reviewed Sep 11, 2024

View reviewed changes

penalosa merged commit 8dcd456 into cloudflare:main Sep 11, 2024
18 checks passed

workers-devprod mentioned this pull request Sep 11, 2024

Version Packages #6669

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(wrangler): Add support for placement hints #6625

feat(wrangler): Add support for placement hints #6625

maxwellpeterson commented Sep 3, 2024 •

edited by penalosa

Loading

changeset-bot bot commented Sep 3, 2024 •

edited

Loading

github-actions bot commented Sep 3, 2024 •

edited

Loading

petebacondarwin Sep 9, 2024

maxwellpeterson Sep 9, 2024 •

edited

Loading

petebacondarwin Sep 10, 2024

petebacondarwin Sep 9, 2024

petebacondarwin Sep 9, 2024

penalosa left a comment •

edited

Loading

	? { mode: "smart", hint: config.placement.hint }
	? config.placement

feat(wrangler): Add support for placement hints #6625

feat(wrangler): Add support for placement hints #6625

Conversation

maxwellpeterson commented Sep 3, 2024 • edited by penalosa Loading

What this PR solves / how to test

Manual testing

Testing error cases

Author has addressed the following

changeset-bot bot commented Sep 3, 2024 • edited Loading

🦋 Changeset detected

github-actions bot commented Sep 3, 2024 • edited Loading

petebacondarwin Sep 9, 2024

Choose a reason for hiding this comment

maxwellpeterson Sep 9, 2024 • edited Loading

Choose a reason for hiding this comment

petebacondarwin Sep 10, 2024

Choose a reason for hiding this comment

petebacondarwin Sep 9, 2024

Choose a reason for hiding this comment

petebacondarwin Sep 9, 2024

Choose a reason for hiding this comment

penalosa left a comment • edited Loading

Choose a reason for hiding this comment

maxwellpeterson commented Sep 3, 2024 •

edited by penalosa

Loading

changeset-bot bot commented Sep 3, 2024 •

edited

Loading

github-actions bot commented Sep 3, 2024 •

edited

Loading

maxwellpeterson Sep 9, 2024 •

edited

Loading

penalosa left a comment •

edited

Loading