You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/12/30 20:30:58 UTC
[GitHub] [airflow] amitb2050 opened a new issue, #28655: AIRFLOW__CELERY__BROKER_URL not supporting multiple sentinel url, instead throwing error
amitb2050 opened a new issue, #28655:
URL: https://github.com/apache/airflow/issues/28655
### Apache Airflow version
2.5.0
### What happened
I am trying to set up Airflow Celery to use sentinel. However, we have multiple sentinel instances up and running. So referring to the examples on the internet, I tried to pass multiple sentinel URL as below in ENV VAR.
`AIRFLOW__CELERY__BROKER_URL='sentinel://redis_sentinel1:26379;sentinel://redis_sentinel2:26379'` (tried with/without quote)
After executing, the worker keeps throwing errors as below:
> Traceback (most recent call last):
> File "<string>", line 1, in <module>
> File "/usr/local/lib/python3.8/urllib/parse.py", line 177, in port
> raise ValueError(message) from None
> **ValueError: Port could not be cast to integer value as '26379;sentinel:'**
>
From the error, it looks like the parser is NOT separating sentinel URLs with a semicolon separator. Is there any workaround for it?
### What you think should happen instead
Using a semicolon separator, sentinel URLs should be separated, and the Redis connection pool should make use of all the sentinel URLs.
### How to reproduce
Try to pass multiple sentinel URLs to `AIRFLOW__CELERY__BROKER_URL` environment variables and then try to bring up the airflow workers
### Operating System
ubuntu
### Versions of Apache Airflow Providers
_No response_
### Deployment
Official Apache Airflow Helm Chart
### Deployment details
_No response_
### Anything else
_No response_
### Are you willing to submit PR?
- [ ] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
--
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: commits-unsubscribe@airflow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] dy957 commented on issue #28655: AIRFLOW__CELERY__BROKER_URL not supporting multiple sentinel url, instead throwing error
Posted by "dy957 (via GitHub)" <gi...@apache.org>.
dy957 commented on issue #28655:
URL: https://github.com/apache/airflow/issues/28655#issuecomment-1480373064
> I know this issue is closed, however I believe I have gotten Airflow working with Redis Sentinel. Here are the configuration details:
>
>
>
> ```bash
>
> # airflow config for celery using redis sentinel
>
>
>
> # note the broker URL password is the redis password, and the redis DB is 0
>
> AIRFLOW__CELERY__BROKER_URL=sentinel://:str0ng_passw0rd@redis-sentinel1:26379/0;sentinel://:str0ng_passw0rd@redis-sentinel2:26379/0;sentinel://:str0ng_passw0rd@redis-sentinel3:26379/0
>
> AIRFLOW__CELERY_BROKER_TRANSPORT_OPTIONS__MASTER_NAME=mymaster
>
> # note this is the password for sentinel
>
> AIRFLOW__CELERY_BROKER_TRANSPORT_OPTIONS__SENTINEL_KWARGS={"password":"str0ng_passw0rd"}
>
> AIRFLOW__CELERY__CELERY_CONFIG_OPTIONS=config.celery_config.CELERY_CONFIG
>
> ```
>
>
>
> The broker transport options are put into celery config using `conf.getsection("celery_broker_transport_options")` which does not load json strings as dicts. Celery expects the `sentinel_kwargs` to be a dict, so I've added this python file to update the broker transport options:
>
>
>
> ```python
>
> # /usr/local/airflow/config/celery_config.py
>
> import json
>
>
>
> from airflow.config_templates.default_celery import DEFAULT_CELERY_CONFIG
>
>
>
> CELERY_CONFIG = DEFAULT_CELERY_CONFIG
>
> if "sentinel_kwargs" in CELERY_CONFIG.get("broker_transport_options", {}):
>
> CELERY_CONFIG["broker_transport_options"]["sentinel_kwargs"] = json.loads(
>
> CELERY_CONFIG["broker_transport_options"]["sentinel_kwargs"]
>
> )
>
> ```
>
>
>
> I've only tested this locally. However, I believe this would work anywhere.
It works for me, thanks
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] amitb2050 commented on issue #28655: AIRFLOW__CELERY__BROKER_URL not supporting multiple sentinel url, instead throwing error
Posted by GitBox <gi...@apache.org>.
amitb2050 commented on issue #28655:
URL: https://github.com/apache/airflow/issues/28655#issuecomment-1370085595
Hi, @potiuk Thanks for your response. I went through issue #28010 which concerns why Sentinel doesn't work with TLS setup. However, this issue is related to the error we get when passing multiple sentinel URLs.
Expectation: Multiple sentinel URLs should be accepted per Celery [documentation](https://docs.celeryq.dev/en/stable/getting-started/backends-and-brokers/redis.html) .
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #28655: AIRFLOW__CELERY__BROKER_URL not supporting multiple sentinel url, instead throwing error
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on issue #28655:
URL: https://github.com/apache/airflow/issues/28655#issuecomment-1458885580
@dintorf - Might be a good idea to contribute that as an example / special case description here: https://airflow.apache.org/docs/apache-airflow/stable/core-concepts/executor/celery.html ?
Just click "Suggest a change on this page"
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] amitb2050 commented on issue #28655: AIRFLOW__CELERY__BROKER_URL not supporting multiple sentinel url, instead throwing error
Posted by GitBox <gi...@apache.org>.
amitb2050 commented on issue #28655:
URL: https://github.com/apache/airflow/issues/28655#issuecomment-1370100072
Thanks, @potiuk for your quick response. Let me study the existing functionality of Airflow CeleryExecutor and see if I can make any modifications to get this working.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] dintorf commented on issue #28655: AIRFLOW__CELERY__BROKER_URL not supporting multiple sentinel url, instead throwing error
Posted by "dintorf (via GitHub)" <gi...@apache.org>.
dintorf commented on issue #28655:
URL: https://github.com/apache/airflow/issues/28655#issuecomment-1458723703
I know this issue is closed, however I believe I have gotten Airflow working with Redis Sentinel. Here are the configuration details:
```bash
# airflow config for celery using redis sentinel
# note the broker URL password is the redis password, and the redis DB is 0
AIRFLOW__CELERY__BROKER_URL=sentinel://:str0ng_passw0rd@redis-sentinel1:26379/0;sentinel://:str0ng_passw0rd@redis-sentinel2:26379/0;sentinel://:str0ng_passw0rd@redis-sentinel3:26379/0
AIRFLOW__CELERY_BROKER_TRANSPORT_OPTIONS__MASTER_NAME=mymaster
# note this is the password for sentinel
AIRFLOW__CELERY_BROKER_TRANSPORT_OPTIONS__SENTINEL_KWARGS={"password":"str0ng_passw0rd"}
AIRFLOW__CELERY__CELERY_CONFIG_OPTIONS=config.celery_config.CELERY_CONFIG
```
The broker transport options are put into celery config using `conf.getsection("celery_broker_transport_options")` which does not load json strings as dicts. Celery expects the `sentinel_kwargs` to be a dict, so I've added this python file to update the broker transport options:
```python
# /usr/local/airflow/config/celery_config.py
import json
from airflow.config_templates.default_celery import DEFAULT_CELERY_CONFIG
CELERY_CONFIG = DEFAULT_CELERY_CONFIG
if "sentinel_kwargs" in CELERY_CONFIG.get("broker_transport_options", {}):
CELERY_CONFIG["broker_transport_options"]["sentinel_kwargs"] = json.loads(
CELERY_CONFIG["broker_transport_options"]["sentinel_kwargs"]
)
```
I've only tested this locally. However, I believe this would work anywhere.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on issue #28655: AIRFLOW__CELERY__BROKER_URL not supporting multiple sentinel url, instead throwing error
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on issue #28655:
URL: https://github.com/apache/airflow/issues/28655#issuecomment-1368081512
Thanks for opening your first issue here! Be sure to follow the issue template!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #28655: AIRFLOW__CELERY__BROKER_URL not supporting multiple sentinel url, instead throwing error
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #28655:
URL: https://github.com/apache/airflow/issues/28655#issuecomment-1370092465
> Hi, @potiuk Thanks for your response. I went through issue https://github.com/apache/airflow/issues/28010 which concerns why Sentinel doesn't work with TLS setup. However, this issue is related to the error we get when passing multiple sentinel URLs.
> Expectation: Multiple sentinel URLs should be accepted per Celery [documentation](https://docs.celeryq.dev/en/stable/getting-started/backends-and-brokers/redis.html) .
Yes - and as you can find in https://github.com/apache/airflow/issues/28010 - they seem to be accepted for the user there. Just SSL configuration seems to be not working. In any way this means that Celery Sentinel (for whatever reason) is not working wiht current Celer approach of Ariflow. I would appreciate if you and other users of Celery could help to investigate why and possibly helped to solve it together. This is the fastest way you can help and give back to the community for the free software you get - you seem to be technical enough and have celery sentinel to test it with and incentive to get it working. PRs fixing that are most welcome. And it is best to keep it in a single open issue (linked to that one so that one is not lost and you can see it refers to the same case).
In short:
* Yes, it seems there are issues with sentinel
* It's better to consolidate it in one issue
* Since this is a niche case, that might become impotant, as a community we'd appreciate those more technology savvy users who want to use sentinel to help with diagnosing and solving it.
Any help there is much appreciated.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #28655: AIRFLOW__CELERY__BROKER_URL not supporting multiple sentinel url, instead throwing error
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #28655:
URL: https://github.com/apache/airflow/issues/28655#issuecomment-1368126959
Basically a duplicate of #28010 - seems that sentinel does not play well with Airflow and the reason for it is unknown. Would be great if someone who is more familiar with it could take a look.
I do not think there is a workaround currently but if sufficient number of those who can try and test it could take a loo at this together, maybe they can figure it out, so better discuss it in one issue.
Closing as duplicate of #28010
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk closed issue #28655: AIRFLOW__CELERY__BROKER_URL not supporting multiple sentinel url, instead throwing error
Posted by GitBox <gi...@apache.org>.
potiuk closed issue #28655: AIRFLOW__CELERY__BROKER_URL not supporting multiple sentinel url, instead throwing error
URL: https://github.com/apache/airflow/issues/28655
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #28655: AIRFLOW__CELERY__BROKER_URL not supporting multiple sentinel url, instead throwing error
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #28655:
URL: https://github.com/apache/airflow/issues/28655#issuecomment-1370094541
BTW. This is how OSS development works. Airflow has > 2300 contributors - and vast majority of them are users fixing the problems they deeply care about. You can become one of them if you help to fix/diagnose it and your PR fixing it gets merged. This is very diferent from commercial software.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org