Resolve "Ewoks install requirements should contain the python and pip version"#65
Conversation
|
In GitLab by @woutdenolf on Nov 6, 2025, 16:22 GMT+1: added 1 commit
Compare with previous version (link unavailable after force-push) |
|
In GitLab by @woutdenolf on Nov 6, 2025, 16:22 GMT+1: added 3 commits
Compare with previous version (link unavailable after force-push) |
|
In GitLab by @woutdenolf on Nov 6, 2025, 18:09 GMT+1: added 1 commit
Compare with previous version (link unavailable after force-push) |
|
In GitLab by @woutdenolf on Nov 6, 2025, 18:21 GMT+1: added 1 commit
Compare with previous version (link unavailable after force-push) |
|
In GitLab by @woutdenolf on Nov 7, 2025, 17:01 GMT+1: added 1 commit
Compare with previous version (link unavailable after force-push) |
|
In GitLab by @woutdenolf on Nov 7, 2025, 18:26 GMT+1: added 1 commit
Compare with previous version (link unavailable after force-push) |
|
In GitLab by @woutdenolf on Nov 8, 2025, 13:04 GMT+1: added 1 commit
Compare with previous version (link unavailable after force-push) |
|
In GitLab by @woutdenolf on Jan 25, 2026, 16:23 GMT+1: added 1 commit
Compare with previous version (link unavailable after force-push) |
|
In GitLab by @woutdenolf on Jan 25, 2026, 16:26 GMT+1: added 11 commits
Compare with previous version (link unavailable after force-push) |
|
In GitLab by @woutdenolf on Jan 25, 2026, 16:32 GMT+1: added 1 commit
Compare with previous version (link unavailable after force-push) |
|
In GitLab by @woutdenolf on Jan 25, 2026, 16:37 GMT+1: added 1 commit
Compare with previous version (link unavailable after force-push) |
|
In GitLab by @woutdenolf on Jan 25, 2026, 16:50 GMT+1: added 1 commit
Compare with previous version (link unavailable after force-push) |
|
In GitLab by @woutdenolf on Jan 25, 2026, 16:57 GMT+1: added 1 commit
Compare with previous version (link unavailable after force-push) |
|
In GitLab by @woutdenolf on Jan 26, 2026, 08:29 GMT+1: added 1 commit
Compare with previous version (link unavailable after force-push) |
|
In GitLab by @woutdenolf on Jan 26, 2026, 09:15 GMT+1: added 1 commit
Compare with previous version (link unavailable after force-push) |
|
In GitLab by @woutdenolf on Jan 26, 2026, 09:16 GMT+1: added 1 commit
Compare with previous version (link unavailable after force-push) |
|
In GitLab by @woutdenolf on Jan 26, 2026, 10:55 GMT+1: added 1 commit
Compare with previous version (link unavailable after force-push) |
9332091 to
cfde0c6
Compare
2468da5 to
99ec2e1
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
88b456c to
082db24
Compare
| class BaseRequirements(BaseModel): | ||
| system: SystemInfo | ||
| python: PythonInfo | ||
| distributions: List[Distribution] |
There was a problem hiding this comment.
Maybe call this python_distributions? Or put distributions in PythonInfo?
There was a problem hiding this comment.
We might not want the model to be too python centric although current everything happens in python.
loichuder
left a comment
There was a problem hiding this comment.
Could you explain a bit more this:
The model has now two lists of dependencies
Are both lists part of the requirements field of the workflow, now ?
When gathering requirements with ewoks convert, are both lists filled? If so, by which package manager?
And when running ewoks install on a single workflow with no requirements, what happens ? Are both lists generated ? Only one ?
Yes. We always gather the list of python distributions using The Pip manager in addition gathers The Pip manager first tried to install this For conda and pixi the second list will not be something we can fully derive from the python distributions but a similar fallback would at least give a chance of installing even if you do not have conda or pixi. Very often we have pure python packages anyway, regardless of what package manager we use on either side (gather or install). So the fallback to deriving something from the distributions is useful. |
What do you mean by "python distributions" and how is it related to Python packages? |
A distribution (the thing you pip install) can contain several packages (the things you import). For example
https://gitlab.esrf.fr/workflow/ewoksapps/ewoksxrpd/-/tree/main/src?ref_type=heads Edit: sorry, your question is in the context of https://docs.python.org/3/library/importlib.metadata.html
Edit: an example Distribution: Pillow |
a9d0334 to
bf49bdc
Compare
bf49bdc to
cd02d86
Compare
cd6cddb to
3878cdc
Compare
|
Following your review I merged all manager specific stuff in the manager classes themselves. This was a huge refactoring so please review again. I also harmonized the CLI ### Added
- `ewoks install`: add `--package-manager-name` and `--package-manager-command` arguments.
- `ewoks convert`: add `--package-manager-name` and `--package-manager-command` arguments.
### Removed
- `ewoks install`: remove `-p/--python` argument.Originally I was just replacing |
|
Note that the manager classes are private API. What is public are the CLI modifications and the structure of the ewoks graph attribute I just realized that since we merged everything in the modules of the manager classes, we would probably move the entire |
3c8e964 to
19dcca9
Compare
| def _install_base_requirements(self, requirements: BaseRequirements) -> bool: | ||
| raise NotImplementedError(f"{self.NAME} installation of python distributions") | ||
|
|
||
| def _get_conda_command(self) -> Tuple[str, ...]: |
There was a problem hiding this comment.
That seems difficult to encompass all conda executables.
I think there is also miniconda for example ?
There was a problem hiding this comment.
The command is always conda or mamba, except for micromamba iirc.
|
I will wait until we release the current RC and then merge this. Thanks! |
Closes #41
Migrated from GitLab: https://gitlab.esrf.fr/workflow/ewoks/ewoks/-/merge_requests/233
Review tip
_requirementsandtestsand review the resttests_requirements: more details below and in github code commentsIf you have experience with other package managers like
uv, please check whether the package manager derived fromBaseManagergoes in the right direction. OnlyPipManageris currently enabled and tested. The others are not used, private and probably contain errors.I think this is better to make a PR per package manager after this PR is merged.
Changes
graph["requirements"].graph["requirements"]list (pip freeze) is still parsed when doingewoks install(converted to a dict for pip).importlib.metadataand the other manage specific (like freeze for pip). This gives the installer a chance event if they do not have the same package manager.ewoks install --python ...is replaced with the more generalewoks install --manager ...Not supported yet but prepare for supporting other package managers in addition to pip: pixi, conda, uv, poetry, pipenv.
Private API: top-level
ewoks._requirements.__init__:add_requirements(graph: TaskGraph) -> None: Add requirements to a workflow definition in-placeget_requirements(graph: TaskGraph) -> BaseRequirements: Extract requirements from a workflow definition.install_requirements(requirements: BaseRequirements) -> None: Install workflow requirements.Private API: implementation
Models
ewoks._requirements.models: the pydantic modelsBaseRequirements: python version, installed distributions, ...PipRequirementsPixiRequirementsReview: have a close look at the models so we avoid going through deprecation cycles because I missed obvious flaws.
Package Managers
ewoks._requirements.managers: package managers to gather requirements metadata and install requirements metadataBaseManagerPipManager: support fallback to installing the distributions if installing the freeze list fails.PixiManagerRequirements metadata
ewoks._requirements.metadata: gathering and parse requirements metadataimportlib