Delete tmpfiles #1135
Delete tmpfiles #1135
Conversation
content_harvester/by_record.py
Outdated
| collection_id, | ||
| request, | ||
| thumbnail_src, | ||
| record['calisphere-id'] | ||
| ) | ||
|
|
||
| [os.remove(filepath) for filepath in tmp_files] |
There was a problem hiding this comment.
So one problem with this route is that tmp_files could be undefined in the case where there's no media and no thumbnail.
Another problem is that, if there is a media file and a thumbnail, any tmp_files returned by create_media_component will be overwritten by the tmp_files returned by create_thumbnail_component, and won't be deleted here at line 116.
There was a problem hiding this comment.
D'oh, of course. How does the new commit look?
There was a problem hiding this comment.
Looks good, but now, in the case where the media_source and the thumbnail_source are the same, you might wind up with duplicates in tmp_files and os.remove will throw a FileNotFoundError when it tries to delete the same file a second time.
[os.remove(filepath) for filepath in set(tmp_files)] would de-dupe the list, or you could explicitly check if os.path.exists(filepath) before removing it.
There was a problem hiding this comment.
Egads, right you are! Pushed another commit.
content_harvester/by_record.py
Outdated
| collection_id, | ||
| request, | ||
| thumbnail_src, | ||
| record['calisphere-id'] | ||
| ) | ||
|
|
||
| [os.remove(filepath) for filepath in tmp_files] |
There was a problem hiding this comment.
Looks good, but now, in the case where the media_source and the thumbnail_source are the same, you might wind up with duplicates in tmp_files and os.remove will throw a FileNotFoundError when it tries to delete the same file a second time.
[os.remove(filepath) for filepath in set(tmp_files)] would de-dupe the list, or you could explicitly check if os.path.exists(filepath) before removing it.
No description provided.