import_ovpack(on_conflict="overwrite") deletes the existing resource before writing, so a failed import loses both versions

Open
  • Is something wrong?
#7,433 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

@Kiko0301 is already working on this.

Since Nov 11, 2026.

  • #7421 by @Kiko0301 — open

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
20/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Stale
Tech stack
python
Domain
backend

Research direction

Read openviking/storage/ovpack/operations.py, especially _remove_existing_root() and the import_ovpack() member-writing loop. PR #7421 describes the proposed backup and restore behavior, and the issue says two regression tests fail on main; inspect those tests and verify that a failed member write preserves the old resource. The issue already has an open pull request.

Written by the indexing model from the issue text.

Description

import_ovpack(..., on_conflict="overwrite") deletes the existing root before writing the package

Bug Description

import_ovpack() (openviking/storage/ovpack/operations.py:263) handles overwrite by calling _remove_existing_root() (operations.py:127) for every existing root and only then iterating the package members with mkdir / write_file_bytes (operations.py:343-368). The delete is not reversible and the member loop is not transactional.

Any failure during the member loop — a storage timeout, a full disk, an ACL error, process cancellation — leaves the destination with the new files partially written and the previous resource permanently deleted. The function raises, but neither the old nor the new version is intact.

Steps to Reproduce

  1. Import a pack at viking://resources/demo, producing a.txt and b.txt.
  2. Import a second pack for the same root with on_conflict="overwrite".
  3. Make write_file_bytes fail for one member (e.g. b.txt) mid-import.

Expected Behavior

The failed import leaves the previous resource at viking://resources/demo intact, so the caller can retry.

Actual Behavior

The previous root was deleted before the member writes; the destination holds a partially written new root, and the previous a.txt / b.txt are gone.

Environment

  • OS: Windows 11
  • OpenViking version/commit: main @ 95d96162

Additional Context

Fix submitted as PR #7421: the existing root is moved to <root>.ovpack-backup-<uuid> before writing, restored if the member loop fails, and discarded after a successful write. Filesystems without mv keep the previous delete-then-write behavior. Two regression tests fail on unmodified main.

Dominant language
Python
Stars
41.8k
Forks
3.4k
Avg merge
19h 29m
Merged PRs (30d)
500

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from volcengine/OpenViking

All issues in volcengine/OpenViking

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.