-
-
Notifications
You must be signed in to change notification settings - Fork 646
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 Poetry macros to generate Python requirements and constraints #10655
Comments
Part of #10655. This macro will read a `Pipfile.lock` and convert everything into a `python_requirement_library`. It won't actually treat this as a true lockfile, i.e. it won't feed the results into the option `--python-setup-requirements-constraints`. That will need to come in a followup. Rather, this acts like the macro `python_requirements()`, just with a Pipfile.lock.
@jsirois : This might relate to some of what you're looking at: would be good to sync with Eric on it when you get back. |
This is a great feature to have, but not going to make it into 2.0. While this workaround isn't ergonomic, Poetry and Pipenv users can in the meantime use the We can pick this back up for 2.1 or 2.2, as our focus for the first 2.x releases is futher improving Python, before we venture out to new languages. |
Another useful capability if integrating with Poetry and Pipenv is to allow for propagating the constraint ranges to the generated setup.py files when packaging a project. So, rather than setting the |
Hi @stuhood (long time no see!), my company is evaluating pants as a build tool for some of our python-based ML repositories, which currently use Poetry for package/dependency management. Is there any way this feature will make it into any of the upcoming Pants 2.X releases? And - especially if that answer is no - would having a few devs like us collaborate on the feature help improve its chances of getting released? Thanks in advance, and let us know how we can help. |
Hey @lucinvitae! Good to hear from you!
It is not currently prioritized: we updated on the roadmap recently over here: https://groups.google.com/g/pants-devel/c/F8Saug3BrFw/m/ZtWDP4YmDwAJ
We would love to collaborate with you on this one! As you've seen, Pants' existing support for lockfiles is relatively limited:
We have work scheduled to resolve the first two issues in the next month or two: #11165 ... but the last issue is what this ticket deals with. There are a few very different designs for this feature, and @Eric-Arellano has offered to help sketch those out in the next few days: if you have any thoughts on how you'd like the integration to work, they'd be very helpful in the meantime! |
Hey @lucinvitae, awesome to hear! As Stu said, we'd love the help and are eager to assist to pull this off. -- The first step is checking that we're on the same page for what Poetry support entails. I've updated this PR's title and description to focus on Poetry, rather than including Pipenv. Could you please check that that's what you're envisioning? Note that Pants will still use Pex to do the actual resolve, which uses pip under the hood. All this PR is doing is automating you as an admin having to run Likewise, this is a lossy conversion. A Poetry lockfile says specifically which file to download, e.g.:
Instead, we will only be able to capture this information:
For example, we would likely convert that into InstructionsWe will be creating a macro, very similar to the
I recommend modeling this as a single macro, something like Unlike those two macros, we will be creating two types of targets: a We should also create two Then, add tests similar to https://github.com/pantsbuild/pants/blob/master/src/python/pants/backend/python/macros/python_requirements_test.py. |
@lucinvitae FYI the prework has landed with #11724, so you should now be fully unblocked. I simplified the above instructions based on that landing :) |
Two thoughts:
|
Thanks @cognifloyd! #2 is particularly compelling, that sounds great and you can add the library to #1 looks interesting, although is likely out of scope. It's much trickier to teach Poetry about Pants imo than to teach Pants how to read Poetry files via poetry-core. |
@Eric-Arellano and @stuhood, thanks for the replies and detailed instructions. Replies/comments below:
e.g.
I'm not sure yet. The "monorepo" approach requires more investigation as to whether or not it suits our internal project's needs. We're just now adding our first source-level dependencies to the internal project. Whether we lose the "hash"-level of detail for the project may not be a huge concern, but we'll see. As for parsing:
It looks like poetry-core is vendoring its dependencies here, which appears to be a subject of some contention, as seen in python-poetry/poetry#2346. One vendored dependency includes tomlkit, which is used as a base library for parsing the
There's some discussion to bring |
Thanks for all that research @lucinvitae! I think it's a non-starter for us to depend on Poetry itself, and we do need to parse poetry.lock to get the constraints file. So, sounds like we need to do our own TOML reading one way or another. At that point, I think we can leave off poetry-core. The spec looks pretty simple to consume for both pyproject.toml and poetry.lock fwict, and we can of course always look to Poetry's code for how they do it. What do you think?
@jsirois mentioned it may be possible to preserve the hashes with Pip and Pex. I think it's out of scope for this ticket, but a possible followup.
I recommend calling it -- We've had a couple more users ask for this Poetry integration in the past two weeks, so it's becoming a higher priority for the project and I'd love to help land this. I'm wondering, how would it sound for you to focus on writing some generic code that reads poetry.lock and/or pyproject.toml, and then I can focus on wiring that up to Pants via the macro? Or perhaps if there are other ways I could help, like focusing on one of the two files while you look at the other? I'm happy to help however :) |
It works straight out of the gate in Pex, so if Pants doesn't get in the way and you're already generating a requirements.txt as part of this work, you could just add in hash checking from the get go as shown here: pex-tool/pex#1310 |
@Eric-Arellano to answer your question about consuming the lockfile: I think using custom logic instead of adding a dependency on poetry from pants sounds reasonable. And Regarding tackling the implementation:
My team is still evaluating a monorepo architecture and hasn't committed to using Pants, so I probably can't commit to delivering this feature by a certain date. If you're comfortable with potentially a long wait, I think your proposal that I write some generic lockfile code would work. Or if - due to increased priority from users requesting this feature - you want to self-assign this issue, please do! |
Great!
Okay, sounds good. I'll try to land this soon, hopefully before y'all make a decision so you can see how the integration would work out for you :)
Please feel free to let us know if there's anything we can help with or you have any questions! Folks are quite friendly on Slack and sometimes share their experience with monorepos, for example. https://www.pantsbuild.org/docs/getting-help#slack |
PR #12174 adds parsing for a Poetry-style pyproject.toml file...it's more or less complete; still welcoming input, though. Estimating ~1 day for code review before landing. |
Sounds great! Maybe also some time for documentation? |
Some orgs use Poetry for dependency management because it has an excellent resolver and lockfile support. While they can translate from Poetry into the pip/Pex world of
requirements.txt
andconstraints.txt
, this requires runningpoetry export
. Pants can automate some of that via macros, similar to thepython_requirements()
macro convertingrequirements.txt
intopython_requirement_library
targets.We want to distinguish between the high-level
pyproject.toml
, where the dependencies you want are declared, vs.poetry.lock
, where all transitive deps are used. The former should be used to generatepython_requirement_library
targets, which get used for things like dependency inference. The latter should be used for a constraints file.Strawman design:
Relates to #11710 for better Pipenv support.
The text was updated successfully, but these errors were encountered: