Flowmachine has incompatible sqlalchemy version

[jc-harrison]

Flowmachine is incompatible with sqlalchemy >= 2.0, due to the direct use of strings in transaction.execute() (and quite possibly other reasons as well):

>>> flowmachine.core.context.get_db().has_table("calls", schema="events")
Traceback (most recent call last):
  File "/root/.local/share/virtualenvs/flowmachine-Lph78dnj/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1410, in execute
    meth = statement._execute_on_connection
AttributeError: 'str' object has no attribute '_execute_on_connection'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/FlowKit-1.18.3/flowmachine/flowmachine/core/connection.py", line 174, in has_table
    return trans.execute(exists_query).fetchall()[0][0]
  File "/root/.local/share/virtualenvs/flowmachine-Lph78dnj/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1412, in execute
    raise exc.ObjectNotExecutableError(statement) from err
sqlalchemy.exc.ObjectNotExecutableError: Not an executable object: "SELECT EXISTS(\n        SELECT * FROM information_schema.tables \n            WHERE table_name='calls'\n         AND table_schema='events')"

We added a sqlalchemy version constraint (174b42e) to work around this, but dependabot immediately bumped the version (4f0f779), and it appears our tests didn't catch this incompatibility. Hence the latest release of the flowmachine docker image (1.18.3) has an incompatible sqlalchemy version and is therefore broken.

[jc-harrison]

Part of the problem here is #6006 - sqlalchemy is still constrained correctly in flowmachine/setup.py, so the sqlalchemy version installed in the integration tests env is compatible with flowmachine. Interestingly though, in this instance the pipenv run python setup.py install in flowmachine.Dockerfile doesn't overwrite the pipenv-installed sqlalchemy with a version compatible with flowmachine/setup.py, for reasons I don't understand. In the 1.18.3 docker build, the build logs imply that the pipenv run python setup.py install step installs sqlalchemy 1.4.47, but the version actually installed in the final image is still 2.0.7.

This doesn't explain why the containerised integration tests are passing (i.e. the end-to-end query tests) - presumably none of them are testing any of the code that doesn't work with sqlalchemy 2.0, but I'm surprised by this.

[jc-harrison]

This doesn't explain why the containerised integration tests are passing (i.e. the end-to-end query tests) - presumably none of them are testing any of the code that doesn't work with sqlalchemy 2.0, but I'm surprised by this.

Ah, no, my mistake - I thought the end-to-end integration tests ran flowmachine in a container, but they don't. So I don't think we actually test the flowmachine docker image at all...

[Thingus]

Related to #6006 here?

[jc-harrison]

The flowmachine unit tests should pick up this issue, but they don't because in CI we install the deps via pipenv install --dev --deploy && pipenv run pip install -e ., which overwrites the pipenv-locked sqlalchemy version with an unpinned version that meets the constraints in flowmachine/setup.py. The pipenv run pip install -e . here is unnecessary, because editable flowmachine install is already included in the dev dependencies in the PIpfile - removing this should cause the flowmachine tests to (correctly) fail, which would block dependabot from bumping sqlalchemy to an incompatible version.

I think there are a number of underlying issues that should be fixed here:

• Remove unnecessary pip install -e . in CI config, so that unit tests are actually run using the same dependencies as are built into the docker image
• Fix Pinned dependencies getting out of sync between Pipfile and setup.py #6006, so that installation from setup.py isn't incompatible with installation from Pipfile
• Move tests in integration_tests/tests/flowmachine_tests and integration_tests/tests/flowmachine_server_tests into flowmachine/tests/integration/ so that they run in the same env as the unit tests (i.e. with the same locked dependencies that are built into the docker image) - this way, there's one less place for dependencies to get out of sync
• In the other integration tests, containerise flowmachine and flowapi, so that we're actually testing the docker images instead of a separately-locked python env just for the integration tests

[jc-harrison]

I think there are a number of underlying issues that should be fixed here:

• Remove unnecessary pip install -e . in CI config, so that unit tests are actually run using the same dependencies as are built into the docker image
• Fix Pinned dependencies getting out of sync between Pipfile and setup.py #6006, so that installation from setup.py isn't incompatible with installation from Pipfile
• Move tests in integration_tests/tests/flowmachine_tests and integration_tests/tests/flowmachine_server_tests into flowmachine/tests/integration/ so that they run in the same env as the unit tests (i.e. with the same locked dependencies that are built into the docker image) - this way, there's one less place for dependencies to get out of sync
• In the other integration tests, containerise flowmachine and flowapi, so that we're actually testing the docker images instead of a separately-locked python env just for the integration tests

The first of these points should be sufficient to resolve the immediate issue here. The second already has its own issue, and I'll open a new issue for the other two.

[jc-harrison]

New issue for integration test changes: #6053