Skip to content

fix: ensure DB connections are fully released on close#146

Merged
monodera merged 4 commits into
mainfrom
fix/db-connection-cleanup
May 22, 2026
Merged

fix: ensure DB connections are fully released on close#146
monodera merged 4 commits into
mainfrom
fix/db-connection-cleanup

Conversation

@monodera
Copy link
Copy Markdown
Contributor

Summary

  • TargetDB.close() was calling session.close() only, which returns connections to SQLAlchemy's internal pool but does not close the underlying TCP sockets to PostgreSQL.
  • On abnormal process termination (unhandled exceptions, SIGINT, etc.) the engine is never garbage-collected, causing idle connections to accumulate on the PostgreSQL server.
  • Added engine.dispose() to close() to fully release all pooled connections to the server.
  • Added __enter__ / __exit__ to support the with statement for safer usage.

Root cause

# Before: session is returned to pool, TCP socket stays open
def close(self):
    self.session.close()

# After: pool is disposed, TCP sockets are closed server-side
def close(self):
    self.session.close()
    self.engine.dispose()

Test plan

  • Verify that after calling db.close(), the connection no longer appears in pg_stat_activity
  • Verify that existing workflows using TargetDB continue to work normally
  • Verify context manager usage: with TargetDB(...) as db: ...

🤖 Generated with Claude Code

TargetDB.close() was only calling session.close(), which returns
connections to SQLAlchemy's pool but does not close the underlying
TCP sockets. On abnormal process termination (exceptions, SIGINT,
etc.) the engine is never garbage-collected, leaving idle connections
on the PostgreSQL server.

Add engine.dispose() to close() so that all pooled connections are
released to the server when the caller is done. Also add __enter__
and __exit__ to support usage as a context manager.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@codacy-production
Copy link
Copy Markdown

codacy-production Bot commented May 22, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 5 complexity · 0 duplication

Metric Results
Complexity 5
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves lifecycle management for TargetDB by fully releasing SQLAlchemy pooled connections when closing, and adds context manager support to encourage safer usage patterns.

Changes:

  • Update TargetDB.close() to call engine.dispose() after session.close() so pooled DB connections are fully released.
  • Add TargetDB.__enter__ / __exit__ to enable with TargetDB(...) as db: usage.
  • Regenerate/update tbls-generated table documentation (index ordering changes only).

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.

File Description
src/targetdb/targetdb.py Disposes SQLAlchemy engine on close and adds context manager support.
docs/tbls/public.target.md Updates generated index listing order/formatting.
docs/tbls/public.sky.md Updates generated index listing order/formatting.
docs/tbls/public.fluxstd.md Updates generated index listing order/formatting.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/targetdb/targetdb.py
Comment thread src/targetdb/targetdb.py Outdated
monodera and others added 2 commits May 21, 2026 17:19
- Wrap session.close() in try/finally so engine.dispose() always runs,
  even if session.close() itself raises an exception.
- Make __enter__() idempotent: skip connect() if a session already exists,
  preventing an engine/session leak when called on an already-connected
  instance (e.g. db.connect(); with db:).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@monodera monodera merged commit 1b7eef3 into main May 22, 2026
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.

2 participants