mikebridge commented on code in PR #44928: URL: https://github.com/apache/superset/pull/44928#discussion_r4221865621
########## superset/versioning/unit_of_work.py: ########## @@ -16,12 +16,39 @@ # under the License. """Runtime capture suppression at Continuum's transaction-scoped write boundary.""" +from itertools import chain + +from sqlalchemy.engine import Connection from sqlalchemy.orm import Session +from sqlalchemy_continuum import versioning_manager from sqlalchemy_continuum.operation import Operations from sqlalchemy_continuum.unit_of_work import UnitOfWork +from sqlalchemy_continuum.utils import is_versioned from superset.versioning.utils import capture_enabled +INITIAL_STATES_KEY: str = "_version_changes_initial_states" + + +def _has_versioned_work(session: Session) -> bool: + """Recognize pending parents, children, deletes, or retained pre-flush state.""" + return bool(session.info.get(INITIAL_STATES_KEY)) or any( + is_versioned(obj) for obj in chain(session.new, session.dirty, session.deleted) Review Comment: Thanks for the detailed comparison. On this tree the performance target is hosts with an installed predicate: when it's `None`, `capture_enabled` does no host lookup, while `_has_versioned_work` still scans pending objects. Could we treat a default-policy fast path as a separate, measured optimization, with tests that preserve deferred decisions and finalizer sample semantics, rather than imply an OSS-default speedup here? The hygiene follow-up is in `d2d69c3632`. -- 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]
