Pinned dependencies getting out of sync between Pipfile and setup.py

[jc-harrison]

Describe the bug
We are duplicating dependency constraints in the various FlowKit python packages (flowapi, flowauth, flowclient, flowkit-jwt-generator, flowmachine; no longer the case for flowetl since #5962) - constraints in setup.py and Pipfile must be manually kept in sync in each case.

This alone is an issue, since it runs the risk that if we update one but forget to update the other, the dependency versions used for testing will be different from those that would be installed via pip install flow<package>. Pipenv's recommendation when using both setup.py and Pipfile is to add the local package as an editable requirement in the Pipfile. This would be different from our current setup, though - currently a non-dev pipenv install will install all of the package dependencies but not the package itself (editable or otherwise).

It becomes more of an issue, though, because dependabot automatically bumps pinned versions in the Pipfiles without changing the corresponding versions in setup.py. Because we editable-install the package in the dev dependencies, this inconsistency leads to an un-lockable env, but for whatever reason that doesn't stop dependabot from making the change (I think because dependabot doesn't correctly handle dependencies of local-path requirements), and it still produces a pipenv install-able Pipfile.lock. I don't think this is a problem for the dev install - locked (upgraded) deps won't be overwritten by versions specified in setup.py, so unit tests will be correctly run using the dependabot-upgraded dependencies - but it is a problem in the docker images (which therefore affects some of the integration tests). In the Dockerfiles we first do a non-dev pipenv install, and then run pipenv run python setup.py install, which will overwrite the pipenv-pinned versions with versions specified in setup.py. Similarly, non-containerised integration tests will use dependencies from the local-path installs of the packages in the integration tests env, which will pick up dependencies from each package's setup.py (not from that package's Pipfile). The result is that dependabot can bump version pins in a Pipfile, and integration tests will continue to use the older versions pinned in setup.py, so Pipfile pins can be "successfully" bumped to versions that would break the integration tests.

Installing the packages without dependencies in the Dockerfiles (e.g. using pip install --no-deps . instead of python setup.py install) would avoid overwriting the locked dependencies, which would resolve some of the problems here. But this then means the pins in setup.py are completely irrelevant for the docker images (but still applicable for the non-containerised integration tests). So ideally we need to also ensure the constraints in setup.py are automatically incorporated into the Pipfiles.

[Thingus]

What do we want our single source of truth to be; setup.py or the Pipfile? Given that setup.py is a Python script underneath it all, would the right move here be to replace the requirements section in setup.py with something that goes and grabs the requirements out of the Pipfile on install?

The result is that dependabot can bump version pins in a Pipfile, and integration tests will continue to use the older versions pinned in setup.py, so Pipfile pins can be "successfully" bumped to versions that would break the integration tests.

Just so I'm clear - this is undesirable behaviour, right? We want the intergration tests to be in step with the rest of the distribution.

[jc-harrison]

What do we want our single source of truth to be; setup.py or the Pipfile? Given that setup.py is a Python script underneath it all, would the right move here be to replace the requirements section in setup.py with something that goes and grabs the requirements out of the Pipfile on install?

I think setup.py should be the source of truth - the other way around feels wrong to me, and not in keeping with the direction of python packaging overall moving towards static config (i.e. pyproject.toml instead of setup.py). The pipenv docs also recommend telling pipenv to lock the setup.py-declared dependencies, not the other way around.

It may be sufficient to stick an editable install of the package in the Pipfile (and remove the pinned requirements duplicated from setup.py) - this is the recommended way to do it, and I can't see an obvious reason for this to be a bad idea. Given the difficulties dependabot has with editable requirements from a local path, though, this change may further break the usefulness of dependabot.

Just so I'm clear - this is undesirable behaviour, right? We want the intergration tests to be in step with the rest of the distribution.

Correct - we want everything to be tested (and deployed) using the pipenv-locked dependencies.

[greenape]

Tbh I think the bot is counter productive at this point, my speculation would be that the way to go is a gh action for doing the bumping.

[jc-harrison]

It's worth noting that pip-tools can lock a setup.py directly, rather than needing a separate input file. Although as I found when switching flowetl over from pipenv to pip-tools, we'd then have to add a fair bit more boilerplate for managing virtual envs.

[Thingus]

So a Pipfile would just be

[packages]
flowmachine = {editable = true, path = "."}

and install_requires gets eveything that was in [packages]?

[jc-harrison]

Yep. In most cases, I think install_requires should already have everything that's in [packages].