[SERVER-1572] If there are no matches, findandmodify() should return an empty result, not raise an exception. Created: 05/Aug/10 Updated: 19/May/14 Resolved: 25/Oct/10 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | 1.5.6 |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Nick Leippe | Assignee: | Mathias Stearn |
| Resolution: | Won't Fix | Votes: | 3 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Environment: |
Ubuntu 10.4, x86-64 |
||
| Issue Links: |
|
||||||||
| Operating System: | ALL | ||||||||
| Participants: | |||||||||
| Description |
|
See comments on (closed) findandmodify() raises pymongo.errors.OperationFailure when there are no matches: File "build/bdist.linux-x86_64/egg/pymongo/database.py", line 294, in command ), ('query', {'started': {'$exists': False}}), ('update', {'$set': {'started': 1280965374}})]) failed: No matching object found According to the mongodb documentation for findandmodify: "This command can be used to atomically modify a document (at most one) and return it." This implies that findandmodify() behaves much like the find() command--it is perfectly acceptable to not have a match at all. IMO, command failure, should be when it finds a match but fails to make the modification, or some other actual but unexpected condition occurs. This is a very common use case--a consumer pulling things off a queue. When the queue is empty, it shouldn't be generating an exception, just returning empty. It yields much nicer code. |
| Comments |
| Comment by Mathias Stearn [ 25/Oct/10 ] |
|
I think the server behavior is correct since find_and_modify on a non-existent object with upsert=false is usually an error and at the very least should be handled differently from normal behavior since no modification has taken place. I helped your issue by adding a helper in python that matches the shell's behavior. That is, returning null/None in that case and {} as the original object when inserting on an upsert. |
| Comment by Mathias Stearn [ 22/Oct/10 ] |
|
I think I will just add a find_and_modify helper to collections in pymongo. That should take care of this. |
| Comment by Nick Leippe [ 05/Aug/10 ] |
|
My last comment turns out is only half correct. The BaseException still has an args attribute that can be analyzed to implement the work around. So, imo, the two solutions are: Creating a specific exception for a single case screams that it shouldn't be an exception. Hitting eof on a fd doesn't raise--you expect to hit it eventually, just like in this case, you expect to exhaust a queue at some point. |
| Comment by Nick Leippe [ 05/Aug/10 ] |
|
In attempting to work around this, I've discovered that this behavior is even more critical to fix. It raises pymongo.errors.OperationFailure, which is derived from BaseException. There are possibly many reasons why this command could legitimately fail and would need to raise this exception. Therefore, in order to ascertain that it is the legitimate reason of not finding a match, the exception needs to be examined. The only data in the exception is the message attribute string which can be tested for the presence of the suffix "failed: No matching object found". Good and dandy right? Except that when doing so yields: DeprecationWarning: BaseException.message has been deprecated as of Python 2.6 Thus, when that attribute is gone, there will be no means of distinguishing this reason for this exception being raised from any other. This work-around will be broken as soon as the BaseException.message attribute is removed. (3.0? I couldn't find anywhere saying when that will be done.) |
| Comment by Nick Leippe [ 05/Aug/10 ] |
|
If there's concern about breaking existing code via a change in behavior, I suggest adding a new, optional parameter, which defaults to the existing behavior such as "required" or "must_exist", or something along those lines. |