Skip to content

Conversation

@rahul-kumar-saini
Copy link
Contributor

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Mar 7, 2023
@rahul-kumar-saini rahul-kumar-saini marked this pull request as ready for review March 7, 2023 19:01
@rahul-kumar-saini rahul-kumar-saini requested review from a team as code owners March 7, 2023 19:01
@rahul-kumar-saini rahul-kumar-saini requested a review from a team as a code owner March 7, 2023 19:01
@rahul-kumar-saini rahul-kumar-saini requested review from a team March 7, 2023 19:01
@rahul-kumar-saini rahul-kumar-saini requested a review from a team as a code owner March 7, 2023 19:01
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.

lgtm, although looks like you need to add tenant ids to sentry.snuba.metrics.get_tag_values


project_id = issue_list[0].project_id
item_ids = [g.id for g in issue_list]
tenant_ids = {"organization_id": issue_list[0].project.organization_id}
Copy link
Member

Choose a reason for hiding this comment

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

Just noting that we preload group projects here

prefetch_related_objects(item_list, "project__organization")
, so this won't cause an extra query.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm new to Django, are you saying I'm causing an extra query by doing .project.organization_id? How can I avoid this if so?

Copy link
Member

Choose a reason for hiding this comment

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

No, just noting for anyone else reading that it won't cause any extra load

@rahul-kumar-saini
Copy link
Contributor Author

lgtm, although looks like you need to add tenant ids to sentry.snuba.metrics.get_tag_values

sentry.snuba.metrics.get_tag_values calls _fetch... which calls run_metrics_query which contains the tenant_ids in the Request.

Lmk if I'm missing any other query somewhere in this stack 😅

@wedamija
Copy link
Member

wedamija commented Mar 7, 2023

lgtm, although looks like you need to add tenant ids to sentry.snuba.metrics.get_tag_values

sentry.snuba.metrics.get_tag_values calls _fetch... which calls run_metrics_query which contains the tenant_ids in the Request.

Lmk if I'm missing any other query somewhere in this stack 😅

This is what's causing the test failure though - something is attempting to pass tenant_ids to sentry.snuba.metrics.get_tag_values and it's not part of the function definition

@rahul-kumar-saini rahul-kumar-saini merged commit ddd37dc into master Mar 8, 2023
@rahul-kumar-saini rahul-kumar-saini deleted the rahul/feat/tagstore_tenant_ids branch March 8, 2023 00:02
@github-actions github-actions bot locked and limited conversation to collaborators Mar 23, 2023
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants