Skip to content

Conversation

@emily-shen
Copy link
Contributor

@emily-shen emily-shen commented Dec 3, 2025

Fixes CC-6386.

The containers control plane needs a DO namespace id (sort of*) when creating a container app in order to link the container app and DO. This is helpfully returned by EWC when the script has a binding to a DO, but not when the DO is simply exported from the script and unbound. For now, we have to list and filter all DO namespace ids, which is not ideal, but there is a PR in flight that should enable search params on the DO list endpoint.

*The container create API can actually accept a script and class name instead, and this is what (future) direct API users would be using.

However, wrangler is in an awkward place because of how we automatically provision containers. Wrangler deploy needs to know whether a particular container + durable object combination already exists, and thus whether we should create a new container app, modify an existing one, or error because you've accidentally tried to reuse the name of an existing container app.

So wrangler will have to get the container's DO's namespace id to make sure it hasn't changed, and at that point we might as well just pass that to the container create API.

(possibly this is something that should be solved EWC side but i think we could follow up with that if we want?)


  • Tests
    • Tests included/updated
    • Tests not necessary because:
  • Public documentation
    • Cloudflare docs PR(s): TODO
    • Documentation not necessary because:
  • Wrangler V3 Backport
    • Wrangler PR:
    • Not necessary because: containers

@changeset-bot
Copy link

changeset-bot bot commented Dec 3, 2025

🦋 Changeset detected

Latest commit: 3a55730

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

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

@pkg-pr-new
Copy link

pkg-pr-new bot commented Dec 3, 2025

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@11512

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@11512

miniflare

npm i https://pkg.pr.new/miniflare@11512

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@11512

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@11512

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@11512

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@11512

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@11512

@cloudflare/workers-utils

npm i https://pkg.pr.new/@cloudflare/workers-utils@11512

wrangler

npm i https://pkg.pr.new/wrangler@11512

commit: 3a55730

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of this code is in the cloudchamber folder but isn't actually used by any cloudchamber paths, so i've moved these functions into the containers folder. (under build.ts and deploy.ts)

@emily-shen emily-shen marked this pull request as ready for review December 4, 2025 10:36
@emily-shen emily-shen requested review from a team as code owners December 4, 2025 10:36
versionId,
accountId,
scriptName,
dryRun: props.dryRun ?? false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not strictly related to this PR but dry run will have already exited by the point this is called so this was unnecessary

@emily-shen emily-shen force-pushed the emily/do-script-name branch from 350145c to 52ead14 Compare December 4, 2025 16:37
@emily-shen emily-shen force-pushed the emily/do-script-name branch from 38d4bd9 to 52cacc5 Compare December 5, 2025 11:16
Copy link
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

LGTM with a few questions

@github-project-automation github-project-automation bot moved this from Untriaged to Approved in workers-sdk Dec 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Approved

Development

Successfully merging this pull request may close these issues.

6 participants