-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[wrangler] Allow deleting KV namespaces by name #12464
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
Conversation
🦋 Changeset detectedLatest commit: 355ab44 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 @petebacondarwin's task —— View job Changeset Review
✅ All changesets look good The changeset
The changeset effectively communicates the new feature to users and includes a practical example. Cuteness Report: This is Cat. Appeared as an emoji in the PR description. Would rate but needs higher resolution for proper assessment. Still appreciated. 🐱/10 |
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: |
fb14dfa to
e86dbef
Compare
|
I have tested this manually using the PR preview to prove that the backend APIs being hit work as expected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e86dbef to
a4f6dc5
Compare
vicb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See inline comments.
Could you please also fix the description ?
When deleting by name, the command will:
List all namespaces for the account
Find the namespace with the matching title
Delete the namespace by its ID
Display both the name and ID in the confirmation message
- explain why list, i.e. because no find by name
- title -> name
- "Display both the name and ID in the confirmation message" doesn't look like this is what is implemented
|
Don't land this until the KV team have reviewed and we have resolved the questions around the |
name vs title is being discussed with the KV team
|
All "title" references have been removed, except for tests that continue to respond with the same format as the API. |
5355b7a to
a45b2ce
Compare
vicb
left a comment
There was a problem hiding this 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 question.
Thanks for the updates
a45b2ce to
9abf561
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
9abf561 to
364599b
Compare
Add ability to delete a KV namespace by providing its name as a positional argument (e.g., `wrangler kv namespace delete my-namespace`). This aligns the delete command with the create and rename commands, which also accept namespace names. The existing --namespace-id and --binding flags continue to work as before. Fixes #4193
- Add namespace name lookup support to getKVNamespaceId() helper
- Return { namespaceId, displayName } instead of just string for better UX
- Update all KV command callers to use the new return type
- Use displayName in log messages for friendlier output (e.g. 'my-binding (abc123)')
- Update test snapshots to reflect the new output format
ae49aa4 to
355ab44
Compare
Fixes #4193.
Allow deleting a KV namespace by providing its name as a positional argument:
This aligns the delete command with the create and rename commands, which also accept namespace names. The existing
--namespace-idand--bindingflags continue to work as before.Implementation
The namespace name lookup logic has been centralized in the
getKVNamespaceId()helper, which now:namespaceparameter for lookup by name{ namespaceId, displayName }instead of just a stringdisplayNameto show both the identifier and ID (e.g.,my-binding (abc123))This improves UX across all KV commands by showing more context in log messages.
A picture of a cute animal (not mandatory, but encouraged)
🐱