Skip to content

chore(API): Remove old unused symbol sources url path#104298

Merged
ceorourke merged 6 commits intomasterfrom
ceorourke/rm-old-symbol-sources-url
Dec 3, 2025
Merged

chore(API): Remove old unused symbol sources url path#104298
ceorourke merged 6 commits intomasterfrom
ceorourke/rm-old-symbol-sources-url

Conversation

@ceorourke
Copy link
Copy Markdown
Member

We have a newer path to use since #56775 so we can remove this one now.

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Dec 2, 2025
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Dec 2, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Dec 2, 2025

🚨 Warning: This pull request contains Frontend and Backend changes!

It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently.

Have questions? Please ask in the #discuss-dev-infra channel.

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 3, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff             @@
##           master   #104298      +/-   ##
===========================================
+ Coverage   80.52%    80.60%   +0.07%     
===========================================
  Files        9342      9339       -3     
  Lines      399115    399216     +101     
  Branches    25560     25560              
===========================================
+ Hits       321391    321790     +399     
+ Misses      77275     76977     -298     
  Partials      449       449              

@ceorourke ceorourke force-pushed the ceorourke/rm-old-symbol-sources-url branch from ea91467 to b69988e Compare December 3, 2025 00:35
@ceorourke ceorourke marked this pull request as ready for review December 3, 2025 01:00
@ceorourke ceorourke requested review from a team as code owners December 3, 2025 01:00
@ceorourke ceorourke requested a review from a team December 3, 2025 01:00
Comment on lines -181 to -204
@responses.activate
def test_proxy_check_region_pinned_url(self) -> None:
responses.add(
responses.GET,
f"{self.REGION.address}/builtin-symbol-sources/",
json={"proxy": True},
)

# No /api/0 as we only include sentry.api.urls.urlpatterns
# and not sentry.web.urls which includes the version prefix
region_pinned = "/builtin-symbol-sources/"
control_url = reverse(
"control-endpoint", kwargs={"organization_slug": self.organization.slug}
)

with override_settings(SILO_MODE=SiloMode.CONTROL, MIDDLEWARE=tuple(self.middleware)):
resp = self.client.get(region_pinned)
assert resp.status_code == 200
resp_json = json.loads(close_streaming_response(resp))
assert resp_json["proxy"] is True

resp = self.client.get(control_url)
assert resp.status_code == 200
assert resp.data["proxy"] is False
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Instead of deleting this test, could the URL name be changed to one we're likely to keep like sentry-js-sdk-loader?

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.

I'm not sure what I'm missing but I tried this and couldn't get it to work - it always 404s. I tried with a couple other pinned URLs too without success. I pushed up the change to use the js sdk loader in case you can see something I'm not.

@ceorourke ceorourke force-pushed the ceorourke/rm-old-symbol-sources-url branch from cfbfe6f to 4a912da Compare December 3, 2025 19:05
Copy link
Copy Markdown
Member

@markstory markstory left a comment

Choose a reason for hiding this comment

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

Thank you 🙇

@ceorourke ceorourke merged commit c6b8785 into master Dec 3, 2025
67 checks passed
@ceorourke ceorourke deleted the ceorourke/rm-old-symbol-sources-url branch December 3, 2025 20:05
@github-actions github-actions bot locked and limited conversation to collaborators Dec 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants