[BUGFIX] Prevent corrupt writes to download archive (#983)
Attempts to make writes to the download archive safer (https://github.com/jmbannon/ytdl-sub/issues/982). `ytdl-sub` will now *copy* the download archive from the working directory to the output directory with a temp name, then perform a *move* to store it with its final expected name. This will drastically lower the window of time where the process could die mid-write and corrupt it on the next read.
This commit is contained in:
parent
74625c293d
commit
30a2ad7a63
4 changed files with 25 additions and 7 deletions
|
|
@ -161,7 +161,7 @@ class SubscriptionDownload(BaseSubscription, ABC):
|
||||||
)
|
)
|
||||||
|
|
||||||
self.download_archive.save_download_mappings()
|
self.download_archive.save_download_mappings()
|
||||||
FileHandler.delete(self.download_archive.working_file_path)
|
FileHandler.delete(self.download_archive.working_ytdl_file_path)
|
||||||
|
|
||||||
@contextlib.contextmanager
|
@contextlib.contextmanager
|
||||||
def _remove_empty_directories_in_output_directory(self):
|
def _remove_empty_directories_in_output_directory(self):
|
||||||
|
|
|
||||||
|
|
@ -89,7 +89,9 @@ class SubscriptionYTDLOptions:
|
||||||
ytdl_options = {}
|
ytdl_options = {}
|
||||||
|
|
||||||
if self._preset.output_options.maintain_download_archive:
|
if self._preset.output_options.maintain_download_archive:
|
||||||
ytdl_options["download_archive"] = self._enhanced_download_archive.working_file_path
|
ytdl_options["download_archive"] = (
|
||||||
|
self._enhanced_download_archive.working_ytdl_file_path
|
||||||
|
)
|
||||||
if self._preset.output_options.keep_max_files:
|
if self._preset.output_options.keep_max_files:
|
||||||
keep_max_files = int(
|
keep_max_files = int(
|
||||||
self._overrides.apply_formatter(self._preset.output_options.keep_max_files)
|
self._overrides.apply_formatter(self._preset.output_options.keep_max_files)
|
||||||
|
|
|
||||||
|
|
@ -385,7 +385,11 @@ class FileHandler:
|
||||||
dst_file_path
|
dst_file_path
|
||||||
Destination file
|
Destination file
|
||||||
"""
|
"""
|
||||||
shutil.copyfile(src=src_file_path, dst=dst_file_path)
|
# Perform the copy by first writing to a temp file, then moving it.
|
||||||
|
# This tries to prevent corrupted writes if the processed dies mid-write,
|
||||||
|
atomic_dst = f"{dst_file_path}-ytdl-sub-incomplete"
|
||||||
|
shutil.copyfile(src=src_file_path, dst=atomic_dst)
|
||||||
|
shutil.move(src=atomic_dst, dst=dst_file_path)
|
||||||
|
|
||||||
@classmethod
|
@classmethod
|
||||||
def move(cls, src_file_path: Union[str, Path], dst_file_path: Union[str, Path]):
|
def move(cls, src_file_path: Union[str, Path], dst_file_path: Union[str, Path]):
|
||||||
|
|
|
||||||
|
|
@ -506,10 +506,19 @@ class EnhancedDownloadArchive:
|
||||||
"""
|
"""
|
||||||
Returns
|
Returns
|
||||||
-------
|
-------
|
||||||
The download mapping's file path in the working directory.
|
The download mapping's file path in the working directory for ytdl usage
|
||||||
"""
|
"""
|
||||||
return str(Path(self.working_directory) / self.file_name)
|
return str(Path(self.working_directory) / self.file_name)
|
||||||
|
|
||||||
|
@property
|
||||||
|
def working_ytdl_file_path(self) -> str:
|
||||||
|
"""
|
||||||
|
Returns
|
||||||
|
-------
|
||||||
|
The download mapping's file path in the working directory for ytdl usage
|
||||||
|
"""
|
||||||
|
return f"{self.working_file_path}-ytdl-archive"
|
||||||
|
|
||||||
@property
|
@property
|
||||||
def mapping(self) -> DownloadMappings:
|
def mapping(self) -> DownloadMappings:
|
||||||
"""
|
"""
|
||||||
|
|
@ -541,7 +550,7 @@ class EnhancedDownloadArchive:
|
||||||
return self
|
return self
|
||||||
|
|
||||||
# Otherwise, create a ytdl download archive file in the working directory.
|
# Otherwise, create a ytdl download archive file in the working directory.
|
||||||
self.mapping.to_download_archive().to_file(self.working_file_path)
|
self.mapping.to_download_archive().to_file(self.working_ytdl_file_path)
|
||||||
|
|
||||||
return self
|
return self
|
||||||
|
|
||||||
|
|
@ -603,15 +612,18 @@ class EnhancedDownloadArchive:
|
||||||
if self._migrated_file_name:
|
if self._migrated_file_name:
|
||||||
self._download_mapping.to_file(output_json_file=self.working_file_path)
|
self._download_mapping.to_file(output_json_file=self.working_file_path)
|
||||||
self.save_file_to_output_directory(
|
self.save_file_to_output_directory(
|
||||||
file_name=self.file_name, output_file_name=self._migrated_file_name
|
file_name=self.file_name, output_file_name=self._migrated_file_name, copy_file=True
|
||||||
)
|
)
|
||||||
|
FileHandler.delete(file_path=self.working_file_path)
|
||||||
|
|
||||||
# and delete the old one if the name differs
|
# and delete the old one if the name differs
|
||||||
if self._file_name != self._migrated_file_name:
|
if self._file_name != self._migrated_file_name:
|
||||||
self.delete_file_from_output_directory(file_name=self.file_name)
|
self.delete_file_from_output_directory(file_name=self.file_name)
|
||||||
# Otherwise, only save if there are changes to the transaction log
|
# Otherwise, only save if there are changes to the transaction log
|
||||||
elif not self.get_file_handler_transaction_log().is_empty:
|
elif not self.get_file_handler_transaction_log().is_empty:
|
||||||
self._download_mapping.to_file(output_json_file=self.working_file_path)
|
self._download_mapping.to_file(output_json_file=self.working_file_path)
|
||||||
self.save_file_to_output_directory(file_name=self.file_name)
|
self.save_file_to_output_directory(file_name=self.file_name, copy_file=True)
|
||||||
|
FileHandler.delete(file_path=self.working_file_path)
|
||||||
return self
|
return self
|
||||||
|
|
||||||
def delete_file_from_output_directory(self, file_name: str):
|
def delete_file_from_output_directory(self, file_name: str):
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue