Dedupe ancestor title requests and lookup once per field#3025
Merged
brianzelip merged 1 commit intoarchivesspace:masterfrom Oct 3, 2023
Merged
Conversation
brianzelip
force-pushed
the
dedupe-ancestor-title-requests
branch
from
d7bc738 to
fab8492
Compare
Collaborator
|
@mark-cooper I re-ran CI here a few times, all failed on the same test. I also get the same failure locally. |
Member
Author
|
@brianzelip I'll sync this up with master and see where things stand -- thx for trying. |
mark-cooper
force-pushed
the
dedupe-ancestor-title-requests
branch
from
fab8492 to
3be8dca
Compare
9 tasks
thimios
pushed a commit
that referenced
this pull request
Apr 4, 2024
This updates: #3025 Additional testing has been done to ensure the instance variable is valid in all views (hence initted in application controller).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR addresses 2 issues we encountered with "search" adjacent performance rendering the context cell (particularly with larger page sizes).
Currently a call to
get_ancestor_titleis made at least twice per "ancestor". Once to see if there's a value, then again to get the value. The PR updates this to just happen once and use the result.Requests for ancestors are repeated per result (record). If a result shares ancestors the same request is being made without reference to any previous lookups for the same "ancestor". To help with this this PR goes with a minimally intrusive approach of introducing a local variable per controller action. It's simple, can be easily replaced or updated in the future for something more sophisticated (it may eventually be better to do something that isn't requesting an entire json record to get a single field), but does the job of reducing the no. of requests to once per "field" (uri), per search request.
Using the ATTracer repository and searching for "bulk":
master (118 requests):
PR (11 requests):
Types of changes
Checklist: