twisted / txmongo

asynchronous python driver for mongo
https://txmongo.readthedocs.io
Apache License 2.0
338 stars 102 forks source link

txmongo not working with azure cosmos db #261

Open JoaquinKeller opened 4 years ago

JoaquinKeller commented 4 years ago

I have a piece of code that works with pymongo and my cosmos DB uri:

from pymongo import MongoClient

mongodb_uri = 'mongodb://....'
client = MongoClient(mongodb_uri)

db = client.foo
collection = db.test
doc = collection.find_one()
print(doc)

However when I try the same with txmongo:

from txmongo.connection import ConnectionPool
from twisted.internet import defer, reactor

@defer.inlineCallbacks
def example():
    mongodb_uri = "mongodb://..."
    client = ConnectionPool(mongodb_uri)

    db = client.foo  
    test = db.test  
    doc = yield test.find_one()
    print(doc)

if __name__ == '__main__':
    example().addCallback(lambda ign: reactor.stop())
    reactor.run()

If the uri is mongodb://localhost:27017 all good but with the cosmos db uri it doesn't work and I get an error message: TxMongo: run time of 1.0s exceeded. (Edit: removed the useless yield of the ConnectionPool call)

IlyaSkriblovsky commented 4 years ago

Hi! Unfortunately I don't have access to CosmoDB to reproduce the issue. Are you sure this isn't related to authentication issue?

JoaquinKeller commented 4 years ago

How can I know it's an authentication issue ? client = yield ConnectionPool(mongodb_uri) goes ok without error, it only gets stuck on doc = yield test.find_one() and start spitting the error message every second

NB: azure cosmos DB has a free access https://devblogs.microsoft.com/cosmosdb/build-apps-for-free-with-azure-cosmos-db-free-tier/

IlyaSkriblovsky commented 4 years ago

ConnectionPool constructor does not connect to mongo, it just returns ConnectionPool instance. Actually, you don't need yield before it. Actual TCP connect is made on the first query, find_one in your case. So auth issues will trigger on the first call too, so please make sure it is not the case. I will try to reproduce it on CosmosDB free tier, thanks for the link.

JoaquinKeller commented 4 years ago

Great ! Thanks for your amazingly fast response. I'll try to capture more error messages on this first call

(NB: the authentication and all connection parameters are embedded in the uri)

JoaquinKeller commented 4 years ago

As I thought that the error might be in the parsing of the uri, I directly called client.authenticate(dbname, username, password) but it did work either....

Also I wasn't able to get a good error message, with the actual reason of why the connection fails. I only get TxMongo: run time of 1.0s exceeded

Any idea of how to get the actual error message ?

IlyaSkriblovsky commented 4 years ago

If you didn't called any actual query yet, authenticate will just remember credentials for future use and return immediately. It will be used later when you call the first query. connection.py:229

Unfortunately I failed to create Azure account, seems like they do not work directly with customers from Russia anymore :cry:

From inspecting the code it seems like DB didnt replied to ismaster command. initialDelay in connection.py:91 is probably equal to 1 and it is the very same "1.0s" timeout in the error message. You can prove it by trying to add retry_delay=5 in ConnectionPool constructor — error message will probably change to run time of 5.0s exceeded.

But this won't answer to the root question: why ismaster response wasn't received. May be it is due to some difference between MongoDB and CosmosDB. I'm very interested to investigate and fix it in TxMongo, but not sure how to do it right now without access to CosmosDB :( May be you can provide some externally accessible minimal CosmosDB installation for testing?

P.S. Is it possible that default 1 second timeout for ismaster reply is too small?... Won't ConnectionPool(..., retry_delay=10) solve the issue? Probably not, but worth trying.

IlyaSkriblovsky commented 4 years ago

Thanks for sharing CosmosDB instance! I've tested connection and found two minor issues and one show-stopper :(

  1. To connect with SSL enabled you must add ssl_context_factory=ClientContextFactory() argument to ConnectionPool constructor (from twisted.internet.ssl import ClientContextFactory). ssl=true URI parameter is ignored for now and txmongo enables or disables SSL depending on whether you passed valid ssl_context_factory.

  2. TxMongo requires you to pass DB name in order to perform authentication. I'm not sure why it is so, may be it is a bug. But if I recall correctly, MongoDB's authentication mechanism works against concrete database, so DB name is required.

  3. But even if you pass thru 1 and 2, it won't work anyways because CosmosDB uses only supports MongoDB wire protocol, while txmongo's find* methods still use legacy wire protocol :(

Historically MongoDB has two variations of wire protocol: legacy is used in MongoDB <2.6 (IIRC) and newer protocol is used in ≥2.6. TxMongo supports newer protocol for insert and update methods, but still use legacy one for find*().

Some time ago I have started to implement sessions & transactions support for TxMongo which includes using new protocol for find methods. But this patch isn't ready yet and due to lack of spare time it lies on a shelf for a couple of months. I will try to extract new-style find methods from it and make it ready for release. I hope I will be able to do it on the next week.

IlyaSkriblovsky commented 4 years ago

Hello, Joaquin,

Could you please test this version with Cosmos DB: https://github.com/twisted/txmongo/pull/262

JoaquinKeller commented 4 years ago

Woaw super cool :-) I'll test that tomorrow, 11pm in my timezone rn

On Thu, Apr 2, 2020 at 9:06 PM Ilya Skriblovsky notifications@github.com wrote:

Hello, Joaquin,

Could you please test this version with Cosmos DB?

262 https://github.com/twisted/txmongo/pull/262

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/twisted/txmongo/issues/261#issuecomment-607834651, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFH53D67IT25236JPGDPELRKSEVXANCNFSM4LRTR7TA .

JoaquinKeller commented 4 years ago

Hi Ilya,

(sorry for the delay I had a family issue)

I am bit noob in git and I didn't find the branch with the commit to test. I can see the commit in github https://github.com/twisted/txmongo/pull/262/commits/b016f7e84bb4081157dfdcbbbd3073d932111500

but when I git show b016f7e84bb4081157dfdcbbbd3073d932111500 I get a fatal: bad object probably because I cannot see/access the branch new-style-find

Best, ---Joaquin

On Thu, Apr 2, 2020 at 10:46 PM Joaquín Keller joaquin.keller@gmail.com wrote:

Woaw super cool :-) I'll test that tomorrow, 11pm in my timezone rn

On Thu, Apr 2, 2020 at 9:06 PM Ilya Skriblovsky notifications@github.com wrote:

Hello, Joaquin,

Could you please test this version with Cosmos DB?

262 https://github.com/twisted/txmongo/pull/262

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/twisted/txmongo/issues/261#issuecomment-607834651, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFH53D67IT25236JPGDPELRKSEVXANCNFSM4LRTR7TA .

IlyaSkriblovsky commented 4 years ago

Sorry for late answer.

new-style-find branch is not in twisted/txmongo repository but in my personal fork: https://github.com/IlyaSkriblovsky/txmongo/tree/new-style-find . You may clone my fork or just download a copy of the branch with Download button on this page.

JoaquinKeller commented 4 years ago

Oh thanks, I was looking in the wrong place...

Ok, I installed it <@txmongo>-<⎇ new-style-find>-> python setup.py install --user I try the same experiment, and I get the same error message: 'TxMongo: run time of 1.0s exceeded.' However the same mongo uri do work with pymongo...

Did you managed to get it work ? Can you share your code ? (hiding the actual uri)

Thanks, -- Joaquín

On Wed, Apr 8, 2020 at 1:33 PM Ilya Skriblovsky notifications@github.com wrote:

Sorry for late answer.

new-style-find branch is not in twisted/txmongo repository but in my personal fork: https://github.com/IlyaSkriblovsky/txmongo/tree/new-style-find . You may clone my fork or just download a copy of the branch with Download button on this page https://github.com/IlyaSkriblovsky/txmongo/tree/new-style-find .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/twisted/txmongo/issues/261#issuecomment-610760299, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFH53GMXSG56OUMWSFRKQTRLQEBTANCNFSM4LRTR7TA .

IlyaSkriblovsky commented 4 years ago

Hmm, that's strange...

I've did following:

  1. git clone git@github.com:IlyaSkriblovsky/txmongo
  2. cd txmongo
  3. git checkout new-style-find
  4. python3 setup.py install --user
  5. pip3 install pyOpenSSL service_identity --user
  6. python3 cosmosdb_tx.py

And it seems to work:

ilya@ilya-330s:~/tmp/txmongo$ python3 cosmosdb_tx.py 
!! [{'_id': ObjectId('5e84f967fd51ec38d09c5c98'), 'x': 42}, {'_id': ObjectId('5e84f967fd51ec38d09c5c99'), 'y': 123}, {'_id': ObjectId('5e8d8a971f68edc59adb035b'), 'x': 42}, {'_id': ObjectId('5e8d8a971f68edc59adb035c'), 'y': 123}, {'_id': ObjectId('5e8d8ab7de43f3643e366b14'), 'x': 42}, {'_id': ObjectId('5e8d8ab7de43f3643e366b15'), 'y': 123}]

cosmosdb_tx.py:

from twisted.internet.ssl import ClientContextFactory
from twisted.internet.task import react
from twisted.internet.defer import *
from twisted.test.ssl_helpers import ClientTLSContext, ServerTLSContext

from txmongo.connection import ConnectionPool

uri = 'mongodb://ql...:pyR...@ql....azure.com:10255/test?ssl=true&replicaSet=globaldb&retrywrites=false&maxIdleTimeMS=120000&appName=@ql...@'

@react
@inlineCallbacks
def main(reactor):
    mc = ConnectionPool(uri, ssl_context_factory=ClientContextFactory())
    db = mc.test
    yield db.coll.insert_many([{'x': 42}, {'y': 123}])
    print('!!', (yield db.coll.find()))

I bet it may be because I've changed something in mongodb:// uri. Please try with the same parameters. If any of them raises questions, please ask and I will recall what I've changed and why.

JoaquinKeller commented 4 years ago

Ok, now it works, your code gave me the indication of where the problem was:

I've added ssl_context_factory=ClientContextFactory() to the ConnectionPool call, and then went on passing the authentication data with the authenticate method, and now it works !

    client = ConnectionPool(mongodb_uri, ssl_context_factory=ClientContextFactory())
    client.authenticate(dbname, username, password)  

Just to be sure, I tried the same code with the official (PyPI) distro of txmongo. And it works as well.

So what failed is that the authentication data is not (properly) extracted from the uri and and therefore this authentication data has to be passed with the authenticate method. Same happens with the ssl=true directive of the uri that is ignored and needs to be explicitly stated.

Actually, I removed all the parameters of the uri and it still worked (with the PyPI version also) mongodb_uri = 'mongodb://MYDBNAME.cosmos.azure.com:10255/ is fine

IlyaSkriblovsky commented 4 years ago

Yes, I've described ssl_context_factory in the post above.

DB name is not required if you explicitly call authenticate(), you are right.

But if PyPI version of txmongo works correctly — this sounds strange.

I get following from the same cosmosdb_tx.py shown above with PyPI version of txmongo:

ilya@ilya-330s:~/txmongo$ ./venv/bin/python cosmosdb_tx.py 
!! [{'cursor': {'firstBatch': [], 'id': 0, 'ns': 'test.coll'}, 'ok': 1.0}]

And this is exactly the issue with old-style/new-style find command implementation. But on new-style-find branch I get the correct array printed.

Are you sure new-style-find branch doesn't interfere with txmongo version installed from PyPI? For example, if you're testing in git-cloned directory, import txmongo will probably load from the local txmongo folder rather than from package installed with pip.

JoaquinKeller commented 4 years ago

You are right, with the PyPI version the data arrive in the wrong form. And yes my bad: You did mention ssl_context_factory in the post above.

Hey, everything is solved :-)

On Wed, Apr 8, 2020 at 10:49 PM Ilya Skriblovsky notifications@github.com wrote:

Yes, I've described ssl_context_factory in the post above https://github.com/twisted/txmongo/issues/261#issuecomment-604081415.

DB name is not required if you explicitly call authenticate(), you are right.

But if PyPI version of txmongo works correctly — this sounds strange.

I get following from the same cosmosdb_tx.py shown above with PyPI version of txmongo:

ilya@ilya-330s:~/txmongo$ ./venv/bin/python cosmosdb_tx.py

!! [{'cursor': {'firstBatch': [], 'id': 0, 'ns': 'test.coll'}, 'ok': 1.0}]

And this is exactly the issue with old-style/new-style find command implementation. But on new-style-find branch I get the correct array printed.

Are you sure new-style-find branch doesn't interfere with txmongo version installed from PyPI? For example, if you're testing in git-cloned directory, import txmongo will probably load from the local txmongo folder rather than from package installed with pip.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/twisted/txmongo/issues/261#issuecomment-611002182, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFH53ERS3L4VWM4CNXZHOLRLSE7DANCNFSM4LRTR7TA .

IlyaSkriblovsky commented 4 years ago

Great!

@psi29a Bret, it would be cool if you could find time to review changes from #262 :)

JoaquinKeller commented 4 years ago

Mmmh, I've never done that before... It would be a first time... I am not understanding half of the code... But it works ! (it seems at least)

psi29a commented 4 years ago

Sure, just give me some time.