🔀 MERGE: SQLAlchemy v1.4 (v2 API) #5122

chrisjsewell · 2021-09-08T23:51:17Z

This PR follows https://docs.sqlalchemy.org/en/14/changelog/migration_20.html (see also https://docs.sqlalchemy.org/en/14/errors.html), and has been previously carried out in aiidateam/disk-objectstore#114:

Added SQLALCHEMY_WARN_20 environmental variable
Adressed all resulting warnings
Add future=True flag for engine and session creation (V1 -> v2 API)

Note, each commit primarily address a single SQLALCHEMY_WARN_20 warning.

One thing to note here is that the Query object, which we use in the SqlaQueryBuilder, has become a long term legacy object, replaced by the direct usage of the select() construct, (see https://docs.sqlalchemy.org/en/14/changelog/migration_20.html#orm-query-unified-with-core-select).
This means that, although it would be good to replace, it is not strictly necessary for the v2 migration (and also not picked up by the RemovedIn20Warnings)

chrisjsewell · 2021-09-09T00:03:34Z

Since we added the SQLALCHEMY_WARN_20 env var in #5103, we have a list of all the code to address (from https://github.com/aiidateam/aiida-core/runs/3550512958):

aiida/backends/sqlalchemy/models/base.py:94
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/models/base.py:94: MovedIn20Warning: The ``declarative_base()`` function is now available as sqlalchemy.orm.declarative_base(). (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    Base = declarative_base(cls=Model, name='Model')  # pylint: disable=invalid-name

tests/test_base_dataclasses.py: 8 warnings
...
  /home/runner/work/aiida-core/aiida-core/aiida/orm/implementation/sqlalchemy/backend.py:86: RemovedIn20Warning: The Session.transaction attribute is considered legacy as of the 1.x series of SQLAlchemy and will be removed in 2.0. For context manager use, use Session.begin().  To access the current root transaction, use Session.get_transaction(). (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    nested = session.transaction.nested

tests/test_base_dataclasses.py: 59 warnings
...
  /home/runner/work/aiida-core/aiida-core/aiida/orm/implementation/sqlalchemy/utils.py:147: RemovedIn20Warning: The Session.transaction attribute is considered legacy as of the 1.x series of SQLAlchemy and will be removed in 2.0. For context manager use, use Session.begin().  To access the current root transaction, use Session.get_transaction(). (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    return get_scoped_session().transaction.nested

tests/test_calculation_node.py: 1 warning
...
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/models/authinfo.py:48: RemovedIn20Warning: "DbAuthInfo" object is being merged into a Session along the backref cascade path for relationship "DbComputer.authinfos"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on this error at: sqlalche.me/e/14/s9r1) (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    super().__init__(*args, **kwargs)

tests/test_dataclasses.py: 21 warnings
...
  /home/runner/work/aiida-core/aiida-core/aiida/orm/implementation/sqlalchemy/querybuilder/joiner.py:198: RemovedIn20Warning: The legacy calling style of select() is deprecated and will be removed in SQLAlchemy 2.0.  Please use the new calling style described at select(). (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    walk = select(selection_walk_list).select_from(join(node1, link1, link1.input_id == node1.id)).where(

tests/test_dataclasses.py: 21 warnings
...
  /home/runner/work/aiida-core/aiida-core/aiida/orm/implementation/sqlalchemy/querybuilder/joiner.py:217: RemovedIn20Warning: The legacy calling style of select() is deprecated and will be removed in SQLAlchemy 2.0.  Please use the new calling style described at select(). (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    select(selection_union_list).select_from(

tests/test_dataclasses.py: 2 warnings
...
  /opt/hostedtoolcache/Python/3.8.11/x64/lib/python3.8/asyncio/base_events.py:654: ResourceWarning: unclosed event loop <_UnixSelectorEventLoop running=False closed=False debug=False>
    _warn(f"unclosed event loop {self!r}", ResourceWarning, source=self)

tests/test_nodes.py: 2 warnings
...
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/models/comment.py:50: RemovedIn20Warning: "DbComment" object is being merged into a Session along the backref cascade path for relationship "DbNode.dbcomments"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on this error at: sqlalche.me/e/14/s9r1) (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    super().__init__(*args, **kwargs)

tests/backends/aiida_sqlalchemy/test_migrations.py: 73 warnings
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/migrations/env.py:39: RemovedIn20Warning: The Connection.connect() method is considered legacy as of the 1.x series of SQLAlchemy and will be removed in 2.0. (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    with connectable.connect() as connection:

tests/backends/aiida_sqlalchemy/test_migrations.py: 11 warnings
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/migrations/versions/ce56d84bcc35_delete_trajectory_symbols_array.py:64: RemovedIn20Warning: The legacy calling style of select() is deprecated and will be removed in SQLAlchemy 2.0.  Please use the new calling style described at select(). (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    select([DbNode.c.id, DbNode.c.uuid]).where(

tests/backends/aiida_sqlalchemy/test_migrations.py: 11 warnings
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/migrations/versions/12536798d4d3_trajectory_symbols_to_attribute.py:67: RemovedIn20Warning: The legacy calling style of select() is deprecated and will be removed in SQLAlchemy 2.0.  Please use the new calling style described at select(). (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    select([DbNode.c.id, DbNode.c.uuid]).where(

tests/backends/aiida_sqlalchemy/test_migrations.py: 73 warnings
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/migrations/env.py:46: RemovedIn20Warning: The .close() method on a so-called 'branched' connection is deprecated as of 1.4, as are 'branched' connections overall, and will be removed in a future release.  If this is a default-handling function, don't close the connection. (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    context.run_migrations()  # pylint: disable=no-member

tests/backends/aiida_sqlalchemy/test_migrations.py: 11 warnings
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/migrations/versions/ea2f50e7f615_dblog_create_uuid_column.py:36: RemovedIn20Warning: Passing a string to Connection.execute() is deprecated and will be removed in version 2.0.  Use the text() construct, or the Connection.exec_driver_sql() method to invoke a driver-level SQL string. (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    id_query = connection.execute('SELECT db_dblog.id FROM db_dblog')

tests/backends/aiida_sqlalchemy/test_migrations.py: 12 warnings
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/migrations/versions/12536798d4d3_trajectory_symbols_to_attribute.py:49: RemovedIn20Warning: The legacy calling style of select() is deprecated and will be removed in SQLAlchemy 2.0.  Please use the new calling style described at select(). (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    select([DbNode.c.id, DbNode.c.uuid]).where(

tests/backends/aiida_sqlalchemy/test_migrations.py: 12 warnings
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/migrations/versions/ce56d84bcc35_delete_trajectory_symbols_array.py:46: RemovedIn20Warning: The legacy calling style of select() is deprecated and will be removed in SQLAlchemy 2.0.  Please use the new calling style described at select(). (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    select([DbNode.c.id, DbNode.c.uuid]).where(

tests/backends/aiida_sqlalchemy/test_migrations.py: 14 warnings
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/migrations/versions/1b8ed3425af9_remove_legacy_workflows.py:61: RemovedIn20Warning: The legacy calling style of select() is deprecated and will be removed in SQLAlchemy 2.0.  Please use the new calling style described at select(). (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    count_workflow = connection.execute(select([func.count()]).select_from(DbWorkflow)).scalar()

tests/backends/aiida_sqlalchemy/test_migrations.py: 14 warnings
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/migrations/versions/1b8ed3425af9_remove_legacy_workflows.py:62: RemovedIn20Warning: The legacy calling style of select() is deprecated and will be removed in SQLAlchemy 2.0.  Please use the new calling style described at select(). (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    count_workflow_data = connection.execute(select([func.count()]).select_from(DbWorkflowData)).scalar()

tests/backends/aiida_sqlalchemy/test_migrations.py: 14 warnings
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/migrations/versions/1b8ed3425af9_remove_legacy_workflows.py:63: RemovedIn20Warning: The legacy calling style of select() is deprecated and will be removed in SQLAlchemy 2.0.  Please use the new calling style described at select(). (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    count_workflow_step = connection.execute(select([func.count()]).select_from(DbWorkflowStep)).scalar()

tests/backends/aiida_sqlalchemy/test_migrations.py: 23 warnings
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/migrations/versions/1feaea71bd5a_migrate_repository.py:48: RemovedIn20Warning: The legacy calling style of select() is deprecated and will be removed in SQLAlchemy 2.0.  Please use the new calling style described at select(). (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    node_count = connection.execute(select([func.count()]).select_from(DbNode)).scalar()

tests/backends/aiida_sqlalchemy/test_migrations.py::TestMigrationSchemaVsModelsSchema::test_model_and_migration_schemas_are_the_same
tests/backends/aiida_sqlalchemy/test_migrations.py::TestMigrationSchemaVsModelsSchema::test_model_and_migration_schemas_are_the_same
  /home/runner/work/aiida-core/aiida-core/tests/backends/aiida_sqlalchemy/test_utils.py:62: RemovedIn20Warning: The Engine.execute() method is considered legacy as of the 1.x series of SQLAlchemy and will be removed in 2.0. All statement execution in SQLAlchemy 2.0 is performed by the Connection.execute() method of Connection, or in the ORM by the Session.execute() method of Session. (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    return bool(engine.execute(text).scalar())

tests/backends/aiida_sqlalchemy/test_migrations.py::TestMigrationSchemaVsModelsSchema::test_model_and_migration_schemas_are_the_same
tests/backends/aiida_sqlalchemy/test_migrations.py::TestMigrationSchemaVsModelsSchema::test_model_and_migration_schemas_are_the_same
  /home/runner/work/aiida-core/aiida-core/tests/backends/aiida_sqlalchemy/test_utils.py:62: RemovedIn20Warning: Passing a string to Connection.execute() is deprecated and will be removed in version 2.0.  Use the text() construct, or the Connection.exec_driver_sql() method to invoke a driver-level SQL string. (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    return bool(engine.execute(text).scalar())

tests/backends/aiida_sqlalchemy/test_migrations.py::TestMigrationSchemaVsModelsSchema::test_model_and_migration_schemas_are_the_same
tests/backends/aiida_sqlalchemy/test_migrations.py::TestMigrationSchemaVsModelsSchema::test_model_and_migration_schemas_are_the_same
  /home/runner/work/aiida-core/aiida-core/tests/backends/aiida_sqlalchemy/test_utils.py:103: RemovedIn20Warning: The Engine.execute() method is considered legacy as of the 1.x series of SQLAlchemy and will be removed in 2.0. All statement execution in SQLAlchemy 2.0 is performed by the Connection.execute() method of Connection, or in the ORM by the Session.execute() method of Session. (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    engine.execute(text)

tests/backends/aiida_sqlalchemy/test_migrations.py::TestMigrationSchemaVsModelsSchema::test_model_and_migration_schemas_are_the_same
tests/backends/aiida_sqlalchemy/test_migrations.py::TestMigrationSchemaVsModelsSchema::test_model_and_migration_schemas_are_the_same
  /home/runner/work/aiida-core/aiida-core/tests/backends/aiida_sqlalchemy/test_utils.py:103: RemovedIn20Warning: Passing a string to Connection.execute() is deprecated and will be removed in version 2.0.  Use the text() construct, or the Connection.exec_driver_sql() method to invoke a driver-level SQL string. (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    engine.execute(text)

tests/backends/aiida_sqlalchemy/test_migrations.py::TestMigrationSchemaVsModelsSchema::test_model_and_migration_schemas_are_the_same
tests/backends/aiida_sqlalchemy/test_migrations.py::TestMigrationSchemaVsModelsSchema::test_model_and_migration_schemas_are_the_same
  /home/runner/work/aiida-core/aiida-core/tests/backends/aiida_sqlalchemy/test_utils.py:103: RemovedIn20Warning: The current statement is being autocommitted using implicit autocommit, which will be removed in SQLAlchemy 2.0. Use the .begin() method of Engine or Connection in order to use an explicit transaction for DML and DDL statements. (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    engine.execute(text)

tests/backends/aiida_sqlalchemy/test_migrations.py::TestMigrationSchemaVsModelsSchema::test_model_and_migration_schemas_are_the_same
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/utils.py:70: RemovedIn20Warning: Using plain strings to indicate SQL statements without using the text() construct is  deprecated and will be removed in version 2.0.  Ensure plain SQL statements are passed using the text() construct. (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    session.execute(

tests/backends/aiida_sqlalchemy/test_migrations.py::TestMigrationSchemaVsModelsSchema::test_model_and_migration_schemas_are_the_same
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/migrations/versions/70c7d732f1b2_delete_dbpath.py:34: RemovedIn20Warning: Passing a string to Connection.execute() is deprecated and will be removed in version 2.0.  Use the text() construct, or the Connection.exec_driver_sql() method to invoke a driver-level SQL string. (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    conn.execute('DROP TRIGGER IF EXISTS autoupdate_tc ON db_dblink')

tests/backends/aiida_sqlalchemy/test_migrations.py::TestMigrationSchemaVsModelsSchema::test_model_and_migration_schemas_are_the_same
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/migrations/versions/70c7d732f1b2_delete_dbpath.py:35: RemovedIn20Warning: Passing a string to Connection.execute() is deprecated and will be removed in version 2.0.  Use the text() construct, or the Connection.exec_driver_sql() method to invoke a driver-level SQL string. (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    conn.execute('DROP FUNCTION IF EXISTS update_tc()')

tests/backends/aiida_sqlalchemy/test_migrations.py::TestMigrationSchemaVsModelsSchema::test_model_and_migration_schemas_are_the_same
tests/backends/aiida_sqlalchemy/test_migrations.py::TestProvenanceRedesignMigration::test_verify_migration
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/migrations/versions/239cea6d2452_provenance_redesign.py:44: RemovedIn20Warning: The legacy calling style of select() is deprecated and will be removed in SQLAlchemy 2.0.  Please use the new calling style described at select(). (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    query_set = connection.execute(select([DbNode.c.type]).where(DbNode.c.type.like('calculation.%'))).fetchall()

tests/backends/aiida_sqlalchemy/test_migrations.py: 82 warnings
  /home/runner/work/aiida-core/aiida-core/tests/backends/aiida_sqlalchemy/test_migrations.py:156: RemovedIn20Warning: The AutomapBase.prepare.reflect parameter is deprecated and will be removed in a future release.  Reflection is enabled when AutomapBase.prepare.autoload_with is passed. (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    base.prepare(bind.engine, reflect=True)

tests/backends/aiida_sqlalchemy/test_migrations.py::TestProvenanceRedesignMigration::test_verify_migration
tests/backends/aiida_sqlalchemy/test_migrations.py::TestProvenanceRedesignMigration::test_verify_migration
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/migrations/versions/239cea6d2452_provenance_redesign.py:57: RemovedIn20Warning: The legacy calling style of select() is deprecated and will be removed in SQLAlchemy 2.0.  Please use the new calling style described at select(). (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    select([DbNode.c.uuid]).where(DbNode.c.type == op.inline_literal(type_string))

tests/backends/aiida_sqlalchemy/test_migrations.py::TestDbLogMigrationRecordCleaning::test_dblog_calculation_node
...
  /home/runner/work/aiida-core/aiida-core/tests/backends/aiida_sqlalchemy/test_migrations.py:782: RemovedIn20Warning: The Row.keys() method is considered legacy as of the 1.x series of SQLAlchemy and will be removed in 2.0. Use the namedtuple standard accessor Row._fields, or for full mapping behavior use  row._mapping.keys()  (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    serialized_param_data = dumps_json([(dict(list(zip(param_data.keys(), param_data))))])

tests/backends/aiida_sqlalchemy/test_migrations.py::TestDbLogMigrationRecordCleaning::test_dblog_calculation_node
...
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/migrations/versions/041a79fc615f_dblog_cleaning.py:117: RemovedIn20Warning: The Row.keys() method is considered legacy as of the 1.x series of SQLAlchemy and will be removed in 2.0. Use the namedtuple standard accessor Row._fields, or for full mapping behavior use  row._mapping.keys()  (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    res.append(dict(list(zip(row.keys(), row))))

tests/backends/aiida_sqlalchemy/test_migrations.py::TestDbLogMigrationRecordCleaning::test_dblog_calculation_node
...
  /home/runner/work/aiida-core/aiida-core/tests/backends/aiida_sqlalchemy/test_migrations.py:795: RemovedIn20Warning: The Row.keys() method is considered legacy as of the 1.x series of SQLAlchemy and will be removed in 2.0. Use the namedtuple standard accessor Row._fields, or for full mapping behavior use  row._mapping.keys()  (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    serialized_leg_wf_logs = dumps_json([(dict(list(zip(leg_wf.keys(), leg_wf))))])

tests/backends/aiida_sqlalchemy/test_migrations.py::TestDbLogMigrationRecordCleaning::test_dblog_calculation_node
...
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/migrations/versions/041a79fc615f_dblog_cleaning.py:98: RemovedIn20Warning: The Row.keys() method is considered legacy as of the 1.x series of SQLAlchemy and will be removed in 2.0. Use the namedtuple standard accessor Row._fields, or for full mapping behavior use  row._mapping.keys()  (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    res.append(dict(list(zip(row.keys(), row))))

tests/backends/aiida_sqlalchemy/test_migrations.py::TestDbLogMigrationRecordCleaning::test_dblog_calculation_node
...
  /home/runner/work/aiida-core/aiida-core/tests/backends/aiida_sqlalchemy/test_migrations.py:808: RemovedIn20Warning: The Row.keys() method is considered legacy as of the 1.x series of SQLAlchemy and will be removed in 2.0. Use the namedtuple standard accessor Row._fields, or for full mapping behavior use  row._mapping.keys()  (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    logs_no_node_list.append((dict(list(zip(log_no_node.keys(), log_no_node)))))

tests/backends/aiida_sqlalchemy/test_migrations.py: 12 warnings
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/migrations/versions/041a79fc615f_dblog_cleaning.py:136: RemovedIn20Warning: The Row.keys() method is considered legacy as of the 1.x series of SQLAlchemy and will be removed in 2.0. Use the namedtuple standard accessor Row._fields, or for full mapping behavior use  row._mapping.keys()  (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    res.append(dict(list(zip(row.keys(), row))))

tests/backends/aiida_sqlalchemy/test_migrations.py::TestDbLogUUIDAddition::test_dblog_unique_uuids
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/migrations/versions/ea2f50e7f615_dblog_create_uuid_column.py:55: RemovedIn20Warning: Passing a string to Connection.execute() is deprecated and will be removed in version 2.0.  Use the text() construct, or the Connection.exec_driver_sql() method to invoke a driver-level SQL string. (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    connection.execute(update_stm)

tests/backends/aiida_sqlalchemy/test_schema.py::TestRelationshipsSQLA::test_user_node_3
  /home/runner/work/aiida-core/aiida-core/tests/backends/aiida_sqlalchemy/test_schema.py:167: SAWarning: Object of type <DbNode> not in session, add operation along 'DbUser.dbnodes' will not proceed
    session.commit()

tests/backends/aiida_sqlalchemy/test_schema.py::TestRelationshipsSQLA::test_user_node_4
  /home/runner/work/aiida-core/aiida-core/tests/backends/aiida_sqlalchemy/test_schema.py:203: SAWarning: Object of type <DbNode> not in session, add operation along 'DbUser.dbnodes' will not proceed
    session.commit()

tests/backends/aiida_sqlalchemy/test_session.py::TestSessionSqla::test_node_access_with_sessions
  /home/runner/work/aiida-core/aiida-core/tests/backends/aiida_sqlalchemy/test_session.py:167: RemovedIn20Warning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    dbnode_reloaded = custom_session.query(sa.models.node.DbNode).get(node.id)

tests/cmdline/commands/test_computer.py::TestVerdiComputerCommands::test_computer_delete
tests/orm/test_computers.py::TestComputer::test_delete
  /home/runner/work/aiida-core/aiida-core/aiida/orm/implementation/sqlalchemy/computers.py:140: RemovedIn20Warning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    session.query(DbComputer).get(pk).delete()

tests/cmdline/commands/test_data.py: 47 warnings
  /home/runner/work/aiida-core/aiida-core/aiida/orm/implementation/sqlalchemy/querybuilder/joiner.py:262: RemovedIn20Warning: The legacy calling style of select() is deprecated and will be removed in SQLAlchemy 2.0.  Please use the new calling style described at select(). (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    walk = select(selection_walk_list).select_from(join(node1, link1, link1.output_id == node1.id)).where(

tests/cmdline/commands/test_data.py: 47 warnings
  /home/runner/work/aiida-core/aiida-core/aiida/orm/implementation/sqlalchemy/querybuilder/joiner.py:278: RemovedIn20Warning: The legacy calling style of select() is deprecated and will be removed in SQLAlchemy 2.0.  Please use the new calling style described at select(). (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    select(selection_union_list).select_from(

tests/cmdline/commands/test_data.py: 12 warnings
  /home/runner/work/aiida-core/aiida-core/aiida/orm/implementation/sqlalchemy/querybuilder/main.py:232: RemovedIn20Warning: ORDER BY columns added implicitly due to DISTINCT is deprecated and will be removed in SQLAlchemy 2.0.  SELECT statements with DISTINCT should be written to explicitly include the appropriate columns in the columns clause (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    for resultrow in query.yield_per(batch_size):  # type: ignore[arg-type] # pylint: disable=not-an-iterable

tests/cmdline/commands/test_devel.py::test_run_sql
  /home/runner/work/aiida-core/aiida-core/aiida/orm/implementation/sqlalchemy/backend.py:137: RemovedIn20Warning: Using plain strings to indicate SQL statements without using the text() construct is  deprecated and will be removed in version 2.0.  Ensure plain SQL statements are passed using the text() construct. (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    queryset = session.execute(query)

tests/cmdline/commands/test_group.py: 56 warnings
...
  /home/runner/work/aiida-core/aiida-core/aiida/orm/implementation/sqlalchemy/groups.py:370: RemovedIn20Warning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    session.query(DbGroup).get(id).delete()

tests/cmdline/commands/test_group.py::TestVerdiGroup::test_create
...
  /home/runner/work/aiida-core/aiida-core/aiida/orm/implementation/sqlalchemy/querybuilder/main.py:621: SADeprecationWarning: Invoking or_() without arguments is deprecated, and will be disallowed in a future release.   For an empty or_() construct, use or_(False, *args).
    expressions.append(or_(*subexpressions))

codecov · 2021-09-09T00:07:37Z

Codecov Report

Merging #5122 (f5dd068) into develop (a295616) will increase coverage by 0.01%.
The diff coverage is 89.24%.

❗ Current head f5dd068 differs from pull request most recent head f926619. Consider uploading reports for the commit f926619 to get more accurate results

@@             Coverage Diff             @@
##           develop    #5122      +/-   ##
===========================================
+ Coverage    80.89%   80.89%   +0.01%     
===========================================
  Files          536      536              
  Lines        37066    37069       +3     
===========================================
+ Hits         29981    29985       +4     
+ Misses        7085     7084       -1

Flag	Coverage Δ
django	`75.74% <46.16%> (+0.01%)`	⬆️
sqlalchemy	`74.84% <89.24%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ns/12536798d4d3_trajectory_symbols_to_attribute.py	`95.66% <ø> (ø)`
...ns/ce56d84bcc35_delete_trajectory_symbols_array.py	`82.15% <0.00%> (ø)`
...m/implementation/sqlalchemy/querybuilder/joiner.py	`91.13% <80.00%> (-0.80%)`	⬇️
...orm/implementation/sqlalchemy/querybuilder/main.py	`84.69% <83.34%> (-0.05%)`	⬇️
aiida/backends/sqlalchemy/migrations/env.py	`80.77% <100.00%> (-0.71%)`	⬇️
...migrations/versions/041a79fc615f_dblog_cleaning.py	`91.31% <100.00%> (ø)`
...s/versions/1b8ed3425af9_remove_legacy_workflows.py	`67.17% <100.00%> (ø)`
...ations/versions/1feaea71bd5a_migrate_repository.py	`83.08% <100.00%> (ø)`
...tions/versions/239cea6d2452_provenance_redesign.py	`95.84% <100.00%> (ø)`
.../migrations/versions/70c7d732f1b2_delete_dbpath.py	`83.34% <100.00%> (ø)`
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a295616...f926619. Read the comment docs.

aiida/orm/implementation/sqlalchemy/utils.py

chrisjsewell · 2021-09-09T13:21:06Z

I've opened sqlalchemy/alembic#908 for:

tests/backends/aiida_sqlalchemy/test_migrations.py: 73 warnings
  /home/runner/work/aiida-core/aiida-core/aiida/backends/sqlalchemy/migrations/env.py:39: RemovedIn20Warning: The Connection.connect() method is considered legacy as of the 1.x series of SQLAlchemy and will be removed in 2.0. (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    with connectable.connect() as connection:

chrisjsewell · 2021-09-10T05:50:34Z

Ok nearly there:

tests/backends/aiida_sqlalchemy/test_migrations.py: 82 warnings
  /home/runner/work/aiida-core/aiida-core/tests/backends/aiida_sqlalchemy/test_migrations.py:156: RemovedIn20Warning: The AutomapBase.prepare.reflect parameter is deprecated and will be removed in a future release.  Reflection is enabled when AutomapBase.prepare.autoload_with is passed. (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    base.prepare(bind.engine, reflect=True)

tests/cmdline/commands/test_data.py: 12 warnings
  /home/runner/work/aiida-core/aiida-core/aiida/orm/implementation/sqlalchemy/querybuilder/main.py:204: RemovedIn20Warning: ORDER BY columns added implicitly due to DISTINCT is deprecated and will be removed in SQLAlchemy 2.0.  SELECT statements with DISTINCT should be written to explicitly include the appropriate columns in the columns clause (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    for resultrow in self.get_session().execute(stmt):

(django only, opened aldjemy/aldjemy#192)

tests/test_base_dataclasses.py: 14 warnings
  /opt/hostedtoolcache/Python/3.8.11/x64/lib/python3.8/site-packages/aldjemy/orm.py:152: RemovedIn20Warning: Calling the mapper() function directly outside of a declarative registry is deprecated. Please use the sqlalchemy.orm.registry.map_imperatively() function for a classical mapping. (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    orm.mapper(sa_model, table, attrs)

chrisjsewell · 2021-09-16T00:52:39Z

Ok, we are down to 3 warnings, none of which preclude the use of the 2.0 API, so I think can be delayed until a later PR:

  /Users/chrisjsewell/Documents/GitHub/aiida-core-origin/tests/backends/aiida_sqlalchemy/test_migrations.py:156: RemovedIn20Warning: The AutomapBase.prepare.reflect parameter is deprecated and will be removed in a future release.  Reflection is enabled when AutomapBase.prepare.autoload_with is passed. (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    base.prepare(bind.engine, reflect=True)

This does not say it will be removed in 2.0, and is still even the recommended way in the docs: https://docs.sqlalchemy.org/en/14/orm/extensions/automap.html#basic-use

  /home/runner/work/aiida-core/aiida-core/aiida/orm/implementation/sqlalchemy/querybuilder/main.py:204: RemovedIn20Warning: ORDER BY columns added implicitly due to DISTINCT is deprecated and will be removed in SQLAlchemy 2.0.  SELECT statements with DISTINCT should be written to explicitly include the appropriate columns in the columns clause (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    for resultrow in self.get_session().execute(stmt):

This won't become an issue until 2.0 is actually out and requires a bit more of a comprehensive fix; basically is you use DISTINCT, then every column used to ORDER BY must also be present in SELECT (see the explanation here: https://blog.jooq.org/how-sql-distinct-and-order-by-are-related/)

  /opt/hostedtoolcache/Python/3.8.11/x64/lib/python3.8/site-packages/aldjemy/orm.py:152: RemovedIn20Warning: Calling the mapper() function directly outside of a declarative registry is deprecated. Please use the sqlalchemy.orm.registry.map_imperatively() function for a classical mapping. (Background on SQLAlchemy 2.0 at: sqlalche.me/e/b8d9)
    orm.mapper(sa_model, table, attrs)

This also is not an immediate problem, and waiting on aldjemy/aldjemy#192

Two final notes:

I wanted to understand a bit more the use for yield_per and partition: Difference between `yield_per` and `partitions` sqlalchemy/sqlalchemy#7030
Back-cascading is now turned off: https://docs.sqlalchemy.org/en/14/errors.html#object-is-being-merged-into-a-session-along-the-backref-cascade; i.e. some objects will not be automatically added to the session under relatively obscure scenarios.
- See also https://docs.sqlalchemy.org/en/14/changelog/migration_14.html#change-5150 and https://groups.google.com/g/sqlalchemy/c/VoY-qEiJA3U
- It's obviously not failing any tests, and I can't think of any obvious reason why this would be an issue for aiida (for example, we already specifically add entities to the session before commiting in the save method)

See: sqlalchemy/alembic#908

Fixes #2475

…eld_per=x)` See: https://docs.sqlalchemy.org/en/14/orm/queryguide.html?highlight=yield%20per#yield-per Using statement execution also inhibits automatic uniquing of results, removing the previous need for patching of `ORMCompileState`.

ramirezfranciscof

Yeap, all good, thanks @chrisjsewell ! I just have a couple of questions to understand a bit better the changes but nothing to change so you can go ahead and merge (also if maybe you can add some very brief comments on the commit message related to these questions it might be good in case someone is also curious while going over this PR)

ramirezfranciscof · 2021-09-16T08:03:13Z

tests/backends/aiida_sqlalchemy/test_migrations.py

@@ -779,7 +779,7 @@ def setUpBeforeMigration(self):
                param_data = session.query(DbLog).filter(DbLog.objpk == param.id
                                                         ).filter(DbLog.objname == 'something.else.'
                                                                  ).with_entities(*cols_to_project).one()
-                serialized_param_data = dumps_json([(dict(list(zip(param_data.keys(), param_data))))])
+                serialized_param_data = dumps_json([param_data._asdict()])


I mean, I know it is more compact and simple, but should we be using private methods of an sqla object?? Is this how they intend you to do it? Why don't they make it a public method then?

Its definitely how they intend it to be used (the warning tells you to use it). It's like us and ProcessBuilderNamespaces; the attributes are intended to primarily be the keys, so we have things lie ._update

Ok, I see. I still find it bizarre that they are suggesting users of the object to access some functionality through a private method. I mean, what makes it "private" then?

Well exactly, thats why personally I think it is never good to be using "attribute dictionaries" in Python (as opposed to e.g. JavaScript where it is a feature of the language)

Mmm I don't understand. My objection was more one of intentionality (as in, the point of prefacing your methods with the underscore is to say "I am using this internally, this is not meant for you, use it at your own peril of messing the object up / losing the feature without deprecation").

In that sense attribute dictionaries are in principle intentionally exposed, what would be the problem with that?

(I mean, I have my own quarrel with attribute dicts when making workflows and crashing against some unexpected problems of where exactly the attribute dict starts having simple dicts, but other than that I recognize it is convenient for the shell)

because if all the attributes on your class are meant to (dynamically) relate to keys of a dictionary, then you cannot have any "static" attributes/methods associated with your class. Here they have "compromised" and have any static methods public but underscored.
The better route would probably have been to use an "external" function (that would call the private methods), as e.g. you do with https://docs.python.org/3/library/dataclasses.html#dataclasses.asdict

aiida/backends/sqlalchemy/migrations/env.py

tests/backends/aiida_sqlalchemy/test_utils.py

aiida/backends/sqlalchemy/migrations/versions/12536798d4d3_trajectory_symbols_to_attribute.py

chrisjsewell · 2021-09-16T08:24:47Z

thanks @ramirezfranciscof!

chrisjsewell commented Sep 9, 2021

View reviewed changes

aiida/orm/implementation/sqlalchemy/utils.py Outdated Show resolved Hide resolved

chrisjsewell closed this Sep 9, 2021

chrisjsewell force-pushed the sqla-v2api branch from 813737a to d33ae8a Compare September 9, 2021 10:52

chrisjsewell reopened this Sep 9, 2021

chrisjsewell force-pushed the sqla-v2api branch 4 times, most recently from 6553ffb to 316e6c1 Compare September 10, 2021 05:18

chrisjsewell linked an issue Sep 10, 2021 that may be closed by this pull request

querybuilder chokes on empty filters #2475

Closed

chrisjsewell force-pushed the sqla-v2api branch from c5ee72d to c372515 Compare September 15, 2021 17:21

chrisjsewell marked this pull request as ready for review September 16, 2021 00:40

chrisjsewell force-pushed the sqla-v2api branch from c275c83 to 6777297 Compare September 16, 2021 00:58

chrisjsewell changed the title ~~⬆️ UPGRADE: sqlalchemy v1.4 (v2 API)~~ 🔀 MERGE: SQLAlchemy v1.4 (v2 API) Sep 16, 2021

chrisjsewell requested review from ltalirz and sphuber September 16, 2021 01:00

chrisjsewell mentioned this pull request Sep 16, 2021

👌 IMPROVE: Add SQLA migration for parity with Django schema #5097

Merged

chrisjsewell added 9 commits September 16, 2021 09:19

SQLA v2 API: Update declarative_base import

1b886e2

SQLA v2 API: .transaction.nested -> .in_nested_transaction()

5001a2f

SQLA v2 API: session.query(O).get(id) -> session.get(O, id)

a95f54c

SQLA v2 API: Replace .execute("...") with .execute(text("..."))

4c6d9a9

SQLA v2 API: Remove branched connection context

3966e7a

See: sqlalchemy/alembic#908

SQLA v2 API: Use Row._as_dict() for dict access

625335f

SQLA v2 API: Replace Engine.execute() with Connection.execute()

c48d617

SQLA v2 API: Replace use of lists in select()

70a7018

SQLA v2 API: Do not build empty query filters

87c96a0

Fixes #2475

chrisjsewell added 4 commits September 16, 2021 09:19

SQLA v2 API: Use transaction context managers

db328dc

SQLA v2 API: add engine & session future=True flag

6aa8281

SQLA v2 API: Suppress known SAWarnings in tests

582a693

chrisjsewell force-pushed the sqla-v2api branch from f5dd068 to f926619 Compare September 16, 2021 07:20

ramirezfranciscof approved these changes Sep 16, 2021

View reviewed changes

chrisjsewell merged commit b31e5f5 into develop Sep 16, 2021

chrisjsewell deleted the sqla-v2api branch September 16, 2021 08:53

ltalirz removed their request for review September 16, 2021 08:57

chrisjsewell mentioned this pull request Mar 12, 2022

🔀 MERGE: Release v2.0.0b1 #5426

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🔀 MERGE: SQLAlchemy v1.4 (v2 API) #5122

🔀 MERGE: SQLAlchemy v1.4 (v2 API) #5122

chrisjsewell commented Sep 8, 2021 •

edited

Loading

chrisjsewell commented Sep 9, 2021 •

edited

Loading

codecov bot commented Sep 9, 2021 •

edited

Loading

chrisjsewell commented Sep 9, 2021

chrisjsewell commented Sep 10, 2021 •

edited

Loading

chrisjsewell commented Sep 16, 2021 •

edited

Loading

ramirezfranciscof left a comment

ramirezfranciscof Sep 16, 2021

chrisjsewell Sep 16, 2021 •

edited

Loading

ramirezfranciscof Sep 16, 2021

chrisjsewell Sep 16, 2021

ramirezfranciscof Sep 16, 2021

chrisjsewell Sep 16, 2021

chrisjsewell commented Sep 16, 2021

🔀 MERGE: SQLAlchemy v1.4 (v2 API) #5122

🔀 MERGE: SQLAlchemy v1.4 (v2 API) #5122

Conversation

chrisjsewell commented Sep 8, 2021 • edited Loading

chrisjsewell commented Sep 9, 2021 • edited Loading

codecov bot commented Sep 9, 2021 • edited Loading

Codecov Report

chrisjsewell commented Sep 9, 2021

chrisjsewell commented Sep 10, 2021 • edited Loading

chrisjsewell commented Sep 16, 2021 • edited Loading

ramirezfranciscof left a comment

Choose a reason for hiding this comment

ramirezfranciscof Sep 16, 2021

Choose a reason for hiding this comment

chrisjsewell Sep 16, 2021 • edited Loading

Choose a reason for hiding this comment

ramirezfranciscof Sep 16, 2021

Choose a reason for hiding this comment

chrisjsewell Sep 16, 2021

Choose a reason for hiding this comment

ramirezfranciscof Sep 16, 2021

Choose a reason for hiding this comment

chrisjsewell Sep 16, 2021

Choose a reason for hiding this comment

chrisjsewell commented Sep 16, 2021

chrisjsewell commented Sep 8, 2021 •

edited

Loading

chrisjsewell commented Sep 9, 2021 •

edited

Loading

codecov bot commented Sep 9, 2021 •

edited

Loading

chrisjsewell commented Sep 10, 2021 •

edited

Loading

chrisjsewell commented Sep 16, 2021 •

edited

Loading

chrisjsewell Sep 16, 2021 •

edited

Loading