Skip to content

feat(tags): Allows tags endpoint to optionally return human readable values #40792

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

Merged

Conversation

edwardgou-sentry
Copy link
Contributor

Adds a readable option to the /tags endpoint that includes more human readable values for certain tag values.
For now, this is only supported by the device tag which converts device model numbers to their more readable marketing names.

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Oct 31, 2022
Copy link
Member

@wmak wmak left a comment

Choose a reason for hiding this comment

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

lgtm, but the 18k file sorta worries me, could we get some performance numbers using the timeit module (or sentry)

Copy link
Member

@wmak wmak left a comment

Choose a reason for hiding this comment

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

Can we include a comment in src/sentry/api/helpers/android_models.py explaining how to update it? (ie. step 1 download file from ___, step 2 .... etc.)

Copy link
Member

@wedamija wedamija left a comment

Choose a reason for hiding this comment

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

I feel like I've seen this mapping (or a similar mapping) before, maybe related to profiling? Are we sure we're not duplicating something that already exists?

@edwardgou-sentry
Copy link
Contributor Author

I feel like I've seen this mapping (or a similar mapping) before, maybe related to profiling? Are we sure we're not duplicating something that already exists?

I think there's been a link to some external mapping being shared on slack but none of that has been committed to the sentry repo yet from what I've been told.
Also did a quick search for some of the values in the mapping in the sentry repo and didnt find any results so I think this would be the first time we introduce a mapping like this?

@edwardgou-sentry edwardgou-sentry requested a review from a team as a code owner November 1, 2022 19:52
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Nov 1, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Nov 1, 2022

🚨 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.

@edwardgou-sentry edwardgou-sentry merged commit 2249c5f into master Nov 2, 2022
@edwardgou-sentry edwardgou-sentry deleted the egou/feat/tags-endpoint-readable-tag-value-option branch November 2, 2022 13:52
@github-actions github-actions bot locked and limited conversation to collaborators Nov 18, 2022
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.

3 participants