-
Notifications
You must be signed in to change notification settings - Fork 20
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
(PXP-10739): Add bucket region to DRS #355
base: master
Are you sure you want to change the base?
Conversation
The style in this PR agrees with This formatting comment was generated automatically by a script in uc-cdis/wool. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This caching strategy isn't super useful when there are many instances of the service running (since all the caches are in-memory and specific to that pod). Since we expect many instances of indexd to be running, I would suggest we look into a more centralized caching solution similar to what's been done in Fence (look for how we dealt with caching creds during signed URL calls or even some of RAS stuff maybe). The idea being: we need a central place for newly spun up instances of indexd to get the cached information. Naive solution (and what we did in Fence) was have a new database table to store the central cache AND in-mem cache in the instances
indexd/index/drivers/alchemy.py
Outdated
bucket_region_info = response.json() | ||
cache.set("bucket_region_info", response.json(), timeout=3600) | ||
else: | ||
print( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the logger for this.
indexd/index/drivers/alchemy.py
Outdated
) | ||
break | ||
else: | ||
print( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the logger.
tests/test_aliases_endpoints.py
Outdated
@@ -1,3 +1,5 @@ | |||
import os |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused import
indexd/app.py
Outdated
@@ -3,6 +3,7 @@ | |||
|
|||
from alembic.config import main as alembic_main | |||
import cdislogging | |||
from distutils.command.config import config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you still need this config here?
Jira Ticket: PXP-xxxx
New Features
Breaking Changes
Bug Fixes
Improvements
Dependency updates
Deployment changes