Skip to content
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 support for poetry v2 #27

Merged
merged 3 commits into from
Jan 10, 2025
Merged

Conversation

DBS-ST-VIT
Copy link
Contributor

Hey @ajkerrigan,
this maybe solves #26.

Actually, we implemented it with legacy support for poetry 1.x.x, but thats something we can also remove. We thought, that it is may be more user friendly with some time with legacy support. Maybe we should also add a deprecation notice for usage with poetry 1.x.x?

Further, there was one deprecation (see here) which has been fixed by actually adapting the current code from poetry. For us, this doesn't look like the best solution, but poetry-plugin-export is expecting a BaseMarker object, which is the reason why we need to work with the marker here and cannot use e.g. the .python_constraint property instead.

Copy link
Member

@ajkerrigan ajkerrigan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm generally good with this, the v2 changes don't look bad at all. Your handling of the python_marker change looks good 👍 .

A couple other comments/suggestions inline related to the upstream rename of PyProjectException --> PyProjectError and how to gracefully handle that.

src/poetry_plugin_freeze/app.py Outdated Show resolved Hide resolved
src/poetry_plugin_freeze/app.py Outdated Show resolved Hide resolved
@DBS-ST-VIT
Copy link
Contributor Author

Hey @ajkerrigan,
many thanks for your input! I updated the branch in our forked repo according your suggestions and also cleaned a little bit up.

May you can have a look on it?

@WhoAmI0501
Copy link

Hi @ajkerrigan ,
i am also waiting for this PR for upgrading to poetry v2. This is the last poetry plugin in my collection that is lacking v2 support. Can you merge this PR?

@RudolfVonKrugstein
Copy link

Oh, this would be cool! I want to upgrade to poetry V2, but need this plugin.

Also handle the upstream PyProjectException-->PyProjectError rename.
Copy link
Member

@ajkerrigan ajkerrigan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with these changes, pushed one minor tweak to make sure we're covering the PyProjectException/PyProjectError/RuntimeError cases. Thanks for working on this one @DBS-ST-VIT 👍

@ajkerrigan ajkerrigan merged commit 60bb6be into cloud-custodian:main Jan 10, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants