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

Cached statistics and all ontologies for ontologies tab and cleaned u… #700

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

henrietteharmse
Copy link
Collaborator

…p logging on backend.

@haideriqbal
Copy link
Collaborator

Hi @henrietteharmse ! I like the idea of caching all ontologies endpoints as it doesn't change during our application lifecycle. However, if I understand it correctly, the current implementation of the endpoint doesn't technically cache the result unless I'm missing something. The new controller will still go to solr to retrieve the ontologies every time it's called.

I suggest we use Spring Cache which is straightforward to set up and can be helpful in our use case. Have a look at https://www.baeldung.com/spring-cache-tutorial

@henrietteharmse
Copy link
Collaborator Author

henrietteharmse commented Jul 15, 2024

The old endpoint is not caching it. I did look at the different Spring Cache solutions. However, we do not need all the functionality these solutions include and thus it is an overkill.

@henrietteharmse
Copy link
Collaborator Author

The new controller will still go to solr to retrieve the ontologies every time it's called.

That is not true. It only calls solr once and never again.

Copy link
Collaborator

@haideriqbal haideriqbal left a comment

Choose a reason for hiding this comment

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

LGTM!

@haideriqbal
Copy link
Collaborator

The new controller will still go to solr to retrieve the ontologies every time it's called.

That is not true. It only calls solr once and never again.

yup! i missed the variable bit here

@@ -252,20 +252,14 @@ export const getOntologies = createAsyncThunk(
export const getAllOntologies = createAsyncThunk(
"ontologies_all",
async (_, { rejectWithValue }) => {
const path = `api/v2/ontologies?size=1`;
const allOntologiesPath = `api/v2/cache/ontologies`;
Copy link
Member

Choose a reason for hiding this comment

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

Is there any reason we can't just make the cached endpoint the only endpoint instead of having a separate API route?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants