Skip to content

Improve download logging #4518

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

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

Flamefire
Copy link
Contributor

@Flamefire Flamefire commented Apr 27, 2024

Log all URLs and remove the "Downloaded ..." message in dry-run mode

Also some refactoring to make the code easier to read/maintain like moving loop invariants out of the loop and using a common variable for the error and warning message.

@Flamefire Flamefire force-pushed the improve_download_logging branch from eecded8 to 8519a24 Compare April 27, 2024 12:02
@boegel boegel added this to the release after 4.9.1 milestone May 22, 2024
@boegel boegel modified the milestones: 4.9.2, release after 4.9.2 Jun 6, 2024
@Flamefire Flamefire force-pushed the improve_download_logging branch from 614e0aa to 97c8199 Compare August 8, 2024 08:00
@boegel boegel modified the milestones: 4.9.3, release after 4.9.3 Aug 28, 2024
@boegel boegel modified the milestones: release after 4.9.4, release after 5.0.0 Mar 17, 2025
The comment about extensions doesn't really apply to the actual behavior.
So give all information we can about what would happen.
No need to duplicate logic of failed downloads.
We want to check for a true-ish value and both `None` and the empty
string are considered false, so shorten the condition
Put the warning-only branch first to avoid the need for `not ...`
In both code paths the same message should be used, so do that more
clearly.
This also avoids an awkward substitution of percent signs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants