Extract ConnectionManager to handle setup connection before using it …#1457
Extract ConnectionManager to handle setup connection before using it …#1457anih wants to merge 2 commits intoMongoEngine:masterfrom
Conversation
then close MongoEngine#1457 .
| class ConnectionManager(object): | ||
| connections_registry = defaultdict(dict) | ||
|
|
||
| def get_and_setup(self, doc_cls, alias=None, collection_name=None): |
There was a problem hiding this comment.
This could use a docstring, especially as a public method. What it doesn, when it should be used, etc.
|
|
||
| @classmethod | ||
| def _get_db(cls, alias): | ||
| """Some Model using other db_alias""" |
There was a problem hiding this comment.
The docstring here is pretty confusing. What is "Some Model"? Why is it using "other db_alias"? Why do we need a private method that only proxies a call to a public get_db?
| conn = get_connection(alias) | ||
| conn.drop_database(db) | ||
|
|
||
| def reset(self): |
There was a problem hiding this comment.
This could use a docstring, too. Additionally, should this method perform any cleanup of the connections before dropping references to them? Will that leave lingering unclosed connections to the database?
|
Hi @anih at a glance this looks great! I haven't deeply read through the code yet, only asked for a few docstrings given that we're working with some public methods we want developers to interact with and understand. Could you add those as well as pull in the master and handle the conflicts?
Absolutely <3 <3 <3 changes that remove cruft from this library AND fix issues at the same time!!! :) |
|
Have you had a chance to read my comments @anih? |
|
Hi @wojcikstefan I had chance but hadn't time to implement them and to finish whole pull request. I will try to do that next week. |
|
Hi @anih have you had some time to look into this PR by any chance? :) |
2 participants
…by Document
Refs #1446, #788, #1487