Merge pull request #7583 from ckan/sqlalchemy-2.0

SQLAlchemy 2.0

.circleci/config.yml

@@ -37,6 +37,8 @@ defaults:
       CKAN_POSTGRES_PWD: pass
       PGPASSWORD: ckan
       PYTEST_COMMON_OPTIONS: -v --ckan-ini=test-core-circle-ci.ini --cov=ckan --cov=ckanext --junitxml=~/junit/result/junit.xml
+      # report usage of deprecated features
+      SQLALCHEMY_WARN_20: 1
   pg_image: &pg_image
     image: postgres:10
     environment:

.github/workflows/pyright.yml

@@ -1,7 +1,7 @@
 name: Check types
 on: [pull_request]
 env:
-  NODE_VERSION: '16'
+  NODE_VERSION: '18'
   PYTHON_VERSION: '3.8'
 
 permissions:
@@ -15,7 +15,7 @@ jobs:
       - uses: actions/setup-python@v2
         with:
           python-version: ${{ env.PYTHON_VERSION }}
-      - uses: actions/setup-node@v2-beta
+      - uses: actions/setup-node@v3
         with:
           node-version: ${{ env.NODE_VERSION }}
       - name: Install python deps

changes/7583.misc

@@ -0,0 +1,30 @@
+:py:meth:`~ckanext.datastore.interfaces.IDatastore.datastore_search` of
+:py:class:`~ckanext.datastore.interfaces.IDatastore` interface is not completely
+compatible with old version.
+
+``where`` key of the ``query_dict`` returned from this method has a different
+format. Before it was a collection of tuples with an SQL where-clause with
+positional/named ``%``-style placeholders on the first position, followed by
+arbitrary number of parameters::
+
+    return {
+        ...,
+        "where": [('"age" BETWEEN %s AND %s', param1, param2, ...), ...]
+    }
+
+Now every element of collection must be a tuple that contains SQL where-clause
+with **named** ``:``-style placeholders and a dict with the values for all the
+placeholders:
+
+
+    return {
+        ...,
+        "where": [(
+            '"age" BETWEEN :my_ext_min AND :my_ext_max',
+            {"my_ext_min": age_between[0], "my_ext_max": age_between[1]},
+        )]
+    }
+
+In order to avoid name conflicts with placeholders from different plugin, don't
+use simple names, i.e. ``val``, ``min``, ``name``, and add unique prefix to all
+the placeholders.

changes/7583.removal

@@ -0,0 +1,16 @@
+SQLAlchemy's Metadata object(:py:attr:`ckan.model.meta.metadata`) is no longer
+bound the the DB engine. [A number of
+operations](https://docs.sqlalchemy.org/en/14/changelog/migration_20.html#implicit-and-connectionless-execution-bound-metadata-removed),
+such as `table.exists()`, `table.create()`, `metadata.create_all()`,
+`metadata.reflect()`, now produce an error
+:py:class:`sqlalchemy.exc.UnboundExecutionError`.
+
+Depending on the situation, following changes may be required:
+
+* Instead of creating tables via custom CLI command or during application
+  startup, use [Alembic
+  migrations](https://docs.ckan.org/en/2.10/extensions/best-practices.html#use-migrations-when-introducing-new-models)
+
+* If there is no other way, change `table.create()`/`table.exists()` to
+  `table.create(engine)`/`table.exists()`. Get `engine` by calling
+  :py:func:`~ckan.model.ensure_engine`.

ckan/authz.py

@@ -344,7 +344,7 @@ def _has_user_permission_for_groups(
     # get any roles the user has for the group
     q: Any = (model.Session.query(model.Member.capacity)
          # type_ignore_reason: attribute has no method
-         .filter(model.Member.group_id.in_(group_ids))  # type: ignore
+         .filter(model.Member.group_id.in_(group_ids))
          .filter(model.Member.table_name == 'user')
          .filter(model.Member.state == 'active')
          .filter(model.Member.table_id == user_id))
@@ -402,7 +402,7 @@ def has_user_permission_for_some_org(
          .filter(model.Member.table_name == 'user')
          .filter(model.Member.state == 'active')
          # type_ignore_reason: attribute has no method
-         .filter(model.Member.capacity.in_(roles))  # type: ignore
+         .filter(model.Member.capacity.in_(roles))
          .filter(model.Member.table_id == user_id))
     group_ids = []
     for row in q:
@@ -416,8 +416,7 @@ def has_user_permission_for_some_org(
         model.Session.query(model.Group)
         .filter(model.Group.is_organization == True)
         .filter(model.Group.state == 'active')
-        # type_ignore_reason: attribute has no method
-        .filter(model.Group.id.in_(group_ids)).exists()  # type: ignore
+        .filter(model.Group.id.in_(group_ids)).exists()
     ).scalar()
 
     return permission_exists
@@ -491,8 +490,7 @@ def user_is_collaborator_on_dataset(
     if capacity:
         if isinstance(capacity, str):
             capacity = [capacity]
-        # type_ignore_reason: attribute has no method
-        q = q.filter(model.PackageMember.capacity.in_(capacity))  # type: ignore
+        q = q.filter(model.PackageMember.capacity.in_(capacity))
 
     return model.Session.query(q.exists()).scalar()
 

ckan/cli/search_index.py

@@ -161,7 +161,10 @@ def rebuild_fast():
     db_url = config['sqlalchemy.url']
     engine = sa.create_engine(db_url)
     package_ids = []
-    result = engine.execute(u"select id from package where state = 'active';")
+    with engine.connect() as conn:
+        result = conn.execute(
+            sa.text("SELECT id FROM package where state = 'active'")
+        )
     for row in result:
         package_ids.append(row[0])
 

ckan/config/environment.py

@@ -252,4 +252,4 @@ def update_config() -> None:
     # Close current session and open database connections to ensure a clean
     # clean environment even if an error occurs later on
     model.Session.remove()
-    model.Session.bind.dispose()
+    model.Session.bind.dispose()  # type: ignore

ckan/lib/dictization/__init__.py

@@ -6,7 +6,7 @@
 
 import sqlalchemy
 from sqlalchemy import Table
-from sqlalchemy.engine import Row  # type: ignore
+from sqlalchemy.engine import Row
 from sqlalchemy.orm import class_mapper
 
 from ckan.model.core import State
@@ -27,7 +27,7 @@ def table_dictize(obj: Any, context: Context, **kw: Any) -> dict[str, Any]:
     result_dict: dict[str, Any] = {}
 
     if isinstance(obj, Row):
-        fields = obj.keys()
+        fields = obj._fields
     else:
         ModelClass = obj.__class__
         table = class_mapper(ModelClass).persist_selectable

ckan/lib/dictization/model_dictize.py

@@ -188,17 +188,17 @@ def package_dictize(
 
     # resources
     res = model.resource_table
-    q = select([res]).where(res.c["package_id"] == pkg.id)
+    q = select(res).where(res.c["package_id"] == pkg.id)
     result = execute(q, res, context)
     result_dict["resources"] = resource_list_dictize(result, context)
     result_dict['num_resources'] = len(result_dict.get('resources', []))
 
     # tags
     tag = model.tag_table
     pkg_tag = model.package_tag_table
-    q = select([tag, pkg_tag.c["state"]],
-               from_obj=pkg_tag.join(tag, tag.c["id"] == pkg_tag.c["tag_id"])
-               ).where(pkg_tag.c["package_id"] == pkg.id)
+    q = select(tag, pkg_tag.c["state"]).join(
+        pkg_tag, tag.c["id"] == pkg_tag.c["tag_id"]
+    ).where(pkg_tag.c["package_id"] == pkg.id)
     result = execute(q, pkg_tag, context)
     result_dict["tags"] = d.obj_list_dictize(result, context,
                                              lambda x: x["name"])
@@ -213,18 +213,21 @@ def package_dictize(
 
     # extras - no longer revisioned, so always provide latest
     extra = model.package_extra_table
-    q = select([extra]).where(extra.c["package_id"] == pkg.id)
+    q = select(extra).where(extra.c["package_id"] == pkg.id)
     result = execute(q, extra, context)
     result_dict["extras"] = extras_list_dictize(result, context)
 
     # groups
     member = model.member_table
     group = model.group_table
-    q = select([group, member.c["capacity"]],
-               from_obj=member.join(group, group.c["id"] == member.c["group_id"])
-               ).where(member.c["table_id"] == pkg.id)\
-                .where(member.c["state"] == 'active') \
-                .where(group.c["is_organization"] == False)
+    q = select(group, member.c["capacity"]).join(
+        member, group.c["id"] == member.c["group_id"]
+    ).where(
+        member.c["table_id"] == pkg.id,
+        member.c["state"] == 'active',
+        group.c["is_organization"] == False
+    )
+
     result = execute(q, member, context)
     context['with_capacity'] = False
     # no package counts as cannot fetch from search index at the same
@@ -235,9 +238,9 @@ def package_dictize(
 
     # owning organization
     group = model.group_table
-    q = select([group]
-               ).where(group.c["id"] == pkg.owner_org) \
-                .where(group.c["state"] == 'active')
+    q = select(group).where(
+        group.c["id"] == pkg.owner_org
+    ).where(group.c["state"] == 'active')
     result = execute(q, group, context)
     organizations = d.obj_list_dictize(result, context)
     if organizations:
@@ -247,11 +250,15 @@ def package_dictize(
 
     # relations
     rel = model.package_relationship_table
-    q = select([rel]).where(rel.c["subject_package_id"] == pkg.id)
+    q = select(
+        rel
+    ).where(rel.c["subject_package_id"] == pkg.id)
     result = execute(q, rel, context)
     result_dict["relationships_as_subject"] = \
         d.obj_list_dictize(result, context)
-    q = select([rel]).where(rel.c["object_package_id"] == pkg.id)
+    q = select(
+        rel
+    ).where(rel.c["object_package_id"] == pkg.id)
     result = execute(q, rel, context)
     result_dict["relationships_as_object"] = \
         d.obj_list_dictize(result, context)

ckan/lib/pagination.py

@@ -53,7 +53,7 @@ class BasePage(List[Any]):
 
     - a sequence
     - an SQLAlchemy query - e.g.: Session.query(MyModel)
-    - an SQLAlchemy select - e.g.: sqlalchemy.select([my_table])
+    - an SQLAlchemy select - e.g.: sqlalchemy.select(my_table)
 
     A "Page" instance maintains pagination logic associated with each
     page, where it begins, what the first/last item on the page is, etc.

ckan/logic/action/create.py

@@ -401,8 +401,7 @@ def resource_view_create(
     last_view = model.Session.query(model.ResourceView)\
         .filter_by(resource_id=resource_id) \
         .order_by(
-            # type_ignore_reason: incomplete SQLAlchemy types
-            model.ResourceView.order.desc()  # type: ignore
+            model.ResourceView.order.desc()
         ).first()
 
     if not last_view:
@@ -610,10 +609,7 @@ def member_create(context: Context,
         filter(model.Member.table_name == obj_type).\
         filter(model.Member.table_id == obj.id).\
         filter(model.Member.group_id == group.id).\
-        order_by(
-            # type_ignore_reason: incomplete SQLAlchemy types
-            model.Member.state.asc()  # type: ignore
-        ).first()
+        order_by(model.Member.state.asc()).first()
     if member:
         user_obj = model.User.get(user)
         if user_obj and member.table_name == u'user' and \