Skip to content
This repository was archived by the owner on Aug 13, 2026. It is now read-only.

Fixes issue #124 - #125

Closed
shuhaowu wants to merge 1 commit into
basho:masterfrom
shuhaowu:patch-for-124
Closed

Fixes issue #124#125
shuhaowu wants to merge 1 commit into
basho:masterfrom
shuhaowu:patch-for-124

Conversation

@shuhaowu

Copy link
Copy Markdown
Contributor

This is what I believe the best behaviour. The get(None) doesn't throw
an error, to be consistent.

However I'm not sure about delete(). To be completely consistent,
.delete() shouldn't throw any errors either, as the key of None doesn't
exists in the database anyway. However, the Pbc client currently throws
a TypeError (actually I would like it better if it didn't throw any
error and just moved on). Any input?

This is what I believe the best behaviour. The get(None) doesn't throw
an error, to be consistent.

However I'm not sure about delete(). To be completely consistent,
.delete() shouldn't throw any errors either, as the key of None doesn't
exists in the database anyway. However, the Pbc client currently throws
a TypeError (actually I would like it better if it didn't throw any
error and just moved on). Any input?
@seancribbs

Copy link
Copy Markdown

@ultimatebuster Sorry for the delay reviewing this. I like the general idea of the change, but doesn't it make sense to raise an error? Maybe something more semantic?

@shuhaowu

Copy link
Copy Markdown
Contributor Author

Well I think it's better to raise an error, as errors are better than silent failures. However, when I was editing this, I was looking for consistency.

Since a lot of code probably do .get(...) and then check if it .exists(), I think it might be okay if we don't raise an error. Though we might fall into the loophole of php/js style silent failures, which will probably result in a major overhaul in the future once these things starts to get out of control.

On the other hand, if we start to raise errors, we have to be consistent and change a lot of silent failures (checkable failures) into raised exceptions, which is kind of a major overhaul already (I'm looking into the option in riak-python-client2)

@seancribbs

Copy link
Copy Markdown

Ugh, this has probably become too stale. @shuhaowu, would you reapply & resubmit after #179 is merged? Thanks.

@seancribbs seancribbs closed this Dec 31, 2012
@shuhaowu

Copy link
Copy Markdown
Contributor Author

Sure.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants