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

Abstract caching and support entrypoints #876

Merged
merged 4 commits into from
Jun 17, 2022
Merged

Abstract caching and support entrypoints #876

merged 4 commits into from
Jun 17, 2022

Conversation

banesullivan
Copy link
Contributor

@banesullivan banesullivan commented Jun 16, 2022

Abstracts the caching mechanism so that external projects can implement a cache interface and use entrypoints to make it available to large_image.

Generally, this is working for django-large-image with girder/django-large-image#32

Todo:

  • Can the MemCache cache be chosen appropriately? Is it ignored when not available and is it chosen over python when available?
  • Should loadCaches() only be called once rather than on every CacheFactory.getCache() call?
  • Address inline TODO statements
  • What is CacheFactory.getCacheSize()and pickAvailableCache() and how are they affected by these changes?
  • What happens if BaseCache.getCache() returns a None cache lock? can/should we support that?
  • Test downstream in django-large-image

@banesullivan banesullivan marked this pull request as ready for review June 16, 2022 23:10
@banesullivan
Copy link
Contributor Author

@manthey, this seems to be working and is tested downstream.I'm ready for a final review if you have time

@banesullivan banesullivan requested a review from manthey June 16, 2022 23:20
@banesullivan
Copy link
Contributor Author

banesullivan commented Jun 16, 2022

Can we also tag 1.14.6 (or should this be 1.15 since there is a new interface) after this merges?

@manthey
Copy link
Member

manthey commented Jun 17, 2022

Excellent.

@manthey
Copy link
Member

manthey commented Jun 17, 2022

What happens if BaseCache.getCache() returns a None cache lock? can/should we support that?

That should work correctly.

@manthey manthey merged commit 58fb8a2 into master Jun 17, 2022
@manthey manthey deleted the caches branch June 17, 2022 12:41
@manthey
Copy link
Member

manthey commented Jun 17, 2022

Can we also tag 1.14.6 (or should this be 1.15 since there is a new interface) after this merges?

It will be 1.15.0 since I think this is enough of a new interface.

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