Skip to content

Comments

Return a clear error when HTTP calls to legacy repo fail, instead of PackageNotFound#1072

Closed
MarcDufresne wants to merge 4 commits intopython-poetry:developfrom
MarcDufresne:develop
Closed

Return a clear error when HTTP calls to legacy repo fail, instead of PackageNotFound#1072
MarcDufresne wants to merge 4 commits intopython-poetry:developfrom
MarcDufresne:develop

Conversation

@MarcDufresne
Copy link
Contributor

@MarcDufresne MarcDufresne commented May 2, 2019

This PR aims to make Poetry a bit more clear about issues when communicating with a legacy repo.

For example, it happened a few times that we would get a PackageNotFound error when trying to install a package from a private repo, when in fact the actual issue was bad credentials. The simple fix is simply to return the HTTP error message instead of a generic PackageNotFound.

With this change you will now see:

[RepositoryError]                                                              
401 Client Error: Unauthorized for url: https://my-repo.com/repo/my-package/

Instead of:

[PackageNotFound]        
Package [my-package] not found.

sdispater
sdispater requested changes May 28, 2019
View reviewed changes
Copy link
Member

@sdispater sdispater left a comment

Choose a reason for hiding this comment

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

Can you add a test covering this?

@MarcDufresne
Copy link
Contributor Author

MarcDufresne commented May 29, 2019

Done! I was hesitant to add tests for _get and _download since they were not already tested and didn't want to take design decisions regarding testing.
However since you asked for them, I took the liberty of adding requests-mock and test both happy and sad path for both methods, let me know what you think.

@brycedrennan brycedrennan added the kind/feature Feature requests/implementations label Aug 17, 2019
@stale
Copy link

stale bot commented Nov 13, 2019

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 13, 2019
@stale
Copy link

stale bot commented Nov 20, 2019

Closing this pull request automatically because it has not had any activity since it has been marked as stale. If you think it is still relevant and should be addressed, feel free to open a new one.

@stale stale bot closed this Nov 20, 2019
@dan-hipschman
Copy link

dan-hipschman commented May 3, 2020

Any objection to reopening and merging this? I was about to make a PR for the same improvement, but this one seems perfectly good. Thanks!

@finswimmer finswimmer reopened this May 4, 2020
@dan-hipschman dan-hipschman mentioned this pull request May 24, 2020
Merged
2 tasks
@finswimmer
Copy link
Member

finswimmer commented Aug 15, 2020

Closing this in favor of #2459

@finswimmer finswimmer closed this Aug 15, 2020
@github-actions
Copy link

github-actions bot commented Mar 1, 2024

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 Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@sdispater sdispater sdispater requested changes

Assignees

No one assigned

Labels

kind/feature Feature requests/implementations

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@MarcDufresne @dan-hipschman @finswimmer @sdispater @brycedrennan