Skip to content

Commit

Permalink
Apply some comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Marishka17 committed Dec 11, 2024
1 parent 2bf9a0b commit fd8658e
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 16 deletions.
11 changes: 5 additions & 6 deletions cvat/apps/dataset_manager/tests/test_rest_api_formats.py
Original file line number Diff line number Diff line change
Expand Up @@ -1483,7 +1483,6 @@ def patched_log_exception(logger=None, exc_info=True):
original_exists,
side_effect(set_condition, export_checked_the_file),
side_effect(wait_condition, clear_process_has_run),
side_effect(sleep, 1),
)

mock_rq_get_current_job.return_value = MagicMock(timeout=5)
Expand Down Expand Up @@ -1517,16 +1516,16 @@ def _clear(*_, file_path: str, file_ctime: str):
):
mock_rq_get_current_job.return_value = MagicMock(timeout=5)

set_condition(clear_process_has_run)
file_is_been_used_error_raised = False
file_is_being_used_error_raised = False
try:
set_condition(clear_process_has_run)
clear_export_cache(
file_path=file_path, file_ctime=file_ctime, logger=MagicMock()
)
except FileIsBeingUsedError:
file_is_been_used_error_raised = True
file_is_being_used_error_raised = True

assert file_is_been_used_error_raised
assert file_is_being_used_error_raised
mock_os_remove.assert_not_called()

# The problem checked is TOCTOU / race condition for file existence check and
Expand All @@ -1538,7 +1537,7 @@ def _clear(*_, file_path: str, file_ctime: str):
# 4. remove removes the actual export file
# Thus, we have no exported file after the successful export.

# note: it is not possibel to achive a situation
# note: it is not possible to achieve the situation
# when clear process deletes newly "re-created by export process"
# file instead of the checked one since file names contain a timestamp.

Expand Down
9 changes: 7 additions & 2 deletions cvat/apps/dataset_manager/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,7 @@ def get_export_cache_lock(
block: bool = True,
acquire_timeout: int | timedelta,
) -> Generator[Lock, Any, Any]:
if acquire_timeout is None:
raise ValueError("Endless waiting for the lock should be avoided")
assert acquire_timeout is not None, "Endless waiting for the lock should be avoided"

if isinstance(acquire_timeout, timedelta):
acquire_timeout = acquire_timeout.total_seconds()
Expand Down Expand Up @@ -236,3 +235,9 @@ def parse_export_file_path(file_path: os.PathLike[str]) -> ParsedExportFilename:
format_repr=basename_match.group('format_tag'),
file_ext=basename_match.group('file_ext'),
)

def extend_export_file_lifetime(file_path: str):
# Update the last modification time to extend the export's lifetime,
# as the last access time is not available on every filesystem.
# As a result, file deletion by the cleaning job will be postponed.
os.utime(file_path, None)
8 changes: 3 additions & 5 deletions cvat/apps/dataset_manager/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
LockNotAvailableError,
current_function_name, get_export_cache_lock,
get_export_cache_dir, make_export_filename,
parse_export_file_path
parse_export_file_path, extend_export_file_lifetime
)
from .util import EXPORT_CACHE_DIR_NAME # pylint: disable=unused-import

Expand Down Expand Up @@ -163,9 +163,7 @@ def export(
acquire_timeout=EXPORT_CACHE_LOCK_ACQUISITION_TIMEOUT,
):
if osp_exists(output_path):
# Update last modification time to prolong the export lifetime
# and postpone the file deleting by the cleaning job
os.utime(output_path, None)
extend_export_file_lifetime(output_path)
return output_path

with tempfile.TemporaryDirectory(dir=cache_dir) as temp_dir:
Expand Down Expand Up @@ -193,7 +191,7 @@ def export(
"and available for downloading for the next {}. "
"Export cache cleaning job is enqueued, id '{}'".format(
db_instance.__class__.__name__.lower(),
db_instance.name if isinstance(db_instance, (Project, Task)) else db_instance.id,
db_instance.id,
dst_format,
output_path,
cache_ttl,
Expand Down
4 changes: 1 addition & 3 deletions cvat/apps/engine/background.py
Original file line number Diff line number Diff line change
Expand Up @@ -329,9 +329,7 @@ def handle_local_download() -> Response:
acquire_timeout=LOCK_ACQUIRE_TIMEOUT,
):
if osp.exists(file_path) and not is_result_outdated():
# Update last update time to prolong the export lifetime
# as the last access time is not available on every filesystem
os.utime(file_path, None)
dm.util.extend_export_file_lifetime(file_path)

return Response(status=status.HTTP_201_CREATED)

Expand Down

0 comments on commit fd8658e

Please sign in to comment.