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

sqlalchemy engine should be created once per process #27897

Open
2 of 3 tasks
pedro-r-marques opened this issue Apr 4, 2024 · 0 comments · May be fixed by #27899
Open
2 of 3 tasks

sqlalchemy engine should be created once per process #27897

pedro-r-marques opened this issue Apr 4, 2024 · 0 comments · May be fixed by #27899
Assignees

Comments

@pedro-r-marques
Copy link

Bug description

According to the SQL Alchemy documentation (https://docs.sqlalchemy.org/en/20/core/connections.html) users of the create_engine API should create the engine once per process. Superset is currently creating sqla Engine objects multiple times in superset/models/core.py:Database class since the database object is created on demand each the chart data is read.

The current approach makes it that the SqlAlchemy pool and connection management functionality doesn't work correctly. By allocating a single Engine object per database per process, superset can reuse all the Sql Alchemy functionality and follow the recommended usage of the API.

How to reproduce the bug

  1. Use the DB_CONNECTION_MUTATOR hook in order to configure a SQLA connection pool (e.g. Queue with size 1)
  2. Observe in the driver (e.g. duckdb SQL engine) that multiple connection to the database are created. This is a direct consequence of the fact that multiple engines are created. After patching the superset code to ensure that a single engine is created per URL (as per SQLA recommended usage) SQLA connection pools work as expected.

Screenshots/recordings

No response

Superset version

master / latest-dev

Python version

3.11

Node version

Not applicable

Browser

Not applicable

Additional context

No response

Checklist

  • I have searched Superset docs and Slack and didn't find a solution to my problem.
  • I have searched the GitHub issue tracker and didn't find a similar bug report.
  • I have checked Superset's logs for errors and if I found a relevant Python stacktrace, I included it here as text in the "additional context" section.
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 a pull request may close this issue.

4 participants