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/05/30 09:47:13 UTC
[GitHub] [airflow] tsugumi-sys opened a new pull request, #24019: Add celery broker configuration example using Amazon SQS
tsugumi-sys opened a new pull request, #24019:
URL: https://github.com/apache/airflow/pull/24019
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
Added because there is no clear configuration example using Amazon SQS as Celery broker #22863.
Note: Dictionary cannot be used in airflow.cfg because [configparser](https://docs.python.org/ja/3/library/configparser.html) cannot parse it ?. So you cannot set [`celery_broker_transport_options`](https://airflow.apache.org/docs/apache-airflow/stable/configurations-ref.html#celery-broker-transport-options) like this.
```ini
[celery_broker_transport_options]
region = ap-northeast-1
predefined_queues = {
"my_q": {
"url": "sqs.ap-northeast-1.amazonaws.com/119814312462/akiranoda-test-sqs-for-airflow",
"access_key_id": "AKIARXZL5KIHBNDHB42J",
"secret_access_key": "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",
}
}
```
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragement file, named `{pr_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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] jedcunningham commented on pull request #24019: Add celery broker configuration example using Amazon SQS
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on PR #24019:
URL: https://github.com/apache/airflow/pull/24019#issuecomment-1195860344
I feel like the situation here is slightly different as it is [officially supported by Celery](https://docs.celeryq.dev/en/stable/getting-started/backends-and-brokers/index.html#broker-overview), and presumably properly tested there. I'm not sure we want to exhaustively test our dependencies. That said, this is a pretty critical piece of the puzzle 🤷
--
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] ferruzzi commented on pull request #24019: Add celery broker configuration example using Amazon SQS
Posted by GitBox <gi...@apache.org>.
ferruzzi commented on PR #24019:
URL: https://github.com/apache/airflow/pull/24019#issuecomment-1194510320
I think it's perfectly reasonable to not want it in the docs if we can't back up the claim with testing.
--
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 pull request #24019: Add celery broker configuration example using Amazon SQS
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #24019:
URL: https://github.com/apache/airflow/pull/24019#issuecomment-1221634790
That comment from @o-nikolas make me think we are NOT ready to make it as an official part of our documentation. But as a blog post was mentioned several times - if either of you would like to write it, I think we can publish it in https://medium.com/apache-airflow
--
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] o-nikolas commented on pull request #24019: Add celery broker configuration example using Amazon SQS
Posted by GitBox <gi...@apache.org>.
o-nikolas commented on PR #24019:
URL: https://github.com/apache/airflow/pull/24019#issuecomment-1211255767
> > We had reports that `predefined_queues` isn't working properly with SQS #11225 (though it's on older Airflow versions) Can you confirm it's working as it should?
>
> I also checked `predefined_queues` option and it didn't work in my local environment. This option is not used in my commit.
Predefined queues, or any other broker transport options can be configured by a plugin to get around the string vs object discrepancy.
--
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 pull request #24019: Add celery broker configuration example using Amazon SQS
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #24019:
URL: https://github.com/apache/airflow/pull/24019#issuecomment-1221635026
I think we can close it now. We can always reopen if we think it is worthwile, but I'd bee rather sceptical on supporting SQS knowing the caveats.
--
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 pull request #24019: Add celery broker configuration example using Amazon SQS
Posted by GitBox <gi...@apache.org>.
potiuk closed pull request #24019: Add celery broker configuration example using Amazon SQS
URL: https://github.com/apache/airflow/pull/24019
--
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 pull request #24019: Add celery broker configuration example using Amazon SQS
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #24019:
URL: https://github.com/apache/airflow/pull/24019#issuecomment-1203107222
FYI. @jedcunningham - this is the kind of problems we might expect https://github.com/apache/airflow/discussions/25486 . I feel rather uncomfortable not having any tests around it. I am not sure even how we would approach diagnosing and fixing similar issues if we do not have any tests like that. There might be many reasons for this kind of failures (celery/sqs incompatibilities, our way of integrating with celery etc.
--
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] tsugumi-sys commented on a diff in pull request #24019: Add celery broker configuration example using Amazon SQS
Posted by GitBox <gi...@apache.org>.
tsugumi-sys commented on code in PR #24019:
URL: https://github.com/apache/airflow/pull/24019#discussion_r912421111
##########
docs/apache-airflow/executor/celery.rst:
##########
@@ -26,6 +26,17 @@ to work, you need to setup a Celery backend (**RabbitMQ**, **Redis**, ...) and
change your ``airflow.cfg`` to point the executor parameter to
``CeleryExecutor`` and provide the related Celery settings.
+If you use Amazon SQS as a Celery broker, set environment variables like this:
+
+.. code-block:: bash
+
+ export AIRFLOW__CELERY__BROKER_URL=sqs://@your_sqs_host
+
+ # Celery's environment variables.
+ export AWS_ACCESS_KEY_ID=your_aws_access_key_id
+ export AWS_SECRET_ACCESS_KEY=your_aws_secret_access_key
+ export AWS_DEFAULT_REGION=your_aws_region
Review Comment:
I add environment variable `AIRFLOW__CELERY__WORKER_ENABLE_REMOTE_CONTROL=true` but new queues are not created.
--
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 pull request #24019: Add celery broker configuration example using Amazon SQS
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #24019:
URL: https://github.com/apache/airflow/pull/24019#issuecomment-1198459759
> I feel like the situation here is slightly different as it is [officially supported by Celery](https://docs.celeryq.dev/en/stable/getting-started/backends-and-brokers/index.html#broker-overview), and presumably properly tested there. I'm not sure we want to exhaustively test our dependencies. That said, this is a pretty critical piece of the puzzle shrug
Actually that might change my view. I did not realize SQS is on-par with RabbitMQ/Redis. On the other Hand MAYBE this is a GREAT opportunity to add a test or two using Moto ?
https://qxf2.com/blog/mocking-out-aws-sqs-using-moto/
Then we would be completely covered and it shoudl be no problem to make it official. @ferruzzi @o-nikolas @vincbeck ? WDYT>
--
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 pull request #24019: Add celery broker configuration example using Amazon SQS
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #24019:
URL: https://github.com/apache/airflow/pull/24019#issuecomment-1194108239
Hmm. I thought a bit about it. And following the dicsusion in https://github.com/apache/airflow/pull/25121 - I think we should not add "offficial documenation" to features that our users tested, but we have no automated tests for and do not know ourselves what might be potential risks, problems etc. I think this falls in the same camp.
@BasPH @jedcunningham @feluelle - WDYT?
I think @tsugumi-sys that a better approach would be that you (for example) write a short blog post about using SQS as a broker, and describe your experiences it - and then we can even add it to "Airlfow" publication on Medium and even link it from the Ecosystem page of Airflow. https://airflow.apache.org/ecosystem/
I think adding it to the official docs in experimental phase when we do not have any tests (nor intention to add them) is IMHO something we should not do - even if we add "Experimental" - because "experimental" means that we are planning to "run" some experiment and turn it into a "full feature" (or drop) when we finish the experiment. This is what happens with:
* MsSQL support as meta-datada DB
* ARM64 support for Docker Image.
In both cases we already have tests in CI and we actively plan to act and gather observations/. For example I think we might beinclined to drop MsSQL if the Survey results will not show significant uptake in MsSQL usage after it's been officially "experimentally" supported in 2.3 https://airflow.apache.org/blog/airflow-survey-2022/ shows a very little usage so far.
Adding information that SQS "works" in the official docs is some kind of statement we make to our users and they might have certain expectations if they follow it.
I think I'd be only inclined to add "experimental" SQS support if we have some tests in our CI making sure that it works (BTW. We have such - simple but regularly running tests for RabbitMQ and Redis).
Others - WDYT?
--
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] github-actions[bot] commented on pull request #24019: Add celery broker configuration example using Amazon SQS
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #24019:
URL: https://github.com/apache/airflow/pull/24019#issuecomment-1141607606
The PR is likely ready to be merged. No tests are needed as no important environment files, nor python files were modified by it. However, committers might decide that full test matrix is needed and add the 'full tests needed' label. Then you should rebase it to the latest main or amend the last commit of the PR, and push it with --force-with-lease.
--
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 pull request #24019: Add celery broker configuration example using Amazon SQS
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on PR #24019:
URL: https://github.com/apache/airflow/pull/24019#issuecomment-1140943154
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 (flake8, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks) 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](https://github.com/apache/airflow/blob/main/docs/apache-airflow/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/BREEZE.rst) 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](https://www.apache.org/foundation/policies/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]( https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#coding-style-and-best-practices).
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
--
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] eladkal commented on pull request #24019: Add celery broker configuration example using Amazon SQS
Posted by GitBox <gi...@apache.org>.
eladkal commented on PR #24019:
URL: https://github.com/apache/airflow/pull/24019#issuecomment-1140991146
We had reports that `predefined_queues` isn't working properly https://github.com/apache/airflow/issues/11225 (though it's on older Airflow versions)
Can you confirm it's working as it should?
--
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] tsugumi-sys commented on a diff in pull request #24019: Add celery broker configuration example using Amazon SQS
Posted by GitBox <gi...@apache.org>.
tsugumi-sys commented on code in PR #24019:
URL: https://github.com/apache/airflow/pull/24019#discussion_r912421111
##########
docs/apache-airflow/executor/celery.rst:
##########
@@ -26,6 +26,17 @@ to work, you need to setup a Celery backend (**RabbitMQ**, **Redis**, ...) and
change your ``airflow.cfg`` to point the executor parameter to
``CeleryExecutor`` and provide the related Celery settings.
+If you use Amazon SQS as a Celery broker, set environment variables like this:
+
+.. code-block:: bash
+
+ export AIRFLOW__CELERY__BROKER_URL=sqs://@your_sqs_host
+
+ # Celery's environment variables.
+ export AWS_ACCESS_KEY_ID=your_aws_access_key_id
+ export AWS_SECRET_ACCESS_KEY=your_aws_secret_access_key
+ export AWS_DEFAULT_REGION=your_aws_region
Review Comment:
I turn the `AIRFLOW__CELERY__WORKER_ENABLE_REMOTE_CONTROL` True and False but new queues are not created...
--
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 a diff in pull request #24019: Add celery broker configuration example using Amazon SQS
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #24019:
URL: https://github.com/apache/airflow/pull/24019#discussion_r914833032
##########
docs/apache-airflow/executor/celery.rst:
##########
@@ -26,6 +26,17 @@ to work, you need to setup a Celery backend (**RabbitMQ**, **Redis**, ...) and
change your ``airflow.cfg`` to point the executor parameter to
``CeleryExecutor`` and provide the related Celery settings.
+If you use Amazon SQS as a Celery broker, set environment variables like this:
Review Comment:
Can you please add a note that this is experimental support/use at own risk. It would be great also to add something along "there are anecdotal evidences that Amazon SQS" might be used as the broker backend.
We do not run any tests for SQS as celery backend (as opposed to rabbitmq and redis - we have integration tests for both).
We do not want to give the users false impression that they can expect it to work and ask as question "I followed the documentation and it does not work" - unless we have CI/tests covering the scenario we are talking about.
Or maybe @ferruzzi @o-nikolas @john-jac - there could be some blog post or documentation from Amazon team to describe it. I believe MWAA uses SQS as broker and maybe one of you (or someone from the amazon developer relations team) coudl write a blog post on "How to use SQS as celery broker for Airflow? " that woudl be nice - and then any questions about it could come to Amazon- I am afraid until we make system tests/integration with Amazon, making a statement that "SQS works and here is the configuration" in Airlfow docs is a little too much "leap of faith" :)
--
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 pull request #24019: Add celery broker configuration example using Amazon SQS
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #24019:
URL: https://github.com/apache/airflow/pull/24019#issuecomment-1252325099
FYI: I think that was a good idea to not "pretend" we support SQS when we have no tests for it. An example of interesting problem with it is here for example: https://github.com/apache/airflow/discussions/26521?converting=1#discussioncomment-3690260
--
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] pingzh commented on a diff in pull request #24019: Add celery broker configuration example using Amazon SQS
Posted by GitBox <gi...@apache.org>.
pingzh commented on code in PR #24019:
URL: https://github.com/apache/airflow/pull/24019#discussion_r889481633
##########
docs/apache-airflow/executor/celery.rst:
##########
@@ -26,6 +26,17 @@ to work, you need to setup a Celery backend (**RabbitMQ**, **Redis**, ...) and
change your ``airflow.cfg`` to point the executor parameter to
``CeleryExecutor`` and provide the related Celery settings.
+If you use Amazon SQS as a Celery broker, set environment variables like this:
+
+.. code-block:: bash
+
+ export AIRFLOW__CELERY__BROKER_URL=sqs://@your_sqs_host
+
+ # Celery's environment variables.
+ export AWS_ACCESS_KEY_ID=your_aws_access_key_id
+ export AWS_SECRET_ACCESS_KEY=your_aws_secret_access_key
+ export AWS_DEFAULT_REGION=your_aws_region
Review Comment:
if will be great if you can also mention this setting https://github.com/apache/airflow/pull/21507
--
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] tsugumi-sys commented on pull request #24019: Add celery broker configuration example using Amazon SQS
Posted by GitBox <gi...@apache.org>.
tsugumi-sys commented on PR #24019:
URL: https://github.com/apache/airflow/pull/24019#issuecomment-1141034408
> We had reports that `predefined_queues` isn't working properly with SQS #11225 (though it's on older Airflow versions) Can you confirm it's working as it should?
I also checked `predefined_queues` option and it didn't work in my local environment. This option is not used in my commit.
--
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] o-nikolas commented on pull request #24019: Add celery broker configuration example using Amazon SQS
Posted by GitBox <gi...@apache.org>.
o-nikolas commented on PR #24019:
URL: https://github.com/apache/airflow/pull/24019#issuecomment-1211353124
> > I feel like the situation here is slightly different as it is [officially supported by Celery](https://docs.celeryq.dev/en/stable/getting-started/backends-and-brokers/index.html#broker-overview), and presumably properly tested there. I'm not sure we want to exhaustively test our dependencies. That said, this is a pretty critical piece of the puzzle shrug
>
> Actually that might change my view. I did not realize SQS is on-par with RabbitMQ/Redis for Celery. On the other Hand MAYBE this is a GREAT opportunity to add a test or two using Moto ?
>
> https://qxf2.com/blog/mocking-out-aws-sqs-using-moto/
>
> Then we would be completely covered and it shoudl be no problem to make it official. @ferruzzi @o-nikolas @vincbeck ? WDYT>
I have a love/hate relationship with using SQS as a celery broker :) (maybe this is for a blog post...)
As a tl;dr: it works, but has a lot of gotchas, and I wouldn't necessarily recommend it generally to airflow users.
Celery is serverless, scales great, and is relatively fast. But it doesn't actually have the same level of support as RabbitMQ and Redis have as a Celery broker. Namely the Monitor and Remote Control APIs are not supported, so while it is stable, it has far less support. Also SQS has the visibility timeout architecturally built into it, which means you can't have tasks that run longer than that timeout without that causing issues (max value for that timeout is 12 hours).
I still support adding automated testing for SQS though, I think that's a worthwhile task.
--
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