-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
ref(sourcemaps): Redesign lookup of source and sourcemaps #45032
Conversation
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.
Minor nit: encode()
should be encode('utf-8')
as discussed.
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.
code and comments are a lot more readable!
This also gave me a couple of ideas for symbolicator. CC @kamilogorek @loewenheim just fyi
|
||
# Cache holding the results of the fetching by url. | ||
self.fetch_by_url_sourceviews = {} | ||
self.fetch_by_url_errors = {} |
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.
is this the only place that stores (cached) download errors?
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.
Yep, it is.
self.allow_scraping = allow_scraping | ||
|
||
def bind_release(self, release=None, dist=None): | ||
"""Updates the fetcher with a project, release and dist.""" |
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.
It doesn't bind the project 👀
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.
@mitsuhiko what is your excuse? 👀🤣
* master: (79 commits) feat(perf-issues): Add performance issue detection timing runner command (#44912) Revert "chore: Investigating org slug already set to a different value (#45134)" fix(hybrid-cloud): Redirect to org restoration page for customer domains (#45159) bug(replays): Fix 500 error when marshaling tags field (#45097) ref(sourcemaps): Redesign lookup of source and sourcemaps (#45032) chore: Investigating org slug already set to a different value (#45134) feat(dynamic-sampling): Implement prioritize by project bias [TET-574] (#42939) feat(dynamic-sampling): Add transaction name prioritize option - (#45034) feat(dyn-sampling): add new bias toggle to project details for prioritise by tx name [TET-717] (#44944) feat(admin) Add admin relay project config view [TET-509] (#45120) Revert "chore(assignment): Add analytics when autoassigning after a manual assignment (#45099)" feat(sourcemaps): Implement new tables supporting debug ids (#44572) ref(js): Remove usage of react-document-title (#45170) chore(py): Consistently name urls using `organization-` prefix (#45180) ref: rename acceptance required checks collector (#45156) chore(assignment): Add analytics when autoassigning after a manual assignment (#45099) feat(source-maps): Update copy for source map debug alerts (#45164) ref(js): Remove custom usage of DocumentTitle (#45165) chore(login): update the login banners (#45151) ref(py): Remove one more legacy project_id from Environment (#45160) ...
This PR fixes a problem introduced in the redesign PR (#45032), which we overlooked the `self.project` parameter in the `Fetcher`.
This PR aims at implementing a new system for looking up source and sourcemaps files for each frame. This mechanism has been designed to account for the future implementation of debug ids.
#sync-getsentry