Skip to content

Comments

Prefer authenticator from repository when available#4488

Closed
kaos wants to merge 1 commit intopython-poetry:masterfrom
kaos:use_repository_authenticator_in_executor
Closed

Prefer authenticator from repository when available#4488
kaos wants to merge 1 commit intopython-poetry:masterfrom
kaos:use_repository_authenticator_in_executor

Conversation

@kaos
Copy link

@kaos kaos commented Sep 8, 2021

Signed-off-by: Andreas Stenius [email protected]

Resolves: #1012 #2593 #3110 #4016
Supersedes: #3349
Introduced: #2595

Feature toggle to side step this issue until resolved:

poetry config experimental.new-installer false
  • Added tests for changed code.

This will ensure that the configured certificates and auth are used when downloading files from a legacy repository.
There are quite a few issues relating to this being broken, so the list of resolves above may be incomplete.

return Page(response.url, response.content, response.headers)

def _download(self, url, dest): # type: (str, str) -> None
return download_file(url, dest, session=self.session)
Copy link
Author

Choose a reason for hiding this comment

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

Removed this, as it is an exact copy of the one from the base class, PypiRepository.

@kaos
Copy link
Author

kaos commented Sep 9, 2021

I can't fathom that of all the issues that has been reported with custom pypi registries and SSL issues, that no one picked up that the new installer can be turned off:
#2595 (comment)

From what I understand, this gives you the same installer that is used in 1.0.10, that several users reports works as it should. Even downgrading to as a work-around for the issues.

@kaos
Copy link
Author

kaos commented Sep 16, 2021

Ping. Any feedback on this?

@absassi
Copy link

absassi commented Oct 13, 2021

I can't fathom that of all the issues that has been reported with custom pypi registries and SSL issues, that no one picked up that the new installer can be turned off: #2595 (comment)

From what I understand, this gives you the same installer that is used in 1.0.10, that several users reports works as it should. Even downgrading to as a work-around for the issues.

@kaos, well, I'm one that recommended either downgrading or setting REQUESTS_CA_BUNDLE both here and in my workplace, because I had no idea that this was caused by the new installer. After all, the new installer comes enabled by default and the docs don't say anything about its bugs nor how to disable it. Thanks for the information and for this PR. I hope it gets reviewed and approved soon.

@kaos
Copy link
Author

kaos commented Oct 13, 2021

@absassi
Thank you. And apologies, re-reading that comment now I see it uses stronger than necessary language. My excuse is that I was rather frustrated at the time (bad excuse as any).

@neersighted neersighted reopened this Nov 10, 2021
@neersighted
Copy link
Member

Closed and re-opened to re-trigger checks being run.

@neersighted neersighted self-assigned this Nov 11, 2021
@neersighted
Copy link
Member

This should have been made obsolete by #5518 -- please test on master and comment/re-open if you think some of these changes are still valid.

@neersighted neersighted closed this Jun 4, 2022
@kaos kaos deleted the use_repository_authenticator_in_executor branch June 4, 2022 10:58
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow root CA bundle to be configured

4 participants