pallets-eco / flask-caching

A caching extension for Flask
https://flask-caching.readthedocs.io
Other
893 stars 193 forks source link

CACHE_OPTIONS has no effect if CACHE_REDIS_URL is set #443

Open dpgaspar opened 1 year ago

dpgaspar commented 1 year ago

Follow up for: https://github.com/pallets-eco/flask-caching/issues/285

If not mistaken the fixed mentioned on the above issue is for Redis sentinel only

Setting the following config:

CACHE_CONFIG = {
    'CACHE_TYPE': 'redis',
    'CACHE_DEFAULT_TIMEOUT': '60*60',  # 1 hour for development
    'CACHE_REDIS_URL': "redis://cache1.us1a.zone:6379/0",
    'CACHE_OPTIONS': {
        'socket_timeout': 1,
        'socket_connect_timeout': 2,
    }
}

it works if we set: 'CACHE_REDIS_URL': "redis://cache1.us1a.zone:6379/0?socket_timeout=1&socket_connect_timeout=2",

on: https://github.com/pallets-eco/flask-caching/blob/master/src/flask_caching/backends/rediscache.py#L86

if we change:

        if redis_url:
            kwargs["host"] = redis_from_url(
                redis_url,
                db=kwargs.pop("db", None),
            )

to:

        if redis_url:
            kwargs["host"] = redis_from_url(
                redis_url,
                db=kwargs.pop("db", None),
                socket_timeout=kwargs.pop("socket_timeout", None),
                socket_connect_timeout=kwargs.pop("socket_connect_timeout", None),
                socket_keepalive=kwargs.pop("socket_keepalive", None),
                socket_keepalive_options=kwargs.pop("socket_keepalive_options", None),
                max_connections=kwargs.pop("max_connections", None),
                username=kwargs.pop("username", None),
                retry=kwargs.pop("retry", None),
                retry_on_timeout=kwargs.pop("retry_on_timeout", False),
                retry_on_error=kwargs.pop("retry_on_error", None),
                redis_connect_func=kwargs.pop("redis_connect_func", None),
            )

works! Not a great solution but we can't pass unknown kwargs, we have to be explicit, this will increase flask-caching dependency with redis-py versions

Happy to open a PR

Environment:

JonathanBrenner commented 1 year ago

Seconding this request. Similarly I am trying to pass in ssl_cert_reqs as a CACHE_OPTION and can see that it is not being passed to redis.

"CACHE_OPTIONS": {
    "ssl": True,
    "ssl_cert_reqs": "none",
},

@dpgaspar To confirm, adding the kwargs as query parameters to your CACHE_REDIS_URL is a workaround that fixed the problem for you?

foarsitter commented 1 month ago

@dpgaspar @JonathanBrenner can you verify that #591 fixes this issue?

JonathanBrenner commented 1 month ago

@foarsitter I no longer have access to the code base with this issue since I changed jobs. Thanks for working on a fix! Hopefully @dpgaspar can verify