-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Catch ZstdError in jlap/repodata.json.zst handling #13559
Conversation
CodSpeed Performance ReportMerging #13559 will create unknown performance changesComparing Summary
|
except (JlapSkipZst, HTTPError) as e: | ||
except (JlapSkipZst, HTTPError, zstandard.ZstdError) as e: | ||
if isinstance(e, zstandard.ZstdError): | ||
# will this include the token |
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.
I think that's reasonable, so maybe sanitize it? Or leave out the URL?
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.
Improved.
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.
Not sure if we actually need the debugging statement, but I concur it's good to catch zstandard decompression exceptions
In general this entire code path could be more clear about whether the user's web server is configured correctly, and which variant they wind up getting. If we don't say something then the user would not know that they have a "404 page that returns a 200 code" for For the unchanged code possibly more HTTP errors should result in "use repodata.json and don't check repodata.json.zst for another week" |
Then lets make this and the other possible cases result in more specific (and actionable) logging statement? Info level? |
3742f23
to
b193003
Compare
Improved log message with sanitized URL, and a bonus sanitized URL in a similar log message. |
e2bbe6b
to
9c73315
Compare
9c73315
to
f24aac3
Compare
* catch ZstdError in jlap/repodata.json.zst handler
Description
Fix #13558
Checklist - did you ...
news
directory (using the template) for the next release's release notes?