Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unified approach to dependency management (requirements.txt) #735

Closed
Darwinkel opened this issue Apr 14, 2023 · 10 comments · Fixed by #1295
Closed

Unified approach to dependency management (requirements.txt) #735

Darwinkel opened this issue Apr 14, 2023 · 10 comments · Fixed by #1295
Assignees
Labels
dependencies Pull requests that update a dependency file

Comments

@Darwinkel
Copy link
Contributor

Darwinkel commented Apr 14, 2023

Is your feature request related to a problem? Please describe.
Currently, we have no standardized approach to dependency management in the various modules, aside from that we use pip's requirements.txt.

Describe the solution you'd like
Some things worth considering:

  • Using Poetry and a lock file (implemented in Keiko and Octopoes by @Lisser)
    • Has some nice features for managing venvs as well
  • A single top-level requirements.txt instead of per module
    • How would this impact the ability to package and run the modules in this repository individually?
  • How to distinguish between production/development/CI dependencies (e.g. linters)?
    • Currently, we use seperate requirements files
  • How can we do this in such a way that we have minimal extra work for Debian packaging?
  • Integration with dependabot is definitely nice to have

Additional context
#729

@Darwinkel Darwinkel added needs refinement dependencies Pull requests that update a dependency file discussion labels Apr 14, 2023
@Darwinkel Darwinkel added this to KAT Apr 14, 2023
@github-project-automation github-project-automation bot moved this to Incoming features / Need assessment in KAT Apr 14, 2023
@praseodym
Copy link
Contributor

I'd be in favour of one single list of dependencies so that we can share a single venv across all modules. This would also greatly speed up builds (e.g. we could base all Docker images off a single Python+venv base image).

I think this should also enable us to work with a single pyproject.toml for Poetry. Poetry also supports dependency groups so we can split out dev/ci/formatter/docs dependencies if we'd like.

@ammar92
Copy link
Contributor

ammar92 commented Apr 18, 2023

I would opt for poetry too and exporting of requirements.txt formats to be used for building the images.

I'd be in favour of one single list of dependencies so that we can share a single venv across all modules. This would also greatly speed up builds (e.g. we could base all Docker images off a single Python+venv base image).

I'm not (yet) in favor of having a single virtual environment for the entire project, because we still consider them as isolated "microservices". And even if we don't see them as such, but as e.g. standalone packages, it would still be easier to have separate environments for both development and deployment. Although your suggestion would indeed improve build speeds.

@dekkers
Copy link
Contributor

dekkers commented Apr 19, 2023

I'm working on moving our Debian packages to properly use Debian packages for the python dependencies. In that case it would be impossible to install the Debian packages if they have conflicting requirements, so I don't see the value of specifying for example the FastAPI version per service if they have to be the same.

@praseodym
Copy link
Contributor

praseodym commented Apr 20, 2023

python-poetry/poetry#936 and python-poetry/poetry#2270 provide some interesting insights on how to set up multiple subprojects (microservices) with Poetry.

@Darwinkel Darwinkel self-assigned this May 3, 2023
@Darwinkel Darwinkel moved this from Incoming features / Need assessment to In Progress in KAT May 3, 2023
@dekkers
Copy link
Contributor

dekkers commented May 16, 2023

I've used pip-compile from pip-tools in the past. It supports defining dependencies in pyproject.toml and also supports test/dev/docs dependencies with project.optional-dependencies. Pip-compile also supports generating hashes with --generate-hashes. So as far as I can see we can do everything with pip-compile that we can do with poetry.

I think the major benefit of pip-compile compared to poetry is that pip-compile only outputs the requirement files. The requirement files are also easy to read and also shows the reverse dependencies of each package (see examples in the pip-tools readme). In constrast poetry has a gigantic and unreadable poetry.lock file that we need to include in git if we want to use poetry.

@ammar92
Copy link
Contributor

ammar92 commented May 16, 2023

python-poetry/poetry#936 and python-poetry/poetry#2270 provide some interesting insights on how to set up multiple subprojects (microservices) with Poetry.

I don't really see how this helps in our case. We have isolated applications, not multiple Python packages within a single project. This also only works for a single environment, which isn't a good idea (yet) imho.

It might work if we really turn everything into a single application, with multiple packages, using the same dependencies, etc.

@Darwinkel Darwinkel moved this from In Progress to Review in KAT Jun 15, 2023
@Darwinkel Darwinkel moved this from Review to Ready for merge in KAT Jun 19, 2023
@dekkers dekkers moved this from Ready for merge to In Progress in KAT Jun 26, 2023
@Darwinkel
Copy link
Contributor Author

Darwinkel commented Jun 27, 2023

Status:

  • project root
  • keiko
  • octopoes
  • boefjes
  • bytes
  • mula
  • rocky

@Darwinkel
Copy link
Contributor Author

Just realised that boefjes requirements are actually somewhat complicated. They are partially generated from the requirements of the boefje modules, which do not include hashing, and in some cases, are not even pinned. We should probably fix this.

image

@Darwinkel Darwinkel moved this from In Progress to Review in KAT Jun 29, 2023
@dekkers dekkers moved this from Review to QA review / functional testing in KAT Jun 30, 2023
@praseodym
Copy link
Contributor

We'll be changing this when we move to new Normalisers and Boefjes runners. For now, maybe integrate everything in the Poetry configuration with clear comments of what dependencies are needed for each plugin?

@Darwinkel Darwinkel moved this from QA review / functional testing to Ready for merge in KAT Jul 3, 2023
@Darwinkel
Copy link
Contributor Author

We'll be changing this when we move to new Normalisers and Boefjes runners. For now, maybe integrate everything in the Poetry configuration with clear comments of what dependencies are needed for each plugin?

I've checked and modified the pipeline a bit in the linked PR. It's fine for now imho and does its job. We'll refactor this when we get to containerization.

@github-project-automation github-project-automation bot moved this from Ready for merge to Done in KAT Jul 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
Archived in project
4 participants