poetry-core
poetry-core copied to clipboard
Path based dev-dependencies break 'install --no-dev' when the dir does not exist
Resolves: python-poetry/poetry#668
- [ ] Added tests for changed code.
- [ ] Updated documentation for changed code.
Hello @sdispater , please take a look and let me know if it makes sense, if so I will provide tests and update docs. Currently marking PR as WIP. 🆙
Hey, please, any feedback? 🙏
Hi @sdispater , is there any chance to get a feedback? Thanks.
Hi @stephsamson , is there any chance to get a feedback from any of the maintainers? Cheers!
Hey, is it possible to get a feedback?
This would be great to merge. Installing all deps takes between 5 and 10 minutes because changes to the source code causes cache busting for docker containers. This leads to every commit consuming N x 10 minutes longer of build time, where N=3 in our small case.
Hi @sdispater , please can you comment on this? 🙏
It would be great to have at least some feedback.
Please @sdispater @abn @dimbleby @radoering @finswimmer @branchvincent, show some love to this good man and review his PR ❤️ This bug is currently a show stopper for using poetry when building docker images. Path dependencies should be installed differently from regular dependencies in Dockerfiles, and sorting path dependencies under dev dependencies is the easiest way to differentiate.
Sorry for the late feedback. I took a quick look and I have some concerns:
- Using
sys.args
at this place doesn't seem right. - There are not only two dependency groups anymore.
--no-dev
will be deprecated in 1.2. - The warning instead of an error is only fine if a lock file exists. If there is no lock file, dev dependencies are required for implicit locking before install.
Unfortunately, some logic to determine if a warning is sufficient seems like a brittle fix that will easily break to me.
Nevertheless, the __init__
of DirectoryDependency
(and FileDependency
) might be too early for checking if the path exists. (URLDependency
and VCSDependency
do not check if the package is available in __init__
). Maybe, there should just be some lazy evaluation: check for existence only if full_path
is used for the first time?
Any progress here?
Since nothing has happened here for quite some time to address the concerns (which might be due to the late feedback), I'm closing this PR in favor of #520, which takes a more general approach. Nevertheless, thanks for your contribution.