Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

txmongo not working with azure cosmos db #261

Open
JoaquinKeller opened this issue Mar 23, 2020 · 19 comments
Open

txmongo not working with azure cosmos db #261

JoaquinKeller opened this issue Mar 23, 2020 · 19 comments

Comments

@JoaquinKeller
Copy link

JoaquinKeller commented Mar 23, 2020

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

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

@JoaquinKeller JoaquinKeller changed the title txmongo not working with azure cosmo db txmongo not working with azure cosmos db Mar 23, 2020
@JoaquinKeller
Copy link
Author

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

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

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

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

IlyaSkriblovsky commented Mar 25, 2020

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 😢

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

IlyaSkriblovsky commented Mar 25, 2020

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

IlyaSkriblovsky commented Apr 2, 2020

Hello, Joaquin,

Could you please test this version with Cosmos DB: #262

@JoaquinKeller
Copy link
Author

JoaquinKeller commented Apr 2, 2020 via email

@JoaquinKeller
Copy link
Author

JoaquinKeller commented Apr 7, 2020 via email

@IlyaSkriblovsky
Copy link
Contributor

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

JoaquinKeller commented Apr 8, 2020 via email

@IlyaSkriblovsky
Copy link
Contributor

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

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

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

JoaquinKeller commented Apr 8, 2020 via email

@IlyaSkriblovsky
Copy link
Contributor

Great!

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

@JoaquinKeller
Copy link
Author

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

psi29a commented Apr 9, 2020

Sure, just give me some time.

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

No branches or pull requests

3 participants