Owen-CH-Leung commented on code in PR #31279: URL: https://github.com/apache/airflow/pull/31279#discussion_r1194523623
########## airflow/providers/redis/provider.yaml: ########## @@ -39,7 +39,7 @@ dependencies: # as well as unquoting URLS with `urllib.parse.unquote`: # https://github.com/redis/redis-py/blob/master/CHANGES # TODO: upgrade to support redis package >=4 - - redis~=3.2 + - redis~=4.5.5 Review Comment: After some digging, I can confirm that the use of redis 4 wouldn't break the existing code base and should be safe to upgrade. The fact that it passes the celery integration test is a hint that it should be ok. For the concerns mentioned in the comment: - **mixins in redis commands** : This has no impact to airflow since `RedisHook` didn't directly use `redis.client`. If we are using `redis.client`, we'd need to switch to `redis.command` - **unquoting URLS with `urllib.parse.unquote`** : This has no impact to airflow as well since we are not using the `from_url` function I've just revised the doc and added a example dag to illustrate the use of the existing 3 redis Operator / sensor. I ran pytest in breeze environment & can confirm that the DAG is good to go -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
