Skip to content

Add icon uploading to objectstore#430

Merged
rbro112 merged 6 commits intomainfrom
ryan/add_icon_uploading_to_objectstore
Dec 12, 2025
Merged

Add icon uploading to objectstore#430
rbro112 merged 6 commits intomainfrom
ryan/add_icon_uploading_to_objectstore

Conversation

@rbro112
Copy link
Copy Markdown
Member

@rbro112 rbro112 commented Oct 24, 2025

Adds app icon uploading to objectstore and passes the returned objectstore id, app_icon_id. to the update API as part of preprocessing.

Likely don't want to merge this as-is as the objectstore code is just copied in, but here nonetheless in prep for merge.

Copy link
Copy Markdown
Member Author

rbro112 commented Oct 24, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@codecov
Copy link
Copy Markdown

codecov bot commented Oct 24, 2025

Codecov Report

❌ Patch coverage is 47.16312% with 149 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.62%. Comparing base (de07a59) to head (265a811).
⚠️ Report is 7 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/launchpad/utils/objectstore/service.py 45.63% 56 Missing ⚠️
src/launchpad/utils/objectstore/metadata.py 39.72% 44 Missing ⚠️
src/launchpad/utils/objectstore/metrics.py 27.27% 32 Missing ⚠️
src/launchpad/artifact_processor.py 50.00% 13 Missing ⚠️
src/launchpad/utils/file_utils.py 78.94% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #430      +/-   ##
==========================================
- Coverage   80.50%   79.62%   -0.88%     
==========================================
  Files         161      162       +1     
  Lines       13383    13562     +179     
  Branches     1406     1440      +34     
==========================================
+ Hits        10774    10799      +25     
- Misses       2078     2229     +151     
- Partials      531      534       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rbro112 rbro112 marked this pull request as draft October 24, 2025 23:16
@rbro112 rbro112 force-pushed the ryan/add_icon_uploading_to_objectstore branch from 91c689a to 265a811 Compare October 30, 2025 18:22
@rbro112 rbro112 changed the title Add icon uploading to objectstore feat(preprod): Add icon uploading to objectstore Oct 30, 2025
@rbro112 rbro112 changed the title feat(preprod): Add icon uploading to objectstore Add icon uploading to objectstore Oct 30, 2025
@jan-auer
Copy link
Copy Markdown
Member

jan-auer commented Dec 5, 2025

Hi, please update objectstore_client to 0.0.14 (or any newer version available at the time)

@rbro112 rbro112 force-pushed the ryan/add_icon_uploading_to_objectstore branch from 7d4bf0d to b801025 Compare December 11, 2025 00:52
@rbro112 rbro112 force-pushed the ryan/add_icon_uploading_to_objectstore branch from b801025 to 2ec1e83 Compare December 11, 2025 23:34
@rbro112 rbro112 marked this pull request as ready for review December 12, 2025 00:14
artifact_id: str,
artifact: Artifact,
) -> str | None:
if self._objectstore_client is None:
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If for some reason objectstore_client is None (should only ever happen if the OBJECSTORE_URL env variable isn't set), this will gracefully fail and log.

We should have everything set up properly following https://github.com/getsentry/ops/blob/bc63aef2e313ac482f1ffbca826fb9273b1aa643/k8s/services/launchpad/deployment.yaml#L34

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe it should log an error not info?



def get_file_size(file_path: Path) -> int:
"""Get file size in bytes.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason a bunch of the comments in this file were removed?

@rbro112 rbro112 merged commit e90b460 into main Dec 12, 2025
21 checks passed
@rbro112 rbro112 deleted the ryan/add_icon_uploading_to_objectstore branch December 12, 2025 17:26
rbro112 added a commit to getsentry/sentry that referenced this pull request Dec 17, 2025
Adds a new preprod `/files/images/<image_id>` endpoint which allows our
frontend to download images from an ID with objectstore. First to be
used with app icons coming soon.

Tested fully E2E locally and all works well. To work, this is reliant on
landing getsentry/launchpad#430 once we get a
published version of the objectstore client, but since nobody will be
consuming this endpoint until we land the stacked PR (#102118) this is
safe to go ahead and merge.

---------

Co-authored-by: Abdullah Khan <60121741+Abdkhan14@users.noreply.github.com>
Co-authored-by: Abdullah Khan <abdullahkhan@PG9Y57YDXQ.local>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants