Conversation
xsh310
added this pull request to stack #768
September 20, 2026 19:23
xsh310
commented
Sep 20, 2026
xsh310
commented
Sep 20, 2026
xsh310
force-pushed
the
skills-mirror-download
branch
from
September 21, 2026 20:34
2314a51 to
ec94820
Compare
write_skill wrote each new file over the existing bundle dir, so a file removed from a skill in Unity Catalog lingered on disk after a re-download. Clear the bundle directory before rewriting it, so the on-disk copy mirrors UC. A write interrupted partway leaves only that one directory incomplete, never a stray copy elsewhere; the next write clears and rebuilds it, so a retry converges on the current bundle. This is the shared writer for every download path, so `ug skills add` re-downloads gain the same mirror semantics. Co-authored-by: Isaac <no-reply@databricks.com>
xsh310
marked this pull request as ready for review
September 21, 2026 20:40
xsh310
force-pushed
the
skills-mirror-download
branch
from
September 21, 2026 20:45
ec94820 to
678305b
Compare
xsh310
added
bug
xsh310
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What did you change, and why?
Change:
write_skillnow clears a skill's bundle directory before rewriting it, so the on-disk copy is a fresh mirror of the current bundle rather than the new files merged on top of the old ones.Why: The writer merged: it wrote every file from the new bundle but never removed files no longer in it, so a file deleted from a skill in Unity Catalog lingered on disk after a re-download. Clearing the directory first makes dropped files vanish. The clear-then-rewrite is also self-healing: a write interrupted partway leaves only that one directory incomplete (never a stray copy elsewhere), and the next write clears and rebuilds it, so a retry always converges on the current bundle. On the launch-refresh path the manifest's
uc_update_timeis advanced only after a successful write, so an interrupted write stays flagged as stale and is retried on the next launch.This is the shared writer for every download path, so
ug skills addre-downloads get the same mirror semantics; the launch-time auto-update stacked on top (#764) relies on it.How do you know it works?
Testing: Added
TestWriteSkillcases for the deleted-upstream-file case, an empty bundle leaving the existing copy untouched, replacing a symlinked bundle without following it to its target, converging from an interrupted previous write (a pre-seeded partial directory), and leaving only the bundle directory behind.uv run ruff check,ruff format --check,ty check src, anduv run pytestall pass locally.This pull request and its description were written by Isaac.