Skip to content

Commit

Permalink
Merge pull request #17122 from jdavcs/dev_sa20_fix22
Browse files Browse the repository at this point in the history
SA2.0 updates: handling "object is being merged into a Session along the backref cascade path"
  • Loading branch information
mvdbeek authored Dec 6, 2023
2 parents 472bc3f + 631e84d commit 9398079
Show file tree
Hide file tree
Showing 11 changed files with 173 additions and 40 deletions.
10 changes: 9 additions & 1 deletion lib/galaxy/managers/workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@
WorkflowInvocation,
WorkflowInvocationStep,
)
from galaxy.model.base import transaction
from galaxy.model.base import (
ensure_object_added_to_session,
transaction,
)
from galaxy.model.index_filter_util import (
append_user_filter,
raw_text_column_filter,
Expand Down Expand Up @@ -818,6 +821,11 @@ def _workflow_from_raw_description(

workflow.has_cycles = True
workflow.steps = steps
# Safeguard: workflow was implicitly merged into this Session prior to SQLAlchemy 2.0.
# when AT LEAST ONE step in steps belonged to a session.
for step in steps:
if ensure_object_added_to_session(workflow, object_in_session=step):
break

comments: List[model.WorkflowComment] = []
comments_by_external_id: Dict[str, model.WorkflowComment] = {}
Expand Down
124 changes: 91 additions & 33 deletions lib/galaxy/model/__init__.py

Large diffs are not rendered by default.

20 changes: 20 additions & 0 deletions lib/galaxy/model/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

from sqlalchemy import event
from sqlalchemy.orm import (
object_session,
scoped_session,
Session,
sessionmaker,
Expand Down Expand Up @@ -167,3 +168,22 @@ def before_flush(session, flush_context, instances):
obj.__create_version__(session)
for obj in versioned_objects(session.deleted):
obj.__create_version__(session, deleted=True)


def ensure_object_added_to_session(object_to_add, *, object_in_session=None, session=None) -> bool:
"""
This function is intended as a safeguard to mimic pre-SQLAlchemy 2.0 behavior.
`object_to_add` was implicitly merged into a Session prior to SQLAlchemy 2.0, which was indicated
by `RemovedIn20Warning` warnings logged while running Galaxy's tests. (See https://github.com/galaxyproject/galaxy/issues/12541)
As part of the upgrade to 2.0, the `cascade_backrefs=False` argument was added to the relevant relationships that turned off this behavior.
This function is called from the code that triggered these warnings, thus emulating the cascading behavior.
The intention is to remove all such calls, as well as this function definition, after the move to SQLAlchemy 2.0.
# Ref: https://docs.sqlalchemy.org/en/14/changelog/migration_14.html#cascade-backrefs-behavior-deprecated-for-removal-in-2-0
"""
if session:
session.add(object_to_add)
return True
if object_in_session and object_session(object_in_session):
object_session(object_in_session).add(object_to_add)
return True
return False
20 changes: 20 additions & 0 deletions lib/galaxy/model/database_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
)
from sqlalchemy.engine import Engine
from sqlalchemy.engine.url import make_url
from sqlalchemy.orm import object_session
from sqlalchemy.sql.compiler import IdentifierPreparer
from sqlalchemy.sql.expression import (
ClauseElement,
Expand Down Expand Up @@ -175,3 +176,22 @@ def _statement_executed_without_error(statement: ClauseElement, engine: Engine)

def is_postgres(url: DbUrl) -> bool:
return url.startswith("postgres")


def ensure_object_added_to_session(object_to_add, *, object_in_session=None, session=None) -> bool:
"""
This function is intended as a safeguard to mimic pre-SQLAlchemy 2.0 behavior.
`object_to_add` was implicitly merged into a Session prior to SQLAlchemy 2.0, which was indicated
by `RemovedIn20Warning` warnings logged while running Galaxy's tests. (See https://github.com/galaxyproject/galaxy/issues/12541)
As part of the upgrade to 2.0, the `cascade_backrefs=False` argument was added to the relevant relationships that turned off this behavior.
This function is called from the code that triggered these warnings, thus emulating the cascading behavior.
The intention is to remove all such calls, as well as this function definition, after the move to SQLAlchemy 2.0.
# Ref: https://docs.sqlalchemy.org/en/14/changelog/migration_14.html#cascade-backrefs-behavior-deprecated-for-removal-in-2-0
"""
if session:
session.add(object_to_add)
return True
if object_in_session and object_session(object_in_session):
object_session(object_in_session).add(object_to_add)
return True
return False
7 changes: 6 additions & 1 deletion lib/galaxy/model/store/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@
ProvidesUserFileSourcesUserContext,
)
from galaxy.files.uris import stream_url_to_file
from galaxy.model.base import transaction
from galaxy.model.base import (
ensure_object_added_to_session,
transaction,
)
from galaxy.model.mapping import GalaxyModelMapping
from galaxy.model.metadata import MetadataCollection
from galaxy.model.orm.util import (
Expand Down Expand Up @@ -1024,6 +1027,7 @@ def _import_workflow_invocations(
imported_invocation = model.WorkflowInvocation()
imported_invocation.user = self.user
imported_invocation.history = history
ensure_object_added_to_session(imported_invocation, object_in_session=history)
workflow_key = invocation_attrs["workflow"]
if workflow_key not in object_import_tracker.workflows_by_key:
raise Exception(f"Failed to find key {workflow_key} in {object_import_tracker.workflows_by_key.keys()}")
Expand All @@ -1045,6 +1049,7 @@ def attach_workflow_step(imported_object, attrs):
for step_attrs in invocation_attrs["steps"]:
imported_invocation_step = model.WorkflowInvocationStep()
imported_invocation_step.workflow_invocation = imported_invocation
ensure_object_added_to_session(imported_invocation, session=self.sa_session)
attach_workflow_step(imported_invocation_step, step_attrs)
restore_times(imported_invocation_step, step_attrs)
imported_invocation_step.action = step_attrs["action"]
Expand Down
6 changes: 5 additions & 1 deletion lib/galaxy/webapps/base/webapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@
from galaxy.managers import context
from galaxy.managers.session import GalaxySessionManager
from galaxy.managers.users import UserManager
from galaxy.model.base import transaction
from galaxy.model.base import (
ensure_object_added_to_session,
transaction,
)
from galaxy.structured_app import (
BasicSharedApp,
MinimalApp,
Expand Down Expand Up @@ -1119,6 +1122,7 @@ def create_new_session(trans, prev_galaxy_session=None, user_for_new_session=Non
if user_for_new_session:
# The new session should be associated with the user
galaxy_session.user = user_for_new_session
ensure_object_added_to_session(galaxy_session, object_in_session=user_for_new_session)
return galaxy_session


Expand Down
6 changes: 5 additions & 1 deletion lib/galaxy/workflow/extract.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@
exceptions,
model,
)
from galaxy.model.base import transaction
from galaxy.model.base import (
ensure_object_added_to_session,
transaction,
)
from galaxy.tool_util.parser import ToolOutputCollectionPart
from galaxy.tools.parameters.basic import (
DataCollectionToolParameter,
Expand Down Expand Up @@ -71,6 +74,7 @@ def extract_workflow(
workflow.stored_workflow = stored
stored.latest_workflow = workflow
trans.sa_session.add(stored)
ensure_object_added_to_session(workflow, session=trans.sa_session)
with transaction(trans.sa_session):
trans.sa_session.commit()
return stored
Expand Down
2 changes: 2 additions & 0 deletions lib/galaxy/workflow/modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
WorkflowStep,
WorkflowStepConnection,
)
from galaxy.model.base import ensure_object_added_to_session
from galaxy.model.dataset_collections import matching
from galaxy.schema.invocation import (
CancelReason,
Expand Down Expand Up @@ -644,6 +645,7 @@ def from_workflow_step(Class, trans, step, **kwds):
def save_to_step(self, step, **kwd):
step.type = self.type
step.subworkflow = self.subworkflow
ensure_object_added_to_session(step, object_in_session=self.subworkflow)

def get_name(self):
if hasattr(self.subworkflow, "name"):
Expand Down
6 changes: 5 additions & 1 deletion lib/galaxy/workflow/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@
WorkflowInvocation,
WorkflowInvocationStep,
)
from galaxy.model.base import transaction
from galaxy.model.base import (
ensure_object_added_to_session,
transaction,
)
from galaxy.schema.invocation import (
CancelReason,
FailureReason,
Expand Down Expand Up @@ -225,6 +228,7 @@ def invoke(self) -> Dict[int, Any]:
workflow_invocation_step = WorkflowInvocationStep()
assert workflow_invocation_step
workflow_invocation_step.workflow_invocation = workflow_invocation
ensure_object_added_to_session(workflow_invocation_step, object_in_session=workflow_invocation)
workflow_invocation_step.workflow_step = step
workflow_invocation_step.state = "new"

Expand Down
6 changes: 5 additions & 1 deletion lib/galaxy/workflow/run_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@
WorkflowRequestInputParameter,
WorkflowRequestStepState,
)
from galaxy.model.base import transaction
from galaxy.model.base import (
ensure_object_added_to_session,
transaction,
)
from galaxy.tools.parameters.meta import expand_workflow_inputs
from galaxy.workflow.resources import get_resource_mapper_function

Expand Down Expand Up @@ -486,6 +489,7 @@ def workflow_run_config_to_request(
workflow_invocation = WorkflowInvocation()
workflow_invocation.uuid = uuid.uuid1()
workflow_invocation.history = run_config.target_history
ensure_object_added_to_session(workflow_invocation, object_in_session=run_config.target_history)

def add_parameter(name: str, value: str, type: WorkflowRequestInputParameter.types) -> None:
parameter = WorkflowRequestInputParameter(
Expand Down
6 changes: 5 additions & 1 deletion test/integration/test_job_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@
from sqlalchemy import select

from galaxy import model
from galaxy.model.base import transaction
from galaxy.model.base import (
ensure_object_added_to_session,
transaction,
)
from galaxy_test.base import api_asserts
from galaxy_test.base.populators import DatasetPopulator
from galaxy_test.driver import integration_util
Expand Down Expand Up @@ -138,6 +141,7 @@ def create_static_job_with_state(self, state):
sa_session.commit()
job = model.Job()
job.history = history
ensure_object_added_to_session(job, object_in_session=history)
job.user = user
job.handler = "unknown-handler"
job.state = state
Expand Down

0 comments on commit 9398079

Please sign in to comment.