[wrangler] preserve containers config when using versions commands#12368
[wrangler] preserve containers config when using versions commands#12368petebacondarwin merged 4 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 4e3eda9 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 |
|
Claude finished @KianNH's task —— View job Changeset Review✅ All changesets look good The changeset file
The description effectively explains what was broken (Containers being disabled) and why (the property was omitted from the API request body). Note: Reviewer petebacondarwin suggested a slightly refined wording in their review comment, which you may want to consider, but the current changeset is already compliant with all requirements. |
create-cloudflare
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
petebacondarwin
left a comment
There was a problem hiding this comment.
Just the change to the changeset but otherwise LGTM.
petebacondarwin
left a comment
There was a problem hiding this comment.
So I think we need some tests updated here?
There was a problem hiding this comment.
so what happens if config.containers (ie the DO containers enabled flag) has changed on a version upload? like if you set containers = true for the first time. i'm guessing that's ok, you just can't have any code that actually depends on containers or it will all break.
separately, if wrangler config for containers (image, w/e) changes, we should detect and warn users that's not going to do a gradual rollout, like we do with DO migrations, and those changes won't apply until you do wrangler deploy next. we should probably advise people to isolate a version upload and containers changes like we do to get around DO migrations and versions.
i'm guessing the desired behaviour is that when we do a version upload, all the container changes apply in one go when you next do a proper deployment. pretty sure that's fine as-is but can someone try it out and verify that's the case?
Co-authored-by: Pete Bacon Darwin <pete@bacondarwin.com>
I think today a warning is fine since it should hopefully be short-lived, we're going to look at something akin to how we have That doesn't solve for users having Worker code that relies on being routed to a specific version of their Container but I think we can only really push users to making sure they're writing backwards/forwards compatible changes. |
|
i can also see a future where we integrate container rollouts with versions gradual rollouts, but i haven't really thought through the implications of that. |
Fixes CC-6846
Commands like
versions uploadwere disabling Containers on the associated Durable Object namespaces since thecontainersproperty was missing from the metadata JSON.I swapped the
containersproperty inCfWorkerInitto use... | undefinedlike the other properties so TS forces the author to either pass it through or intentionally disregard it.A picture of a cute animal (not mandatory, but encouraged)