Skip to content

[AIRFLOW-4847] Update config template to reflect supporting different Celery pool implementation#5477

Merged
ashb merged 2 commits into
apache:masterfrom
XD-DENG:refine_celery_pool_config
Jun 26, 2019
Merged

[AIRFLOW-4847] Update config template to reflect supporting different Celery pool implementation#5477
ashb merged 2 commits into
apache:masterfrom
XD-DENG:refine_celery_pool_config

Conversation

@XD-DENG
Copy link
Copy Markdown
Member

@XD-DENG XD-DENG commented Jun 25, 2019

Jira

Description

The support to different Celery pool implementation has been added in #4308 (https://github.com/apache/airflow/blob/master/airflow/bin/cli.py#L1086-L1087).

But it's not reflected in the default_airflow.cfg yet, while it's the main portal of config options to most users. Currently the only way for users to get to know the existence of this option is to check source code of bin/cli.py (like what I have done).

This situation can be addressed by adding it into default_airflow.cfg and mentioning this change in UPDATING.md.

… Celery pool implementation

The support to different Celery pool implementation has been added
in apache#4308.

But it's not reflected in the default_airflow.cfg yet, while it's
the main portal of config options to most users.
@XD-DENG XD-DENG requested review from ashb, feng-tao and kaxil June 25, 2019 09:15
Comment thread UPDATING.md Outdated
### `pool` config option in Celery section to support different Celery pool implementation

The new `pool` config option allows users to choose different pool
implementation. Default value is "prefork", while choices include "prefok" (default),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
implementation. Default value is "prefork", while choices include "prefok" (default),
implementation. Default value is "prefork", while choices include "prefork" (default),

Copy link
Copy Markdown
Member

@kaxil kaxil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 typo but otherwise LGTM

@XD-DENG
Copy link
Copy Markdown
Member Author

XD-DENG commented Jun 25, 2019

Typo is addressed. Thanks @kaxil !

@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #5477 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5477   +/-   ##
=======================================
  Coverage   79.11%   79.11%           
=======================================
  Files         489      489           
  Lines       30682    30682           
=======================================
  Hits        24275    24275           
  Misses       6407     6407

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d9ad100...ccde909. Read the comment docs.

@XD-DENG
Copy link
Copy Markdown
Member Author

XD-DENG commented Jun 26, 2019

@OmerJog unfortunately that's the best I can find in Celery's documentation. Actually they are enough from my point of view. Users are supposed to change this config only given they understand the differences, which can not really be covered by one or two links, and they should do their research.

@ashb ashb merged commit dd08ae3 into apache:master Jun 26, 2019
ashb pushed a commit to ashb/airflow that referenced this pull request Jun 26, 2019
… Celery pool implementation (apache#5477)

The support to different Celery pool implementation has been added
in apache#4308.

But it's not reflected in the default_airflow.cfg yet, while it's
the main portal of config options to most users.

(cherry picked from commit dd08ae3)
andriisoldatenko pushed a commit to andriisoldatenko/airflow that referenced this pull request Jul 26, 2019
… Celery pool implementation (apache#5477)

The support to different Celery pool implementation has been added
in apache#4308.

But it's not reflected in the default_airflow.cfg yet, while it's
the main portal of config options to most users.
wmorris75 pushed a commit to modmed-external/incubator-airflow that referenced this pull request Jul 29, 2019
… Celery pool implementation (apache#5477)

The support to different Celery pool implementation has been added
in apache#4308.

But it's not reflected in the default_airflow.cfg yet, while it's
the main portal of config options to most users.
dharamsk pushed a commit to postmates/airflow that referenced this pull request Aug 8, 2019
… Celery pool implementation (apache#5477)

The support to different Celery pool implementation has been added
in apache#4308.

But it's not reflected in the default_airflow.cfg yet, while it's
the main portal of config options to most users.

(cherry picked from commit dd08ae3)
@XD-DENG XD-DENG deleted the refine_celery_pool_config branch December 4, 2019 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants