-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
ci: Enable Objectstore #103483
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ci: Enable Objectstore #103483
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #103483 +/- ##
===========================================
+ Coverage 80.61% 80.63% +0.01%
===========================================
Files 9280 9280
Lines 396184 396165 -19
Branches 25250 25247 -3
===========================================
+ Hits 319402 319458 +56
+ Misses 76322 76247 -75
Partials 460 460 |
Swatinem
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉 🥔
| if report.stored_id: | ||
| session = get_attachments_session(self.project.organization_id, self.project.id) | ||
| storage_url = session.object_url(report.stored_id) | ||
| storage_url = maybe_rewrite_objectstore_url(storage_url) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of rewriting the the URL here, can we set the objectstore.config option to point to objectstore in tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you did that you would run into the opposite problem, i.e. sentry wouldn't be able to reach http://objectstore:8888, because sentry is not running in docker.
backend-cimode of devservicesrequires_objectstoredecorator if the service is not running