Skip to content

Conversation

eriknaslund
Copy link

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
Copy link
Member

Choose a reason for hiding this comment

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

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

@ajdavis
Copy link
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.

@eriknaslund
Copy link
Author

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
@user0007
Copy link

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.

@ajdavis
Copy link
Member

ajdavis commented Feb 21, 2012

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

@user0007
Copy link

Great, thanks!

@rozza
Copy link
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")

@eriknaslund
Copy link
Author

@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).

@jonashaag
Copy link

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

@behackett
Copy link
Member

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants