Skip to content

Commit

Permalink
Fix table/__table__/HasTable for models
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jdavcs committed Apr 3, 2024
1 parent 8a8a422 commit d46eb8d
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 18 deletions.
4 changes: 2 additions & 2 deletions lib/galaxy/managers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)


# -----------------------------------------------------------------------------
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/galaxy/managers/datasets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
16 changes: 7 additions & 9 deletions lib/galaxy/model/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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):
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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(
Expand Down
8 changes: 5 additions & 3 deletions test/unit/data/model/test_mapping_testing_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 5 additions & 3 deletions test/unit/data/model/test_model_testing_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit d46eb8d

Please sign in to comment.