ref(snapshots): Key manifest entries by relative path instead of hash#3241
Merged
NicoHinderling merged 9 commits intomasterfrom Mar 25, 2026
Merged
ref(snapshots): Key manifest entries by relative path instead of hash#3241NicoHinderling merged 9 commits intomasterfrom
NicoHinderling merged 9 commits intomasterfrom
Conversation
Switch the snapshot manifest dictionary key from content hash to relative file path. The content hash is preserved in the image metadata extra field, enabling the backend to support both key formats during the transition. Co-Authored-By: Claude <[email protected]>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Use path_as_url to normalize backslashes to forward slashes on Windows, ensuring manifest keys are platform-independent. Co-Authored-By: Claude <[email protected]>
Fixes clippy::str_to_string lint. Co-Authored-By: Claude <[email protected]>
Switch manifest key from relative path to file name (basename). When multiple images share the same file name from different directories, keep the first and warn with the full relative paths of all collisions so the user can debug and resolve duplicates. Co-Authored-By: Claude <[email protected]>
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
The manifest key is now the image file name, so storing it again inside the value is redundant. Remove the field from ImageMetadata and let the backend derive it from the key instead. Co-Authored-By: Claude <[email protected]>
Move the collision check before the file open and upload queue so colliding images are not uploaded unnecessarily. Co-Authored-By: Claude <[email protected]>
Use manifest_entries.len() instead of the initial images count so the success and error messages reflect skipped collisions. Co-Authored-By: Claude <[email protected]>
The collision warning now lists all paths sharing a filename (kept and excluded) on one line per group, making it clear which image was retained and which were skipped. Co-Authored-By: Claude <[email protected]>
rbro112
approved these changes
Mar 25, 2026
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| .or_default() | ||
| .push(relative_path); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
"Uploading" count includes collision-skipped images
Low Severity
The new collision detection in upload_images() can skip images, making the actual upload count (manifest_entries.len()) smaller than images.len(). The "Uploaded" and error messages inside upload_images() were updated to use manifest_entries.len(), but the "Uploading N image files" message in execute() (line 118) still uses images.len(). When collisions occur, users see e.g. "Uploading 5 image files" followed by "Uploaded 3 image files" — a confusing mismatch that didn't exist before this PR since hash-based keying had no collisions.
lcian
approved these changes
Mar 25, 2026
loewenheim
approved these changes
Mar 25, 2026
NicoHinderling
added a commit
to getsentry/sentry
that referenced
this pull request
Mar 25, 2026
…on (#111467) Switch the snapshot manifest to use the dict key as the image file name, removing the redundant `image_file_name` field from `ImageMetadata`. All consumers now derive the image name from the manifest key instead of reading it from the value. Changes: - Remove `image_file_name` from `ImageMetadata` model - `categorize_image_diff` uses dict key as the image name - `_build_base_images` and API endpoint derive file name from key - `content_hash` field used for objectstore lookups (falls back to key for old hash-keyed manifests) - Tests rewritten to use filename-keyed manifests Companion CLI PR: getsentry/sentry-cli#3241 Co-authored-by: Claude Opus 4.6 <[email protected]>
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
4 participants
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.
Switch the snapshot manifest dictionary key from content hash to image
file name (basename). This enables the backend to use the key directly
as the image identifier, removing the redundant
image_file_namefieldfrom manifest values.
Changes:
image_file_nameremoved fromImageMetadata— derived from keycontent_hashstored in image metadataextrafor objectstore lookupswarning listing all excluded paths
path_as_urlfor cross-platform consistencyCompanion backend PR: getsentry/sentry#111467