CI build step relies on pyproject.toml's unpinned dependencies

[StevenMaude]

The problem

Our build CI check failed a few times at the end of July, because of the following:

1. pymssql is one of our dependencies. It uses Cython.
2. The pymssql maintainers had pushed a new release to PyPI.
3. The new pymssql release did not, at the time our build CI check ran, have the wheels published: Build packages for all platforms and publish to pypi uniformly, rather than platform-by-platform. pymssql/pymssql#832 ¹
4. Our requirements for use in development are pinned to explicit versions in requirements.*.txt files. But pip install . uses the dependencies in pyproject.toml.
5. pyproject.toml has unpinned dependencies, and so pip install . uses the latest version of pymssql:

   ehrql/pyproject.toml

   Lines 14 to 26 in 4770ec6

   	dependencies = [
   	"pyarrow",
   	"sqlalchemy",
   	"structlog",
   	# Database driver for MS-SQL
   	"pymssql",
   	# Gives us isolation from the system version of SQLite and means we don't
   	# need to worry about e.g. some versions of SQLite missing the `FLOOR`
   	# function.
   	"sqlean.py",
   	]

6. Because there was no wheel of the latest pymssql, our build CI step tried to compile the latest version of pymssql from source.
7. The pip install of the ehrQL source failed because pymssql could not be installed. The pymssql build failed because the build dependencies were not all setup.

Solutions

Possible solutions

1. Specify the dependencies and dev dependencies with pinned versions in pyproject.toml. It is still possible to keep these dependencies separated out in pyproject.toml.
   • We can also still generate requirements.*.txt files with hashes.
   • This is probably the better solution in the long run. But does Dependabot still have issues by doing this?
     • I previously tried moving the dependencies into pyproject.toml previously (Production dependencies not getting updated by Dependabot #964), and it looked like transitive dependencies weren't always getting updated.
     • Does Dependabot rebuild the requirements.*.txt files, as well as pyproject.toml or only update one of these files?
2. Drop the build step from CI entirely.
   • Maybe valid if we're only ever intending anyone to run the Docker image, rather than do a pip install?
   • However, there is a valid use case for a pip install version of ehrQL: auto-completion in an editor or IDE. I've used this when testing out dev containers.
3. Something else that someone other than me thinks of.

Probable not-a-solution

Installing the requirements from the requirements.*.txt files in CI, and then doing pip install . for ehrql. This would have fixed the build step. But is probably not a good workaround. Someone doing a pip install themselves would still have encountered a failure.

Original issue

Not checked conclusively, but we were getting the following happening on new PRs in the CI build step.

Example:

Building wheels for collected packages: opensafely-ehrql, pymssql
  Building wheel for opensafely-ehrql (pyproject.toml): started
  Building wheel for opensafely-ehrql (pyproject.toml): finished with status 'done'
  Created wheel for opensafely-ehrql: filename=opensafely_ehrql-2+local-py3-none-any.whl size=139521 sha256=e1a096bff7d09275af01e9535db4b45d17169fd7423f3a7d4960f48b858a0b86
  Stored in directory: /tmp/pip-ephem-wheel-cache-27hsiz0f/wheels/f3/93/b2/2e8e07ec9c2f16c3df02264d0dba776c5efb0835029cde6f0e
  Building wheel for pymssql (pyproject.toml): started
  Building wheel for pymssql (pyproject.toml): finished with status 'error'
  error: subprocess-exited-with-error
  
  × Building wheel for pymssql (pyproject.toml) did not run successfully.
  │ exit code: 1
  ╰─> [43 lines of output]
      setup.py: platform.system() => Linux
      setup.py: platform.architecture() => ('64bit', 'ELF')
      setup.py: platform.libc_ver() => ('glibc', '2.35')
      setup.py: include_dirs => []
      setup.py: library_dirs => []
      running bdist_wheel
      running build
      running build_py
      creating build
      creating build/lib.linux-x86_64-cpython-311
      creating build/lib.linux-x86_64-cpython-311/pymssql
      copying src/pymssql/__init__.py -> build/lib.linux-x86_64-cpython-311/pymssql
      running build_ext
      warning: src/pymssql/_mssql.pyx:27:0: The 'DEF' statement is deprecated and will be removed in a future Cython version. Consider using global variables, constants, and in-place literals instead. See https://github.com/cython/cython/issues/4310
      warning: src/pymssql/_mssql.pyx:28:0: The 'DEF' statement is deprecated and will be removed in a future Cython version. Consider using global variables, constants, and in-place literals instead. See https://github.com/cython/cython/issues/4310
      warning: src/pymssql/_mssql.pyx:29:0: The 'DEF' statement is deprecated and will be removed in a future Cython version. Consider using global variables, constants, and in-place literals instead. See https://github.com/cython/cython/issues/4310
      warning: src/pymssql/_mssql.pyx:30:0: The 'DEF' statement is deprecated and will be removed in a future Cython version. Consider using global variables, constants, and in-place literals instead. See https://github.com/cython/cython/issues/4310
      warning: src/pymssql/_mssql.pyx:31:0: The 'DEF' statement is deprecated and will be removed in a future Cython version. Consider using global variables, constants, and in-place literals instead. See https://github.com/cython/cython/issues/4310
      warning: src/pymssql/_mssql.pyx:32:0: The 'DEF' statement is deprecated and will be removed in a future Cython version. Consider using global variables, constants, and in-place literals instead. See https://github.com/cython/cython/issues/4310
      warning: src/pymssql/_mssql.pyx:35:0: The 'DEF' statement is deprecated and will be removed in a future Cython version. Consider using global variables, constants, and in-place literals instead. See https://github.com/cython/cython/issues/4310
      warning: src/pymssql/_mssql.pyx:36:0: The 'DEF' statement is deprecated and will be removed in a future Cython version. Consider using global variables, constants, and in-place literals instead. See https://github.com/cython/cython/issues/4310
      warning: src/pymssql/_mssql.pyx:38:0: The 'DEF' statement is deprecated and will be removed in a future Cython version. Consider using global variables, constants, and in-place literals instead. See https://github.com/cython/cython/issues/4310
      warning: src/pymssql/_mssql.pyx:82:0: The 'IF' statement is deprecated and will be removed in a future Cython version. Consider using runtime conditions or C macros instead. See https://github.com/cython/cython/issues/4310
      warning: src/pymssql/_mssql.pyx:230:0: The 'DEF' statement is deprecated and will be removed in a future Cython version. Consider using global variables, constants, and in-place literals instead. See https://github.com/cython/cython/issues/4310
      warning: src/pymssql/_mssql.pyx:264:4: The 'IF' statement is deprecated and will be removed in a future Cython version. Consider using runtime conditions or C macros instead. See https://github.com/cython/cython/issues/4310
      warning: src/pymssql/_mssql.pyx:333:4: The 'IF' statement is deprecated and will be removed in a future Cython version. Consider using runtime conditions or C macros instead. See https://github.com/cython/cython/issues/4310
      warning: src/pymssql/_mssql.pyx:831:8: The 'IF' statement is deprecated and will be removed in a future Cython version. Consider using runtime conditions or C macros instead. See https://github.com/cython/cython/issues/4310
      warning: src/pymssql/_mssql.pyx:919:8: The 'IF' statement is deprecated and will be removed in a future Cython version. Consider using runtime conditions or C macros instead. See https://github.com/cython/cython/issues/4310
      warning: src/pymssql/_mssql.pyx:1005:12: The 'IF' statement is deprecated and will be removed in a future Cython version. Consider using runtime conditions or C macros instead. See https://github.com/cython/cython/issues/4310
      warning: src/pymssql/_mssql.pyx:1379:16: The 'IF' statement is deprecated and will be removed in a future Cython version. Consider using runtime conditions or C macros instead. See https://github.com/cython/cython/issues/4310
      warning: src/pymssql/_mssql.pyx:1713:8: The 'IF' statement is deprecated and will be removed in a future Cython version. Consider using runtime conditions or C macros instead. See https://github.com/cython/cython/issues/4310
      Compiling src/pymssql/_mssql.pyx because it changed.
      [1/1] Cythonizing src/pymssql/_mssql.pyx
      building 'pymssql._mssql' extension
      creating build/temp.linux-x86_64-cpython-311
      creating build/temp.linux-x86_64-cpython-311/src
      creating build/temp.linux-x86_64-cpython-311/src/pymssql
      gcc -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/opt/hostedtoolcache/Python/3.11.4/x64/include/python3.11 -c src/pymssql/_mssql.c -o build/temp.linux-x86_64-cpython-311/src/pymssql/_mssql.o -DMSDBLIB
      src/pymssql/_mssql.c:1127:10: fatal error: sqlfront.h: No such file or directory
       1127 | #include "sqlfront.h"
            |          ^~~~~~~~~~~~
      compilation terminated.
      error: command '/usr/bin/gcc' failed with exit code 1
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
  ERROR: Failed building wheel for pymssql
ERROR: Could not build wheels for pymssql, which is required to install pyproject.toml-based projects

This failure meant PRs wouldn't get a ✔️ because the build step failed.

Footnotes

1. ⚠️ While that issue is closed, it has not been addressed. The same problem could still break our build in future. If other dependencies we use need to be compiled, the same problem could apply with them too. ↩

[StevenMaude]

I think this was a transient issue with pymssql, which is used for the pyproject.toml install here, as it's one of the dependencies.

This build process doesn't use pinned dependencies.

Think this issue to blame, and now fixed. At least, there are wheels for the current pymssql version. They haven't yet, I don't believe, changed their release process to make wheels available when the release is published, so this doesn't happen again.

[StevenMaude]

Have manually re-run the CI failed jobs on the affected PRs, and these are now working.

[StevenMaude]

We could still consider reworking how we manage the dependencies. Maybe the requirements should all go into pyproject.toml, and then generate requirements.*.txt files from them.

I believe I tried this before and had issues with transitive dependencies not getting updated, but maybe it just works now? However, Dependabot also has issues with not updating multiple source of requirements in Python repositories, unfortunately, that I don't know are fixed. See #964 for more details.

[inglesp]

Closing for now, as this is unlikely to affect users. Thanks @StevenMaude for the write-up -- if this rears its head again, we'll not be starting from square one.