From d46eb8d504baffb82ce54aff16813e4289a83379 Mon Sep 17 00:00:00 2001 From: John Davis Date: Fri, 8 Mar 2024 16:05:04 -0500 Subject: [PATCH] Fix table/__table__/HasTable for models HasTable was a hack to accommodate declarative mapping without changing thousands of lines that referred to the `table` attribute of model instances. The one type:ignore added to managers.datasets can be removed after we map DatasetInstance models declaratively. --- lib/galaxy/managers/base.py | 4 ++-- lib/galaxy/managers/datasets.py | 2 +- lib/galaxy/model/__init__.py | 16 +++++++--------- .../data/model/test_mapping_testing_utils.py | 8 +++++--- test/unit/data/model/test_model_testing_utils.py | 8 +++++--- 5 files changed, 20 insertions(+), 18 deletions(-) diff --git a/lib/galaxy/managers/base.py b/lib/galaxy/managers/base.py index be487b4da675..5f3c4736cf3c 100644 --- a/lib/galaxy/managers/base.py +++ b/lib/galaxy/managers/base.py @@ -193,7 +193,7 @@ def get_object(trans, id, class_name, check_ownership=False, check_accessible=Fa # ============================================================================= -U = TypeVar("U", bound=model._HasTable) +U = TypeVar("U", bound=model.Base) # ----------------------------------------------------------------------------- @@ -999,7 +999,7 @@ class ModelFilterParser(HasAModelManager): # (as the model informs how the filter params are parsed) # I have no great idea where this 'belongs', so it's here for now - model_class: Type[model._HasTable] + model_class: Type[Union[model.Base, model.DatasetInstance]] parsed_filter = parsed_filter orm_filter_parsers: OrmFilterParsersType fn_filter_parsers: FunctionFilterParsersType diff --git a/lib/galaxy/managers/datasets.py b/lib/galaxy/managers/datasets.py index eb137a2bcfc2..d70ed422eb9e 100644 --- a/lib/galaxy/managers/datasets.py +++ b/lib/galaxy/managers/datasets.py @@ -338,7 +338,7 @@ def serialize_permissions(self, item, key, user=None, **context): # ============================================================================= AKA DatasetInstanceManager class DatasetAssociationManager( - base.ModelManager[model.DatasetInstance], + base.ModelManager[model.DatasetInstance], # type:ignore[type-var] secured.AccessibleManagerMixin, secured.OwnableManagerMixin, deletable.PurgableManagerMixin, diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 4099f769f029..59dd1941e54d 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -230,13 +230,6 @@ from galaxy.tools import DefaultToolState from galaxy.workflow.modules import WorkflowModule - class _HasTable: - table: FromClause - __table__: FromClause - -else: - _HasTable = object - def get_uuid(uuid: Optional[Union[UUID, str]] = None) -> UUID: if isinstance(uuid, UUID): @@ -246,12 +239,13 @@ def get_uuid(uuid: Optional[Union[UUID, str]] = None) -> UUID: return UUID(str(uuid)) -class Base(_HasTable, DeclarativeBase): +class Base(DeclarativeBase): __abstract__ = True metadata = MetaData(naming_convention=NAMING_CONVENTION) mapper_registry.metadata = metadata registry = mapper_registry __init__ = mapper_registry.constructor + table: FromClause @classmethod def __declare_last__(cls): @@ -4466,7 +4460,7 @@ def datatype_for_extension(extension, datatypes_registry=None) -> "Data": return ret -class DatasetInstance(RepresentById, UsesCreateAndUpdateTime, _HasTable): +class DatasetInstance(RepresentById, UsesCreateAndUpdateTime): """A base class for all 'dataset instances', HDAs, LDAs, etc""" states = Dataset.states @@ -4479,6 +4473,8 @@ class DatasetInstance(RepresentById, UsesCreateAndUpdateTime, _HasTable): copied_from_library_dataset_dataset_association: Optional["LibraryDatasetDatasetAssociation"] validated_states = DatasetValidatedState + table: FromClause + __table__: FromClause def __init__( self, @@ -11164,6 +11160,7 @@ def __repr__(self): "hidden_beneath_collection_instance_id", ForeignKey("history_dataset_collection_association.id"), nullable=True ), ) +HistoryDatasetAssociation.__table__ = HistoryDatasetAssociation.table LibraryDatasetDatasetAssociation.table = Table( "library_dataset_dataset_association", @@ -11208,6 +11205,7 @@ def __repr__(self): Column("user_id", Integer, ForeignKey("galaxy_user.id"), index=True), Column("message", TrimmedString(255)), ) +LibraryDatasetDatasetAssociation.__table__ = LibraryDatasetDatasetAssociation.table mapper_registry.map_imperatively( diff --git a/test/unit/data/model/test_mapping_testing_utils.py b/test/unit/data/model/test_mapping_testing_utils.py index 346ec2421403..cc99df4a4a2e 100644 --- a/test/unit/data/model/test_mapping_testing_utils.py +++ b/test/unit/data/model/test_mapping_testing_utils.py @@ -10,8 +10,8 @@ UniqueConstraint, ) from sqlalchemy.orm import registry +from sqlalchemy.sql.expression import FromClause -from galaxy.model import _HasTable from galaxy.model.unittest_utils.mapping_testing_utils import ( collection_consists_of_objects, has_index, @@ -75,14 +75,16 @@ def test_collection_consists_of_objects(session): @mapper_registry.mapped -class Foo(_HasTable): +class Foo: + __table__: FromClause __tablename__ = "foo" id = Column(Integer, primary_key=True) field1 = Column(Integer) @mapper_registry.mapped -class Bar(_HasTable): +class Bar: + __table__: FromClause __tablename__ = "bar" id = Column(Integer, primary_key=True) field1 = Column(Integer) diff --git a/test/unit/data/model/test_model_testing_utils.py b/test/unit/data/model/test_model_testing_utils.py index 94b7768de24f..7bbf02d62ae9 100644 --- a/test/unit/data/model/test_model_testing_utils.py +++ b/test/unit/data/model/test_model_testing_utils.py @@ -13,8 +13,8 @@ ) from sqlalchemy.exc import NoResultFound from sqlalchemy.orm import registry +from sqlalchemy.sql.expression import FromClause -from galaxy.model import _HasTable from galaxy.model.unittest_utils.model_testing_utils import ( dbcleanup, dbcleanup_wrapper, @@ -156,14 +156,16 @@ def managed(a, b): @mapper_registry.mapped -class Foo(_HasTable): +class Foo: + __table__: FromClause __tablename__ = "foo" id = Column(Integer, primary_key=True) field1 = Column(Integer) @mapper_registry.mapped -class Bar(_HasTable): +class Bar: + __table__: FromClause __tablename__ = "bar" id = Column(Integer, primary_key=True) field1 = Column(Integer)