aminghadersohi commented on code in PR #44262:
URL: https://github.com/apache/superset/pull/44262#discussion_r4064893029
##########
tests/unit_tests/commands/deletion_retention/test_prune_audit.py:
##########
@@ -429,3 +433,710 @@ def test_pruning_shares_the_audit_writers_clock() -> None:
from superset.commands.deletion_retention import audit
assert prune_audit.utc_now is audit.utc_now
+
+
[email protected](
+ "dialect", [postgresql.dialect(), mysql.dialect(), sqlite.dialect()]
+)
+def test_repeat_history_scope_is_only_present_for_locked_rechecks(dialect:
Any) -> None:
+ """Limit history to candidate entities only when a batch scope is
supplied."""
+ table: sa.Table = prune_audit.PurgeAuditLog.__table__
+ now: datetime = datetime(2026, 1, 1)
+ scope_entities: list[tuple[str, str | None]] = [
+ ("dashboard", "b"),
+ ("chart", "a"),
+ ("chart", "a"),
+ ]
+ scoped: sa.sql.Select = sa.select(table.c.id).where(
+ *prune_audit._duplicate_predicates(table, now,
scope_entities=scope_entities)
+ )
+ unscoped: sa.sql.Select = sa.select(table.c.id).where(
+ *prune_audit._duplicate_predicates(table, now)
+ )
+ scoped_sql: str = str(
+ scoped.compile(dialect=dialect, compile_kwargs={"literal_binds": True})
+ )
+ unscoped_sql: str = str(unscoped.compile(dialect=dialect))
+ assert scoped_sql.count("entity_type IN ('chart', 'dashboard')") == 3
+ assert scoped_sql.count("entity_uuid IN ('a', 'b')") == 3
+ assert "repeat_scope" not in scoped_sql
+ assert "entity_type IN" not in unscoped_sql
+ assert "entity_uuid IN" not in unscoped_sql
+
+
+def test_maximum_literal_scope_executes_with_sqlite_bind_budget() -> None:
+ """Account for repeated positional scope binds at the 100-id ceiling."""
+ engine: sa.Engine = sa.create_engine("sqlite://")
+ metadata: sa.MetaData = sa.MetaData()
+ table: sa.Table = prune_audit.PurgeAuditLog.__table__.to_metadata(metadata)
+ ids: list[UUID] = [uuid4() for _ in range(prune_audit.MAX_BATCH_SIZE)]
+ pairs: list[tuple[str, str | None]] = [
+ (f"type-{i}", f"entity-{i}") for i in range(len(ids))
+ ]
+ query: sa.sql.Select = sa.select(table.c.id).where(
+ table.c.id.in_(ids),
+ *prune_audit._duplicate_predicates(
+ table, datetime(2026, 1, 1), scope_entities=pairs
+ ),
+ )
+ compiled: sa.sql.compiler.Compiled = query.compile(
+ dialect=sqlite.dialect(), compile_kwargs={"render_postcompile": True}
+ )
+ assert 300 < len(compiled.params) < 350
+ assert compiled.positiontup is not None
+ assert 700 < len(compiled.positiontup) < 750
+ try:
+ metadata.create_all(engine)
+ with engine.connect() as connection:
+ assert list(connection.scalars(query)) == []
+ finally:
+ engine.dispose()
+
+
[email protected](
+ ("backend", "version", "uses_window"),
+ [
+ ("mysql", (5, 7, 44), False),
+ ("mysql", (8, 0, 36), True),
+ ("mysql", (8, 4, 0), True),
+ ("postgresql", (17, 0), True),
+ ("sqlite", (3, 45, 0), True),
+ ],
+)
[email protected]("initialized", [True, False])
+def test_repeat_dispatch_uses_metadata_server_version(
+ backend: str,
+ version: tuple[int, ...],
+ uses_window: bool,
+ initialized: bool,
+) -> None:
+ """Route compatible MySQL servers by version, including first
connection."""
+ dialect: sa.engine.Dialect = mysql.dialect()
+ dialect.name = backend
+ dialect.server_version_info = version if initialized else None
+ connected_dialect: sa.engine.Dialect = mysql.dialect()
+ connected_dialect.name = backend
+ connected_dialect.server_version_info = version
+ table: sa.Table = prune_audit.PurgeAuditLog.__table__
+ mock_db: MagicMock
+ with patch.object(prune_audit, "db") as mock_db:
+ mock_db.session.get_bind.return_value.dialect = dialect
+ mock_db.session.connection.return_value.dialect = connected_dialect
+ query: sa.sql.Select = sa.select(table.c.id).where(
+ prune_audit._repeats_an_earlier_block(table, datetime(2026, 2, 1))
+ )
+ sql: str = str(query.compile(dialect=mysql.dialect())).lower()
+ assert ("lag(" in sql) is uses_window
+ assert mock_db.session.connection.call_count == (
+ 1 if backend == "mysql" and not initialized else 0
+ )
+
+
[email protected](
+ ("history", "repeat_indices"),
+ [
+ pytest.param(
+ [
+ ("chart", STATUS_BLOCKED, 1, "same"),
+ ("dashboard", STATUS_BLOCKED, 2, "other"),
+ ("chart", STATUS_BLOCKED, 3, "same"),
+ ],
+ [2],
+ id="partition-reason-isolation",
+ ),
+ pytest.param(
+ [
+ ("dashboard", STATUS_CONFIRMED, 1, None),
+ ("chart", STATUS_BLOCKED, 5, "same"),
+ ("chart", STATUS_CONFIRMED, 6, None),
+ ("chart", STATUS_BLOCKED, 7, "same"),
+ ("chart", STATUS_BLOCKED, 8, "same"),
+ ],
+ [4],
+ id="boundary-join-isolation",
+ ),
+ pytest.param(
+ [
+ ("dashboard", STATUS_BLOCKED, 1, "same"),
+ ("chart", STATUS_BLOCKED, 2, "same"),
+ ("chart", STATUS_BLOCKED, 3, "same"),
+ ],
+ [2],
+ id="predecessor-type-isolation",
+ ),
+ pytest.param(
+ [
+ ("chart", STATUS_BLOCKED, 5, "same"),
+ ("chart", STATUS_BLOCKED, 7, "same"),
+ ("dashboard", STATUS_CONFIRMED, 9, None),
+ ],
+ [1],
+ id="foreign-newer-streak-breaker",
Review Comment:
Same function, other half (`prune_audit.py:326`): dropping
`boundary.entity_uuid == target.entity_uuid` survives all 152 unit tests and
the 40-seed equivalence test — two charts, different UUIDs, one confirmed
newer, and the other's "blocked since" row ages out. Fixture hardcodes the
UUID, no fence.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
