search

thoth-station / micropipenv

A lightweight wrapper for pip to support requirements.txt, Pipenv and Poetry lock files or converting them to pip-tools compatible output. Designed for containerized Python applications but not limited to them.
https://pypi.org/project/micropipenv/
GNU Lesser General Public License v3.0
234 stars 25 forks source link

Groups support in Poetry lock files #185

Closed fridex closed 2 years ago

fridex commented 3 years ago

Is your feature request related to a problem? Please describe.

Poetry introduced dependency groups feature that will need few adjustments in the source code to work properly:

https://python-poetry.org/blog/announcing-poetry-1.2.0a2/#dependency-groups

See also A note about the dev-dependencies section part for dev-dependencies handling. It might be also needed to rethink how we integrate with dependency groups when mapping internally to Pipfile.lock (we will probably not support them, but dev group can be mapped to dev-dependencies implicitly).

goern commented 3 years ago

/triage accepted /priority important-soon /kind feature

@fridex as always: feel free to reprioritize!

sesheta commented 3 years ago

Rotten issues close after 30d of inactivity. Reopen the issue with /reopen. Mark the issue as fresh with /remove-lifecycle rotten.

/close

sesheta commented 3 years ago

@sesheta: Closing this issue.

In response to [this](https://github.com/thoth-station/micropipenv/issues/185#issuecomment-917588319): >Rotten issues close after 30d of inactivity. >Reopen the issue with `/reopen`. >Mark the issue as fresh with `/remove-lifecycle rotten`. > >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
fridex commented 3 years ago

/reopen /remove-lifecycle rotten

sesheta commented 3 years ago

@fridex: Reopened this issue.

In response to [this](https://github.com/thoth-station/micropipenv/issues/185#issuecomment-917891177): >/reopen >/remove-lifecycle rotten Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
sesheta commented 2 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

/lifecycle stale

fridex commented 2 years ago

/remove-lifecycle stale

goern commented 2 years ago

@Gregory-Pereira @fridex what is missing to get this merged? its lingering around for some time now...

frenzymadness commented 2 years ago

Groups' support in poetry is not yet officially released. It will be in 1.2 but I'm not sure about their schedule. The last release is 1.2.0a2 from August 2021.

sesheta commented 2 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

/lifecycle stale

sesheta commented 2 years ago

Stale issues rot after 30d of inactivity. Mark the issue as fresh with /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

/lifecycle rotten

frenzymadness commented 2 years ago

Poetry 1.2.0 beta 2 has been released 16 days ago. It might be a good time to take a look at this.

sesheta commented 2 years ago

Rotten issues close after 30d of inactivity. Reopen the issue with /reopen. Mark the issue as fresh with /remove-lifecycle rotten.

/close

sesheta commented 2 years ago

@sesheta: Closing this issue.

In response to [this](https://github.com/thoth-station/micropipenv/issues/185#issuecomment-1193096628): >Rotten issues close after 30d of inactivity. >Reopen the issue with `/reopen`. >Mark the issue as fresh with `/remove-lifecycle rotten`. > >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
frenzymadness commented 2 years ago

/reopen

sesheta commented 2 years ago

@frenzymadness: Reopened this issue.

In response to [this](https://github.com/thoth-station/micropipenv/issues/185#issuecomment-1193275760): >/reopen Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
frenzymadness commented 2 years ago

/remove-lifecycle rotten

frenzymadness commented 2 years ago

Poetry 1.2.1 has been released 4 days ago. I have to take a look at this.

shifqu commented 2 years ago

Hello

I think I am experiencing a bug due to the addition of poetry's group dependencies.

Poetry.lock hash 'OBFUSCATED' does not correspond to hash computed based on pyproject.toml 'OBFUSCATED', aborting deployment

Note: I replaced the hashes, not that it's a security issue, but I replaced them anyway :)

We are using poetry 1.2.2 and python 3.8.

If you need more information, feel free to request it here. If you think this is a separate issue, I'd be happy to open a new ticket for it.

Thanks in advance.

frenzymadness commented 2 years ago

Thanks for your interest @shifqu . Could you please provide your poetry.lock and pyproject.toml files? Feel free to create some artificial ones just to reproduce the issue.

shifqu commented 2 years ago

Hello, I am having trouble reproducing this with an artificial poetry.lock/pyproject.toml as it normally is a step in our deployment to openshift, which uses source2image, which in turn uses micropipenv.

I cannot send the real poetry.lock since it includes urls to internal locations. But I figure this might be the issue. The source of our packages is not pypi, but an internal mirror. Could this have to do with the incompatible hash generation?

shifqu commented 2 years ago

So, after checking our toml file for differences, and reading a bit of code, I was able to come up with a mvp to reproduce the issue.

Poetry 1.2 no longer uses tool.poetry.dev-dependencies, but rather tool.poetry.group.dev.dependencies. So baiscally to reproduce:

verify("poetry")

This will result in an error.
```bash
Poetry.lock hash 'dfb302d9454a0be70f1fedd6989ced7714cd7a5e6649cfb64e745bed1db87c06' does not correspond to hash computed based on pyproject.toml '9ea646a9539b841f3a1fa36deb7ccbf9f9067ecee05345e5421ea1731bea6fe6', aborting deployment

Then I figured I'd try the same, but this time with old-style pyproject.toml (so tool.poetry.dev-dependencies). Manually change tool.poetry.group.dev.dependencies to tool.poetry.dev-dependencies in your pyproject.toml and then run poetry lock to update your lock file accordingly. If we run main.py again, no more error is raised.

So basically, the error originates from _compute_poetry_hash Unfortunately, extending relevant_keys is not possible, since we need multiple keys. Something like checking if tool.poetry.group.dev.dependencies exists and then fetching them separately would resolve this.

Note: I added the example to reproduce as a zip-file to this comment. mpe_mvp.zip

frenzymadness commented 2 years ago

Thanks a lot for the investigation and all the information. I'll take a look at this ASAP.

frenzymadness commented 2 years ago

@shifqu thanks again, you basically did all the work here. I looked into it and found the implementation of the hash verification in poetry which helped me to understand how it should work here because our code is basically a copy of the original one from poetry. The change is proposed in the linked PR with a description in the comment. Could you please take a look at it and possibly test it? I've also added a test for the new pyproject.toml and poetry.lock produced by the latest version of poetry.

Do you know about any other use case of poetry groups we don't have covered in micropipenv? I'm not an active poetry user so any advice about what to test and what to prepare micropipenv for is very appreciated.

shifqu commented 2 years ago

@frenzymadness thanks! I made a short review on your PR, but am closing one comment, as re-reading your last comment made me realize that just getting the groups key is sufficient.

I will try to find some time to test the PR.

So far, I have only come across groups when using dependencies, so AFAIK we got the groups covered with your PR.

shifqu commented 2 years ago

I just tested the PR from your personal repository and can confirm that my previous test now succeeds 👌

I then ran the command poetry add --group extra isort, which created the group.extra.dependencies section in my pyproject.toml. I then ran the verify function again, which did not throw an error, so the arbitrary groups are all handled by your current PR! 🙌

The pyproject dependency part looks like this:

[tool.poetry.dependencies]
python = "^3.10"
micropipenv = {path = "path/to/your/public/micropipenv-poetry_group"}

[tool.poetry.group.dev.dependencies]
black = "^22.10.0"

[tool.poetry.group.extra.dependencies]
isort = "^5.10.1"