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

Cache table "isil2opac_hbzid" #1911

Closed
wants to merge 2 commits into from

Conversation

dr0i
Copy link
Member

@dr0i dr0i commented Sep 26, 2023

Don't make a URL lookup to get the concordance every time an item is shown. Load thie concordance from a gitsubmodule table and cache it as HashMap.

Follows #1893.

Don't make a URL lookup to get the concordance every time an item is shown.
Load thie concordance from a gitsubmodule table and cache it as HashMap.

Follows #1893.
This is necessary for repos that are already cloned and needs to work
with gitsubmodules.
@dr0i dr0i requested a review from fsteeg September 26, 2023 13:00
@dr0i dr0i added the Web-App label Sep 26, 2023
@fsteeg
Copy link
Member

fsteeg commented Sep 26, 2023

Loading the file in Global and setting a static field on the Lobid controller seems more complicated than needed. Play has a built-in cache which I think would make sense to use here. It's already used elsewhere in the controller, see:

return Cache.getOrElse("res.label." + id, getLabel, Application.ONE_DAY);

@fsteeg fsteeg assigned dr0i and unassigned fsteeg Sep 26, 2023
@dr0i
Copy link
Member Author

dr0i commented Sep 26, 2023

I don't think this makes sense as it would invoke the loading of the file hundreds of time.
I think play's cache makes sense for caching websites, not files. Or have I missed something?

@dr0i dr0i assigned fsteeg and unassigned dr0i Sep 26, 2023
@fsteeg
Copy link
Member

fsteeg commented Sep 26, 2023

Well it would load the file only once, and after that use the cached version. It's not about a browser cache it that's what you are thinking of. It's a server side cache.

@fsteeg fsteeg assigned dr0i and unassigned fsteeg Sep 26, 2023
@TobiasNx
Copy link
Contributor

@dr0i @fsteeg I am not sure I we should proceed with this PR because we are about to add this info to the record metadata: #1888 and #1906

@dr0i
Copy link
Member Author

dr0i commented Sep 28, 2023

Definitely not :) . See #1868.
Closing.

@dr0i dr0i closed this Sep 28, 2023
@dr0i dr0i deleted the 1893-useSubmodulesDataForIsil2opac_hbzid branch May 27, 2024 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants