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

DR-2874 WIP adding dalli cache for rights #71

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

chrisb87
Copy link
Member

@chrisb87 chrisb87 commented Apr 17, 2024

This is a work-in-progress for adding a RepoAPI response cache (here, for rights calls).

Some problems still to solve:

  • Installing gems: I couldn't find how to install the dalli gem in the container. Perhaps switching to the jruby base image is preferred but I had trouble with that too (the jruby base image was complaining it was built with an older version of java than our cantaloupe jar).
  • Cache key: This branch uses a key that includes the image ID and the IP of the requestor. This works here because the rights call that is cached is cached for every derivative size. Perhaps moving the cache call to the “authorize” method and using other identifiers for the cache key, like “is this IP in full_res_file_access or not" and/or derivative size, is a better way to go.
  • Refactor RepoAPI? When considering where to make the call that is cached, and what cache key to use, we should consider if we actually want to refactor repoAPI and provide a different method that is more convenient for caching here.
  • Setup Amazon elastic cache: my account did not have permissions to experiment with an actual AWS elastic cache instance, so this only used a local memcached with docker-compose
  • Memcached username/password: Set this up for the local dockerized memcached client to show how we get environment variables to authenticate
  • Cache expiration: make a expiration time that is random within a given range?

@chrisb87 chrisb87 added the work in progress Work in progress, do not merge label Apr 17, 2024
@croyfish
Copy link
Contributor

croyfish commented Apr 24, 2024

This is some good exploration. I did notice that the build is failing, and we would need to think about how to best manage the new gems and what specifications we'd require for a new memcached instance.

Is it also worth thinking about if we can move the business logic / rights processing logic entirely into repo so that it could return only a boolean response for any combination of IPs + image_id + derivative_type?

Should we also consider whether Cantaloupe can become aware (via request or direct access?) of the full list of on-prem IP addresses and only send the ips if they are in that list? It would probably prevent a lot of redundant cache keys both in repo and in the new cache being added here.

@chrisb87 chrisb87 force-pushed the DR-2874-rights-cache branch from 22a7170 to 2f7e023 Compare June 11, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work in progress Work in progress, do not merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants