-
Notifications
You must be signed in to change notification settings - Fork 203
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
Fixes #3354 - Handles FileNotFoundError for assets #3355
Conversation
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.
Thanks, this is a nice simple improvement.
How hard would it be to write some unit tests for this? Give it a fixture file, assert expected checksum, give it a non-existant file, assert missing_file
returned?
@miketaylr do you remember if there was a reason to put the get_checksum inside? it's slightly weird. |
This was done a long time ago :) |
you might have left out a word. inside _____? |
@miketaylr do you remember if there was a rationale for doing this nesting instead of two flat functions? def bust_cache(file_path):
def get_checksum(file_path):
…
return
return |
Let's see the test coverage.
We can do better. Let me complete this PR. Do not merge for now. |
I have switched to draft to not break anything. |
ok better the 1% missing is because we have one case which is not tested because the function doesn't make sense. I will open another issue for it. |
opened in #3356 |
Ah, no -- no memory. Nest functions seems nicer and more pythonic. |
Thanks @karlcow |
This PR fixes issue #3354
Proposed PR background
Basically in case the actual files where missing, we got a FileNotFound spiraling up and returning a 500.
This now will help a bit more by failing gracefully. The resource still does not exist but we will be able to see it in the network panel of devtools. The uris will carry a
?missing_file
?