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 2019/12/26 19:52:52 UTC
[GitHub] [airflow] mik-laj opened a new pull request #6905: [AIRFLOW-6361]
Run LocalTaskJob directly in Celery task
mik-laj opened a new pull request #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905
Hello,
Celery runs the CLI first command, which contains LocalTaskJob. LocalTaskJob is responsible for starting the next user-code process. This level of isolation is redundant because LocalTaskJob doesn't execute unsafe code. The first command is run by a new process creation, not by a fork, so this is an expensive operation.
According to preliminary measurements, this change results in an increase in performance close to 30%.
I will provide more information in PR.
Best regards
Kamil Bregula
After:
```
real 0m38.394s
user 0m4.340s
sys 0m1.600s
real 0m38.355s
user 0m4.700s
sys 0m1.340s
real 0m38.675s
user 0m4.760s
sys 0m1.530s
real 0m38.488s
user 0m4.770s
sys 0m1.280s
real 0m38.434s
user 0m4.600s
sys 0m1.390s
real 0m38.378s
user 0m4.500s
sys 0m1.270s
real 0m38.106s
user 0m4.200s
sys 0m1.100s
real 0m38.082s
user 0m4.170s
sys 0m1.030s
real 0m38.173s
user 0m4.290s
sys 0m1.340s
real 0m38.161s
user 0m4.460s
sys 0m1.370s
```
Before:
```
real 0m53.488s
user 0m5.140s
sys 0m1.700s
real 1m8.288s
user 0m6.430s
sys 0m2.200s
real 0m53.371s
user 0m5.330s
sys 0m1.630s
real 0m58.939s
user 0m6.470s
sys 0m1.730s
real 0m53.255s
user 0m4.950s
sys 0m1.640s
real 0m58.802s
user 0m5.970s
sys 0m1.790s
real 0m58.449s
user 0m5.380s
sys 0m1.580s
real 0m53.308s
user 0m5.120s
sys 0m1.430s
real 0m53.485s
user 0m5.220s
sys 0m1.290s
real 0m53.387s
user 0m5.020s
sys 0m1.590s
```
Make sure you have checked _all_ steps below.
### Jira
- [ ] My PR addresses the following [Airflow Jira](https://issues.apache.org/jira/browse/AIRFLOW/) issues and references them in the PR title. For example, "\[AIRFLOW-XXX\] My Airflow PR"
- https://issues.apache.org/jira/browse/AIRFLOW-6361
- In case you are fixing a typo in the documentation you can prepend your commit with \[AIRFLOW-XXX\], code changes always need a Jira issue.
- In case you are proposing a fundamental code change, you need to create an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)).
- In case you are adding a dependency, check if the license complies with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
### Description
- [ ] Here are some details about my PR, including screenshots of any UI changes:
### Tests
- [ ] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason:
### Commits
- [ ] My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
1. Subject is limited to 50 characters (not including Jira issue reference)
1. Subject does not end with a period
1. Subject uses the imperative mood ("add", not "adding")
1. Body wraps at 72 characters
1. Body explains "what" and "why", not "how"
### Documentation
- [ ] In case of new functionality, my PR adds documentation that describes how to use it.
- All the public functions and the classes in the PR contain docstrings that explain what it does
- If you implement backwards incompatible changes, please leave a note in the [Updating.md](https://github.com/apache/airflow/blob/master/UPDATING.md) so we can assign it to a appropriate 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
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on issue #6905: [AIRFLOW-6361] Run
LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569624565
I created a separate PR with a sequence diagram. https://github.com/apache/airflow/pull/6961
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569145791
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=h1) Report
> Merging [#6905](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/7d97d16c47ba181839fd583b8653d2872dc4290b?src=pr&el=desc) will **decrease** coverage by `0.17%`.
> The diff coverage is `83.33%`.
[](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6905 +/- ##
==========================================
- Coverage 84.7% 84.52% -0.18%
==========================================
Files 680 679 -1
Lines 38556 38492 -64
==========================================
- Hits 32659 32536 -123
- Misses 5897 5956 +59
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/bin/cli.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9iaW4vY2xpLnB5) | `96.7% <100%> (+2.65%)` | :arrow_up: |
| [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `88.11% <33.33%> (-0.33%)` | :arrow_down: |
| [airflow/operators/postgres\_operator.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvcG9zdGdyZXNfb3BlcmF0b3IucHk=) | `100% <0%> (ø)` | :arrow_up: |
| [airflow/operators/mysql\_operator.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXlzcWxfb3BlcmF0b3IucHk=) | `100% <0%> (ø)` | :arrow_up: |
| [airflow/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXlzcWxfdG9faGl2ZS5weQ==) | `100% <0%> (ø)` | :arrow_up: |
| [airflow/operators/generic\_transfer.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvZ2VuZXJpY190cmFuc2Zlci5weQ==) | `100% <0%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| ... and [21 more](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=footer). Last update [7d97d16...a992c13](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on issue #6905: [AIRFLOW-6361] Run
LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569717105
> @ashb I tested locally and this does not happen because a new Dagbag is created each time CLI command is called.
Wait, do we? In on my phone so checking the code is hard right now, but we used to have a single module-global dagbag object in cli.py. Where in the code does the dagbag get (re)created now?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] stale[bot] commented on issue #6905: [AIRFLOW-6361] Run
LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
stale[bot] commented on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-589070002
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on a change in pull request #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#discussion_r363027926
##########
File path: airflow/bin/cli.py
##########
@@ -1063,3 +1063,23 @@ def _add_subcommand(cls, subparsers, sub):
def get_parser():
"""Calls static method inside factory which creates argument parser"""
return CLIFactory.get_parser()
+
+
+def exec_airflow_command(command_to_exec: List[str]):
Review comment:
We should change the StandardTaskRunner to use this new method too please
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on issue #6905: [AIRFLOW-6361] Run
LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569688311
Broadly sounds good, however as it stands this PR would load the dag in to the long running Celery worker process.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] nuclearpinguin commented on a change in pull request
#6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
nuclearpinguin commented on a change in pull request #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#discussion_r361700738
##########
File path: airflow/executors/celery_executor.py
##########
@@ -59,17 +58,17 @@
@app.task
-def execute_command(command_to_exec: str) -> None:
+def execute_command(command_to_exec: List[str]) -> None:
"""Executes command."""
log = LoggingMixin().log
log.info("Executing command in Celery: %s", command_to_exec)
- env = os.environ.copy()
try:
- subprocess.check_call(command_to_exec, stderr=subprocess.STDOUT,
- close_fds=True, env=env)
- except subprocess.CalledProcessError as e:
- log.exception('execute_command encountered a CalledProcessError')
- log.error(e.output)
+ parser = CLIFactory.get_parser()
+ # drop "airflow"
+ command_to_exec = command_to_exec[1:]
+ args = parser.parse_args(command_to_exec)
+ args.func(args)
Review comment:
If we use it only here then value of such function is poor, but I think we can re-use it in other places. Then further refactoring of executing task will be easier because only this function will have to be refactored.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569145791
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=h1) Report
> Merging [#6905](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/d0fbb523d617316a35461e6ee53781a7a716092c?src=pr&el=desc) will **decrease** coverage by `0.18%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6905 +/- ##
==========================================
- Coverage 84.62% 84.44% -0.19%
==========================================
Files 679 679
Lines 38497 38504 +7
==========================================
- Hits 32578 32514 -64
- Misses 5919 5990 +71
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `88.51% <100%> (-0.08%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/config\_templates/airflow\_local\_settings.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb25maWdfdGVtcGxhdGVzL2FpcmZsb3dfbG9jYWxfc2V0dGluZ3MucHk=) | `71.73% <0%> (ø)` | :arrow_up: |
| [airflow/models/taskreschedule.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvdGFza3Jlc2NoZWR1bGUucHk=) | `100% <0%> (ø)` | :arrow_up: |
| [airflow/configuration.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb25maWd1cmF0aW9uLnB5) | `92.65% <0%> (+0.05%)` | :arrow_up: |
| [airflow/executors/kubernetes\_executor.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMva3ViZXJuZXRlc19leGVjdXRvci5weQ==) | `57.48% <0%> (+0.09%)` | :arrow_up: |
| ... and [5 more](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=footer). Last update [d0fbb52...9492ed9](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on a change in pull request #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#discussion_r363034814
##########
File path: airflow/bin/cli.py
##########
@@ -1063,3 +1063,23 @@ def _add_subcommand(cls, subparsers, sub):
def get_parser():
"""Calls static method inside factory which creates argument parser"""
return CLIFactory.get_parser()
+
+
+def exec_airflow_command(command_to_exec: List[str]):
Review comment:
https://github.com/apache/airflow/blob/master/airflow/task/task_runner/standard_task_runner.py#L71-L84
This will not be easy because StandardTaskRunner also sets the title of the process. For me it is a slightly different case.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on a change in pull request #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#discussion_r362019838
##########
File path: airflow/executors/celery_executor.py
##########
@@ -63,13 +62,9 @@ def execute_command(command_to_exec: CommandType) -> None:
"""Executes command."""
log = LoggingMixin().log
log.info("Executing command in Celery: %s", command_to_exec)
- env = os.environ.copy()
try:
- subprocess.check_call(command_to_exec, stderr=subprocess.STDOUT,
Review comment:
Environment variables will be preserved - since it's the same process :)
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] nuclearpinguin commented on a change in pull request
#6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
nuclearpinguin commented on a change in pull request #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#discussion_r361687056
##########
File path: airflow/executors/celery_executor.py
##########
@@ -59,17 +58,17 @@
@app.task
-def execute_command(command_to_exec: str) -> None:
+def execute_command(command_to_exec: List[str]) -> None:
"""Executes command."""
log = LoggingMixin().log
log.info("Executing command in Celery: %s", command_to_exec)
- env = os.environ.copy()
try:
- subprocess.check_call(command_to_exec, stderr=subprocess.STDOUT,
- close_fds=True, env=env)
- except subprocess.CalledProcessError as e:
- log.exception('execute_command encountered a CalledProcessError')
- log.error(e.output)
+ parser = CLIFactory.get_parser()
+ # drop "airflow"
+ command_to_exec = command_to_exec[1:]
+ args = parser.parse_args(command_to_exec)
+ args.func(args)
Review comment:
Maybe we can put this logic into a function? It would help in future refactor.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569145791
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=h1) Report
> Merging [#6905](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/7d97d16c47ba181839fd583b8653d2872dc4290b?src=pr&el=desc) will **decrease** coverage by `0.17%`.
> The diff coverage is `83.33%`.
[](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6905 +/- ##
==========================================
- Coverage 84.7% 84.52% -0.18%
==========================================
Files 680 679 -1
Lines 38556 38492 -64
==========================================
- Hits 32659 32536 -123
- Misses 5897 5956 +59
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/bin/cli.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9iaW4vY2xpLnB5) | `96.7% <100%> (+2.65%)` | :arrow_up: |
| [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `88.11% <33.33%> (-0.33%)` | :arrow_down: |
| [airflow/operators/postgres\_operator.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvcG9zdGdyZXNfb3BlcmF0b3IucHk=) | `100% <0%> (ø)` | :arrow_up: |
| [airflow/operators/mysql\_operator.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXlzcWxfb3BlcmF0b3IucHk=) | `100% <0%> (ø)` | :arrow_up: |
| [airflow/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXlzcWxfdG9faGl2ZS5weQ==) | `100% <0%> (ø)` | :arrow_up: |
| [airflow/operators/generic\_transfer.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvZ2VuZXJpY190cmFuc2Zlci5weQ==) | `100% <0%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| ... and [21 more](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=footer). Last update [7d97d16...a992c13](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on issue #6905: [AIRFLOW-6361] Run
LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569495320
@mik-laj - should we add this graph to the documentation once merged? It's SUUUPER useful! And maybe we can add the first graph to the documentation in v1-10-test - so that it can be released with 1.10.8 docs ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on issue #6905: [AIRFLOW-6361] Run
LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569797159
@ashb I added note about plugins in `celery.rst` file. Although I will try to remove this limitation in the future. It doesn't seem difficult. The next step is to decouple the executor from the CLI.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on issue #6905: [AIRFLOW-6361] Run
LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569692530
@ashb I tested locally and this does not happen because a new Dagbag is created each time CLI command is called. I would also like to make LocalTaskJob independent of DAG and operator and only use information from TaskInstance. It seems to me that there is no need to load the entire DAG in LocalTaskJob. This can further increase performance.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on issue #6905: [AIRFLOW-6361] Run
LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569246664
Really nice! This looks really nice - unless there is some hidden side effect in the CeleryExecutor, this looks like really nice improvement. LocalTaskJob uses task runner underneath that provides appropriate isolation between tasks already so indeed that looks like an unnecessary overhead to start yet another process. I'd love others to take a look but this one looks great for me!
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569145791
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=h1) Report
> Merging [#6905](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/7d97d16c47ba181839fd583b8653d2872dc4290b?src=pr&el=desc) will **increase** coverage by `0.03%`.
> The diff coverage is `70%`.
[](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6905 +/- ##
==========================================
+ Coverage 84.7% 84.74% +0.03%
==========================================
Files 680 680
Lines 38556 38755 +199
==========================================
+ Hits 32659 32841 +182
- Misses 5897 5914 +17
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/task/task\_runner/standard\_task\_runner.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy90YXNrL3Rhc2tfcnVubmVyL3N0YW5kYXJkX3Rhc2tfcnVubmVyLnB5) | `73.58% <0%> (+6.91%)` | :arrow_up: |
| [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `88.11% <100%> (-0.33%)` | :arrow_down: |
| [airflow/bin/cli.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9iaW4vY2xpLnB5) | `92.78% <73.33%> (-1.27%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/utils/file.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9maWxlLnB5) | `86.3% <0%> (-5.17%)` | :arrow_down: |
| [airflow/utils/cli.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9jbGkucHk=) | `68.75% <0%> (-2.95%)` | :arrow_down: |
| ... and [90 more](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=footer). Last update [7d97d16...530125d](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569145791
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=h1) Report
> Merging [#6905](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6fffa5b0d7840727a96dc1765a0166656bc7ea52?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6905 +/- ##
==========================================
- Coverage 84.72% 84.44% -0.29%
==========================================
Files 679 679
Lines 38505 38504 -1
==========================================
- Hits 32623 32514 -109
- Misses 5882 5990 +108
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `88.51% <100%> (-0.08%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/bin/cli.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9iaW4vY2xpLnB5) | `96.05% <0%> (+2.63%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=footer). Last update [6fffa5b...9492ed9](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on a change in pull request #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#discussion_r361695192
##########
File path: airflow/executors/celery_executor.py
##########
@@ -59,17 +58,17 @@
@app.task
-def execute_command(command_to_exec: str) -> None:
+def execute_command(command_to_exec: List[str]) -> None:
"""Executes command."""
log = LoggingMixin().log
log.info("Executing command in Celery: %s", command_to_exec)
- env = os.environ.copy()
try:
- subprocess.check_call(command_to_exec, stderr=subprocess.STDOUT,
- close_fds=True, env=env)
- except subprocess.CalledProcessError as e:
- log.exception('execute_command encountered a CalledProcessError')
- log.error(e.output)
+ parser = CLIFactory.get_parser()
+ # drop "airflow"
+ command_to_exec = command_to_exec[1:]
+ args = parser.parse_args(command_to_exec)
+ args.func(args)
Review comment:
I wanted to delete CLI references completely during another refactoring, but it makes sense to create functions now.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569145791
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=h1) Report
> Merging [#6905](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6fffa5b0d7840727a96dc1765a0166656bc7ea52?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6905 +/- ##
==========================================
- Coverage 84.72% 84.44% -0.29%
==========================================
Files 679 679
Lines 38505 38504 -1
==========================================
- Hits 32623 32514 -109
- Misses 5882 5990 +108
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `88.51% <100%> (-0.08%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/bin/cli.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9iaW4vY2xpLnB5) | `96.05% <0%> (+2.63%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=footer). Last update [6fffa5b...9492ed9](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj edited a comment on issue #6905: [AIRFLOW-6361]
Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569453949
To show the process of execution and communication between processes, I drew two sequence diagrams. I hope it will be helpful.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569145791
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=h1) Report
> Merging [#6905](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/7d97d16c47ba181839fd583b8653d2872dc4290b?src=pr&el=desc) will **decrease** coverage by `0.1%`.
> The diff coverage is `92.85%`.
[](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6905 +/- ##
==========================================
- Coverage 84.52% 84.42% -0.11%
==========================================
Files 680 680
Lines 38556 38561 +5
==========================================
- Hits 32591 32555 -36
- Misses 5965 6006 +41
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `88.11% <100%> (-0.33%)` | :arrow_down: |
| [airflow/bin/cli.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9iaW4vY2xpLnB5) | `95.69% <90.9%> (+1.65%)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/jobs/scheduler\_job.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL3NjaGVkdWxlcl9qb2IucHk=) | `89.28% <0%> (+1.17%)` | :arrow_up: |
| [airflow/utils/helpers.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9oZWxwZXJzLnB5) | `82.75% <0%> (+6.32%)` | :arrow_up: |
| ... and [3 more](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=footer). Last update [7d97d16...79aa35e](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on issue #6905: [AIRFLOW-6361] Run
LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569495874
Yes. I wanna to add it to documentation. It's only early draft now. I working on it yet.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] kaxil commented on a change in pull request #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#discussion_r362009139
##########
File path: airflow/executors/celery_executor.py
##########
@@ -63,13 +62,9 @@ def execute_command(command_to_exec: CommandType) -> None:
"""Executes command."""
log = LoggingMixin().log
log.info("Executing command in Celery: %s", command_to_exec)
- env = os.environ.copy()
try:
- subprocess.check_call(command_to_exec, stderr=subprocess.STDOUT,
Review comment:
When `os.fork` is used in `StandardTaskRunner` it is fine. But for cases where `CAN_FORK=False`, this would fail
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] kaxil commented on a change in pull request #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#discussion_r362006526
##########
File path: airflow/bin/cli.py
##########
@@ -1063,3 +1063,26 @@ def _add_subcommand(cls, subparsers, sub):
def get_parser():
"""Calls static method inside factory which creates argument parser"""
return CLIFactory.get_parser()
+
+
+def exec_airflow_command(command_to_exec: List[str]):
+ """
+ Execute command using CLI.
+
+ The command is run in the current process and thread.
+
+ :param command_to_exec: list of program arguments. The first element should contain the "airflow" element.
+ :type command_to_exec List[str]
+ """
+ parser = CLIFactory.get_parser()
+
+ if not command_to_exec:
Review comment:
Do we need this? As `command_to_exec` is a required parameter.
Or do you want to raise any error when it is `None` or `[]`, in which case we should update the error msg
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj opened a new pull request #6905: [AIRFLOW-6361]
Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
mik-laj opened a new pull request #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905
Hello,
The executor runs multiple processes to perform one task. Many processes have a very short life cycle, so the process of starting it is a significant overhead.
Firstly, the Celery executor trigger Celery tasks - app.task. This task runs the CLI command (first process), which contains LocalTaskJob. LocalTaskJob runs the separate command (second process) that executes user-code. This level of isolation is redundant because LocalTaskJob doesn't execute unsafe code. The first command is run by a new process creation, not by a fork, so this is an expensive operation. I suggest running code from the first process as part of the celery task to reduce the need to create new processes.
The code currently uses CLIFactory to run the LocalTaskJob It is better to do this without unnecessary dependence on CLI, but it is a big change and I plan to do it in a separate PR.
WIP PR: https://github.com/mik-laj/incubator-airflow/pull/10 (Travis green :-D )
Performance benchmark:
===================
Example DAG from Airflow with unneeded sleep instructions deleted.
```python
"""Example DAG demonstrating the usage of the BashOperator."""
from datetime import timedelta
import airflow
from airflow.models import DAG
from airflow.operators.bash_operator import BashOperator
from airflow.operators.dummy_operator import DummyOperator
args = {
'owner': 'airflow',
'start_date': airflow.utils.dates.days_ago(2),
}
dag = DAG(
dag_id='example_bash_operator',
default_args=args,
schedule_interval='0 0 * * *',
dagrun_timeout=timedelta(minutes=60),
)
run_this_last = DummyOperator(
task_id='run_this_last',
dag=dag,
)
# [START howto_operator_bash]
run_this = BashOperator(
task_id='run_after_loop',
bash_command='echo 1',
dag=dag,
)
# [END howto_operator_bash]
run_this >> run_this_last
for i in range(3):
task = BashOperator(
task_id='runme_' + str(i),
bash_command='echo "{{ task_instance_key_str }}",
dag=dag,
)
task >> run_this
# [START howto_operator_bash_template]
also_run_this = BashOperator(
task_id='also_run_this',
bash_command='echo "run_id={{ run_id }} | dag_run={{ dag_run }}"',
dag=dag,
)
# [END howto_operator_bash_template]
also_run_this >> run_this_last
if __name__ == "__main__":
dag.cli()
```
```python
import airflow
from airflow import DAG
from airflow.models import DagBag
dagbag = airflow.models.DagBag()
dag: DAG = dagbag.get_dag("example_bash_operator")
dag.clear()
dag.run()
```
Environment: Brreze
```
unset AIRFLOW__CORE__DAGS_FOLDER
unset AIRFLOW__CORE__UNIT_TEST_MODE
chmod -R 777 /root
sudo -E su airflow
export AIRFLOW__CORE__EXECUTOR="CeleryExecutor"
export AIRFLOW__CELERY__BROKER_URL="redis://redis:6379/0"
export AIRFLOW__CELERY__WORKER_CONCURRENCY=8
seq 1 10 | xargs -n 1 -I {} bash -c "time python /files/benchmark_speed.py > /dev/null 2>&1" | grep '^(real\|user\|sys)';
```
Result:
|Fn. | After | Before | Change|
|--------|-------|--------|-------|
|AVERAGE | 56.48 | 38.32 | -32% |
|VAR | 23.60 | 0.04 | -98% |
|MAX | 68.29 | 38.68 | -43% |
|MIN | 53.26 | 38.08 | -28% |
|STDEV | 4.86 | 0.19 | -96%. |
Raw data
After:
```
real 0m38.394s
user 0m4.340s
sys 0m1.600s
real 0m38.355s
user 0m4.700s
sys 0m1.340s
real 0m38.675s
user 0m4.760s
sys 0m1.530s
real 0m38.488s
user 0m4.770s
sys 0m1.280s
real 0m38.434s
user 0m4.600s
sys 0m1.390s
real 0m38.378s
user 0m4.500s
sys 0m1.270s
real 0m38.106s
user 0m4.200s
sys 0m1.100s
real 0m38.082s
user 0m4.170s
sys 0m1.030s
real 0m38.173s
user 0m4.290s
sys 0m1.340s
real 0m38.161s
user 0m4.460s
sys 0m1.370s
```
Before:
```
real 0m53.488s
user 0m5.140s
sys 0m1.700s
real 1m8.288s
user 0m6.430s
sys 0m2.200s
real 0m53.371s
user 0m5.330s
sys 0m1.630s
real 0m58.939s
user 0m6.470s
sys 0m1.730s
real 0m53.255s
user 0m4.950s
sys 0m1.640s
real 0m58.802s
user 0m5.970s
sys 0m1.790s
real 0m58.449s
user 0m5.380s
sys 0m1.580s
real 0m53.308s
user 0m5.120s
sys 0m1.430s
real 0m53.485s
user 0m5.220s
sys 0m1.290s
real 0m53.387s
user 0m5.020s
sys 0m1.590s
```
---
Link to JIRA issue: https://issues.apache.org/jira/browse/AIRFLOW-6361
- [x] Description above provides context of the change
- [x] Commit message starts with `[AIRFLOW-NNNN]`, where AIRFLOW-NNNN = JIRA ID*
- [x] Unit tests coverage for changes (not needed for documentation changes)
- [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [x] Relevant documentation is updated including usage instructions.
- [x] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
(*) For document-only changes, no JIRA issue is needed. Commit message starts `[AIRFLOW-XXXX]`.
---
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).
Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] kaxil commented on a change in pull request #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#discussion_r362007109
##########
File path: airflow/executors/celery_executor.py
##########
@@ -63,13 +62,9 @@ def execute_command(command_to_exec: CommandType) -> None:
"""Executes command."""
log = LoggingMixin().log
log.info("Executing command in Celery: %s", command_to_exec)
- env = os.environ.copy()
try:
- subprocess.check_call(command_to_exec, stderr=subprocess.STDOUT,
Review comment:
This will mean that Environment Variables won't be preserved.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on a change in pull request #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#discussion_r361698886
##########
File path: airflow/executors/celery_executor.py
##########
@@ -59,17 +58,17 @@
@app.task
-def execute_command(command_to_exec: str) -> None:
+def execute_command(command_to_exec: List[str]) -> None:
"""Executes command."""
log = LoggingMixin().log
log.info("Executing command in Celery: %s", command_to_exec)
- env = os.environ.copy()
try:
- subprocess.check_call(command_to_exec, stderr=subprocess.STDOUT,
- close_fds=True, env=env)
- except subprocess.CalledProcessError as e:
- log.exception('execute_command encountered a CalledProcessError')
- log.error(e.output)
+ parser = CLIFactory.get_parser()
+ # drop "airflow"
+ command_to_exec = command_to_exec[1:]
+ args = parser.parse_args(command_to_exec)
+ args.func(args)
Review comment:
In my opinion, this will not affect it in any way. The code I am writing now will delete after refactoring. Now, when I create new functions I will also have to write tests, so this code will be more than 3 lines.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] nuclearpinguin commented on a change in pull request
#6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
nuclearpinguin commented on a change in pull request #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#discussion_r361696059
##########
File path: airflow/executors/celery_executor.py
##########
@@ -59,17 +58,17 @@
@app.task
-def execute_command(command_to_exec: str) -> None:
+def execute_command(command_to_exec: List[str]) -> None:
"""Executes command."""
log = LoggingMixin().log
log.info("Executing command in Celery: %s", command_to_exec)
- env = os.environ.copy()
try:
- subprocess.check_call(command_to_exec, stderr=subprocess.STDOUT,
- close_fds=True, env=env)
- except subprocess.CalledProcessError as e:
- log.exception('execute_command encountered a CalledProcessError')
- log.error(e.output)
+ parser = CLIFactory.get_parser()
+ # drop "airflow"
+ command_to_exec = command_to_exec[1:]
+ args = parser.parse_args(command_to_exec)
+ args.func(args)
Review comment:
Having a function will make it easier imho ✅
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569145791
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=h1) Report
> Merging [#6905](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/7d97d16c47ba181839fd583b8653d2872dc4290b?src=pr&el=desc) will **decrease** coverage by `0.1%`.
> The diff coverage is `92.85%`.
[](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6905 +/- ##
==========================================
- Coverage 84.52% 84.42% -0.11%
==========================================
Files 680 680
Lines 38556 38561 +5
==========================================
- Hits 32591 32555 -36
- Misses 5965 6006 +41
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `88.11% <100%> (-0.33%)` | :arrow_down: |
| [airflow/bin/cli.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9iaW4vY2xpLnB5) | `95.69% <90.9%> (+1.65%)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/jobs/scheduler\_job.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL3NjaGVkdWxlcl9qb2IucHk=) | `89.28% <0%> (+1.17%)` | :arrow_up: |
| [airflow/utils/helpers.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9oZWxwZXJzLnB5) | `82.75% <0%> (+6.32%)` | :arrow_up: |
| ... and [3 more](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=footer). Last update [7d97d16...79aa35e](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on issue #6905: [AIRFLOW-6361] Run
LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569453949
To show the process of execution and communication between processes, I drew two sequence diagrams. I hope it will be helpful.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj edited a comment on issue #6905: [AIRFLOW-6361]
Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569447444
No. Behaviour in this situation is completely identical. DAG was earlier and is still reloaded before running LocalTaskJob and before executing a raw task. However I think, this can be optimized so that LocalTaskJob doesn't need to have access to DAG. it has all the necessary information in the database, but this will be part of another PR.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io commented on issue #6905: [AIRFLOW-6361] Run
LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569145791
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=h1) Report
> Merging [#6905](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6fffa5b0d7840727a96dc1765a0166656bc7ea52?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6905 +/- ##
==========================================
- Coverage 84.72% 84.44% -0.29%
==========================================
Files 679 679
Lines 38505 38504 -1
==========================================
- Hits 32623 32514 -109
- Misses 5882 5990 +108
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `88.51% <100%> (-0.08%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/bin/cli.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9iaW4vY2xpLnB5) | `96.05% <0%> (+2.63%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=footer). Last update [6fffa5b...9492ed9](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on issue #6905: [AIRFLOW-6361] Run
LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569447444
No. Behaviour in this situation is completely identical. DAG was earlier and is still reloaded before running LocalTaskJob and before executing a raw 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569145791
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=h1) Report
> Merging [#6905](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6fffa5b0d7840727a96dc1765a0166656bc7ea52?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6905 +/- ##
==========================================
- Coverage 84.72% 84.44% -0.29%
==========================================
Files 679 679
Lines 38505 38504 -1
==========================================
- Hits 32623 32514 -109
- Misses 5882 5990 +108
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `88.51% <100%> (-0.08%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/bin/cli.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9iaW4vY2xpLnB5) | `96.05% <0%> (+2.63%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=footer). Last update [6fffa5b...9492ed9](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] yuqian90 commented on issue #6905: [AIRFLOW-6361] Run
LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
yuqian90 commented on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569293366
The improvements sound attractive. One question, does this mean every time a change is made to the tasks, the worker needs to be restarted to pick up those changes?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] ashb edited a comment on issue #6905: [AIRFLOW-6361] Run
LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
ashb edited a comment on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569688311
Broadly sounds good, however as it stands this PR would load the dag in to the long running Celery worker process. Which, when coupled with my fork change _might_ mean that this stops tasks automatically picking up dag file changes!
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569145791
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=h1) Report
> Merging [#6905](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/d0fbb523d617316a35461e6ee53781a7a716092c?src=pr&el=desc) will **decrease** coverage by `0.18%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6905 +/- ##
==========================================
- Coverage 84.62% 84.44% -0.19%
==========================================
Files 679 679
Lines 38497 38504 +7
==========================================
- Hits 32578 32514 -64
- Misses 5919 5990 +71
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `88.51% <100%> (-0.08%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/config\_templates/airflow\_local\_settings.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb25maWdfdGVtcGxhdGVzL2FpcmZsb3dfbG9jYWxfc2V0dGluZ3MucHk=) | `71.73% <0%> (ø)` | :arrow_up: |
| [airflow/models/taskreschedule.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvdGFza3Jlc2NoZWR1bGUucHk=) | `100% <0%> (ø)` | :arrow_up: |
| [airflow/configuration.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb25maWd1cmF0aW9uLnB5) | `92.65% <0%> (+0.05%)` | :arrow_up: |
| [airflow/executors/kubernetes\_executor.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMva3ViZXJuZXRlc19leGVjdXRvci5weQ==) | `57.48% <0%> (+0.09%)` | :arrow_up: |
| ... and [5 more](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=footer). Last update [d0fbb52...9492ed9](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569145791
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=h1) Report
> Merging [#6905](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/7d97d16c47ba181839fd583b8653d2872dc4290b?src=pr&el=desc) will **decrease** coverage by `0.17%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6905 +/- ##
==========================================
- Coverage 84.7% 84.52% -0.18%
==========================================
Files 680 679 -1
Lines 38556 38492 -64
==========================================
- Hits 32659 32536 -123
- Misses 5897 5956 +59
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `88.11% <100%> (-0.33%)` | :arrow_down: |
| [airflow/bin/cli.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9iaW4vY2xpLnB5) | `96.7% <100%> (+2.65%)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/utils/cli.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9jbGkucHk=) | `68.75% <0%> (-2.95%)` | :arrow_down: |
| [airflow/cli/commands/dag\_command.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9jbGkvY29tbWFuZHMvZGFnX2NvbW1hbmQucHk=) | `86.2% <0%> (-0.51%)` | :arrow_down: |
| [airflow/cli/commands/task\_command.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9jbGkvY29tbWFuZHMvdGFza19jb21tYW5kLnB5) | `69.93% <0%> (-0.29%)` | :arrow_down: |
| ... and [7 more](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=footer). Last update [7d97d16...a992c13](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569145791
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=h1) Report
> Merging [#6905](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6fffa5b0d7840727a96dc1765a0166656bc7ea52?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6905 +/- ##
==========================================
- Coverage 84.72% 84.44% -0.29%
==========================================
Files 679 679
Lines 38505 38504 -1
==========================================
- Hits 32623 32514 -109
- Misses 5882 5990 +108
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `88.51% <100%> (-0.08%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/bin/cli.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9iaW4vY2xpLnB5) | `96.05% <0%> (+2.63%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=footer). Last update [6fffa5b...9492ed9](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] stale[bot] closed pull request #6905: [AIRFLOW-6361] Run
LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
stale[bot] closed pull request #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on issue #6905: [AIRFLOW-6361] Run
LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569713841
> @ashb I tested locally and this does not happen because a new Dagbag is created each time CLI command is called.
Oh yes, we recreate a new dag bag. The only thing that would be preloaded then is modules or plugins. This is okay, but should be written down somewhere. (I'm not sure where - we don't have anything like this in the docs right now)
> I would also like to make LocalTaskJob independent of DAG and operator and only use information from TaskInstance. It seems to me that there is no need to load the entire DAG in LocalTaskJob. This can further increase performance.
Yes totally!
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569145791
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=h1) Report
> Merging [#6905](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/7d97d16c47ba181839fd583b8653d2872dc4290b?src=pr&el=desc) will **increase** coverage by `0.03%`.
> The diff coverage is `70%`.
[](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6905 +/- ##
==========================================
+ Coverage 84.7% 84.74% +0.03%
==========================================
Files 680 680
Lines 38556 38755 +199
==========================================
+ Hits 32659 32841 +182
- Misses 5897 5914 +17
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/task/task\_runner/standard\_task\_runner.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy90YXNrL3Rhc2tfcnVubmVyL3N0YW5kYXJkX3Rhc2tfcnVubmVyLnB5) | `73.58% <0%> (+6.91%)` | :arrow_up: |
| [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `88.11% <100%> (-0.33%)` | :arrow_down: |
| [airflow/bin/cli.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9iaW4vY2xpLnB5) | `92.78% <73.33%> (-1.27%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/utils/file.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9maWxlLnB5) | `86.3% <0%> (-5.17%)` | :arrow_down: |
| [airflow/utils/cli.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9jbGkucHk=) | `68.75% <0%> (-2.95%)` | :arrow_down: |
| ... and [90 more](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=footer). Last update [7d97d16...530125d](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] kaxil commented on a change in pull request #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#discussion_r362008246
##########
File path: airflow/executors/celery_executor.py
##########
@@ -63,13 +62,9 @@ def execute_command(command_to_exec: CommandType) -> None:
"""Executes command."""
log = LoggingMixin().log
log.info("Executing command in Celery: %s", command_to_exec)
- env = os.environ.copy()
try:
- subprocess.check_call(command_to_exec, stderr=subprocess.STDOUT,
Review comment:
That would also mean all the Connections created via Environment variables won't work unless LocalTaskJob copies the current vars
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on issue #6905: [AIRFLOW-6361] Run
LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569718112
@ashb
1. https://github.com/apache/airflow/blob/master/airflow/cli/commands/task_command.py#L142
2. https://github.com/apache/airflow/blob/master/airflow/utils/cli.py#L155
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569145791
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=h1) Report
> Merging [#6905](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/d0fbb523d617316a35461e6ee53781a7a716092c?src=pr&el=desc) will **decrease** coverage by `0.18%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6905 +/- ##
==========================================
- Coverage 84.62% 84.44% -0.19%
==========================================
Files 679 679
Lines 38497 38504 +7
==========================================
- Hits 32578 32514 -64
- Misses 5919 5990 +71
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `88.51% <100%> (-0.08%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/config\_templates/airflow\_local\_settings.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb25maWdfdGVtcGxhdGVzL2FpcmZsb3dfbG9jYWxfc2V0dGluZ3MucHk=) | `71.73% <0%> (ø)` | :arrow_up: |
| [airflow/models/taskreschedule.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvdGFza3Jlc2NoZWR1bGUucHk=) | `100% <0%> (ø)` | :arrow_up: |
| [airflow/configuration.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb25maWd1cmF0aW9uLnB5) | `92.65% <0%> (+0.05%)` | :arrow_up: |
| [airflow/executors/kubernetes\_executor.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMva3ViZXJuZXRlc19leGVjdXRvci5weQ==) | `57.48% <0%> (+0.09%)` | :arrow_up: |
| ... and [5 more](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=footer). Last update [d0fbb52...9492ed9](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6905:
[AIRFLOW-6361] Run LocalTaskJob directly in Celery task
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6905: [AIRFLOW-6361] Run LocalTaskJob directly in Celery task
URL: https://github.com/apache/airflow/pull/6905#issuecomment-569145791
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=h1) Report
> Merging [#6905](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/7d97d16c47ba181839fd583b8653d2872dc4290b?src=pr&el=desc) will **decrease** coverage by `0.17%`.
> The diff coverage is `83.33%`.
[](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6905 +/- ##
==========================================
- Coverage 84.7% 84.52% -0.18%
==========================================
Files 680 679 -1
Lines 38556 38492 -64
==========================================
- Hits 32659 32536 -123
- Misses 5897 5956 +59
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/bin/cli.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9iaW4vY2xpLnB5) | `96.7% <100%> (+2.65%)` | :arrow_up: |
| [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `88.11% <33.33%> (-0.33%)` | :arrow_down: |
| [airflow/operators/postgres\_operator.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvcG9zdGdyZXNfb3BlcmF0b3IucHk=) | `100% <0%> (ø)` | :arrow_up: |
| [airflow/operators/mysql\_operator.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXlzcWxfb3BlcmF0b3IucHk=) | `100% <0%> (ø)` | :arrow_up: |
| [airflow/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXlzcWxfdG9faGl2ZS5weQ==) | `100% <0%> (ø)` | :arrow_up: |
| [airflow/operators/generic\_transfer.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvZ2VuZXJpY190cmFuc2Zlci5weQ==) | `100% <0%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| ... and [21 more](https://codecov.io/gh/apache/airflow/pull/6905/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=footer). Last update [7d97d16...a992c13](https://codecov.io/gh/apache/airflow/pull/6905?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services