AutoReconnect error should be raised instead of TypeError when ReadPrefrence.SECONDARY is used #99

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
6 participants

eKIK commented Feb 17, 2012

This fix is for a bug where non-strings are joined using .join(...) in case the user has ReadPreference.SECONDARY. This causes a TypeError to be thrown instead of the expected AutoReconnect error.

@@ -28,6 +28,7 @@
from bson.son import SON
from bson.tz_util import utc
+from mock import Mock, patch
@rozza

rozza Feb 17, 2012

Member

This is a new dependancy, can you confirm which mocking library this is?

Member

ajdavis commented Feb 17, 2012

This is an important fix, thanks a ton. But can we test it without adding the mock dependency? Right now pymongo depends on nothing but nose to run its tests.

eKIK commented Feb 19, 2012

Thanks a lot for the feedback.

I simply used Mock (http://pypi.python.org/pypi/mock / http://www.voidspace.org.uk/python/mock/) since it's my usual mocking library of choice, and I didn't see an obvious way to trigger this error without mocks. I completely see the value of only depending on nose though.

Please feel free to incorporate just my code fix, but write the unit test differently. I'm afraid I'm at a loss on how to do it nicely though.

jonashaag added a commit to jonashaag/mongo-python-driver that referenced this pull request Feb 19, 2012

Sorry for writting here but where can I submit an issue?

There is a bug in distinct() method. This method should allow to use custom query => http://www.mongodb.org/display/DOCS/Aggregation#Aggregation-Distinct

db.address.distinct( "zip-code" , { age : 30 } )

but pymongo does not provide this future, why?

Thanks.

Member

ajdavis commented Feb 21, 2012

@user0007 you can submit a bug at https://jira.mongodb.org/

Great, thanks!

Member

rozza commented Feb 21, 2012

@user0007 - just to let you know the syntax is different, but you can achieve it by doing a find then distinct:

db.address.find({"age": 30}).distinct("zip-code")

eKIK commented Feb 21, 2012

@jonashaag your manual patching looks good to me. I'm happy to close my pull request if you open one instead. If there's a more proper way to do things, please let me know (I'm a git / github n00b).

Never mind -- the commiters can choose to adopt all our patches or squash them together or whatnot.

Member

behackett commented Feb 22, 2012

Thanks for reporting this (and for the patch, I merged it). Luckily you should only hit this bug if your entire replica set is unreachable which somewhat mitigates the risk.

I applied Jonas' patch to fix up the test. Thanks Jonas!

@behackett behackett closed this Feb 22, 2012

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