SQLAlchemy 2.0 #17180
Closed
SQLAlchemy 2.0 #17180
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jdavcs
added
kind/enhancement
area/database
jdavcs
force-pushed
the
dev_sa20
branch
2 times, most recently
from
036a728 to
ab7c9a5
Compare
jdavcs
force-pushed
the
dev_sa20
branch
3 times, most recently
from
6efeb8c to
19be969
Compare
jdavcs
force-pushed
the
dev_sa20
branch
6 times, most recently
from
32ad1b0 to
a7e68ba
Compare
jdavcs
force-pushed
the
dev_sa20
branch
7 times, most recently
from
6b87c52 to
1870645
Compare
jdavcs
force-pushed
the
dev_sa20
branch
8 times, most recently
from
e0bd349 to
207299f
Compare
Also, minor SA2.0 syntax fix
1. In 2.0, when the statement contains "returning", the result type is ChunkedIteratorResult, which does not have the rowcount attr, becuase: 2. result.rowcount should not be used for statements containting the returning clause Ref: https://docs.sqlalchemy.org/en/20/core/connections.html#sqlalchemy.engine.CursorResult.rowcount
Otherwise there's an idle transaction left in the database (+locks)
Same as prev. commit: otherwise db locks are left
This restores the behavior under SQLAlchemy 1.4 (Note that we set the pool for sqlite only if it's not an in-memory db
jdavcs
added a commit
to jdavcs/galaxy
that referenced
this pull request
Mar 7, 2024
Upgrade SQLAlchemy to 2.0 This conflicts with dependency requirements for sqlalchemy-graphene (used only in toolshed, new WIP client) Remove RemovedIn20Warning from config This does not exist in SQLAlchemy 2.0 Update import path for DeclarativeMeta Move declaration of injected attrs into constructor Remove unused import For context: https://github.com/galaxyproject/galaxy/pull/14717/files#r1486979280 Also, remove model attr type hints that conflict with SA2.0 Apply Mapped/mapped_column to model definitions Included models: galaxy, tool shed, tool shed install Column types: DateTime Integer Boolan Unicode String (Text/TEXT/TrimmedString/VARCHAR) UUID Numeric NOTE on typing of nullability: db schema != python app - Mapped[datetime] specifies correct type for the python app; - nullable=True specifies correct mapping to the db schema (that's what the CREATE TABLE sql statement will reflect). mapped_column.nullable takes precedence over typing annotation of Mapped. So, if we have: foo: Mapped[str] = mapped_column(String, nullable=True) - that means that the foo db field will allow NULL, but the python app will not allow foo = None. And vice-versa: bar: Mapped[Optional[str]] = mapped_column(String, nullable=False) - the bar db field is NOT NULL, but bar = None is OK. This might need to be applied to other column definitions, but for now this addresses specific mypy errors. Ref: https://docs.sqlalchemy.org/en/20/orm/declarative_tables.html#mapped-column-derives-the-datatype-and-nullability-from-the-mapped-annotation Add typing to JSON columns, fix related mypy errors Columns: MutableJSONType JSONType DoubleEncodedJsonType TODO: I think we need a type alias for json-typed columns: bytes understand iteration, but not access by key. Use correct type hints to define common model attrs Start applying Mapped to relationship definitions in the model Remove column declaration from HasTags parent class Fix SA2.0 error: wrap sql in text() Fix SA2.0 error: pass bind to create_all Fix SA2.0 error: use Row._mapping for keyed attribute access Ref: https://docs.sqlalchemy.org/en/20/changelog/migration_20.html#result-rows-act-like-named-tuples Fix SA2.0 error: show password in url SA 1.4: str(url) renders connection string with password SA 2.0: str(url) renders connection string WITHOUT password Solution: Use render_as_string(hide_password=False) Fix SA2.0 error: use attribute_keyed_dict Replaces attribute_mapped_collection (SA20) Fix SA2.0 error: make select stmt a subquery Rename varable to fix mypy Fix SA2.0 error: explicitly use subquery() for select-from argument Fix SA2.0 error: replase session.bind with session.get_bind() Fix SA2.0 error: joinedload does not take str args Fix use of table model attribute - Use __table__ (SA attr) instead of table (galaxy attr) on mapped classes - Drop .table and .table.c where redundant Fix bug: fix HistoryAudit model It is not a child of RepresentById becuase it does not and should not have an id attr. Duplicating the __repr__ definition in the HistoryAudit class is a temporary fix: a proper fix requires changing all models (id and __repr__ should be split into 2 mixins): to be done in a follow-up PR. Fix bug: check if template.fields is not null before iterating Fix bug: call unique() on result, not select stmt Fix bug: do not pass subquery to in_ Fix bug/typo: use select_from Fix bug: if using alias on ORM entity, use __table__ as valid FromClause Fix bug: HDAH model is not serializable (caught by mypy) Fix typing error: migrations.base Fix typing error: managers.secured This fixed 58 mypy errors! Fix typing error: session type Fix typing error: use Session instead of scoped_session No need to pass around scoped_session as arguments Fix typing error: sharable Fix SA2.0 error: sqlalchemy exceptions import; minor mypy fix Mypy: type-ignore: this is never SessionlessContext Mypy: use verbose assignment to help mypy Mypy: add assert stmt Mypy: add assert to ensure seesion is not None Calling that method when a User obj is not attached to a session should not happen. Mypy: return 0 if no results Mypy: type-ignore: scoped_session vs. install_model_session We use the disctinction for DI. Mypy: refactor to one-liner Mypy: add assert stmts where we know session returns an object Mypy: rename wfi_step > wfi_step_sq when it becomes a subquery Job search refactor: factor out build_job_subquery Job search refactor: build_stmt_for_hda Job search refactor: build_stmt_for_ldda Job search refactor: build_stmt_for_hdca Job search refactor: build_stmt_for_dce Job search refactor: rename query >> stmt Mypy: add anno for Lists; type-ignore for HDAs Note: type-ignore is due to imperative mapping of HDAs (and LDDAs). This will be removed once we map those models declaratively Mypy: managers.histories Mypy: model.deferred Mypy: arg passed to template can be None Mypy: celery tasks type ignore arg: we need to map DatasetInstance classes declaratively for that to work correctly. Mypy: type-ignore hda attr-defined error Need to map declaratively to remove this Convert visualization manager index query to SA Core Mypy: session is not none Mypy: type-ignore what requires more refactoring Mypy: type-ignore hda, ldda attrs: need declarative mapping Also, minor SA2.0 syntax fix Mypy: type-ignores to handle late evaluation of relationship arguments Mypy: type-ignore column property assignments (type is correct) Mypy: typing errors, misc. fixes Mypy: all statements are reachable Mypy: need to map hda declaratively, then its parent is model.Base Fix typing errors: sharable, secured Fix package mypy errors Fix SA2.0 error: celery task 1. In 2.0, when the statement contains "returning", the result type is ChunkedIteratorResult, which does not have the rowcount attr, becuase: 2. result.rowcount should not be used for statements containting the returning clause Ref: https://docs.sqlalchemy.org/en/20/core/connections.html#sqlalchemy.engine.CursorResult.rowcount Wrap call to ensure session is closed Otherwise there's an idle transaction left in the database (+locks) Ensure session is closed on TS Registry load Same as prev. commit: otherwise db locks are left Fix SA2.0 error: list arg to select; mypy Use NullPool for sqlite engines This restores the behavior under SQLAlchemy 1.4 (Note that we set the pool for sqlite only if it's not an in-memory db Help mypy: job is never None
jdavcs
added a commit
to jdavcs/galaxy
that referenced
this pull request
Mar 8, 2024
Upgrade SQLAlchemy to 2.0 This conflicts with dependency requirements for sqlalchemy-graphene (used only in toolshed, new WIP client) Remove RemovedIn20Warning from config This does not exist in SQLAlchemy 2.0 Update import path for DeclarativeMeta Move declaration of injected attrs into constructor Remove unused import For context: https://github.com/galaxyproject/galaxy/pull/14717/files#r1486979280 Also, remove model attr type hints that conflict with SA2.0 Apply Mapped/mapped_column to model definitions Included models: galaxy, tool shed, tool shed install Column types: DateTime Integer Boolan Unicode String (Text/TEXT/TrimmedString/VARCHAR) UUID Numeric NOTE on typing of nullability: db schema != python app - Mapped[datetime] specifies correct type for the python app; - nullable=True specifies correct mapping to the db schema (that's what the CREATE TABLE sql statement will reflect). mapped_column.nullable takes precedence over typing annotation of Mapped. So, if we have: foo: Mapped[str] = mapped_column(String, nullable=True) - that means that the foo db field will allow NULL, but the python app will not allow foo = None. And vice-versa: bar: Mapped[Optional[str]] = mapped_column(String, nullable=False) - the bar db field is NOT NULL, but bar = None is OK. This might need to be applied to other column definitions, but for now this addresses specific mypy errors. Ref: https://docs.sqlalchemy.org/en/20/orm/declarative_tables.html#mapped-column-derives-the-datatype-and-nullability-from-the-mapped-annotation Add typing to JSON columns, fix related mypy errors Columns: MutableJSONType JSONType DoubleEncodedJsonType TODO: I think we need a type alias for json-typed columns: bytes understand iteration, but not access by key. Use correct type hints to define common model attrs Start applying Mapped to relationship definitions in the model Remove column declaration from HasTags parent class Fix SA2.0 error: wrap sql in text() Fix SA2.0 error: pass bind to create_all Fix SA2.0 error: use Row._mapping for keyed attribute access Ref: https://docs.sqlalchemy.org/en/20/changelog/migration_20.html#result-rows-act-like-named-tuples Fix SA2.0 error: show password in url SA 1.4: str(url) renders connection string with password SA 2.0: str(url) renders connection string WITHOUT password Solution: Use render_as_string(hide_password=False) Fix SA2.0 error: use attribute_keyed_dict Replaces attribute_mapped_collection (SA20) Fix SA2.0 error: make select stmt a subquery Rename varable to fix mypy Fix SA2.0 error: explicitly use subquery() for select-from argument Fix SA2.0 error: replase session.bind with session.get_bind() Fix SA2.0 error: joinedload does not take str args Fix use of table model attribute - Use __table__ (SA attr) instead of table (galaxy attr) on mapped classes - Drop .table and .table.c where redundant Fix bug: fix HistoryAudit model It is not a child of RepresentById becuase it does not and should not have an id attr. Duplicating the __repr__ definition in the HistoryAudit class is a temporary fix: a proper fix requires changing all models (id and __repr__ should be split into 2 mixins): to be done in a follow-up PR. Fix bug: check if template.fields is not null before iterating Fix bug: call unique() on result, not select stmt Fix bug: do not pass subquery to in_ Fix bug/typo: use select_from Fix bug: if using alias on ORM entity, use __table__ as valid FromClause Fix bug: HDAH model is not serializable (caught by mypy) Fix typing error: migrations.base Fix typing error: managers.secured This fixed 58 mypy errors! Fix typing error: session type Fix typing error: use Session instead of scoped_session No need to pass around scoped_session as arguments Fix typing error: sharable Fix SA2.0 error: sqlalchemy exceptions import; minor mypy fix Mypy: type-ignore: this is never SessionlessContext Mypy: use verbose assignment to help mypy Mypy: add assert stmt Mypy: add assert to ensure seesion is not None Calling that method when a User obj is not attached to a session should not happen. Mypy: return 0 if no results Mypy: type-ignore: scoped_session vs. install_model_session We use the disctinction for DI. Mypy: refactor to one-liner Mypy: add assert stmts where we know session returns an object Mypy: rename wfi_step > wfi_step_sq when it becomes a subquery Job search refactor: factor out build_job_subquery Job search refactor: build_stmt_for_hda Job search refactor: build_stmt_for_ldda Job search refactor: build_stmt_for_hdca Job search refactor: build_stmt_for_dce Job search refactor: rename query >> stmt Mypy: add anno for Lists; type-ignore for HDAs Note: type-ignore is due to imperative mapping of HDAs (and LDDAs). This will be removed once we map those models declaratively Mypy: managers.histories Mypy: model.deferred Mypy: arg passed to template can be None Mypy: celery tasks type ignore arg: we need to map DatasetInstance classes declaratively for that to work correctly. Mypy: type-ignore hda attr-defined error Need to map declaratively to remove this Convert visualization manager index query to SA Core Mypy: session is not none Mypy: type-ignore what requires more refactoring Mypy: type-ignore hda, ldda attrs: need declarative mapping Also, minor SA2.0 syntax fix Mypy: type-ignores to handle late evaluation of relationship arguments Mypy: type-ignore column property assignments (type is correct) Mypy: typing errors, misc. fixes Mypy: all statements are reachable Mypy: need to map hda declaratively, then its parent is model.Base Fix typing errors: sharable, secured Fix package mypy errors Fix SA2.0 error: celery task 1. In 2.0, when the statement contains "returning", the result type is ChunkedIteratorResult, which does not have the rowcount attr, becuase: 2. result.rowcount should not be used for statements containting the returning clause Ref: https://docs.sqlalchemy.org/en/20/core/connections.html#sqlalchemy.engine.CursorResult.rowcount Wrap call to ensure session is closed Otherwise there's an idle transaction left in the database (+locks) Ensure session is closed on TS Registry load Same as prev. commit: otherwise db locks are left Fix SA2.0 error: list arg to select; mypy Use NullPool for sqlite engines This restores the behavior under SQLAlchemy 1.4 (Note that we set the pool for sqlite only if it's not an in-memory db Help mypy: job is never None
4 tasks
jdavcs
added a commit
to jdavcs/galaxy
that referenced
this pull request
Mar 11, 2024
Upgrade SQLAlchemy to 2.0 This conflicts with dependency requirements for sqlalchemy-graphene (used only in toolshed, new WIP client) Remove RemovedIn20Warning from config This does not exist in SQLAlchemy 2.0 Update import path for DeclarativeMeta Move declaration of injected attrs into constructor Remove unused import For context: https://github.com/galaxyproject/galaxy/pull/14717/files#r1486979280 Also, remove model attr type hints that conflict with SA2.0 Apply Mapped/mapped_column to model definitions Included models: galaxy, tool shed, tool shed install Column types: DateTime Integer Boolan Unicode String (Text/TEXT/TrimmedString/VARCHAR) UUID Numeric NOTE on typing of nullability: db schema != python app - Mapped[datetime] specifies correct type for the python app; - nullable=True specifies correct mapping to the db schema (that's what the CREATE TABLE sql statement will reflect). mapped_column.nullable takes precedence over typing annotation of Mapped. So, if we have: foo: Mapped[str] = mapped_column(String, nullable=True) - that means that the foo db field will allow NULL, but the python app will not allow foo = None. And vice-versa: bar: Mapped[Optional[str]] = mapped_column(String, nullable=False) - the bar db field is NOT NULL, but bar = None is OK. This might need to be applied to other column definitions, but for now this addresses specific mypy errors. Ref: https://docs.sqlalchemy.org/en/20/orm/declarative_tables.html#mapped-column-derives-the-datatype-and-nullability-from-the-mapped-annotation Add typing to JSON columns, fix related mypy errors Columns: MutableJSONType JSONType DoubleEncodedJsonType TODO: I think we need a type alias for json-typed columns: bytes understand iteration, but not access by key. Use correct type hints to define common model attrs Start applying Mapped to relationship definitions in the model Remove column declaration from HasTags parent class Fix SA2.0 error: wrap sql in text() Fix SA2.0 error: pass bind to create_all Fix SA2.0 error: use Row._mapping for keyed attribute access Ref: https://docs.sqlalchemy.org/en/20/changelog/migration_20.html#result-rows-act-like-named-tuples Fix SA2.0 error: show password in url SA 1.4: str(url) renders connection string with password SA 2.0: str(url) renders connection string WITHOUT password Solution: Use render_as_string(hide_password=False) Fix SA2.0 error: use attribute_keyed_dict Replaces attribute_mapped_collection (SA20) Fix SA2.0 error: make select stmt a subquery Rename varable to fix mypy Fix SA2.0 error: explicitly use subquery() for select-from argument Fix SA2.0 error: replase session.bind with session.get_bind() Fix SA2.0 error: joinedload does not take str args Fix use of table model attribute - Use __table__ (SA attr) instead of table (galaxy attr) on mapped classes - Drop .table and .table.c where redundant Fix bug: fix HistoryAudit model It is not a child of RepresentById becuase it does not and should not have an id attr. Duplicating the __repr__ definition in the HistoryAudit class is a temporary fix: a proper fix requires changing all models (id and __repr__ should be split into 2 mixins): to be done in a follow-up PR. Fix bug: check if template.fields is not null before iterating Fix bug: call unique() on result, not select stmt Fix bug: do not pass subquery to in_ Fix bug/typo: use select_from Fix bug: if using alias on ORM entity, use __table__ as valid FromClause Fix bug: HDAH model is not serializable (caught by mypy) Fix typing error: migrations.base Fix typing error: managers.secured This fixed 58 mypy errors! Fix typing error: session type Fix typing error: use Session instead of scoped_session No need to pass around scoped_session as arguments Fix typing error: sharable Fix SA2.0 error: sqlalchemy exceptions import; minor mypy fix Mypy: type-ignore: this is never SessionlessContext Mypy: use verbose assignment to help mypy Mypy: add assert stmt Mypy: add assert to ensure seesion is not None Calling that method when a User obj is not attached to a session should not happen. Mypy: return 0 if no results Mypy: type-ignore: scoped_session vs. install_model_session We use the disctinction for DI. Mypy: refactor to one-liner Mypy: add assert stmts where we know session returns an object Mypy: rename wfi_step > wfi_step_sq when it becomes a subquery Job search refactor: factor out build_job_subquery Job search refactor: build_stmt_for_hda Job search refactor: build_stmt_for_ldda Job search refactor: build_stmt_for_hdca Job search refactor: build_stmt_for_dce Job search refactor: rename query >> stmt Mypy: add anno for Lists; type-ignore for HDAs Note: type-ignore is due to imperative mapping of HDAs (and LDDAs). This will be removed once we map those models declaratively Mypy: managers.histories Mypy: model.deferred Mypy: arg passed to template can be None Mypy: celery tasks type ignore arg: we need to map DatasetInstance classes declaratively for that to work correctly. Mypy: type-ignore hda attr-defined error Need to map declaratively to remove this Convert visualization manager index query to SA Core Mypy: session is not none Mypy: type-ignore what requires more refactoring Mypy: type-ignore hda, ldda attrs: need declarative mapping Also, minor SA2.0 syntax fix Mypy: type-ignores to handle late evaluation of relationship arguments Mypy: type-ignore column property assignments (type is correct) Mypy: typing errors, misc. fixes Mypy: all statements are reachable Mypy: need to map hda declaratively, then its parent is model.Base Fix typing errors: sharable, secured Fix package mypy errors Fix SA2.0 error: celery task 1. In 2.0, when the statement contains "returning", the result type is ChunkedIteratorResult, which does not have the rowcount attr, becuase: 2. result.rowcount should not be used for statements containting the returning clause Ref: https://docs.sqlalchemy.org/en/20/core/connections.html#sqlalchemy.engine.CursorResult.rowcount Wrap call to ensure session is closed Otherwise there's an idle transaction left in the database (+locks) Ensure session is closed on TS Registry load Same as prev. commit: otherwise db locks are left Fix SA2.0 error: list arg to select; mypy Use NullPool for sqlite engines This restores the behavior under SQLAlchemy 1.4 (Note that we set the pool for sqlite only if it's not an in-memory db Help mypy: job is never None
jdavcs
added a commit
to jdavcs/galaxy
that referenced
this pull request
Mar 11, 2024
Upgrade SQLAlchemy to 2.0 This conflicts with dependency requirements for sqlalchemy-graphene (used only in toolshed, new WIP client) Remove RemovedIn20Warning from config This does not exist in SQLAlchemy 2.0 Update import path for DeclarativeMeta Move declaration of injected attrs into constructor Remove unused import For context: https://github.com/galaxyproject/galaxy/pull/14717/files#r1486979280 Also, remove model attr type hints that conflict with SA2.0 Apply Mapped/mapped_column to model definitions Included models: galaxy, tool shed, tool shed install Column types: DateTime Integer Boolan Unicode String (Text/TEXT/TrimmedString/VARCHAR) UUID Numeric NOTE on typing of nullability: db schema != python app - Mapped[datetime] specifies correct type for the python app; - nullable=True specifies correct mapping to the db schema (that's what the CREATE TABLE sql statement will reflect). mapped_column.nullable takes precedence over typing annotation of Mapped. So, if we have: foo: Mapped[str] = mapped_column(String, nullable=True) - that means that the foo db field will allow NULL, but the python app will not allow foo = None. And vice-versa: bar: Mapped[Optional[str]] = mapped_column(String, nullable=False) - the bar db field is NOT NULL, but bar = None is OK. This might need to be applied to other column definitions, but for now this addresses specific mypy errors. Ref: https://docs.sqlalchemy.org/en/20/orm/declarative_tables.html#mapped-column-derives-the-datatype-and-nullability-from-the-mapped-annotation Add typing to JSON columns, fix related mypy errors Columns: MutableJSONType JSONType DoubleEncodedJsonType TODO: I think we need a type alias for json-typed columns: bytes understand iteration, but not access by key. Use correct type hints to define common model attrs Start applying Mapped to relationship definitions in the model Remove column declaration from HasTags parent class Fix SA2.0 error: wrap sql in text() Fix SA2.0 error: pass bind to create_all Fix SA2.0 error: use Row._mapping for keyed attribute access Ref: https://docs.sqlalchemy.org/en/20/changelog/migration_20.html#result-rows-act-like-named-tuples Fix SA2.0 error: show password in url SA 1.4: str(url) renders connection string with password SA 2.0: str(url) renders connection string WITHOUT password Solution: Use render_as_string(hide_password=False) Fix SA2.0 error: use attribute_keyed_dict Replaces attribute_mapped_collection (SA20) Fix SA2.0 error: make select stmt a subquery Rename varable to fix mypy Fix SA2.0 error: explicitly use subquery() for select-from argument Fix SA2.0 error: replase session.bind with session.get_bind() Fix SA2.0 error: joinedload does not take str args Fix use of table model attribute - Use __table__ (SA attr) instead of table (galaxy attr) on mapped classes - Drop .table and .table.c where redundant Fix bug: fix HistoryAudit model It is not a child of RepresentById becuase it does not and should not have an id attr. Duplicating the __repr__ definition in the HistoryAudit class is a temporary fix: a proper fix requires changing all models (id and __repr__ should be split into 2 mixins): to be done in a follow-up PR. Fix bug: check if template.fields is not null before iterating Fix bug: call unique() on result, not select stmt Fix bug: do not pass subquery to in_ Fix bug/typo: use select_from Fix bug: if using alias on ORM entity, use __table__ as valid FromClause Fix bug: HDAH model is not serializable (caught by mypy) Fix typing error: migrations.base Fix typing error: managers.secured This fixed 58 mypy errors! Fix typing error: session type Fix typing error: use Session instead of scoped_session No need to pass around scoped_session as arguments Fix typing error: sharable Fix SA2.0 error: sqlalchemy exceptions import; minor mypy fix Mypy: type-ignore: this is never SessionlessContext Mypy: use verbose assignment to help mypy Mypy: add assert stmt Mypy: add assert to ensure seesion is not None Calling that method when a User obj is not attached to a session should not happen. Mypy: return 0 if no results Mypy: type-ignore: scoped_session vs. install_model_session We use the disctinction for DI. Mypy: refactor to one-liner Mypy: add assert stmts where we know session returns an object Mypy: rename wfi_step > wfi_step_sq when it becomes a subquery Job search refactor: factor out build_job_subquery Job search refactor: build_stmt_for_hda Job search refactor: build_stmt_for_ldda Job search refactor: build_stmt_for_hdca Job search refactor: build_stmt_for_dce Job search refactor: rename query >> stmt Mypy: add anno for Lists; type-ignore for HDAs Note: type-ignore is due to imperative mapping of HDAs (and LDDAs). This will be removed once we map those models declaratively Mypy: managers.histories Mypy: model.deferred Mypy: arg passed to template can be None Mypy: celery tasks type ignore arg: we need to map DatasetInstance classes declaratively for that to work correctly. Mypy: type-ignore hda attr-defined error Need to map declaratively to remove this Convert visualization manager index query to SA Core Mypy: session is not none Mypy: type-ignore what requires more refactoring Mypy: type-ignore hda, ldda attrs: need declarative mapping Also, minor SA2.0 syntax fix Mypy: type-ignores to handle late evaluation of relationship arguments Mypy: type-ignore column property assignments (type is correct) Mypy: typing errors, misc. fixes Mypy: all statements are reachable Mypy: need to map hda declaratively, then its parent is model.Base Fix typing errors: sharable, secured Fix package mypy errors Fix SA2.0 error: celery task 1. In 2.0, when the statement contains "returning", the result type is ChunkedIteratorResult, which does not have the rowcount attr, becuase: 2. result.rowcount should not be used for statements containting the returning clause Ref: https://docs.sqlalchemy.org/en/20/core/connections.html#sqlalchemy.engine.CursorResult.rowcount Wrap call to ensure session is closed Otherwise there's an idle transaction left in the database (+locks) Ensure session is closed on TS Registry load Same as prev. commit: otherwise db locks are left Fix SA2.0 error: list arg to select; mypy Use NullPool for sqlite engines This restores the behavior under SQLAlchemy 1.4 (Note that we set the pool for sqlite only if it's not an in-memory db Help mypy: job is never None
4 tasks
jdavcs
added a commit
to jdavcs/galaxy
that referenced
this pull request
Mar 20, 2024
Upgrade SQLAlchemy to 2.0 This conflicts with dependency requirements for sqlalchemy-graphene (used only in toolshed, new WIP client) Remove RemovedIn20Warning from config This does not exist in SQLAlchemy 2.0 Update import path for DeclarativeMeta Move declaration of injected attrs into constructor Remove unused import For context: https://github.com/galaxyproject/galaxy/pull/14717/files#r1486979280 Also, remove model attr type hints that conflict with SA2.0 Apply Mapped/mapped_column to model definitions Included models: galaxy, tool shed, tool shed install Column types: DateTime Integer Boolan Unicode String (Text/TEXT/TrimmedString/VARCHAR) UUID Numeric NOTE on typing of nullability: db schema != python app - Mapped[datetime] specifies correct type for the python app; - nullable=True specifies correct mapping to the db schema (that's what the CREATE TABLE sql statement will reflect). mapped_column.nullable takes precedence over typing annotation of Mapped. So, if we have: foo: Mapped[str] = mapped_column(String, nullable=True) - that means that the foo db field will allow NULL, but the python app will not allow foo = None. And vice-versa: bar: Mapped[Optional[str]] = mapped_column(String, nullable=False) - the bar db field is NOT NULL, but bar = None is OK. This might need to be applied to other column definitions, but for now this addresses specific mypy errors. Ref: https://docs.sqlalchemy.org/en/20/orm/declarative_tables.html#mapped-column-derives-the-datatype-and-nullability-from-the-mapped-annotation Add typing to JSON columns, fix related mypy errors Columns: MutableJSONType JSONType DoubleEncodedJsonType TODO: I think we need a type alias for json-typed columns: bytes understand iteration, but not access by key. Use correct type hints to define common model attrs Start applying Mapped to relationship definitions in the model Remove column declaration from HasTags parent class Fix SA2.0 error: wrap sql in text() Fix SA2.0 error: pass bind to create_all Fix SA2.0 error: use Row._mapping for keyed attribute access Ref: https://docs.sqlalchemy.org/en/20/changelog/migration_20.html#result-rows-act-like-named-tuples Fix SA2.0 error: show password in url SA 1.4: str(url) renders connection string with password SA 2.0: str(url) renders connection string WITHOUT password Solution: Use render_as_string(hide_password=False) Fix SA2.0 error: use attribute_keyed_dict Replaces attribute_mapped_collection (SA20) Fix SA2.0 error: make select stmt a subquery Rename varable to fix mypy Fix SA2.0 error: explicitly use subquery() for select-from argument Fix SA2.0 error: replase session.bind with session.get_bind() Fix SA2.0 error: joinedload does not take str args Fix use of table model attribute - Use __table__ (SA attr) instead of table (galaxy attr) on mapped classes - Drop .table and .table.c where redundant Fix bug: fix HistoryAudit model It is not a child of RepresentById becuase it does not and should not have an id attr. Duplicating the __repr__ definition in the HistoryAudit class is a temporary fix: a proper fix requires changing all models (id and __repr__ should be split into 2 mixins): to be done in a follow-up PR. Fix bug: check if template.fields is not null before iterating Fix bug: call unique() on result, not select stmt Fix bug: do not pass subquery to in_ Fix bug/typo: use select_from Fix bug: if using alias on ORM entity, use __table__ as valid FromClause Fix bug: HDAH model is not serializable (caught by mypy) Fix typing error: migrations.base Fix typing error: managers.secured This fixed 58 mypy errors! Fix typing error: session type Fix typing error: use Session instead of scoped_session No need to pass around scoped_session as arguments Fix typing error: sharable Fix SA2.0 error: sqlalchemy exceptions import; minor mypy fix Mypy: type-ignore: this is never SessionlessContext Mypy: use verbose assignment to help mypy Mypy: add assert stmt Mypy: add assert to ensure seesion is not None Calling that method when a User obj is not attached to a session should not happen. Mypy: return 0 if no results Mypy: type-ignore: scoped_session vs. install_model_session We use the disctinction for DI. Mypy: refactor to one-liner Mypy: add assert stmts where we know session returns an object Mypy: rename wfi_step > wfi_step_sq when it becomes a subquery Job search refactor: factor out build_job_subquery Job search refactor: build_stmt_for_hda Job search refactor: build_stmt_for_ldda Job search refactor: build_stmt_for_hdca Job search refactor: build_stmt_for_dce Job search refactor: rename query >> stmt Mypy: add anno for Lists; type-ignore for HDAs Note: type-ignore is due to imperative mapping of HDAs (and LDDAs). This will be removed once we map those models declaratively Mypy: managers.histories Mypy: model.deferred Mypy: arg passed to template can be None Mypy: celery tasks type ignore arg: we need to map DatasetInstance classes declaratively for that to work correctly. Mypy: type-ignore hda attr-defined error Need to map declaratively to remove this Convert visualization manager index query to SA Core Mypy: session is not none Mypy: type-ignore what requires more refactoring Mypy: type-ignore hda, ldda attrs: need declarative mapping Also, minor SA2.0 syntax fix Mypy: type-ignores to handle late evaluation of relationship arguments Mypy: type-ignore column property assignments (type is correct) Mypy: typing errors, misc. fixes Mypy: all statements are reachable Mypy: need to map hda declaratively, then its parent is model.Base Fix typing errors: sharable, secured Fix package mypy errors Fix SA2.0 error: celery task 1. In 2.0, when the statement contains "returning", the result type is ChunkedIteratorResult, which does not have the rowcount attr, becuase: 2. result.rowcount should not be used for statements containting the returning clause Ref: https://docs.sqlalchemy.org/en/20/core/connections.html#sqlalchemy.engine.CursorResult.rowcount Wrap call to ensure session is closed Otherwise there's an idle transaction left in the database (+locks) Ensure session is closed on TS Registry load Same as prev. commit: otherwise db locks are left Fix SA2.0 error: list arg to select; mypy Use NullPool for sqlite engines This restores the behavior under SQLAlchemy 1.4 (Note that we set the pool for sqlite only if it's not an in-memory db Help mypy: job is never None
jdavcs
added a commit
to jdavcs/galaxy
that referenced
this pull request
Mar 22, 2024
Upgrade SQLAlchemy to 2.0 This conflicts with dependency requirements for sqlalchemy-graphene (used only in toolshed, new WIP client) Remove RemovedIn20Warning from config This does not exist in SQLAlchemy 2.0 Update import path for DeclarativeMeta Move declaration of injected attrs into constructor Remove unused import For context: https://github.com/galaxyproject/galaxy/pull/14717/files#r1486979280 Also, remove model attr type hints that conflict with SA2.0 Apply Mapped/mapped_column to model definitions Included models: galaxy, tool shed, tool shed install Column types: DateTime Integer Boolan Unicode String (Text/TEXT/TrimmedString/VARCHAR) UUID Numeric NOTE on typing of nullability: db schema != python app - Mapped[datetime] specifies correct type for the python app; - nullable=True specifies correct mapping to the db schema (that's what the CREATE TABLE sql statement will reflect). mapped_column.nullable takes precedence over typing annotation of Mapped. So, if we have: foo: Mapped[str] = mapped_column(String, nullable=True) - that means that the foo db field will allow NULL, but the python app will not allow foo = None. And vice-versa: bar: Mapped[Optional[str]] = mapped_column(String, nullable=False) - the bar db field is NOT NULL, but bar = None is OK. This might need to be applied to other column definitions, but for now this addresses specific mypy errors. Ref: https://docs.sqlalchemy.org/en/20/orm/declarative_tables.html#mapped-column-derives-the-datatype-and-nullability-from-the-mapped-annotation Add typing to JSON columns, fix related mypy errors Columns: MutableJSONType JSONType DoubleEncodedJsonType TODO: I think we need a type alias for json-typed columns: bytes understand iteration, but not access by key. Use correct type hints to define common model attrs Start applying Mapped to relationship definitions in the model Remove column declaration from HasTags parent class Fix SA2.0 error: wrap sql in text() Fix SA2.0 error: pass bind to create_all Fix SA2.0 error: use Row._mapping for keyed attribute access Ref: https://docs.sqlalchemy.org/en/20/changelog/migration_20.html#result-rows-act-like-named-tuples Fix SA2.0 error: show password in url SA 1.4: str(url) renders connection string with password SA 2.0: str(url) renders connection string WITHOUT password Solution: Use render_as_string(hide_password=False) Fix SA2.0 error: use attribute_keyed_dict Replaces attribute_mapped_collection (SA20) Fix SA2.0 error: make select stmt a subquery Rename varable to fix mypy Fix SA2.0 error: explicitly use subquery() for select-from argument Fix SA2.0 error: replase session.bind with session.get_bind() Fix SA2.0 error: joinedload does not take str args Fix use of table model attribute - Use __table__ (SA attr) instead of table (galaxy attr) on mapped classes - Drop .table and .table.c where redundant Fix bug: fix HistoryAudit model It is not a child of RepresentById becuase it does not and should not have an id attr. Duplicating the __repr__ definition in the HistoryAudit class is a temporary fix: a proper fix requires changing all models (id and __repr__ should be split into 2 mixins): to be done in a follow-up PR. Fix bug: check if template.fields is not null before iterating Fix bug: call unique() on result, not select stmt Fix bug: do not pass subquery to in_ Fix bug/typo: use select_from Fix bug: if using alias on ORM entity, use __table__ as valid FromClause Fix bug: HDAH model is not serializable (caught by mypy) Fix typing error: migrations.base Fix typing error: managers.secured This fixed 58 mypy errors! Fix typing error: session type Fix typing error: use Session instead of scoped_session No need to pass around scoped_session as arguments Fix typing error: sharable Fix SA2.0 error: sqlalchemy exceptions import; minor mypy fix Mypy: type-ignore: this is never SessionlessContext Mypy: use verbose assignment to help mypy Mypy: add assert stmt Mypy: add assert to ensure seesion is not None Calling that method when a User obj is not attached to a session should not happen. Mypy: return 0 if no results Mypy: type-ignore: scoped_session vs. install_model_session We use the disctinction for DI. Mypy: refactor to one-liner Mypy: add assert stmts where we know session returns an object Mypy: rename wfi_step > wfi_step_sq when it becomes a subquery Job search refactor: factor out build_job_subquery Job search refactor: build_stmt_for_hda Job search refactor: build_stmt_for_ldda Job search refactor: build_stmt_for_hdca Job search refactor: build_stmt_for_dce Job search refactor: rename query >> stmt Mypy: add anno for Lists; type-ignore for HDAs Note: type-ignore is due to imperative mapping of HDAs (and LDDAs). This will be removed once we map those models declaratively Mypy: managers.histories Mypy: model.deferred Mypy: arg passed to template can be None Mypy: celery tasks type ignore arg: we need to map DatasetInstance classes declaratively for that to work correctly. Mypy: type-ignore hda attr-defined error Need to map declaratively to remove this Convert visualization manager index query to SA Core Mypy: session is not none Mypy: type-ignore what requires more refactoring Mypy: type-ignore hda, ldda attrs: need declarative mapping Also, minor SA2.0 syntax fix Mypy: type-ignores to handle late evaluation of relationship arguments Mypy: type-ignore column property assignments (type is correct) Mypy: typing errors, misc. fixes Mypy: all statements are reachable Mypy: need to map hda declaratively, then its parent is model.Base Fix typing errors: sharable, secured Fix package mypy errors Fix SA2.0 error: celery task 1. In 2.0, when the statement contains "returning", the result type is ChunkedIteratorResult, which does not have the rowcount attr, becuase: 2. result.rowcount should not be used for statements containting the returning clause Ref: https://docs.sqlalchemy.org/en/20/core/connections.html#sqlalchemy.engine.CursorResult.rowcount Wrap call to ensure session is closed Otherwise there's an idle transaction left in the database (+locks) Ensure session is closed on TS Registry load Same as prev. commit: otherwise db locks are left Fix SA2.0 error: list arg to select; mypy Use NullPool for sqlite engines This restores the behavior under SQLAlchemy 1.4 (Note that we set the pool for sqlite only if it's not an in-memory db Help mypy: job is never None
jdavcs
added a commit
to jdavcs/galaxy
that referenced
this pull request
Mar 25, 2024
Upgrade SQLAlchemy to 2.0 This conflicts with dependency requirements for sqlalchemy-graphene (used only in toolshed, new WIP client) Remove RemovedIn20Warning from config This does not exist in SQLAlchemy 2.0 Update import path for DeclarativeMeta Move declaration of injected attrs into constructor Remove unused import For context: https://github.com/galaxyproject/galaxy/pull/14717/files#r1486979280 Also, remove model attr type hints that conflict with SA2.0 Apply Mapped/mapped_column to model definitions Included models: galaxy, tool shed, tool shed install Column types: DateTime Integer Boolan Unicode String (Text/TEXT/TrimmedString/VARCHAR) UUID Numeric NOTE on typing of nullability: db schema != python app - Mapped[datetime] specifies correct type for the python app; - nullable=True specifies correct mapping to the db schema (that's what the CREATE TABLE sql statement will reflect). mapped_column.nullable takes precedence over typing annotation of Mapped. So, if we have: foo: Mapped[str] = mapped_column(String, nullable=True) - that means that the foo db field will allow NULL, but the python app will not allow foo = None. And vice-versa: bar: Mapped[Optional[str]] = mapped_column(String, nullable=False) - the bar db field is NOT NULL, but bar = None is OK. This might need to be applied to other column definitions, but for now this addresses specific mypy errors. Ref: https://docs.sqlalchemy.org/en/20/orm/declarative_tables.html#mapped-column-derives-the-datatype-and-nullability-from-the-mapped-annotation Add typing to JSON columns, fix related mypy errors Columns: MutableJSONType JSONType DoubleEncodedJsonType TODO: I think we need a type alias for json-typed columns: bytes understand iteration, but not access by key. Use correct type hints to define common model attrs Start applying Mapped to relationship definitions in the model Remove column declaration from HasTags parent class Fix SA2.0 error: wrap sql in text() Fix SA2.0 error: pass bind to create_all Fix SA2.0 error: use Row._mapping for keyed attribute access Ref: https://docs.sqlalchemy.org/en/20/changelog/migration_20.html#result-rows-act-like-named-tuples Fix SA2.0 error: show password in url SA 1.4: str(url) renders connection string with password SA 2.0: str(url) renders connection string WITHOUT password Solution: Use render_as_string(hide_password=False) Fix SA2.0 error: use attribute_keyed_dict Replaces attribute_mapped_collection (SA20) Fix SA2.0 error: make select stmt a subquery Rename varable to fix mypy Fix SA2.0 error: explicitly use subquery() for select-from argument Fix SA2.0 error: replase session.bind with session.get_bind() Fix SA2.0 error: joinedload does not take str args Fix use of table model attribute - Use __table__ (SA attr) instead of table (galaxy attr) on mapped classes - Drop .table and .table.c where redundant Fix bug: fix HistoryAudit model It is not a child of RepresentById becuase it does not and should not have an id attr. Duplicating the __repr__ definition in the HistoryAudit class is a temporary fix: a proper fix requires changing all models (id and __repr__ should be split into 2 mixins): to be done in a follow-up PR. Fix bug: check if template.fields is not null before iterating Fix bug: call unique() on result, not select stmt Fix bug: do not pass subquery to in_ Fix bug/typo: use select_from Fix bug: if using alias on ORM entity, use __table__ as valid FromClause Fix bug: HDAH model is not serializable (caught by mypy) Fix typing error: migrations.base Fix typing error: managers.secured This fixed 58 mypy errors! Fix typing error: session type Fix typing error: use Session instead of scoped_session No need to pass around scoped_session as arguments Fix typing error: sharable Fix SA2.0 error: sqlalchemy exceptions import; minor mypy fix Mypy: type-ignore: this is never SessionlessContext Mypy: use verbose assignment to help mypy Mypy: add assert stmt Mypy: add assert to ensure seesion is not None Calling that method when a User obj is not attached to a session should not happen. Mypy: return 0 if no results Mypy: type-ignore: scoped_session vs. install_model_session We use the disctinction for DI. Mypy: refactor to one-liner Mypy: add assert stmts where we know session returns an object Mypy: rename wfi_step > wfi_step_sq when it becomes a subquery Job search refactor: factor out build_job_subquery Job search refactor: build_stmt_for_hda Job search refactor: build_stmt_for_ldda Job search refactor: build_stmt_for_hdca Job search refactor: build_stmt_for_dce Job search refactor: rename query >> stmt Mypy: add anno for Lists; type-ignore for HDAs Note: type-ignore is due to imperative mapping of HDAs (and LDDAs). This will be removed once we map those models declaratively Mypy: managers.histories Mypy: model.deferred Mypy: arg passed to template can be None Mypy: celery tasks type ignore arg: we need to map DatasetInstance classes declaratively for that to work correctly. Mypy: type-ignore hda attr-defined error Need to map declaratively to remove this Convert visualization manager index query to SA Core Mypy: session is not none Mypy: type-ignore what requires more refactoring Mypy: type-ignore hda, ldda attrs: need declarative mapping Also, minor SA2.0 syntax fix Mypy: type-ignores to handle late evaluation of relationship arguments Mypy: type-ignore column property assignments (type is correct) Mypy: typing errors, misc. fixes Mypy: all statements are reachable Mypy: need to map hda declaratively, then its parent is model.Base Fix typing errors: sharable, secured Fix package mypy errors Fix SA2.0 error: celery task 1. In 2.0, when the statement contains "returning", the result type is ChunkedIteratorResult, which does not have the rowcount attr, becuase: 2. result.rowcount should not be used for statements containting the returning clause Ref: https://docs.sqlalchemy.org/en/20/core/connections.html#sqlalchemy.engine.CursorResult.rowcount Wrap call to ensure session is closed Otherwise there's an idle transaction left in the database (+locks) Ensure session is closed on TS Registry load Same as prev. commit: otherwise db locks are left Fix SA2.0 error: list arg to select; mypy Use NullPool for sqlite engines This restores the behavior under SQLAlchemy 1.4 (Note that we set the pool for sqlite only if it's not an in-memory db Help mypy: job is never None
jdavcs
added a commit
to jdavcs/galaxy
that referenced
this pull request
Mar 29, 2024
Upgrade SQLAlchemy to 2.0 This conflicts with dependency requirements for sqlalchemy-graphene (used only in toolshed, new WIP client) Remove RemovedIn20Warning from config This does not exist in SQLAlchemy 2.0 Update import path for DeclarativeMeta Move declaration of injected attrs into constructor Remove unused import For context: https://github.com/galaxyproject/galaxy/pull/14717/files#r1486979280 Also, remove model attr type hints that conflict with SA2.0 Apply Mapped/mapped_column to model definitions Included models: galaxy, tool shed, tool shed install Column types: DateTime Integer Boolan Unicode String (Text/TEXT/TrimmedString/VARCHAR) UUID Numeric NOTE on typing of nullability: db schema != python app - Mapped[datetime] specifies correct type for the python app; - nullable=True specifies correct mapping to the db schema (that's what the CREATE TABLE sql statement will reflect). mapped_column.nullable takes precedence over typing annotation of Mapped. So, if we have: foo: Mapped[str] = mapped_column(String, nullable=True) - that means that the foo db field will allow NULL, but the python app will not allow foo = None. And vice-versa: bar: Mapped[Optional[str]] = mapped_column(String, nullable=False) - the bar db field is NOT NULL, but bar = None is OK. This might need to be applied to other column definitions, but for now this addresses specific mypy errors. Ref: https://docs.sqlalchemy.org/en/20/orm/declarative_tables.html#mapped-column-derives-the-datatype-and-nullability-from-the-mapped-annotation Add typing to JSON columns, fix related mypy errors Columns: MutableJSONType JSONType DoubleEncodedJsonType TODO: I think we need a type alias for json-typed columns: bytes understand iteration, but not access by key. Use correct type hints to define common model attrs Start applying Mapped to relationship definitions in the model Remove column declaration from HasTags parent class Fix SA2.0 error: wrap sql in text() Fix SA2.0 error: pass bind to create_all Fix SA2.0 error: use Row._mapping for keyed attribute access Ref: https://docs.sqlalchemy.org/en/20/changelog/migration_20.html#result-rows-act-like-named-tuples Fix SA2.0 error: show password in url SA 1.4: str(url) renders connection string with password SA 2.0: str(url) renders connection string WITHOUT password Solution: Use render_as_string(hide_password=False) Fix SA2.0 error: use attribute_keyed_dict Replaces attribute_mapped_collection (SA20) Fix SA2.0 error: make select stmt a subquery Rename varable to fix mypy Fix SA2.0 error: explicitly use subquery() for select-from argument Fix SA2.0 error: replase session.bind with session.get_bind() Fix SA2.0 error: joinedload does not take str args Fix use of table model attribute - Use __table__ (SA attr) instead of table (galaxy attr) on mapped classes - Drop .table and .table.c where redundant Fix bug: fix HistoryAudit model It is not a child of RepresentById becuase it does not and should not have an id attr. Duplicating the __repr__ definition in the HistoryAudit class is a temporary fix: a proper fix requires changing all models (id and __repr__ should be split into 2 mixins): to be done in a follow-up PR. Fix bug: check if template.fields is not null before iterating Fix bug: call unique() on result, not select stmt Fix bug: do not pass subquery to in_ Fix bug/typo: use select_from Fix bug: if using alias on ORM entity, use __table__ as valid FromClause Fix bug: HDAH model is not serializable (caught by mypy) Fix typing error: migrations.base Fix typing error: managers.secured This fixed 58 mypy errors! Fix typing error: session type Fix typing error: use Session instead of scoped_session No need to pass around scoped_session as arguments Fix typing error: sharable Fix SA2.0 error: sqlalchemy exceptions import; minor mypy fix Mypy: type-ignore: this is never SessionlessContext Mypy: use verbose assignment to help mypy Mypy: add assert stmt Mypy: add assert to ensure seesion is not None Calling that method when a User obj is not attached to a session should not happen. Mypy: return 0 if no results Mypy: type-ignore: scoped_session vs. install_model_session We use the disctinction for DI. Mypy: refactor to one-liner Mypy: add assert stmts where we know session returns an object Mypy: rename wfi_step > wfi_step_sq when it becomes a subquery Job search refactor: factor out build_job_subquery Job search refactor: build_stmt_for_hda Job search refactor: build_stmt_for_ldda Job search refactor: build_stmt_for_hdca Job search refactor: build_stmt_for_dce Job search refactor: rename query >> stmt Mypy: add anno for Lists; type-ignore for HDAs Note: type-ignore is due to imperative mapping of HDAs (and LDDAs). This will be removed once we map those models declaratively Mypy: managers.histories Mypy: model.deferred Mypy: arg passed to template can be None Mypy: celery tasks type ignore arg: we need to map DatasetInstance classes declaratively for that to work correctly. Mypy: type-ignore hda attr-defined error Need to map declaratively to remove this Convert visualization manager index query to SA Core Mypy: session is not none Mypy: type-ignore what requires more refactoring Mypy: type-ignore hda, ldda attrs: need declarative mapping Also, minor SA2.0 syntax fix Mypy: type-ignores to handle late evaluation of relationship arguments Mypy: type-ignore column property assignments (type is correct) Mypy: typing errors, misc. fixes Mypy: all statements are reachable Mypy: need to map hda declaratively, then its parent is model.Base Fix typing errors: sharable, secured Fix package mypy errors Fix SA2.0 error: celery task 1. In 2.0, when the statement contains "returning", the result type is ChunkedIteratorResult, which does not have the rowcount attr, becuase: 2. result.rowcount should not be used for statements containting the returning clause Ref: https://docs.sqlalchemy.org/en/20/core/connections.html#sqlalchemy.engine.CursorResult.rowcount Wrap call to ensure session is closed Otherwise there's an idle transaction left in the database (+locks) Ensure session is closed on TS Registry load Same as prev. commit: otherwise db locks are left Fix SA2.0 error: list arg to select; mypy Use NullPool for sqlite engines This restores the behavior under SQLAlchemy 1.4 (Note that we set the pool for sqlite only if it's not an in-memory db Help mypy: job is never None
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ref #12541.
This PR enables SQLAlchemy 2.0 without changing any behavior. It contains bug fixes, SA 2.0 fixes, typing (not mypy-specific) error fixes, mypy error fixes, and relevant refactoring. Mypy error fixes are edits that help mypy or type-ignores that are (a) false positives, (b) require DatasetInstance models to be mapped declaratively (which will allow us to add appropriate type hints to their mapped attributes), or (c) require nontrivial refactoring or analysis that go beyond the scope of this PR, which is about enabling SA.20. To the best of my knowledge, none of the type-ignores silence mypy errors caused by incorrect usage of SQLAlchemy-related code.
There's more SA2.0-specific typing and refactoring that can be applied to the model definition. However, I think it's best to leave it for follow-up PRs because (a) much is dependent on remapping the DatasetInstance and TS RepositoryMetadata models declaratively (for which we need to rename the metadata attribute in those classes, which is difficult), and (b) there's more new/better SA2.0 syntax, patterns, abstractions to consider and apply to our code base than could fit in one PR.
The commits have been cleaned-up. The names are self-explanatory; more details are provided in commit descriptions were needed.
The PR is very large - sorry about that! Please let me know if I can reorganize it in any way to simplify review.
How to test the changes?
(Select all options that apply)
License