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

Add cache for collections #86

Merged
merged 1 commit into from
Apr 30, 2021
Merged

Conversation

mmacata
Copy link
Collaborator

@mmacata mmacata commented Apr 29, 2021

With this PR, the response to /collections is cached directly in the openeo-grassgis-driver without the need for nginx or similar to handle the cache.
The cache will be filled on application startup and can be cleared with /collections?cache=false

Related to #85

@mmacata mmacata requested a review from metzm April 29, 2021 08:39
@mmacata mmacata mentioned this pull request Apr 29, 2021
Copy link
Collaborator

@metzm metzm left a comment

Choose a reason for hiding this comment

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

It seems that the cache is not filled at application startup, but when the endpoint /collections is used for the first time after startup. Is this correct? In this case it would be nice if the collections cache would be already filled on application startup.

@mmacata
Copy link
Collaborator Author

mmacata commented Apr 30, 2021

Yes this is correct. I wanted to avoid a long startup time, because processes are fetched on startup as well. When it takes too long the application never starts but runs in a timeout. The way it is now, the whole application would not be affected and there is more time for collections to be collected. I would prefer to keep it this way and maybe add a persistent file based or redis cache in the future which survives a restart of the application.

@metzm
Copy link
Collaborator

metzm commented Apr 30, 2021

[...] I would prefer to keep it this way and maybe add a persistent file based or redis cache in the future which survives a restart of the application.

Very good idea, makes sense.

Copy link
Collaborator

@metzm metzm left a comment

Choose a reason for hiding this comment

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

Ready for merging IMHO.

@mmacata mmacata merged commit 3e210b4 into Open-EO:master Apr 30, 2021
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.

2 participants