MongoEngine / mongoengine

A Python Object-Document-Mapper for working with MongoDB
http://mongoengine.org
MIT License
4.23k stars 1.23k forks source link

connect() does not use connection settings previously registered as default #1770

Open iogakos opened 6 years ago

iogakos commented 6 years ago

Hi,

I use register_connection() to register the default connection settings and connect() to establish a connection afterwards. When calling connect() without an alias, I would expect to connect using the default settings previously defined with register_connection(), though connect() ignores the default alias and falls back to the sane defaults:

>>> import mongoengine as m
>>> m.register_connection('default', db='mongoenginetest', host='mongomock://user:pass@localhost1/mongoenginetest')
>>> conn = m.connect(db='mongoenginetest')
>>> conn
MongoClient(host=['localhost:27017'], document_class=dict, tz_aware=False, connect=True, read_preference=Primary())

Note, the host used when registering the default alias and the host of the established client are different. connect() ends up overwriting the default connection settings.

get_connection() seems to respect the default connection settings instead:

>>> import mongoengine as m
>>> m.register_connection('default', db='mongoenginetest', host='mongomock://user:pass@localhost1/mongoenginetest')
>>> conn = m.connection.get_connection()
>>> conn
mongomock.MongoClient('mongodb://user:pass@localhost1/mongoenginetest', 27017)

I am a bit confused whether this is an expected behaviour or not. My proposed fix is changing connect() to look at _connection_settings instead of _connections before registering a connection which doesn't seem to break any of the existing tests.

bagerard commented 5 years ago

Hi @wojcikstefan, I'd like to have your input on this. I've always wondered why we were exposing register_connection (as opposed to just exposing connect), the only valuable reason that I see is to allow users to register the settings and delay the actual creation of the pymongo connections until they are actually required (which they can only do by calling get_connection() today, as shown by this issue). Are there other use cases?

Exposing register_connection brings a few potential bugs like this one due to the de-synchronization of the different globals that maintain the connections (_connections, _connection_details, etc)

I believe it will be less confusing to only expose connect(), let me know what you think

srl295 commented 3 years ago

@bagerard just a note, for something like a test situation, the app code may call connect() and disconnect() multiple times, register_connection() lets me set the default prior to other code being called. Hope it helps.