Respect poetry lockfile in Synapse container, if present #1220
Conversation
Signed-off-by: Sean Quah <[email protected]>
`--no-use-pep517` is incompatible with a poetry-using `pyproject.toml`. Signed-off-by: Sean Quah <[email protected]>
Signed-off-by: Sean Quah <[email protected]>
Code to be uncommented once Synapse develop is on poetry. Signed-off-by: Sean Quah <[email protected]>
Poetry will want to create `.venv/` inside of the Synapse source directory, which means we have to make a copy to avoid writing to the `/src` mounted volume. Signed-off-by: Sean Quah <[email protected]>
Signed-off-by: Sean Quah <[email protected]>
squahtx
force-pushed
the
squah/pyproject-poetry-contribs
branch
from
4fc5b04 to
9b6d8b7
Compare
|
CI is failing because it's running against old containers. If desired, I could split this into two PRs, one for the Dockerfile and one for the scripts, but neither makes much sense in isolation. |
squahtx
changed the title
scripts/synapse_sytest.sh
Outdated
| else | ||
| # We've already created the virtualenv, but lets double check we have all | ||
| # deps. | ||
| /venv/bin/pip install -q --upgrade --no-cache-dir "$SYNAPSE_SOURCE"[redis] |
There was a problem hiding this comment.
I'm surprised we only install redis here, rather than all!
scripts/synapse_sytest.sh
Outdated
| poetry install --extras redis | ||
| popd | ||
| else | ||
| # Install Synapse and dependencies using pip. As of pip 20.1, this will | ||
| # try to build Synapse in-tree, which means writing changes to the source | ||
| # directory. | ||
| # The virtual env will already be populated with dependencies from the | ||
| # Docker build. | ||
| echo "Installing Synapse using pip..." | ||
| /venv/bin/pip install -q --upgrade --no-cache-dir /synapse[redis] |
There was a problem hiding this comment.
I think this should be all rather than redis---a bug from before. (If any extra other than redis changed since the container was built, that won't be picked up in this script---I think?)
There was a problem hiding this comment.
(to be addressed in a PR I will insert after #1219 but before this one)
| # Install Synapse and dependencies using pip. As of pip 20.1, this will | ||
| # try to build Synapse in-tree, which means writing changes to the source | ||
| # directory. | ||
| # The virtual env will already be populated with dependencies from the | ||
| # Docker build. | ||
| echo "Installing Synapse using pip..." | ||
| /venv/bin/pip install -q --upgrade --no-cache-dir /synapse[all] |
There was a problem hiding this comment.
I guess we will end up nuking this once poetry has landed?
There was a problem hiding this comment.
Not entirely sure. I'm half thinking that we should keep compatibility for pre-poetry versions of synapse. Then again you can just specify a pre-poetry branch of sytest?
There was a problem hiding this comment.
I'd be inclined to leave it for a few releases and then do the nuking. One for the future, I suppose.
Install pinned dependencies using poetry if a
poetry.lockfile ispresent in the Synapse source directory, otherwise fall back to pip.
The Synapse container installs Synapse dependencies twice, once at
Docker build time and once at test time. The former installation is
intended to speed up the latter. To make this poetry-compatible, we
have to convince poetry to use the same virtual env both times. By
using the
virtualenvs.in-projectconfig option, we can get poetry touse a virtual env at
./.venv. The source directory is going to be amounted volume and writing into it is impolite, so we have to make a
copy of it. For simplicity, we ditch the tarball for non-poetry
installs and always make a copy.
To make offline mode work, we have to make a cached copy of a
poetry-corewheel available to pip. For simplicity, we do the samething for
setuptoolsandwheelfor pre-poetry Synapses.Once Synapse develop has been converted to poetry, the TODO in
docker/synapse.Dockerfilecan be resolved.NB: This blocks progress on poetry-izing Synapse, since some of the CI
steps can't be updated until the Synapse container includes poetry.
Can be reviewed commit by commit. The first commit is from #1219.
To test the container changes locally: