Skip to content

Commit c25755a

Browse files
committed
Fix backups for tasks using a mounted file share
Currently, backing up such a task succeeds, but restoring fails with "No such file or directory: <root>/share/manifest.jsonl". IMO, the problem is not really in the restore logic, but in the backup logic. Share task backups are "heavyweight", in that they include a copy of the media files. But the `storage` field in `task.json` is still set to "share". As a result, CVAT tries to import it as a share task, and that logic is broken. I think we should handle such backups consistently with heavyweight cloud storage backups, and set `storage` to "local". The restore logic will then work perfectly well. In the future, if we happen to implement lightweight backups for share tasks, we can use `storage: "share"` for those.
1 parent 57ceda9 commit c25755a

File tree

4 files changed

+42
-8
lines changed

4 files changed

+42
-8
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
### Fixed
2+
3+
- Backups of tasks created from a mounted file share no longer fail to import
4+
(<https://github.com/cvat-ai/cvat/pull/9972>)

cvat/apps/engine/backup.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -619,7 +619,10 @@ def serialize_data():
619619
]
620620
data['validation_layout'] = validation_params
621621

622-
if self._db_data.storage == StorageChoice.CLOUD_STORAGE and not self._lightweight:
622+
if (
623+
self._db_data.storage == StorageChoice.SHARE
624+
or self._db_data.storage == StorageChoice.CLOUD_STORAGE and not self._lightweight
625+
):
623626
data["storage"] = StorageChoice.LOCAL
624627

625628
return self._prepare_data_meta(data)
Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
services:
2-
cvat_worker_import:
3-
volumes:
4-
- ./tests/mounted_file_share:/home/django/share:rw
5-
cvat_server:
6-
volumes:
7-
- ./tests/mounted_file_share:/home/django/share:rw
8-
cvat_worker_chunks:
2+
cvat_server: &share-config
93
volumes:
104
- ./tests/mounted_file_share:/home/django/share:rw
5+
cvat_worker_chunks: *share-config
6+
cvat_worker_import: *share-config
7+
cvat_worker_export: *share-config

tests/python/rest_api/test_tasks.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1298,6 +1298,33 @@ def test_cannot_export_backup_for_task_without_data(self, tasks):
12981298

12991299
assert "Backup of a task without data is not allowed" in str(capture.value.body)
13001300

1301+
@pytest.mark.with_external_services
1302+
def test_can_export_and_import_backup_task_with_mounted_share(self):
1303+
share_content = [
1304+
"images/image_1.jpg",
1305+
"images/image_2.jpg",
1306+
]
1307+
task_spec = {
1308+
"name": "Task with files from mounted share",
1309+
"labels": [{"name": "car"}],
1310+
}
1311+
data_spec = {"image_quality": 75, "server_files": share_content}
1312+
task_id, _ = create_task(self.user, task_spec, data_spec)
1313+
1314+
task = self.client.tasks.retrieve(task_id)
1315+
1316+
filename = self.tmp_dir / f"share_task_{task.id}_backup.zip"
1317+
task.download_backup(filename)
1318+
1319+
with zipfile.ZipFile(filename, "r") as zf:
1320+
files_in_data = {
1321+
name.removeprefix("data/") for name in zf.namelist() if name.startswith("data/")
1322+
}
1323+
1324+
assert files_in_data == {"manifest.jsonl", *share_content}
1325+
1326+
self._test_can_restore_task_from_backup(task_id)
1327+
13011328
@pytest.mark.with_external_services
13021329
@pytest.mark.parametrize("lightweight_backup", [True, False])
13031330
def test_can_export_and_import_backup_task_with_cloud_storage(self, lightweight_backup):
@@ -1404,6 +1431,9 @@ def _test_can_restore_task_from_backup(self, task_id: int, lightweight_backup: b
14041431
assert new_meta["storage"] == ("cloud_storage" if lightweight_backup else "local")
14051432
assert new_meta["cloud_storage_id"] is None
14061433
exclude_regex_paths.extend([r"root\['cloud_storage_id'\]", r"root\['storage'\]"])
1434+
if old_meta["storage"] == "share":
1435+
assert new_meta["storage"] == "local"
1436+
exclude_regex_paths.append(r"root\['storage'\]")
14071437

14081438
assert (
14091439
DeepDiff(

0 commit comments

Comments
 (0)