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

Conversation

@Swatinem
Copy link
Contributor

This nulls out the get_deleted_objects, which is used by the django admin to display a list of objects, and more importantly related objects being deleted.

As in particular with big owners/repos, actually building the list of all the related objects is unreasonably slow, so slow in fact that showing the confirmation dialog times out.

To not block deletes of deeply nested object hierarchies, this will just return empty lists instead.


This should fix https://github.com/codecov/internal-issues/issues/1302

This nulls out the `get_deleted_objects`, which is used by the django admin to display a list of objects, and more importantly related objects being deleted.

As in particular with big owners/repos, actually building the list of all the related objects is unreasonably slow, so slow in fact that showing the confirmation dialog times out.

To not block deletes of deeply nested object hierarchies, this will just return empty lists instead.
@Swatinem Swatinem requested a review from a team April 16, 2025 13:27
@Swatinem Swatinem self-assigned this Apr 16, 2025
@seer-by-sentry
Copy link
Contributor

seer-by-sentry bot commented Apr 16, 2025

🚨 Sentry detected 4 potential issues in your recent changes 🚨

Lower risk findings
  • Line 648, codecov_auth/admin.py: Function is missing a type annotation

    • get_deleted_objects now returns empty values, potentially skipping important deletion logic.
  • Line 107, core/admin.py: Function is missing a type annotation

    • get_deleted_objects returns empty values, potentially skipping important deletion logic.
  • Line 661, codecov_auth/admin.py: Function is missing a type annotation for one or more arguments

    • save_related is missing type annotations for form and formsets arguments.
  • Line 104, core/admin.py: Function is missing a type annotation for one or more arguments

    • delete_model is missing type annotations for request and obj arguments.

Did you find this useful? React with a 👍 or 👎

@Swatinem Swatinem enabled auto-merge April 17, 2025 06:45
@Swatinem Swatinem added this pull request to the merge queue Apr 17, 2025
@codecov-notifications
Copy link

codecov-notifications bot commented Apr 17, 2025

Codecov Report

Attention: Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
codecov_auth/admin.py 0.00% 1 Missing ⚠️
core/admin.py 50.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@codecov
Copy link

codecov bot commented Apr 17, 2025

Codecov Report

Attention: Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 96.31%. Comparing base (c2edbb3) to head (2f0d116).
Report is 2 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
codecov_auth/admin.py 0.00% 1 Missing ⚠️
core/admin.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1289      +/-   ##
==========================================
- Coverage   96.35%   96.31%   -0.04%     
==========================================
  Files         488      488              
  Lines       16919    16927       +8     
==========================================
+ Hits        16302    16304       +2     
- Misses        617      623       +6     
Flag Coverage Δ
unit 96.31% <33.33%> (-0.04%) ⬇️

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.

Merged via the queue into main with commit 28f8d8b Apr 17, 2025
19 of 24 checks passed
@Swatinem Swatinem deleted the swatinem/lightweight-delete branch April 17, 2025 06:53
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.

3 participants