Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion scripts/benchmark_migration.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def find_models(module: ModuleType) -> list[type[Model]]: # noqa: C901
# where the current model is out-of-sync with the existing table after a
# downgrade
sqlalchemy_uri = current_app.config["SQLALCHEMY_DATABASE_URI"]
engine = create_engine(sqlalchemy_uri, future=True)
engine = create_engine(sqlalchemy_uri)
Base = automap_base() # noqa: N806
Base.prepare(engine, reflect=True)
seen = set()
Expand Down
3 changes: 0 additions & 3 deletions superset/cli/test_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,9 +280,6 @@ def test_sqlalchemy_dialect(
"""
Test the SQLAlchemy dialect, making sure it supports everything Superset needs.
"""
if "future" not in engine_kwargs:
engine_kwargs["future"] = True

engine = create_engine(sqlalchemy_uri, **engine_kwargs)
dialect = engine.dialect

Expand Down
2 changes: 1 addition & 1 deletion superset/commands/streaming_export/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ def _execute_query_and_stream(
delimiter = csv_export_config.get("sep", ",")
decimal_separator = csv_export_config.get("decimal", ".")

with db.session(future=True) as session:
with db.session() as session:
# Merge database to prevent DetachedInstanceError
merged_database = session.merge(database)

Expand Down
1 change: 0 additions & 1 deletion superset/db_engine_specs/gsheets.py
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,6 @@ def validate_parameters(
}
}
},
future=True,
)
conn = engine.connect()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Connection resource leak

The connection at line 393 is never explicitly closed and has no finally block or context manager. Multiple early returns (lines 414, 425) bypass any potential cleanup. Compare to the correct pattern at line 525: with engine.connect() as conn:. This causes a resource leak where the database connection remains open after the function returns.

Code Review Run #d015f4


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

idx = 0
Expand Down
4 changes: 2 additions & 2 deletions superset/migrations/shared/catalogs.py
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ def upgrade_catalog_perms(engines: set[str] | None = None) -> None:

"""
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

# The Database model has an eager-loaded (``lazy="joined"``) ``ssh_tunnel``
# backref. Eager-loading it here would SELECT every column on ``ssh_tunnels``,
Expand Down Expand Up @@ -581,7 +581,7 @@ def downgrade_catalog_perms(engines: set[str] | None = None) -> None:
WARNING: models (datasets and charts) not in the default catalog are deleted!
"""
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

# See upgrade_catalog_perms: avoid eager-loading the ``ssh_tunnel`` backref so the
# query stays schema-safe across migration revisions.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class Slice(Base):
def upgrade():
bind = op.get_bind()
op.add_column("slices", sa.Column("perm", sa.String(length=2000), nullable=True))
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

# Use Slice class defined here instead of models.Slice
for slc in session.query(Slice).all():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def upgrade():
)

bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

# don't use models.DruidMetric
# because it assumes the context is consistent with the application
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ class Dashboard(AuditMixin, Base):

def upgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

objects = session.query(Slice).all()
objects += session.query(Dashboard).all()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class Slice(Base):
def upgrade():
bind = op.get_bind()
op.add_column("slices", sa.Column("datasource_id", sa.Integer()))
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

for slc in session.query(Slice).all():
if slc.druid_datasource_id:
Expand All @@ -63,7 +63,7 @@ def upgrade():

def downgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)
for slc in session.query(Slice).all():
if slc.datasource_type == "druid":
slc.druid_datasource_id = slc.datasource_id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class Database(Base):

def upgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

for obj in session.query(Database).all():
obj.allow_run_sync = True
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class Slice(Base):

def upgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

slices = session.query(Slice).all()
slice_len = len(slices)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class Url(Base):

def upgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

urls = session.query(Url).all()
urls_len = len(urls)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class Slice(Base):

def upgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

for slc in session.query(Slice).filter(Slice.viz_type.like("deck_%")):
params = json.loads(slc.params)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class Slice(Base):

def upgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

for slc in session.query(Slice).filter(
or_(Slice.viz_type.like("line"), Slice.viz_type.like("bar"))
Expand Down Expand Up @@ -75,7 +75,7 @@ def upgrade():

def downgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

for slc in session.query(Slice).filter(
or_(Slice.viz_type.like("line"), Slice.viz_type.like("bar"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class Dashboard(Base):

def upgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

dashboards = session.query(Dashboard).all()
for i, dashboard in enumerate(dashboards):
Expand All @@ -68,7 +68,7 @@ def upgrade():

def downgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

dashboards = session.query(Dashboard).all()
for i, dashboard in enumerate(dashboards):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def upgrade():
),
)

session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

# Use Slice class defined here instead of models.Slice
for tbl in session.query(Table).all():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class Slice(Base):

def upgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

slices = session.query(Slice).filter_by(viz_type="cal_heatmap").all()
slice_len = len(slices)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class Slice(Base):

def upgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

for slc in session.query(Slice).all():
try:
Expand All @@ -63,7 +63,7 @@ def upgrade():

def downgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

for slc in session.query(Slice).all():
try:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class Slice(Base):

def upgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

for slc in session.query(Slice).all():
try:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class Database(Base):


def replace(source, target):
with db.Session(bind=op.get_bind(), future=True) as session:
with db.Session(bind=op.get_bind()) as session:
with session.begin():
query = (
session.query(Slice, Database)
Expand All @@ -80,7 +80,7 @@ def replace(source, target):

for slc, database in query:
try:
engine = create_engine(database.sqlalchemy_uri, future=True)
engine = create_engine(database.sqlalchemy_uri)

if engine.dialect.identifier_preparer._double_percents:
params = json.loads(slc.params)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class Slice(Base):

def upgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

for slc in session.query(Slice).all():
try:
Expand All @@ -66,7 +66,7 @@ def upgrade():

def downgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

for slc in session.query(Slice).all():
try:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class Slice(Base):

def upgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

for slc in session.query(Slice).filter(Slice.viz_type == "pie").all():
try:
Expand All @@ -68,7 +68,7 @@ def upgrade():

def downgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

for slc in session.query(Slice).filter(Slice.viz_type == "pie").all():
try:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ def compute_time_compare(granularity, periods):

def upgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

for chart in session.query(Slice):
params = json.loads(chart.params or "{}")
Expand Down Expand Up @@ -163,7 +163,7 @@ def upgrade():

def downgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

for chart in session.query(Slice):
params = json.loads(chart.params or "{}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ class TableColumn(BaseColumnMixin, Base):

def upgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

tables = [
Annotation,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class TableColumn(BaseColumnMixin, Base):

def upgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

# Delete the orphaned columns records.
for record in session.query(DruidColumn).all():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class SqlMetric(BaseMetricMixin, Base):

def upgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

# Delete the orphaned metrics records.
for record in session.query(DruidMetric).all():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ def scan_dashboard_positions_data(positions):

def upgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

dashboards = session.query(Dashboard).all()
for i, dashboard in enumerate(dashboards):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def is_v2_dash(positions):

def upgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

dashboards = session.query(Dashboard).all()
for i, dashboard in enumerate(dashboards): # noqa: B007
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class Dashboard(Base):

def upgrade(): # noqa: C901
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

dashboards = session.query(Dashboard).all()
for i, dashboard in enumerate(dashboards):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def upgrade_slice(slc):

def upgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

filter_box_slices = session.query(Slice).filter_by(viz_type="filter_box")
for slc in filter_box_slices.all():
Expand All @@ -90,7 +90,7 @@ def upgrade():

def downgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

filter_box_slices = session.query(Slice).filter_by(viz_type="filter_box")
for slc in filter_box_slices.all():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def upgrade():

bind = op.get_bind()
insp = sa.engine.reflection.Inspector.from_engine(bind)
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

tables = session.query(SqlaTable).all()
for table in tables:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ class TableColumn(BaseColumnMixin, Base):

def upgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

tables = [
Annotation,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def add_parent_ids(node, layout):

def upgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

dashboards = session.query(Dashboard).all()
for i, dashboard in enumerate(dashboards):
Expand All @@ -88,7 +88,7 @@ def upgrade():

def downgrade():
bind = op.get_bind()
session = db.Session(bind=bind, future=True)
session = db.Session(bind=bind)

dashboards = session.query(Dashboard).all()
for i, dashboard in enumerate(dashboards):
Expand Down
Loading
Loading