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

Disable SQLAlchemy connection pooling #98

Closed
wants to merge 2 commits into from
Closed

Disable SQLAlchemy connection pooling #98

wants to merge 2 commits into from

Conversation

RazerM
Copy link
Contributor

@RazerM RazerM commented Jan 24, 2017

The Database class provides no way to use it, and the current
implementation means that a connection is held by the pool even after
db.close().

See also #67.

The Database class provides no way to use it, and the current
implementation means that a connection is held by the pool even after
db.close()
@kennethreitz
Copy link
Owner

aren't there major advantages to using the connection pooling with the current implementation?

@RazerM
Copy link
Contributor Author

RazerM commented Feb 5, 2017

No, Database creates an engine then immediately asks that engine for a connection:

        self._engine = create_engine(self.db_url, **kwargs)

        # Connect to the database.
        self.db = self._engine.connect()

Later, Database.close returns that connection to the pool. This means that the engine held by Database has a connection open for as long as it lives, even after calling close.

Connection pooling doesn't help since within the lifecycle of Database, one connection is checked out, then later returned.

@dmitrytokarev
Copy link

@RazerM please resolve conflicts. then we can ping Kenneth to merge this

@dmitrytokarev
Copy link

@kennethreitz could you review this PR?

@kennethreitz
Copy link
Owner

Sounds like we should fix db.close() then

@RazerM
Copy link
Contributor Author

RazerM commented Aug 27, 2017

The way to fix db.close() is by not creating a connection pool that isn't used. It's only keeping a connection open because create_engine's default arguments include pool_size=5, which is "the number of connections to keep open inside the connection pool".

@kennethreitz
Copy link
Owner

One of the goals of this library was to provide proper connection pooling, @RazerM. It sounds like I messed that up. Can we fix that easily?

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.

None yet

3 participants