-
Notifications
You must be signed in to change notification settings - Fork 10
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
Make redis and fakeredis optional dependencies #57
base: dev
Are you sure you want to change the base?
Conversation
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.
to avoid having exception raised we should do something like https://github.com/developmentseed/cogeo-mosaic/blob/main/cogeo_mosaic/backends/az.py#L17-L24 in
https://github.com/developmentseed/titiler-xarray/blob/dev/titiler/xarray/redis_pool.py#L4-L5
and then https://github.com/developmentseed/cogeo-mosaic/blob/main/cogeo_mosaic/backends/az.py#L40-L43 in the class methods
Sure @vincentsarago I'll make that change in a bit. Also changing accordingly the infra code, will ping you. Thanks ! |
d68c30b
to
7e44466
Compare
1b01a88
to
546e089
Compare
if settings.enable_cache: | ||
titiler_env.update( | ||
{"TITILER_XARRAY_CACHE_HOST": redis_cluster.attr_redis_endpoint_address} | ||
) |
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.
can we move this block up, so we don't have 2 settings.enable_cache
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.
not-mandatory if you want to merge
@emileten do we have anything to do here before merging? |
Partially addresses package size issue for lambda : #56