[Bugfix] Drop service/2/ from the service configuration during dstack preset export - #4160
Closed
peterschmidt85 wants to merge 2 commits into
Closed
[Bugfix] Drop service/2/ from the service configuration during dstack preset export#4160peterschmidt85 wants to merge 2 commits into
service/2/ from the service configuration during dstack preset export#4160peterschmidt85 wants to merge 2 commits into
Conversation
dstack preset export copied a preset's files keeping their store paths, so the session record layout (service/<k>/, trials/<n>/) leaked into the exported artifact. Files now export without the record prefix: service/2/patches/fix.patch lands at patches/fix.patch next to the configuration. When dropping the prefixes would collide, the full record layout is kept. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
service/2/ from the service configuration during dstack preset export
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
Closing: any rewriting of the stored paths needs collision guards, while the record layout is the only design that needs none, since its files already exist at those relative paths on the exporting filesystem. The prefix also carries provenance: the export names the recorded attempt that produced it. |
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
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.
Fixes an issue introduced in #4152.
Steps to reproduce
service/<k>/patches/, where<k>is the final-service attempt number.dstack preset export 6900d9d7 -f qwen38-service.dstack.yml.Actual behaviour
The exported configuration and the copied files keep the store-internal record layout, leaking the attempt numbering into the artifact:
Expected behaviour
The export keeps only the structure under the record prefix, with the files next to the configuration:
Collision fallback
Dropping the prefixes can merge paths that the record layout kept apart. Before writing anything, the export checks for that and keeps the full record layout for all files when:
service/1/patches/fix.patchandservice/2/patches/fix.patchboth becomingpatches/fix.patch;Paths are compared case-insensitively: on macOS and Windows
Fix.patchandfix.patchare the same file, so a case-sensitive check would let one copy silently overwrite the other. Each case has a test; verified end to end by deploying an exported preset on an MI300X and receiving a completion from the patched server.🤖 Generated with Claude Code