Upgrade redis

[Owen-CH-Leung]

Upgrade redis to 4.5.5. This PR fixes: #29175

passed all the existing 5 tests for the redis provider

breeze testing tests --test-type "Providers[redis]"

---

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

[boring-cyborg]

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:

• Pay attention to the quality of your code (ruff, mypy and type annotations). Our pre-commits will help you with that.
• In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
• Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
• Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
• Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
• Be sure to read the Airflow Coding style.
  Apache Airflow is a community-driven project and together we are making it better 🚀.
  In case of doubts contact the developers at:
  Mailing List: dev@airflow.apache.org
  Slack: https://s.apache.org/airflow-slack

[uranusjr]

Can 3.x still be supported? Also ~=4.5.5 limits the version range to only >=4.5.5,<4.6.0, which is likely not the intention?

[Owen-CH-Leung]

Thanks for your comment. How does redis>=3.2,<4.5.5 sound to you ?

[pankajkoti]

do we wish to have an upper bound? can we not keep just >=3.2
I do not have enough context on this. Do we know if there was there a reason that we were not able to support redis >= 3.3.0 and now we're able to do so? Do we have enough tests in our provider to test the mixins and unquoting URLS with urllib.parse.unquote as mentioned in the comment?

[Owen-CH-Leung]

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

[potiuk]

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.

[Owen-CH-Leung]

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

[potiuk]

Suggested change

	# Upgrading redis package >= 4 doesn't require code changes
	# Changes in Redis 4 only brings into impact when we are directly importing redis.client
	# to execute the commands, which we are not doing.
	# The issue with unquoting URLS with `urllib.parse.unquote` is also not applicable
	# to airflow, because we are not using the from_url function
	# Upgrading redis package >= 4 doesn't require code changes
	# Changes in Redis 4 only brings into impact when we are directly importing redis.client
	# to execute the commands, which we are not doing.
	# The issue with unquoting URLS with `urllib.parse.unquote` is also not applicable
	# to airflow, because we are not using the from_url function

I think what @eladkal ment (and Also what I think) - you can remove the whole comment now and simply make it part of commit message. There is no point in leaving in the code, but the commit message will carry the trace of this being verified.

[eladkal]

yeah sorry for not being clear.
If version is OK no need to further explain why it's OK.
We explain only things that we need to consider for future work

[Owen-CH-Leung]

Thanks all. I removed the comments and make it as part of commit msg. I'm fixing the static check error and the build-doc error that I get from the CI tests. Will let you guys know once they are fixed.

[potiuk]

This is cool! Thanks for being receptive :)

[boring-cyborg]

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.