`trino.sqlalchemy` module breaks `coverage` in some situations

[evansd]

It used to be possible to check the coverage of a specific module by a specific set of tests using something like:

pytest --cov=ehrql.utils.functools_utils tests/unit/utils/test_functools_utils.py

However now doing so results in this cryptic error:

ImportError while loading conftest '/home/dave/projects/ebmdatalab/ehrql/tests/conftest.py'.
tests/conftest.py:8: in <module>
    import ehrql
ehrql/__init__.py:4: in <module>
    from ehrql.measures import INTERVAL, Measures
ehrql/measures/__init__.py:1: in <module>
    from ehrql.measures.calculate import (
ehrql/measures/calculate.py:4: in <module>
    from ehrql.measures.measures import get_all_group_by_columns
ehrql/measures/measures.py:5: in <module>
    from ehrql.query_language import (
ehrql/query_language.py:13: in <module>
    from ehrql.query_model.population_validation import validate_population_definition
ehrql/query_model/population_validation.py:1: in <module>
    from ehrql.query_engines.in_memory import InMemoryQueryEngine
ehrql/query_engines/in_memory.py:6: in <module>
    from ehrql.query_engines.in_memory_database import (
ehrql/query_engines/in_memory_database.py:11: in <module>
    from ehrql.utils.orm_utils import table_has_one_row_per_patient
ehrql/utils/orm_utils.py:7: in <module>
    from sqlalchemy.orm import declarative_base
../../../.virtualenvs/ebm-ehrql/lib/python3.11/site-packages/sqlalchemy/orm/__init__.py:21: in <module>
    from . import mapper as mapperlib
../../../.virtualenvs/ebm-ehrql/lib/python3.11/site-packages/sqlalchemy/orm/mapper.py:47: in <module>
    from . import attributes
../../../.virtualenvs/ebm-ehrql/lib/python3.11/site-packages/sqlalchemy/orm/attributes.py:38: in <module>
    from . import collections
../../../.virtualenvs/ebm-ehrql/lib/python3.11/site-packages/sqlalchemy/orm/collections.py:128: in <module>
    from .base import NO_KEY
../../../.virtualenvs/ebm-ehrql/lib/python3.11/site-packages/sqlalchemy/orm/base.py:428: in <module>
    @inspection._inspects(object)
../../../.virtualenvs/ebm-ehrql/lib/python3.11/site-packages/sqlalchemy/inspection.py:131: in decorate
    raise AssertionError(
E   AssertionError: Type <class 'object'> is already registered

For some reason, Coverage ends up importing modules twice and this is causing some kind of SQLAlchemy registration hook to fire a second time and then to error.

Using git-bisect I tracked it down to this commit which suggests that the Trino SQLAlchemy connector is at fault. I also note we needed to silence a deprecation warning here, so that might be related.

The trino-python-client project looks reasonably active so hopeful they'd accept a patch to fix this. (I think it's unlikely they'd diagnose and fix it for us on the basis of a bug report as I can't see it being a huge priority for them.)

[inglesp]

This works for me:

$ pytest --cov=ehrql.utils.functools_utils tests/unit/utils/test_functools_utils.py
============================= test session starts ==============================
platform linux -- Python 3.11.2, pytest-7.4.2, pluggy-1.0.0
rootdir: /home/inglesp/work/ebmdatalab/ehrql
configfile: pyproject.toml
plugins: xdist-3.3.1, cov-4.1.0, hypothesis-6.87.3, mock-3.11.1
collected 5 items

tests/unit/utils/test_functools_utils.py .....                           [100%]

------------------------------ coverage ------------------------------
Name    Stmts   Miss Branch BrPart  Cover
-----------------------------------------
TOTAL      20      0      2      0   100%

1 file skipped due to complete coverage.

Required test coverage of 100.0% reached. Total coverage: 100.00%

============================== 5 passed in 0.09s ===============================

[inglesp]

@evansd can we close?

[evansd]

I still get the same error, though it's useful information that you don't.

[inglesp]

I now get this error as well.

[inglesp]

And now I don't!

[inglesp]

Aha! It depends on whether you've got a local editable install of ehrql...

[inglesp]

Right, this one's fun.

The problem's only indirectly related to Trino, and the exception is actually caused by a nasty interaction between the following:

• the way pytest handles warnings,
• the way Coverage.py handles editable installs, and
• some import-time caching in SQLAlchemy.

See inglesp/pytest-warnings-coverage-sqlalchemy for a minimal example of the problem.

The problem was introduced to this repo when we started filtering a deprecation warning from SQLAlchemy here.

Instead of filtering the warning, we can address it quite easily: a job for tomorrow.