review and improve error handling when invoking makedirs #1365
Conversation
Test Results239 tests 234 ✅ 1m 35s ⏱️ For more details on these failures, see this check. Results for commit 98c55eb. ♻️ This comment has been updated with latest results. |
|
I'm a bit worried about this one. Deleting a file that already exists and replacing it with a directory seems like kind of surprising behavior, at least to me. It's probably a pretty rare situation, so maybe it's not a big deal. I think there should at least be a warning logged when we delete the file, because that could help someone troubleshoot if they run into this. |
|
The way the mirroring works, fine tuned order of operations is not assured. for All data types, not just files, we have a lot of code that deletes old versions of the files.... This is going a step further. I'm not super certain about it either, .. if you want can revert that (requires separate patch... since the changes are all in one commit.) Want me to remove that file with the same name remove code? |
|
oh... it will still solve the client problem, because the improved error handling is what was essential. |
|
OK... I was testing the hpc-mirroring example, and the rm causes crashes and data loss when mirroring the linux kernel. I will remove that stuff. |
If (usual case) there are multiple files in a directory, then there is a race condition to create the directory, multiple instances will see the directory is not there, and will run makedirs to create it. the "loser" just won't create the directory... but with unlink... the unlink fails (because directory exists.) and then the operation fails. (not retried? FIXME?) So I think it's better to leave that failure mode to humans.
client ran into some error/crashes related to makedirs in downloading.
Reviewed all invocations.