-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Support multiple poetry versions #1621
Conversation
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).
d572152
to
ffbd4ee
Compare
Hey @tomzx ! Thanks for starting a work here. ✨ ✨ ✨ I noticed a comment from @sobolevn here in #1556:
And I am also wondering if support for pre-release Poetry versions should be kept. 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! |
I sort of agree. There's no point in supporting |
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. |
Yes, it makes sense. |
Is there anything I can do to help move this forward? |
@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. |
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! |
Closing this out as we've been running poetry v1 for a good while now. |
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.