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

Implement PIP_TRUSTED_HOSTS logic from importing requirements to specified --index URL #5572

Closed
kalebmckale opened this issue Jan 21, 2023 · 6 comments
Labels
Contributor Candidate The issue has been identified/triaged and contributions are welcomed/encouraged. Type: Enhancement 💡 This is a feature or enhancement request. Type: Possible Bug This issue describes a possible bug in pipenv.

Comments

@kalebmckale
Copy link
Contributor

kalebmckale commented Jan 21, 2023

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

The larger problem I'd ideally like to solve is to have newly created Pipfiles contain the correct source information and not be required to first fail, to have to edit the Pipfile, and then to attempt again.

Specifically, when adding an index via command line, if the index URL begins with https, verify_ssl is automatically set to true. However, if using private indices, this may be neither necessary nor preferred.

Describe the solution you'd like

The logic currently used when parsing requirements files in core.py that checks environment variable PIP_TRUSTED_HOSTS when determining the value for verify_ssl could also be implemented with index URLs specified at the command line.

Describe alternatives you've considered

Alternatively, one could pre-generate a blank Pipfile template with the correct sources and not use pipenv to create Pipfile. This would not be preferred.

Additional context

I'm happy to make the simple changes and create a pull request (see kalebmckale/pipenv@7c5dcde). I've already successfully tested this locally on my system, but I understand it needs to run though the full gamut of possible platforms, etc.

kalebmckale added a commit to kalebmckale/pipenv that referenced this issue Jan 21, 2023
Duplicated logic around line 204 in core.py to allow users to specify
index via --index command-line option and validate against
PIP_TRUSTED_HOSTS when determining verify_ssl value.
@matteius matteius added Type: Enhancement 💡 This is a feature or enhancement request. Type: Possible Bug This issue describes a possible bug in pipenv. Contributor Candidate The issue has been identified/triaged and contributions are welcomed/encouraged. labels Jan 26, 2023
kalebmckale added a commit to kalebmckale/pipenv that referenced this issue Feb 18, 2023
Duplicated logic around line 204 in core.py to allow users to specify
index via --index command-line option and validate against
PIP_TRUSTED_HOSTS when determining verify_ssl value.
@matteius
Copy link
Member

@kalebmckale Would you still be interested in taking on any fixes related to this?

@kalebmckale
Copy link
Contributor Author

kalebmckale commented Feb 18, 2023

@matteius I would love to be able to confidently create pull requests, but unfortunately I don't have a good development environment at home. Right now, I really just do programming on my iPad, and iSH isn't quite there with its functionality although it's pretty nice to have. I think the latest issue I ran into was the inability to use pyenv to install other versions of Python because iSH has yet to figure out (or perhaps just hasn't released a fix to) shell redirection, i.e. <(...). tl;dr: I'm happy to do the coding, but I cannot really test it at home.

P.S. I just rebased the changes I made for this branch on the new main. So, theoretically, it's ready to be tested to go.

@matteius
Copy link
Member

@kalebmckale opening a PR that is editable by the maintainers would be a good way to capture your ideas and possibly allow us to push them across the line. There is so much going on for me that pipenv is a slice of it, and while I'd love to get the issue reports under 100 in the next year, it will require as much help as possible.

kalebmckale added a commit to kalebmckale/pipenv that referenced this issue Feb 18, 2023
Duplicated logic from `import_requirements()` to `do_install()` in
`core.py` to allow users to specify index via `--index` command-line
option and validate against `PIP_TRUSTED_HOSTS` when determining
`verify_ssl` value.
@kalebmckale
Copy link
Contributor Author

@matteius Okay, I think I did it correctly. It's technically my first PR on GitHub. I wasn't sure what extension to use for the news item but decided to go with feature.

kalebmckale added a commit to kalebmckale/pipenv that referenced this issue Mar 10, 2023
Duplicated logic from `import_requirements()` to `do_install()` in
`core.py` to allow users to specify index via `--index` command-line
option and validate against `PIP_TRUSTED_HOSTS` when determining
`verify_ssl` value.
kalebmckale added a commit to kalebmckale/pipenv that referenced this issue Mar 10, 2023
Duplicated logic from `import_requirements()` to `do_install()` in
`core.py` to allow users to specify index via `--index` command-line
option and validate against `PIP_TRUSTED_HOSTS` when determining
`verify_ssl` value.
kalebmckale added a commit to kalebmckale/pipenv that referenced this issue Mar 10, 2023
Duplicated logic from `import_requirements()` in requirements.py
to `do_install()` in `install.py` to allow users to specify index
via `--index` command-line option and validate against
`PIP_TRUSTED_HOSTS` when determining `verify_ssl` value.
kalebmckale added a commit to kalebmckale/pipenv that referenced this issue Mar 19, 2023
Duplicated logic from `import_requirements()` in requirements.py
to `do_install()` in `install.py` to allow users to specify index
via `--index` command-line option and validate against
`PIP_TRUSTED_HOSTS` when determining `verify_ssl` value.

Update news file to be more descriptive.
oz123 pushed a commit that referenced this issue Mar 20, 2023
Duplicated logic from `import_requirements()` in requirements.py
to `do_install()` in `install.py` to allow users to specify index
via `--index` command-line option and validate against
`PIP_TRUSTED_HOSTS` when determining `verify_ssl` value.

Update news file to be more descriptive.
@kalebmckale
Copy link
Contributor Author

@matteius @oz123 This can be closed / resolved?

@kalebmckale
Copy link
Contributor Author

@matteius @oz123 I figured out how to close the issue. So embarrassing! 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Contributor Candidate The issue has been identified/triaged and contributions are welcomed/encouraged. Type: Enhancement 💡 This is a feature or enhancement request. Type: Possible Bug This issue describes a possible bug in pipenv.
Projects
None yet
Development

No branches or pull requests

2 participants