Skip to content
This repository was archived by the owner on Jun 13, 2025. It is now read-only.

Avoid N+1 queries fetching Upload.errors#1263

Merged
Swatinem merged 2 commits intomainfrom
swatinem/uploaderror-nplus1
Apr 2, 2025
Merged

Avoid N+1 queries fetching Upload.errors#1263
Swatinem merged 2 commits intomainfrom
swatinem/uploaderror-nplus1

Conversation

@Swatinem
Copy link
Contributor

@Swatinem Swatinem commented Apr 1, 2025

Previously, this was going through an unnecessarily complex Java-esque design pattern, just to execute a query that can be much better served by using a prefetch_related Django combinator.


This should fix codecov/engineering-team#3399

@Swatinem Swatinem requested a review from adrian-codecov April 1, 2025 13:00
@Swatinem Swatinem self-assigned this Apr 1, 2025
@Swatinem Swatinem requested review from a team as code owners April 1, 2025 13:00
@Swatinem Swatinem force-pushed the swatinem/uploaderror-nplus1 branch from 8efa5ff to 662e742 Compare April 1, 2025 13:01
@seer-by-sentry
Copy link
Contributor

seer-by-sentry bot commented Apr 1, 2025

✅ Sentry found no issues in your recent changes ✅

Copy link
Contributor

@joseph-sentry joseph-sentry left a comment

Choose a reason for hiding this comment

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

one comment about using queryset_to_connection, otherwise lgtm

Base automatically changed from swatinem/repo-resolver-fields to main April 1, 2025 15:05
Previously, this was going through an unnecessarily complex Java-esque design pattern, just to execute a query that can be much better served by using a `prefetch_related` Django combinator.
@Swatinem Swatinem force-pushed the swatinem/uploaderror-nplus1 branch from 662e742 to c224f1a Compare April 1, 2025 15:10
@Swatinem Swatinem requested a review from joseph-sentry April 1, 2025 15:11
repository_id__in=[self.repo1.pk, self.repo2.pk],
)
)
).reverse()
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 have no idea whats going on here. For some reason I don’t understand, the assertion order for this completely unrelated code is switched around.
And its also not a flake either, as I have retried this 3 times, and its failing with the same error.

@Swatinem Swatinem force-pushed the swatinem/uploaderror-nplus1 branch from a91935a to 9f9cefb Compare April 1, 2025 16:25
@codecov
Copy link

codecov bot commented Apr 1, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.31%. Comparing base (defc6a5) to head (9f9cefb).
Report is 2 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1263      +/-   ##
==========================================
- Coverage   96.32%   96.31%   -0.01%     
==========================================
  Files         493      490       -3     
  Lines       16933    16916      -17     
==========================================
- Hits        16310    16293      -17     
  Misses        623      623              
Flag Coverage Δ
unit 96.31% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@codecov-notifications
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@Swatinem Swatinem added this pull request to the merge queue Apr 2, 2025
Merged via the queue into main with commit 55ca1e7 Apr 2, 2025
23 of 24 checks passed
@Swatinem Swatinem deleted the swatinem/uploaderror-nplus1 branch April 2, 2025 15:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[API] - Improve N+1 ReportUploadError Query resolve time

2 participants