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

Support multiple poetry versions #1621

Closed

Conversation

tomzx
Copy link
Contributor

@tomzx tomzx commented Jan 10, 2020

Install poetry 0.12.17 for pre-1.0.0 lock files.
In the case that a poetry.lock file is discovered with a
metadata.hashes entry, we should use poetry 0.12.17 to update
the lockfile in order to preserve version compatibility. Otherwise
assume that the user wants to use the latest poetry version (1.0.0
at this time).

This attempts to tackle #1571. There are a few issues with this PR, but this is to get the discussion started. I'd like to add tests to confirm it works as expected, however I'm not a ruby/rspec expert so I'd need some help. I think that most of the tests in python/spec/dependabot/python/update_checker/poetry_version_resolver_spec.rb could be executed both under 0.12.7 and 1.0.0, which would require creating a separate set of fixtures for each version.

One thing I'd like to discuss is where the downgrading logic should go. In my tests, it seems it will do the downgrade when it checks the first package. I think however that it would make sense to execute the downgrade before that, however I'm unsure where that place is. I put the code there simply because it was the path where the crash was occurring and all the information necessary was there. There's also a bit of code about installing a version of python that may not have been installed yet and installing the requirements (poetry amongst them), so I thought it was not too bad.

In the case that a poetry.lock file is discovered with a
metadata.hashes entry, we should use poetry 0.12.17 to update
the lockfile in order to preserve version compatibility. Otherwise
assume that the user wants to use the latest poetry version (1.0.0
at this time).
@tomzx tomzx force-pushed the features/multiple-poetry-versions branch from d572152 to ffbd4ee Compare January 10, 2020 03:08
@amureki
Copy link
Contributor

amureki commented Jan 31, 2020

Hey @tomzx !

Thanks for starting a work here. ✨ ✨ ✨
I switched from pipenv to Poetry (1.0+) and also struggling with dependabot now.

I noticed a comment from @sobolevn here in #1556:

In case making a support for several versions is a hard thing, then I suggest to drop poetry@0.x support in favour of poetry@1.x support. Because update process is easy for developers.

And I am also wondering if support for pre-release Poetry versions should be kept.
This adds complexity of maintaining for a tiny value. Let me explain:
I do believe that Poetry was discovered and being used by developers who are actively maintaining their projects and got tired of pipenv/other tools. This group of people most likely switched to a stable 1.+ version already.
But even if they are not, instead of keeping compatibility, we should kindly suggest them to iterate and update versions (since those are users who are using dependabot - tool to keep their stacks up-to-date, why packaging tooling should be an exception there?)

I am happy to put some of my time in this and try to help here, though I am not yet familiar with Ruby stack. Maybe also with the help and opinion of @feelepxyz we can get this going!

@sobolevn
Copy link
Contributor

I sort of agree. There's no point in supporting poetry<1.0 since for devs transition is just automatic. poetry itself converts old files to new ones.

@tomzx
Copy link
Contributor Author

tomzx commented Jan 31, 2020

Given that we might be upgrading someone's lockfile from a pre-1.0.0 format to a 1.0.0+ format, should there be some sort of warning as part of the PR to let them know or is it their responsibility to review the changes and notice that themselves?

I agree that supporting pre-1.0.0 and 1.x versions is probably not very useful for people using dependabot. I would thus suggest closing this and merging any of the automated PRs that bump to 1.0.x (1.0.3 was released today), which will take care of adding support for any 1.x versions, as well as upgrading pre-1.0.0 lock files.

@sobolevn
Copy link
Contributor

sobolevn commented Feb 1, 2020

Yes, it makes sense.

@jtbeach
Copy link
Contributor

jtbeach commented Feb 20, 2020

Is there anything I can do to help move this forward?

@sobolevn
Copy link
Contributor

@greysteil I would love to help with this one (but I don't know Ruby)! dependabot is not working for me for more than a month now.

@greysteil
Copy link
Contributor

I'm afraid I don't work on Dependabot anymore (I'm focussed on other security products at GitHub), but @feelepxyz and the team should be able to help!

@feelepxyz feelepxyz changed the base branch from master to main June 24, 2020 11:26
@feelepxyz
Copy link
Contributor

Closing this out as we've been running poetry v1 for a good while now.

@feelepxyz feelepxyz closed this Dec 18, 2020
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.

None yet

6 participants