mikebridge commented on code in PR #41076: URL: https://github.com/apache/superset/pull/41076#discussion_r3574958428
########## superset/daos/version.py: ########## @@ -0,0 +1,69 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Backward-compat façade for the entity-versioning DAO surface. + +The actual implementation lives in :mod:`superset.versioning.queries` +(read side: list/get/resolve/find/UUID derivation). This module +re-exports it under a single ``VersionDAO`` class plus the module-level +UUID helpers so existing callers keep working without changes. (The +write side — restore + audit stamping — ships in a later PR; only the +read surface is wired here.) + +New code should import from the versioning sub-modules directly. +""" + +from __future__ import annotations + +from superset.versioning.queries import ( + current_live_transaction_id, + current_live_version_uuid, + current_version_number, + derive_version_uuid, + derive_version_uuid as _derive_version_uuid, # noqa: F401 + find_active_by_uuid, + get_version, + list_change_records_batch, + list_versions, + resolve_version_uuid, + VERSION_UUID_NAMESPACE, +) + +# Re-exports for ``from superset.daos.version import …`` consumers. +__all__ = [ + "VERSION_UUID_NAMESPACE", + "VersionDAO", + "derive_version_uuid", +] Review Comment: Addressed by subsequent commits; the flagged code is no longer current after the rebase onto master. Resolving as outdated. ########## superset/datasets/api.py: ########## @@ -440,17 +504,69 @@ def put(self, pk: int) -> Response: # This validates custom Schema with custom validations except ValidationError as error: return self.response_400(message=error.messages) + + # Live version identifiers before the update (empty + query-free when + # ``ENABLE_VERSIONING_CAPTURE`` is off). + old_info = current_entity_version_info(SqlaTable, pk) Review Comment: Addressed by subsequent commits; the flagged code is no longer current after the rebase onto master. Resolving as outdated. ########## superset/datasets/api.py: ########## @@ -440,17 +504,69 @@ def put(self, pk: int) -> Response: # This validates custom Schema with custom validations except ValidationError as error: return self.response_400(message=error.messages) + + # Live version identifiers before the update (empty + query-free when + # ``ENABLE_VERSIONING_CAPTURE`` is off). + old_info = current_entity_version_info(SqlaTable, pk) + try: + # Two commands, two commits, two Continuum transactions for an + # ``override_columns`` save — deliberately NOT merged into one + # transaction. A single-transaction design was attempted and + # reverted: ``DBEventLogger`` writes request logs through the + # SHARED scoped session and calls ``commit()`` / + # ``rollback()`` on it mid-request (superset/utils/log.py), + # so any save held uncommitted across a logged sub-action can + # be committed half-done (Postgres/MySQL) or rolled back + # entirely on a transient logger failure (SQLite's + # "database is locked"). Until the event logger gets its own + # session, per-command commit boundaries are the only shape + # whose failure modes are honest. Consequence the + # version-history UI must tolerate: one logical save can + # surface as two version transactions stamped the same second. changed_model = UpdateDatasetCommand(pk, item, override_columns).run() + # Capture the post-update identifiers BEFORE the refresh: + # RefreshDatasetCommand commits its own transaction, so reading + # afterwards would attribute the refresh's version to the + # user's update (and old→new would span two transactions). + new_info = current_entity_version_info( + SqlaTable, changed_model.id, changed_model.uuid + ) Review Comment: Addressed by subsequent commits; the flagged code is no longer current after the rebase onto master. Resolving as outdated. ########## superset/datasets/api.py: ########## @@ -440,17 +504,69 @@ def put(self, pk: int) -> Response: # This validates custom Schema with custom validations except ValidationError as error: return self.response_400(message=error.messages) + + # Live version identifiers before the update (empty + query-free when + # ``ENABLE_VERSIONING_CAPTURE`` is off). + old_info = current_entity_version_info(SqlaTable, pk) + try: + # Two commands, two commits, two Continuum transactions for an + # ``override_columns`` save — deliberately NOT merged into one + # transaction. A single-transaction design was attempted and + # reverted: ``DBEventLogger`` writes request logs through the + # SHARED scoped session and calls ``commit()`` / + # ``rollback()`` on it mid-request (superset/utils/log.py), + # so any save held uncommitted across a logged sub-action can + # be committed half-done (Postgres/MySQL) or rolled back + # entirely on a transient logger failure (SQLite's + # "database is locked"). Until the event logger gets its own + # session, per-command commit boundaries are the only shape + # whose failure modes are honest. Consequence the + # version-history UI must tolerate: one logical save can + # surface as two version transactions stamped the same second. changed_model = UpdateDatasetCommand(pk, item, override_columns).run() + # Capture the post-update identifiers BEFORE the refresh: + # RefreshDatasetCommand commits its own transaction, so reading + # afterwards would attribute the refresh's version to the + # user's update (and old→new would span two transactions). + new_info = current_entity_version_info( + SqlaTable, changed_model.id, changed_model.uuid + ) + etag_version_uuid = new_info.version_uuid Review Comment: Addressed by subsequent commits; the flagged code is no longer current after the rebase onto master. Resolving as outdated. ########## scripts/seed_junction_load.py: ########## @@ -0,0 +1,679 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +# ---------------------------------------------------------------------- +# Stress-test data generator for the composite-PK migration (sc-105349). +# +# Bulk-inserts synthetic parent rows and many-to-many junction rows for +# the eight association tables that the composite-PK migration touches. +# Useful for measuring migration runtime at varying scales — run this at +# 100K / 1M / 5M / 10M rows and time the migration at each scale to +# verify the O(N log N) extrapolation. +# +# Idempotent: rerunning with the same target is a no-op; rerunning with +# a higher target adds rows up to the new total. Batched bulk INSERTs +# (10K rows per statement) make it fast on Postgres, MySQL, and SQLite. +# +# Usage (inside the Superset container): +# +# docker exec superset-superset-1 \\ +# /app/.venv/bin/python /app/scripts/seed_junction_load.py \\ +# --dashboard-slices 1000000 \\ +# --slice-user 100000 \\ +# --dashboard-user 100000 +# +# Run with no flags for the defaults shown below. Use ``--dry-run`` to +# print the planned inserts without writing anything. +# +# The script connects via Superset's standard ``DATABASE_*`` env vars +# (or ``SUPERSET__SQLALCHEMY_DATABASE_URI`` if set), so it works +# automatically inside the Superset container regardless of which +# metadata DB backend is in use. + +from __future__ import annotations + +import argparse +import logging +import os +import sys +import time +from contextlib import contextmanager +from typing import Iterator +from uuid import uuid4 + +import sqlalchemy as sa +from sqlalchemy.engine import Connection, Engine + +logger = logging.getLogger("seed_junction_load") Review Comment: Addressed by subsequent commits; the flagged code is no longer current after the rebase onto master. Resolving as outdated. ########## scripts/seed_junction_load.py: ########## @@ -0,0 +1,679 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +# ---------------------------------------------------------------------- +# Stress-test data generator for the composite-PK migration (sc-105349). +# +# Bulk-inserts synthetic parent rows and many-to-many junction rows for +# the eight association tables that the composite-PK migration touches. +# Useful for measuring migration runtime at varying scales — run this at +# 100K / 1M / 5M / 10M rows and time the migration at each scale to +# verify the O(N log N) extrapolation. +# +# Idempotent: rerunning with the same target is a no-op; rerunning with +# a higher target adds rows up to the new total. Batched bulk INSERTs +# (10K rows per statement) make it fast on Postgres, MySQL, and SQLite. +# +# Usage (inside the Superset container): +# +# docker exec superset-superset-1 \\ +# /app/.venv/bin/python /app/scripts/seed_junction_load.py \\ +# --dashboard-slices 1000000 \\ +# --slice-user 100000 \\ +# --dashboard-user 100000 +# +# Run with no flags for the defaults shown below. Use ``--dry-run`` to +# print the planned inserts without writing anything. +# +# The script connects via Superset's standard ``DATABASE_*`` env vars +# (or ``SUPERSET__SQLALCHEMY_DATABASE_URI`` if set), so it works +# automatically inside the Superset container regardless of which +# metadata DB backend is in use. + +from __future__ import annotations + +import argparse +import logging +import os +import sys +import time +from contextlib import contextmanager +from typing import Iterator +from uuid import uuid4 + +import sqlalchemy as sa +from sqlalchemy.engine import Connection, Engine + +logger = logging.getLogger("seed_junction_load") + +# Bulk INSERT batch size. Larger values = fewer statements but more memory. +BATCH = 10_000 Review Comment: Addressed by subsequent commits; the flagged code is no longer current after the rebase onto master. Resolving as outdated. -- 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]
