Fixes #3354 - Handles FileNotFoundError for assets#3355
Fixes #3354 - Handles FileNotFoundError for assets#3355miketaylr merged 4 commits intowebcompat:masterfrom
Conversation
miketaylr
left a comment
There was a problem hiding this comment.
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?