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

Add warning for smart placement w/assets #7568

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/tidy-kiwis-arrive.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": minor
---

Warn users when using smart placement with Workers + Assets and `serve_directly` is set to `false`
49 changes: 49 additions & 0 deletions packages/wrangler/src/__tests__/deploy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4687,6 +4687,55 @@ addEventListener('fetch', event => {});`
`);
});

it("should warn when using smart placement with Worker first", async () => {
const assets = [
{ filePath: ".assetsignore", content: "*.bak\nsub-dir" },
{ filePath: "file-1.txt", content: "Content of file-1" },
{ filePath: "file-2.bak", content: "Content of file-2" },
{ filePath: "file-3.txt", content: "Content of file-3" },
{ filePath: "sub-dir/file-4.bak", content: "Content of file-4" },
{ filePath: "sub-dir/file-5.txt", content: "Content of file-5" },
];
writeAssets(assets, "assets");
writeWorkerSource({ format: "js" });
writeWranglerConfig({
main: "index.js",
assets: {
directory: "assets",
experimental_serve_directly: false,
binding: "ASSETS",
},
placement: {
mode: "smart",
},
});
const bodies: AssetManifest[] = [];
await mockAUSRequest(bodies);
mockSubDomainRequest();
mockUploadWorkerRequest({
expectedAssets: {
jwt: "<<aus-completion-token>>",
config: {
serve_directly: false,
},
},
expectedMainModule: "index.js",
expectedBindings: [{ name: "ASSETS", type: "assets" }],
});

await runWrangler("deploy");

expect(std.warn).toMatchInlineSnapshot(`
"▲ [WARNING] Turning on Smart Placement in a Worker that is using assets and serve_directly set to false means that your entire Worker could be moved to run closer to your data source, and all requests will go to that Worker before serving assets.

This could result in poor performance as round trip times could increase when serving assets.

Read more: https://developers.cloudflare.com/workers/static-assets/binding/#smart-placement

"
`);
});

it("should warn if experimental_serve_directly=false but no binding is provided", async () => {
const assets = [
{ filePath: ".assetsignore", content: "*.bak\nsub-dir" },
Expand Down
12 changes: 12 additions & 0 deletions packages/wrangler/src/assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,18 @@ export function validateAssetsArgsAndConfig(
);
}

// Smart placement turned on when using assets
if (
config?.placement?.mode === "smart" &&
config?.assets?.experimental_serve_directly === false
) {
logger.warn(
"Turning on Smart Placement in a Worker that is using assets and serve_directly set to false means that your entire Worker could be moved to run closer to your data source, and all requests will go to that Worker before serving assets.\n" +
"This could result in poor performance as round trip times could increase when serving assets.\n\n" +
"Read more: https://developers.cloudflare.com/workers/static-assets/binding/#smart-placement"
);
}

// User Worker ahead of assets, but no assets binding provided
if (
"legacy" in args
Expand Down
Loading