Include rediss to the list of supported URL schemes #31028

vlazuka · 2023-05-02T20:11:42Z

This pull request fixes: #31027

…rl connections

boring-cyborg · 2023-05-02T20:11:45Z

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

vlazuka · 2023-05-02T22:05:04Z

Tagging @ephraimbuddy @pierrejeambrun @josh-fell

jedcunningham

There are some other checks against redis:// in that same file. Can you do the same for the rest of them?

airflow/config_templates/default_celery.py

vlazuka

changes are added

airflow/config_templates/default_celery.py

vlazuka · 2023-05-05T22:23:08Z

Tagging @uranusjr @josh-fell @jedcunningham @rozza @hussein-awala @potiuk to make it reviewed faster

pankajkoti

Did you get a chance to test this E2E with both redis and rediss? If yes, will it possible to share some logs/screenshot of the working solution.

airflow/config_templates/default_celery.py

vlazuka · 2023-05-12T21:22:08Z

@pankajkoti

It was like this:

And the error went away:

Referring to this Issue I created before

uranusjr

Looks fine as far as I can tell

hussein-awala

LGTM

airflow/config_templates/default_celery.py

hussein-awala · 2023-05-15T05:01:40Z

Can you install pre-commit to fix static checks (doc) and fix the failed tests?

vlazuka · 2023-05-25T23:51:43Z

It seems I can't merge without @jedcunningham because you requested changes and you need to review them know

stale

hussein-awala · 2023-05-26T05:14:00Z

Actually the PR is blocked by the failed CI, you need to fix the static checks as I explained before (doc)

uranusjr · 2023-05-26T07:40:54Z

airflow/config_templates/default_celery.py

-    result_backend = f'db+{conf.get("database", "SQL_ALCHEMY_CONN")}'
+    result_backend = "db+{}".format(conf.get("database", "SQL_ALCHEMY_CONN"))


Why change this?

airflow/config_templates/default_celery.py

vlazuka · 2023-05-26T20:29:43Z

Hello @hussein-awala,

Thank you for pointing at this. CI allows to merge the pr though (hm).
Actually the failure is from the pr 30352 by @ORuteMa

TBH I don't understand why we create a new variable broker_transport_options_for_celery there

@uranusjr take a look, please

Thanks

airflow/config_templates/default_celery.py

uranusjr · 2023-05-29T07:32:07Z

I added some changes, this should be enough to convince Mypy.

ORuteMa · 2023-05-29T15:08:16Z

Hello @hussein-awala,

Thank you for pointing at this. CI allows to merge the pr though (hm). Actually the failure is from the pr 30352 by @ORuteMa

TBH I don't understand why we create a new variable broker_transport_options_for_celery there

@uranusjr take a look, please

Thanks

Hello, it used in the situation that you have sentinel and redis server both password-authed.

vlazuka · 2023-06-07T17:03:28Z

When is this going to be merged and released?

boring-cyborg · 2023-06-07T17:09:49Z

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

--------- Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com> (cherry picked from commit 471fdac)

Update default_celery.py to include rediss to the list of supported u…

344369d

…rl connections

vlazuka mentioned this pull request May 2, 2023

Airflow doesn't recognize rediss:... url to point to a Redis broker #31027

Closed

2 tasks

jedcunningham previously requested changes May 2, 2023

View reviewed changes

josh-fell reviewed May 3, 2023

View reviewed changes

airflow/config_templates/default_celery.py Outdated Show resolved Hide resolved

Update default_celery.py

bafbe2e

vlazuka requested review from jedcunningham and josh-fell May 3, 2023 09:22

Update default_celery.py

b76565a

uranusjr reviewed May 3, 2023

View reviewed changes

airflow/config_templates/default_celery.py Outdated Show resolved Hide resolved

Update default_celery.py

13541de

vlazuka requested a review from uranusjr May 3, 2023 19:56

Merge branch 'main' into patch-1

8063582

uranusjr changed the title ~~Update default_celery.py to include rediss to the list of supported u…~~ Include rediss to the list of supported URL schemes May 3, 2023

Merge branch 'main' into patch-1

63c93e4

vlazuka commented May 4, 2023

View reviewed changes

airflow/config_templates/default_celery.py Outdated Show resolved Hide resolved

vlazuka added 2 commits May 4, 2023 15:54

Merge branch 'main' into patch-1

e6e44b4

Merge branch 'main' into patch-1

0121521

pankajkoti reviewed May 7, 2023

View reviewed changes

uranusjr reviewed May 7, 2023

View reviewed changes

airflow/config_templates/default_celery.py Outdated Show resolved Hide resolved

vlazuka added 3 commits May 12, 2023 10:29

Merge branch 'main' into patch-1

815311e

typo fixed

6e81217

Merge branch 'main' into patch-1

1486557

uranusjr approved these changes May 15, 2023

View reviewed changes

hussein-awala approved these changes May 15, 2023

View reviewed changes

airflow/config_templates/default_celery.py Show resolved Hide resolved

Merge branch 'main' into patch-1

cb9b2c6

vlazuka and others added 3 commits May 25, 2023 15:07

Merge branch 'main' into patch-1

dc78cef

code from sentinel changes is fixed + typo fixes

c812664

Merge branch 'main' into patch-1

4b87b60

vlazuka requested a review from hussein-awala May 25, 2023 23:54

uranusjr reviewed May 26, 2023

View reviewed changes

airflow/config_templates/default_celery.py Outdated Show resolved Hide resolved

vlazuka and others added 3 commits May 26, 2023 10:54

Merge branch 'main' into patch-1

f57b2f8

Merge branch 'main' into patch-1

0104dbc

bringing back f-string and fixing sentinel

b8c4426

uranusjr reviewed May 29, 2023

View reviewed changes

airflow/config_templates/default_celery.py Outdated Show resolved Hide resolved

Add back annotation to resolve Mypy error

662689d

uranusjr reviewed May 29, 2023

View reviewed changes

airflow/config_templates/default_celery.py Outdated Show resolved Hide resolved

MInimize diff

08164b6

uranusjr approved these changes May 29, 2023

View reviewed changes

Merge branch 'main' into patch-1

a70c66f

potiuk merged commit 471fdac into apache:main Jun 7, 2023

eladkal added this to the Airflow 2.6.2 milestone Jun 8, 2023

eladkal added the type:bug-fix Changelog: Bug Fixes label Jun 8, 2023

eladkal pushed a commit that referenced this pull request Jun 8, 2023

Include rediss to the list of supported URL schemes (#31028)

71bcb66

--------- Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com> (cherry picked from commit 471fdac)

eladkal mentioned this pull request Jun 13, 2023

Status of testing of Apache Airflow 2.6.2rc2 #31867

Closed

62 tasks

		result_backend = f'db+{conf.get("database", "SQL_ALCHEMY_CONN")}'
		result_backend = "db+{}".format(conf.get("database", "SQL_ALCHEMY_CONN"))

Include rediss to the list of supported URL schemes #31028

Include rediss to the list of supported URL schemes #31028

Uh oh!

Conversation

vlazuka commented May 2, 2023 • edited by eladkal Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

boring-cyborg bot commented May 2, 2023

Uh oh!

vlazuka commented May 2, 2023

Uh oh!

jedcunningham left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vlazuka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vlazuka commented May 5, 2023

Uh oh!

pankajkoti left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vlazuka commented May 12, 2023

Uh oh!

uranusjr left a comment

Choose a reason for hiding this comment

Uh oh!

hussein-awala left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hussein-awala commented May 15, 2023

Uh oh!

vlazuka commented May 25, 2023

Uh oh!

hussein-awala commented May 26, 2023

Uh oh!

uranusjr May 26, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vlazuka commented May 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

uranusjr commented May 29, 2023

Uh oh!

ORuteMa commented May 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vlazuka commented Jun 7, 2023

Uh oh!

boring-cyborg bot commented Jun 7, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

vlazuka commented May 2, 2023 •

edited by eladkal

Loading

vlazuka commented May 26, 2023 •

edited

Loading

ORuteMa commented May 29, 2023 •

edited

Loading