Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

[WIP] Use Poetry to manage Synapse's dependencies #12337

Closed
wants to merge 42 commits into from

Conversation

DMRobertson
Copy link
Contributor

@DMRobertson DMRobertson commented Mar 30, 2022

Reviewable commit-by-commit. Hopefully. Ignoring the lockfile, the diffstats for each individual commit should hopefully be managable.

I'd appreciate eyes on the output from CI. I'd like to ensure that it's still reasonably easy to follow, and clear that we're using the poetry locked environment rather than pip-installing the newest version of everything.

This set of changes:

  • uses poetry rather than setuptools to declare Synapse's project metadata and dependencies;
  • updates the docker image and debian package to use dependency versions managed in the poetry lockfile;
  • runs CI in the locked poetry environment; and
  • updates docs to strongly recommend using poetry for development purposes.

Included at the start are a couple of DEBUG commits which will ensure that the CI run for
5f8db3e will build all debian packages and run the twisted CI job. Before this is merged, they'll need to be undone.

Closes #11537.

TODO:

  • bump version number to appropriate version before merging,
  • compare the wheels and sdists generated on develop and on this branch.
  • CI that pip installs the latest released version of everything, to anticipate problems from people installing from wheels. (We'll no longer get notifications of this via our CI). This is probably best done in a separate PR.

David Robertson and others added 15 commits March 30, 2022 14:07
- Nuke python_dependencies and setup.py
- Allow commiting poetry.lock
- Initial lockfile pulling in the latest version of deps at the time of locking
Missed in #12088. We got away with it because we were indirectly
importing it in other places. Without this, we encounter pain in `poetry
export`; and besides, we should be explicit about our direct
dependencies.
See comment in the toml file:

>  The duplication here is awful. I hate hate hate hate hate it.

I don't think that we're going to be adding many extras any time soon,
and I think this is the least bad option.
After #12107 it's much easier for black, isort and flake8 to find the
scripts we want them to lint.
In particular the documentaion for contributors should only advocate
poetry.

I am not thrilled at the propsect of now having N+1 installation
methods---but at the very least we ought to mention poetry here.
debian/changelog Outdated Show resolved Hide resolved
docker/Dockerfile-dhvirtualenv Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
- Stop always running twisted trunk on this branch
- Stop building all devs on this branch
- Run olddeps after linting
@DMRobertson DMRobertson marked this pull request as ready for review March 30, 2022 17:28
@DMRobertson DMRobertson requested a review from a team as a code owner March 30, 2022 17:28
@babolivier babolivier self-assigned this Mar 31, 2022
Copy link
Contributor

@squahtx squahtx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

skimmed the changes - would appreciate more eyes on this!

.ci/scripts/test_old_deps.sh Outdated Show resolved Hide resolved
.github/workflows/tests.yml Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@babolivier babolivier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for working on this! Really looking forward to seeing this PR land.
Not sure if the failing CI job is a flake or not...

docker/Dockerfile Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
synapse/__init__.py Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
docs/code_style.md Outdated Show resolved Hide resolved
docs/development/contributing_guide.md Outdated Show resolved Hide resolved
docs/upgrade.md Outdated Show resolved Hide resolved
docs/upgrade.md Outdated Show resolved Hide resolved
Co-authored-by: Sean Quah <[email protected]>
Co-authored-by: Brendan Abolivier <[email protected]>
David Robertson and others added 2 commits March 31, 2022 16:39
DMRobertson pushed a commit that referenced this pull request Apr 8, 2022
instead, configure the tools to find sensible targets.

Pulled out from #12337; part (but not all) of
e53e99e. Related: #12107.
@DMRobertson DMRobertson marked this pull request as draft April 8, 2022 12:45
DMRobertson pushed a commit that referenced this pull request Apr 11, 2022
Olddeps and twisted trunk tests are handled in separate PRs.

The PyPy config is a best-effort only; it's completely untested.

Pulled out from #12337.
DMRobertson pushed a commit that referenced this pull request Apr 12, 2022
* Run "main" trial tests under poetry

Olddeps and twisted trunk tests are handled in separate PRs.

The PyPy config is a best-effort only; it's completely untested.

Pulled out from #12337.

* Changelog
@squahtx squahtx removed their request for review April 13, 2022 13:37
David Robertson added 3 commits April 14, 2022 10:42
I think these may have been retained in merge confusion.
we did need it to `install` though, (I think?)
@DMRobertson DMRobertson changed the title Use Poetry to manage Synapse's dependencies [WIP] Use Poetry to manage Synapse's dependencies Apr 14, 2022
@DMRobertson
Copy link
Contributor Author

I've ripped out what I need to from this. Many thanks all for you eyes and words.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lock Dependencies in Synapse
6 participants