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 2021/03/10 15:45:40 UTC
[GitHub] [airflow] leonsmith opened a new pull request #14699: Feature: Move celery.default_queue to operators.default_queue to allow re-use between executors
leonsmith opened a new pull request #14699:
URL: https://github.com/apache/airflow/pull/14699
The `default_queue` config option resides in the `celery` section.
We already have an operators config section which would be a better place to specify a default.
This also allows other executors to re-use the default queue functionality without having to set celery specific config.
<!--
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: https://github.com/apache/airflow/issues/14696
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/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 [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] leonsmith commented on pull request #14699: Feature: Move celery.default_queue to operators.default_queue to allow re-use between executors
Posted by GitBox <gi...@apache.org>.
leonsmith commented on pull request #14699:
URL: https://github.com/apache/airflow/pull/14699#issuecomment-798792371
Sure @turbaszek might need a little assistance as they look pretty unrelated at a surface glance?
Is master always supposed to be green? I can re-base and see if any of them change
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil merged pull request #14699: Feature: Move celery.default_queue to operators.default_queue to allow re-use between executors
Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #14699:
URL: https://github.com/apache/airflow/pull/14699
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #14699: Feature: Move celery.default_queue to operators.default_queue to allow re-use between executors
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #14699:
URL: https://github.com/apache/airflow/pull/14699#discussion_r592359306
##########
File path: airflow/configuration.py
##########
@@ -159,6 +159,7 @@ class AirflowConfigParser(ConfigParser): # pylint: disable=too-many-ancestors
('metrics', 'statsd_datadog_tags'): ('scheduler', 'statsd_datadog_tags', '2.0.0'),
('metrics', 'statsd_custom_client_path'): ('scheduler', 'statsd_custom_client_path', '2.0.0'),
('scheduler', 'parsing_processes'): ('scheduler', 'max_threads', '1.10.14'),
+ ('operators', 'default_queue'): ('celery', 'default_queue', '2.1.0'),
Review comment:
Please put 2.1 though, yes. Saves us having to change it later.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on pull request #14699: Feature: Move celery.default_queue to operators.default_queue to allow re-use between executors
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #14699:
URL: https://github.com/apache/airflow/pull/14699#issuecomment-795630224
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/master/CONTRIBUTING.rst)
Here are some useful points:
- Pay attention to the quality of your code (flake8, pylint and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/master/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/master/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/master/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/master/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #14699: Feature: Move celery.default_queue to operators.default_queue to allow re-use between executors
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #14699:
URL: https://github.com/apache/airflow/pull/14699#discussion_r592358314
##########
File path: airflow/configuration.py
##########
@@ -159,6 +159,7 @@ class AirflowConfigParser(ConfigParser): # pylint: disable=too-many-ancestors
('metrics', 'statsd_datadog_tags'): ('scheduler', 'statsd_datadog_tags', '2.0.0'),
('metrics', 'statsd_custom_client_path'): ('scheduler', 'statsd_custom_client_path', '2.0.0'),
('scheduler', 'parsing_processes'): ('scheduler', 'max_threads', '1.10.14'),
+ ('operators', 'default_queue'): ('celery', 'default_queue', '2.1.0'),
Review comment:
I have a feeling that it doesn't matter much now, because the release manager decides whether this change will be included in the next release.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] leonsmith edited a comment on pull request #14699: Feature: Move celery.default_queue to operators.default_queue to allow re-use between executors
Posted by GitBox <gi...@apache.org>.
leonsmith edited a comment on pull request #14699:
URL: https://github.com/apache/airflow/pull/14699#issuecomment-798875272
The test suit looked like it mostly passed last night (after a rebase onto master), 2 of the actions look like they got killed as they exited with a 137 code.
No idea why they have all reset this morning & its now saying it doesn't have permissions.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] leonsmith commented on a change in pull request #14699: Feature: Move celery.default_queue to operators.default_queue to allow re-use between executors
Posted by GitBox <gi...@apache.org>.
leonsmith commented on a change in pull request #14699:
URL: https://github.com/apache/airflow/pull/14699#discussion_r591643024
##########
File path: airflow/configuration.py
##########
@@ -159,6 +159,7 @@ class AirflowConfigParser(ConfigParser): # pylint: disable=too-many-ancestors
('metrics', 'statsd_datadog_tags'): ('scheduler', 'statsd_datadog_tags', '2.0.0'),
('metrics', 'statsd_custom_client_path'): ('scheduler', 'statsd_custom_client_path', '2.0.0'),
('scheduler', 'parsing_processes'): ('scheduler', 'max_threads', '1.10.14'),
+ ('operators', 'default_queue'): ('celery', 'default_queue', '2.1.0'),
Review comment:
I'm not sure what version should be put here, guessing this would have to go as a new minor?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #14699: Feature: Move celery.default_queue to operators.default_queue to allow re-use between executors
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #14699:
URL: https://github.com/apache/airflow/pull/14699#issuecomment-798786446
@leonsmith could you please take a look at the CI issues?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] leonsmith commented on a change in pull request #14699: Feature: Move celery.default_queue to operators.default_queue to allow re-use between executors
Posted by GitBox <gi...@apache.org>.
leonsmith commented on a change in pull request #14699:
URL: https://github.com/apache/airflow/pull/14699#discussion_r593938106
##########
File path: airflow/config_templates/default_test.cfg
##########
@@ -64,6 +64,7 @@ auth_backend = airflow.api.auth.backend.default
[operators]
default_owner = airflow
+default_queue = default
Review comment:
Removed as suggested, this applied to the old config value under the celery section (it was set to the same value as airflow.cfg)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #14699: Feature: Move celery.default_queue to operators.default_queue to allow re-use between executors
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14699:
URL: https://github.com/apache/airflow/pull/14699#issuecomment-798786397
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #14699: Feature: Move celery.default_queue to operators.default_queue to allow re-use between executors
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #14699:
URL: https://github.com/apache/airflow/pull/14699#discussion_r593918938
##########
File path: airflow/config_templates/default_test.cfg
##########
@@ -64,6 +64,7 @@ auth_backend = airflow.api.auth.backend.default
[operators]
default_owner = airflow
+default_queue = default
Review comment:
```suggestion
```
We don't need to include defaults here if it's the same default from airflow.cfg
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #14699: Feature: Move celery.default_queue to operators.default_queue to allow re-use between executors
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #14699:
URL: https://github.com/apache/airflow/pull/14699#issuecomment-810678003
@leonsmith Can you do a rebase? I would like to merge this change.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #14699: Feature: Move celery.default_queue to operators.default_queue to allow re-use between executors
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #14699:
URL: https://github.com/apache/airflow/pull/14699#discussion_r593938567
##########
File path: airflow/config_templates/default_test.cfg
##########
@@ -64,6 +64,7 @@ auth_backend = airflow.api.auth.backend.default
[operators]
default_owner = airflow
+default_queue = default
Review comment:
Yeah, removing makes sense, just no need to add it to the new section
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] leonsmith commented on pull request #14699: Feature: Move celery.default_queue to operators.default_queue to allow re-use between executors
Posted by GitBox <gi...@apache.org>.
leonsmith commented on pull request #14699:
URL: https://github.com/apache/airflow/pull/14699#issuecomment-810850096
@mik-laj rebased as requested π
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] leonsmith commented on pull request #14699: Feature: Move celery.default_queue to operators.default_queue to allow re-use between executors
Posted by GitBox <gi...@apache.org>.
leonsmith commented on pull request #14699:
URL: https://github.com/apache/airflow/pull/14699#issuecomment-798875272
The test suit looked like it mostly passed last night, 2 of the actions look like they got killed as they exited with a 137 code.
No idea why they have all reset this morning & its now saying it doesn't have permissions.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org