Skip to content

Implement Delete Operation Support #24

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

Merged
merged 3 commits into from
Aug 2, 2023

Conversation

dkz2
Copy link
Contributor

@dkz2 dkz2 commented Aug 2, 2023

This PR addresses the need to extend our current Memcached functionality with the addition of a delete operation. This operation allows users to remove specific key-value pairs from the cache directly. This PR will close #18.

Motivation:
To broaden our interaction capabilities with Memcached servers. The delete operation offers users more control over the stored data.

Modifications:

  • Added a new DeleteCommand to our MemcachedRequest struct.
  • Updated our MemcachedRequestEncoder to handle delete operation request
  • Implemented a new a delete method to our MemcachedConnection that allows for deleting a key-value pair from memcached.
  • Added Unit and Integration test.

Results:
With the addition of the delete operation support, our API now allows users to directly remove key value pairs from cache, enhancing the versatility and control of data within the memcached server.

Comment on lines 271 to 274
case .NF:
return false
default:
return false
Copy link
Contributor

Choose a reason for hiding this comment

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

I am kinda leaning towards throwing an error in the case where the key was not found. Also in the default case we should throw.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In that case we probably do not need to return bool anymore?

Copy link
Contributor

Choose a reason for hiding this comment

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

Exactly

@dkz2 dkz2 marked this pull request as ready for review August 2, 2023 12:42
return false
throw MemcachedConnectionError.unexpectedNilResponse
Copy link
Contributor

Choose a reason for hiding this comment

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

We should throw a new error here to indicate that the key didn't exist

@FranzBusch FranzBusch merged commit 526c67d into swift-server:main Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Delete Operation Support
2 participants