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

WIP feat(redis): redis client and caching logic #264

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

Avantol13
Copy link
Contributor

⚠️ Work in progress ⚠️

  • Describe what this pull request does.
  • Add short descriptive bullet points for each section if relevant. Keep in mind that they will be parsed automatically to generate official release notes.
  • Test manually.
  • Maintain or increase the test coverage (if relevant).
  • Update the documentation, or justify if not needed.

-->

New Features

Breaking Changes

Bug Fixes

Improvements

Dependency updates

Deployment changes

@github-actions
Copy link

The style in this PR agrees with black. ✔️

This formatting comment was generated automatically by a script in uc-cdis/wool.

Comment on lines +223 to +228
# check redis
json_record = flask.current_app.redis_client.get(record)
if json_record:
json_record = json.loads(json_record)
else:
# get from db
Copy link
Contributor

Choose a reason for hiding this comment

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

in fence i do: check redis; if not in redis: call indexd so i think we could skip checking redis again here. 3 cases:

  • request to indexd from another service: we can assume the other service already checked the cache
  • request from a user: now that the request reached indexd, it's too late to improve performance
    • the record is in redis: getting from redis instead of the DB doesn't improve performance (i think)
    • it's not in redis: performance is decreased since indexd checks both

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.

None yet

3 participants