potiuk commented on code in PR #31279: URL: https://github.com/apache/airflow/pull/31279#discussion_r1193562398
########## 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: Yes `>=3.2.0' is best. As long as we are reasonably sure that our tests are covering basic integraiton, not having an upper-binding is strongly preferred - the automated constraint management and upgrading to latest released version (and our canary builds failing in case of problems detected) work best in this case. > I think the existing test suites isn't enough to cover the concerns mentioned in the comment. I'll try to enrich that in this PR To be honest - I am not even sure if that comment is relevant. So **maybe** just checking that it is not relevant is enough. I think it was put there out of abundance of caution. I think our usage of redis in the operators is very limited so just having an actually executed and tested example dag with our current redis would be more than enough to consider this "cool". Another thing we should consider together with that test is to bring redis server in our tests to later version, https://github.com/apache/airflow/blob/main/scripts/ci/docker-compose/integration-celery.yml#LL32C1-L33C1 Redis 5 we have does not get security fixes any more https://endoflife.date/redis - which means EOL. I would consider bumping it to at least 6.2 version. Our integration tests are more of "smoke" tests so we do not comprehensively test integration with multiple versions of the software we run, but theoretically we could even consider running matrix of tests for 6.0, 6.2, 7.0 of redis as well. -- 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]
