Skip to content
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

perf(dx): let developers know that they need to enable index on links #21745

Merged
merged 4 commits into from
Aug 3, 2023

Conversation

barredterra
Copy link
Collaborator

@barredterra barredterra commented Jul 19, 2023

For DocTypes linked on a dashboard, get_open_count() and (via click) the list view filtered by this link will be called many times. Hence, indexing likely pays off.

@github-actions github-actions bot added the add-test-cases Add test case to validate fix or enhancement label Jul 19, 2023
@codecov
Copy link

codecov bot commented Jul 19, 2023

Codecov Report

Merging #21745 (9a3eef3) into develop (d1c3877) will increase coverage by 2.16%.
The diff coverage is 100.00%.

❗ Current head 9a3eef3 differs from pull request most recent head c83b8b2. Consider uploading reports for the commit c83b8b2 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #21745      +/-   ##
===========================================
+ Coverage    61.88%   64.05%   +2.16%     
===========================================
  Files          765      763       -2     
  Lines        72988    69526    -3462     
  Branches      6292     6281      -11     
===========================================
- Hits         45170    44536     -634     
+ Misses       24275    21455    -2820     
+ Partials      3543     3535       -8     
Flag Coverage Δ
server 69.14% <100.00%> (+3.88%) ⬆️

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

@stale
Copy link

stale bot commented Jul 27, 2023

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

@stale stale bot added the inactive label Jul 27, 2023
@ankush ankush removed the inactive label Jul 28, 2023
@ankush ankush requested a review from a team July 31, 2023 09:30
@ankush ankush removed the add-test-cases Add test case to validate fix or enhancement label Aug 2, 2023
@ankush ankush changed the title perf: enable indexing for dashboard links perf(dx): let developers know that they need to enable index on links Aug 3, 2023
Many edge cases which can't be checked on-the-fly. Best to just notify
developer that they should enable indexing.
@ankush
Copy link
Member

ankush commented Aug 3, 2023

I ran this and it enabled way too many indexes (not a lot of them required indexing 🥴 ), we need a better solution for all link/dynamic link indexing 😔

For now I've made this a nudge to developer, they can enable it if required.

@ankush ankush merged commit fa73d5e into frappe:develop Aug 3, 2023
16 checks passed
@barredterra barredterra deleted the index-dashboard-links branch August 3, 2023 07:41
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 18, 2023
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.

None yet

2 participants