S3 (28): Scope S3 object keys by repository name alone - #856
Open
CleanCut wants to merge 1 commit into
Open
Conversation
Moving a repository to another namespace no longer implies re-keying every object it has stored in S3. The version store previously built each repo's object prefix from the `<namespace>/<name>` tail of its path, so a namespace transfer would have had to copy every blob to a new key; the prefix is now the repository's name alone, and a transfer leaves stored objects untouched. S3 is pre-GA, so there is no stored data to migrate. This has to land before the first production S3 write, including the file->S3 migration, or migrated objects land under the old prefix and need an expensive re-key later. `OxenError::S3PrefixUnresolvable` now reports a missing or non-UTF-8 final path component, which is what can actually go wrong once the parent directory is no longer read. SelfHosting.md documents the new prefix. ENG-1171
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughS3 repository object prefixes now use ChangesS3 repository prefix simplification
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
CleanCut
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/liboxen/src/storage/version_store.rs`:
- Around line 611-623: Extend the S3 version-store tests alongside
create_version_store_s3_without_a_repo_directory_name_errors with a
Unix-specific path whose final component contains invalid UTF-8 bytes, and
assert create_version_store returns Err(OxenError::S3PrefixUnresolvable(_)).
Guard the test with the appropriate platform-specific configuration and
construct the path using raw bytes so the non-UTF-8 branch is exercised.
- Around line 535-542: Update the S3 prefix construction in the version-store
initialization around S3VersionStore::new so it uses a globally unique,
persisted immutable repository identifier rather than only repo_dir.file_name().
Preserve that identifier when the repository moves between namespaces, and add a
regression test proving repositories with the same name in different namespaces
use isolated prefixes and cannot cross-read or cross-delete objects.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4ce440c6-4184-4d5b-a6ce-4106558d52e5
📒 Files selected for processing (4)
SelfHosting.mdcrates/liboxen/src/error.rscrates/liboxen/src/storage/s3.rscrates/liboxen/src/storage/version_store.rs
CleanCut
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.
Moving a repository to another namespace no longer implies re-keying every object it has stored in S3. The version store previously built each repo's object prefix from the
<namespace>/<name>tail of its path, so a namespace transfer would have had to copy every blob to a new key; the prefix is now the repository's name alone, and a transfer leaves stored objects untouched.S3 is pre-GA, so there is no stored data to migrate.
ENG-1171