-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Add option to skip installing directory dependencies #6845
Add option to skip installing directory dependencies #6845
Conversation
I think this fixes #668 |
@radoering while we wait for review on python-poetry/poetry-core#520, would you mind taking a look at this and giving some thoughts? Thanks! |
If I understand correctly, the main motivation for this feature is the Docker cache. Before taking a look into the implementation details, I'd like to here the opinions of other maintainers if this feature makes sense for them. Especially @neersighted because he has much more experience with Docker than I do. Just one question: Does it make sense to lump directory and file dependencies together (since both are path dependencies) or should it only consider directory dependencies? |
Great question. I’m not super familiar with workflows around file dependencies, I’ve never used them. The fundamental question as far as caches are concerned is “does this likely change every build (source code) or only rarely (dependencies)?”. |
I'm not convinced this is useful, as in the use-case shown here (not building a package, just using Poetry to set up an environment), groups could easily be used. I don't think path dependencies should be special at all; encouraging users to think of them as 'special' takes us down a path we have strictly wanted to avoid (e.g. leaking metadata beyond the Core Metadata spec between layers of the dependency tree because Poetry is used all the way down), and reduces the interoperability of Poetry with the rest of the ecosystem. I think that documenting the use of this pattern, but with groups, can help the people who are trying to do monorepos today, and that a workspace-like approach like suggested in #2270 is what we should explore, instead of trying to tear down the Core Metadata/PEP 517 wall between Poetry projects. |
could you explain how? I use dependency groups in conjunction with path dependencies (each group is just a path dependency thus allowing selecting/deselecting subprojects; see https://github.com/adriangb/python-monorepo/tree/main/poetry) in a real world monorepo and it works great aside from docker caching. I don’t see how “using dependency groups” solves the same problem this PR does? |
Hmm, I did not realize that this PR was targeted at the pattern in your example repo there. While I brought up the overall direction for better supporting monorepos as kind of a extension of my thinking re: this PR, I did not realize that is at the core of what you want to do. I think that makes my objection firmer -- special support for using path dependencies in this way (with a superproject that delegates to subprojects) is not desirable because it is ultimately incomplete unless we tear down the metadata barrier, and all the designs along that line have been rejected to date. It's possible we decide that the subproject approach is not tenable and go that way after all, but I think it would have to be comprehensive and not piecemeal. It would probably be better if it was a new dependency type if that were the case, instead of overloading Regarding the use case you have, I'm afraid I still don't see how what you want to do is not possible with dependency groups. Could one not do the following (once python-poetry/poetry-core#520 is in, with all your groups set to optional)? FROM python:3.11
COPY pyproject.toml poetry.lock .
RUN pip install poetry && poetry install --no-root
COPY src/ ./src
RUN poetry install --only-root
COPY folder-of-path-deps/ folder-of-path-deps/
RUN poetry install --no-root --with <path-dep-groups> |
I haven’t heard any objection to my design. If you can think of any issues let's discuss in #6850.
Yes this is a small isolated piece, but that is by design. Part of the attractive of this design is that it can be implemented in small incremental steps, most of which except maybe this one are already completed and we’re beneficial to Poetry regardless of the acceptance of the proposed monorepo design. So I wouldn’t really call it “piecemeal”.
I don’t think things will work properly without this PR but I’m away from a computer to confirm, I can test later or feel free to give it a crack with my example repo. |
Ok so I had a chance to look at your example and test in my example repo. The issue with the solution you proposed is that you never install the transitive 3rd party dependencies (which may even be all 3rd party dependencies if there is no "root" pacakge/dependencies), so there's no layer that has "all of the things that are expensive to install but don't change often" which is what we want for good caching. I updated my example repo to use this branch (adriangb/python-monorepo@0250dda) and tested that indeed doing So at least as proposed dependency groups are not an alternative to this PR. |
pre-commit.ci autofix |
97580c6
to
0d502a1
Compare
0d502a1
to
2aa8f1f
Compare
2aa8f1f
to
97b890d
Compare
@radoering I renamed |
I think this will also be useful in CI workflows (so not Docker but similar) since the same principle of transitive deps changing less than source code apply. |
…_skip_directory_flag
@rodering, I addressed all of the feedback and got the branch in sync with the default branch. Apologies if the commits got messy, I'm working from plane wifi. |
Deploy preview for website ready! ✅ Preview Built with commit f41c035. |
Thank you for pushing this across the finish line Randy! |
This PR contains the following updates: | Package | Update | Change | |---|---|---| | [poetry](https://python-poetry.org/) ([source](https://github.com/python-poetry/poetry), [changelog](https://python-poetry.org/history/)) | minor | `1.4.2` -> `1.5.0` | --- ### Release Notes <details> <summary>python-poetry/poetry</summary> ### [`v1.5.0`](https://github.com/python-poetry/poetry/blob/HEAD/CHANGELOG.md#​150---2023-05-19) [Compare Source](python-poetry/poetry@1.4.2...1.5.0) ##### Added - **Introduce the new source priorities `explicit` and `supplemental`** ([#​7658](python-poetry/poetry#7658), [#​6879](python-poetry/poetry#6879)). - **Introduce the option to configure the priority of the implicit PyPI source** ([#​7801](python-poetry/poetry#7801)). - Add handling for corrupt cache files ([#​7453](python-poetry/poetry#7453)). - Improve caching of URL and git dependencies ([#​7693](python-poetry/poetry#7693), [#​7473](python-poetry/poetry#7473)). - Add option to skip installing directory dependencies ([#​6845](python-poetry/poetry#6845), [#​7923](python-poetry/poetry#7923)). - Add `--executable` option to `poetry env info` ([#​7547](python-poetry/poetry#7547)). - Add `--top-level` option to `poetry show` ([#​7415](python-poetry/poetry#7415)). - Add `--lock` option to `poetry remove` ([#​7917](python-poetry/poetry#7917)). - Add experimental `POETRY_REQUESTS_TIMEOUT` option ([#​7081](python-poetry/poetry#7081)). - Improve performance of wheel inspection by avoiding unnecessary file copy operations ([#​7916](python-poetry/poetry#7916)). ##### Changed - **Remove the old deprecated installer and the corresponding setting `experimental.new-installer`** ([#​7356](python-poetry/poetry#7356)). - **Introduce `priority` key for sources and deprecate flags `default` and `secondary`** ([#​7658](python-poetry/poetry#7658)). - Deprecate `poetry run <entry point>` if the entry point was not previously installed via `poetry install` ([#​7606](python-poetry/poetry#7606)). - Only write the lock file if the installation succeeds ([#​7498](python-poetry/poetry#7498)). - Do not write the unused package category into the lock file ([#​7637](python-poetry/poetry#7637)). ##### Fixed - Fix an issue where Poetry's internal pyproject.toml continually grows larger with empty lines ([#​7705](python-poetry/poetry#7705)). - Fix an issue where Poetry crashes due to corrupt cache files ([#​7453](python-poetry/poetry#7453)). - Fix an issue where the `Retry-After` in HTTP responses was not respected and retries were handled inconsistently ([#​7072](python-poetry/poetry#7072)). - Fix an issue where Poetry silently ignored invalid groups ([#​7529](python-poetry/poetry#7529)). - Fix an issue where Poetry does not find a compatible Python version if not given explicitly ([#​7771](python-poetry/poetry#7771)). - Fix an issue where the `direct_url.json` of an editable install from a git dependency was invalid ([#​7473](python-poetry/poetry#7473)). - Fix an issue where error messages from build backends were not decoded correctly ([#​7781](python-poetry/poetry#7781)). - Fix an infinite loop when adding certain dependencies ([#​7405](python-poetry/poetry#7405)). - Fix an issue where pre-commit hooks skip pyproject.toml files in subdirectories ([#​7239](python-poetry/poetry#7239)). - Fix an issue where pre-commit hooks do not use the expected Python version ([#​6989](python-poetry/poetry#6989)). - Fix an issue where an unclear error message is printed if the project name is the same as one of its dependencies ([#​7757](python-poetry/poetry#7757)). - Fix an issue where `poetry install` returns a zero exit status even though the build script failed ([#​7812](python-poetry/poetry#7812)). - Fix an issue where an existing `.venv` was not used if `in-project` was not set ([#​7792](python-poetry/poetry#7792)). - Fix an issue where multiple extras passed to `poetry add` were not parsed correctly ([#​7836](python-poetry/poetry#7836)). - Fix an issue where `poetry shell` did not send a newline to `fish` ([#​7884](python-poetry/poetry#7884)). - Fix an issue where `poetry update --lock` printed operations that were not executed ([#​7915](python-poetry/poetry#7915)). - Fix an issue where `poetry add --lock` did perform a full update of all dependencies ([#​7920](python-poetry/poetry#7920)). - Fix an issue where `poetry shell` did not work with `nushell` ([#​7919](python-poetry/poetry#7919)). - Fix an issue where subprocess calls failed on Python 3.7 ([#​7932](python-poetry/poetry#7932)). - Fix an issue where keyring was called even though the password was stored in an environment variable ([#​7928](python-poetry/poetry#7928)). ##### Docs - Add information about what to use instead of `--dev` ([#​7647](python-poetry/poetry#7647)). - Promote semantic versioning less aggressively ([#​7517](python-poetry/poetry#7517)). - Explain Poetry's own versioning scheme in the FAQ ([#​7517](python-poetry/poetry#7517)). - Update documentation for configuration with environment variables ([#​6711](python-poetry/poetry#6711)). - Add details how to disable the virtualenv prompt ([#​7874](python-poetry/poetry#7874)). - Improve documentation on whether to commit `poetry.lock` ([#​7506](python-poetry/poetry#7506)). - Improve documentation of `virtualenv.create` ([#​7608](python-poetry/poetry#7608)). ##### poetry-core ([`1.6.0`](https://github.com/python-poetry/poetry-core/releases/tag/1.6.0)) - Improve error message for invalid markers ([#​569](python-poetry/poetry-core#569)). - Increase robustness when deleting temporary directories on Windows ([#​460](python-poetry/poetry-core#460)). - Replace `tomlkit` with `tomli`, which changes the interface of some *internal* classes ([#​483](python-poetry/poetry-core#483)). - Deprecate `Package.category` ([#​561](python-poetry/poetry-core#561)). - Fix a performance regression in marker handling ([#​568](python-poetry/poetry-core#568)). - Fix an issue where wildcard version constraints were not handled correctly ([#​402](python-poetry/poetry-core#402)). - Fix an issue where `poetry build` created duplicate Python classifiers if they were specified manually ([#​578](python-poetry/poetry-core#578)). - Fix an issue where local versions where not handled correctly ([#​579](python-poetry/poetry-core#579)). </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS44Mi4wIiwidXBkYXRlZEluVmVyIjoiMzUuODIuMCIsInRhcmdldEJyYW5jaCI6Im1hc3RlciJ9--> Reviewed-on: https://git.walbeck.it/walbeck-it/docker-python-poetry/pulls/717 Co-authored-by: renovate-bot <bot@walbeck.it> Co-committed-by: renovate-bot <bot@walbeck.it>
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This serves the same purpose as
--only-root
but for path deps (which just like the "root" will bust caches if any source file changes).If/once python-poetry/poetry-core#520 gets merged this will allow writing a Dockerfile along the lines of:
I chose
--no-path
because of it's parallels to--no-root
, but I'm open to other options.Closes #6845
Closes #6850
Closes #3671