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

Make link logging more consistent #7390

Open
xavfernandez opened this issue Nov 22, 2019 · 3 comments · May be fixed by #11083
Open

Make link logging more consistent #7390

xavfernandez opened this issue Nov 22, 2019 · 3 comments · May be fixed by #11083
Labels
C: logging Information Logging state: awaiting PR Feature discussed, PR is needed type: refactor Refactoring code

Comments

@xavfernandez
Copy link
Member

What's the problem this feature will solve?

Links are sometimes logged via their __str__ method (cf

"HTTP error %s while getting %s", exc.response.status_code, link,
), sometimes with custom logic like in
if link.netloc == PyPI.file_storage_domain:
url = link.show_url
else:
url = link.url_without_fragment
redacted_url = redact_auth_from_url(url)

And on top of that we have a special case for PyPI downloads.

Like Chris pointed out in #7384 (comment) this special case (reintroduced via #7225) doesn't help with debugging.

Describe the solution you'd like

Always use the same logic (a Link property (or __str__) when logging a link, ideally with identical logic when dealing wih filepath or VCS urls.

This could involve always using link.show_url for info logs and provide the full (redacted) url in --verbose mode.

Alternative Solutions

Statu quo.

@triage-new-issues triage-new-issues bot added the S: needs triage Issues/PRs that need to be triaged label Nov 22, 2019
@xavfernandez xavfernandez added type: refactor Refactoring code C: logging Information Logging and removed S: needs triage Issues/PRs that need to be triaged labels Nov 22, 2019
@xavfernandez xavfernandez added the state: needs discussion This needs some more discussion label Nov 22, 2019
@pradyunsg
Copy link
Member

I'd say we can drop the discussion needed and relabel this as awaiting PR.

@xavfernandez xavfernandez added state: awaiting PR Feature discussed, PR is needed and removed state: needs discussion This needs some more discussion labels Nov 22, 2019
@andyalbert
Copy link

Hello, I tried to pick up this issue, but this code snippet is not there anymore on the main branch, I think we can close this issue.

@uranusjr
Copy link
Member

uranusjr commented Aug 6, 2021

There is still custom logic in e.g. network/download.py. You might find more by searching redact_auth_from_url through the code base and identify usages like this:

url = link.url  # Or e.g. url_without_fragment
redact_auth_from_url(url)

@q0w q0w linked a pull request May 3, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: logging Information Logging state: awaiting PR Feature discussed, PR is needed type: refactor Refactoring code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants