-
Notifications
You must be signed in to change notification settings - Fork 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
Add name
key to sources
in Pipfile
s
#7744
Conversation
Likely there will be hash mismatches in the fixture lockfiles, so they'll need regenerating... if so, probably best to only do what's necessary to get past CI... as no point in regenerating in this PR using an older version of |
9b638cf
to
62f4d33
Compare
883d742
to
d97b36b
Compare
@@ -1,12 +1,13 @@ | |||
{ | |||
"_meta": { | |||
"hash": { | |||
"sha256": "c402ea48092e9d467af51a483bb8dd8ad0620e11c94f009dcd433f97a99d45db" | |||
"sha256": "e76ae491d793d659f05c7f7eab261fd6167dc062efcba08f17be68e73eb87665" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was the only lockfile that the tests required to have an updated hash.
I strongly suspect the hashes are outdated for the other fixture lockfiles, but again, we can regenerate them all after updating to latest pipenv
once we drop Python 3.7.
d97b36b
to
26d416e
Compare
ae4e7bd
to
dd9672c
Compare
name
key to Pipfile
's sources
name
key to sources
in Pipfile
s
7b0c471
to
781896c
Compare
Newer versions of `pipenv` require any specified `sources` to be explicitly `name`'d. More context here: pypa/pipenv#5370 (comment) This has been true since at least Sept 2022. Our version of `pipenv` is from April of 2022, so it doesn't complain. But it will as soon as we upgrade. Also, for sources that dynamically injects into the `Pipfile`, they need a name. These sources are stripped from the final `Pipfile` / `Pipfile.lock` during the `FileUpdater#post_process_lockfile` method, so all we need is a placeholder `name` to placate `pipenv`. Long term, we may want to add custom error handling to flag this missing key as a `Dependabot::DependencyFileNotResolvable` error. But I decided that was out of scope for now as this PR does not generate the error... that will not happen until we upgrade to newer `pipenv`. And even at that point, our first priority will be upgrading, and then from there handling any new errors that start popping up. And even then, most of the active users of `pipenv` are unlikely to see this error because they're likely running a newer version of `pipenv`.
781896c
to
16be5d4
Compare
We got a report via a support ticket that opened a PR that included Unfortunately, I can't repro and the affected user's repo is a private company so I can't access their repo and they are migrating to a different python package manager so decided it wasn't worthwhile for them to try to create a reproducible example. The one observation they made:
If anyone else encounters this, please feel free to file a new issue, preferably here in |
We are having the same issue but unfortunately also in a private repository 😞 It looks like older versions of
We also have private libraries, so not sure I'll be able to create a reproducible example. |
@juanitosvq Can you at least create a new issue describing what you're seeing? The But this isn't a conversation we should be having on an already merged PR... 😁 |
Newer versions of `pipenv` require any specified `sources` to be explicitly `name`'d. More context here: pypa/pipenv#5370 (comment) This has been true since at least Sept 2022. Our version of `pipenv` is from April of 2022, so it doesn't complain. But it will as soon as we upgrade. Also, for sources that dynamically injects into the `Pipfile`, they need a name. These sources are stripped from the final `Pipfile` / `Pipfile.lock` during the `FileUpdater#post_process_lockfile` method, so all we need is a placeholder `name` to placate `pipenv`. Long term, we may want to add custom error handling to flag this missing key as a `Dependabot::DependencyFileNotResolvable` error. But I decided that was out of scope for now as this PR does not generate the error... that will not happen until we upgrade to newer `pipenv`. And even at that point, our first priority will be upgrading, and then from there handling any new errors that start popping up. And even then, most of the active users of `pipenv` are unlikely to see this error because they're likely running a newer version of `pipenv`.
Newer versions of
pipenv
require any specifiedsources
to beexplicitly
name
'd.More context here: pypa/pipenv#5370 (comment)
This has been true since at least Sept 2022. Our version of
pipenv
isfrom April of 2022, so it doesn't complain. But it will as soon as we upgrade.
Also, for sources that dynamically injects into the
Pipfile
, they need a name. These sources are stripped from the finalPipfile
/Pipfile.lock
during theFileUpdater#post_process_lockfile
method, so all we need is a placeholder
name
to placatepipenv
.Long term, we may want to add custom error handling to flag this
missing key as a
Dependabot::DependencyFileNotResolvable
error.But I decided that was out of scope for now as this PR does not generate
the error... that will not happen until we upgrade to newer
pipenv
.And even at that point, our first priority will be upgrading, and then
from there handling any new errors that start popping up.
And even then, most of the active users of
pipenv
are unlikely to see thiserror because they're likely running a newer version of
pipenv
.