Skip to content
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

storage: fix to allow all values in storage #9

Closed
wants to merge 1 commit into from

Conversation

Tha-Robert
Copy link

Also fixes race condtion in get where we chech if it exits and
maybe cull value before we return it.

Signed-off-by: Robert Marklund robert.marklund@ericsson.com

Also fixes race condtion in get where we chech if it exits and
maybe cull value before we return it.

Signed-off-by: Robert Marklund <robert.marklund@ericsson.com>
@bmuller
Copy link
Owner

bmuller commented Sep 15, 2015

Hey @Tha-Robert - I see that this would allow the storage of a null (None) value, but why is this desired? I could perhaps see if this was an attempt to create a delete function - but that would be another discussion.

@Tha-Robert
Copy link
Author

For our purposes we need it for lazy delete functionality, not needed but preferred.

Second I dislike having values in storage that can have too meanings the actual value and no value exists. It takes a way one value that cant be stored in the storage. This ways its gets more generic, and fail proof.

//R

@bmuller
Copy link
Owner

bmuller commented Sep 16, 2015

Deleting a key/value and storing a null value are not the same thing, so this wouldn't get you the ability to delete. For the second case, it could be solved by not doing anything if someone attempts to set a key/value where the value is None, which would mean that a None result from a get means only one thing - that the key doesn't exist. If I made that change instead, the API wouldn't need to change. What do you think about that solution?

@bmuller
Copy link
Owner

bmuller commented Jan 12, 2019

Closed from inactivity. Please feel free to reopen if you still see a need and want to discuss. Thanks!

@bmuller bmuller closed this Jan 12, 2019
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.

2 participants