feat: add support for deleting an item from cli

momentohq / momento-cli

Official CLI for Momento Serverless Cache

Apache License 2.0

48 stars 10 forks source link

feat: add support for deleting an item from cli #296

Closed pratik151192 closed 1 year ago

pratik151192 commented 1 year ago

Added support to delete an item for a cache. Since we already had delete as a cache subcommand to delete the cache, I went with delete-item for this one. Let me know if you have better suggestions.

How was this change tested?

Manually ran ./target/debug/momento cache delete-item for a key I had inserted and verified it got deleted from the cache. A non-existent key doesn't throw an error.
Added a test case for command verification similar to other commands.

cprice404 commented 1 year ago

@kvcache @eaddingtonwhite @danielamiao are you all okay with momento cache delete-item as the name for this subcommand?

eaddingtonwhite commented 1 year ago

delete-item makes sense to me I think. 👍

danielamiao commented 1 year ago

I'm OK with it, it's a bit too long for my liking but I don't have a better suggestion

kvcache commented 1 year ago

invalidate
remove

did you consider these single word command names?

pratik151192 commented 1 year ago

@kvcache I did consider remove. I preferred the word delete in the name as all our SDKs and API references use that word to get rid of a key. I am not opposed to remove though. @cprice404 thoughts?

Invalidate is something I didn't consider, however, it seems more of a policy than an operation. Invalidate can also indicate upsert and not necessarily a delete.

cprice404 commented 1 year ago

i vote for consistency with the SDKs over brevity, especially since we have tab completion. so i'd rather not introduce a new term for now?

I think probably in the future before we 1.0 the CLI we should reorganize the top-level subcommands so that we have something like momento cache create and momento item set, so all of the item commands are in their own namespace and it's not confusing between the cache control plane operations. at that point in time it can be just momento item delete. but for now I think it would be confusing to use a word other than delete given that that is what it's called in the SDKs.

@kvcache can you live with delete-item for now?

kvcache commented 1 year ago

I'm not going to block this pr. I disagree with the name but it's not a big deal: The one you want is not ambiguous or confusing, or even wrong.