[GitHub] [airflow] kaxil commented on issue #6697: [AIRFLOW-6135] Extract DAG processing from SchedulerJob into separate class

2019-12-02 Thread GitBox
kaxil commented on issue #6697: [AIRFLOW-6135] Extract DAG processing from 
SchedulerJob into separate class
URL: https://github.com/apache/airflow/pull/6697#issuecomment-560911546
 
 
   > @kaxil I agree, but for now I am trying to solve one problem. This change 
is already very big - +2,256 −2,169 In the near future I will probably make 
other changes in the schedule to improve it. This is one of the first because 
customers complain about problems with it.
   
   Sure, I am definitely fine with it :) 
   
   Keep the PRs flowing, you have been doing a really good work


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] codecov-io edited a comment on issue #6702: [AIRFLOW-6140] Add missing types for some core classes. Depends on [AIRFLOW-6004]

2019-12-02 Thread GitBox
codecov-io edited a comment on issue #6702: [AIRFLOW-6140] Add missing types 
for some core classes. Depends on [AIRFLOW-6004]
URL: https://github.com/apache/airflow/pull/6702#issuecomment-560257785
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6702?src=pr=h1) 
Report
   > Merging 
[#6702](https://codecov.io/gh/apache/airflow/pull/6702?src=pr=desc) into 
[master](https://codecov.io/gh/apache/airflow/commit/91eb87112b343ce4c18a994a41703bf793125dca?src=pr=desc)
 will **increase** coverage by `0.03%`.
   > The diff coverage is `84.66%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/airflow/pull/6702/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/6702?src=pr=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master#6702  +/-   ##
   ==
   + Coverage   83.82%   83.85%   +0.03% 
   ==
 Files 668  669   +1 
 Lines   3756937726 +157 
   ==
   + Hits3149431637 +143 
   - Misses   6075 6089  +14
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/airflow/pull/6702?src=pr=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[airflow/configuration.py](https://codecov.io/gh/apache/airflow/pull/6702/diff?src=pr=tree#diff-YWlyZmxvdy9jb25maWd1cmF0aW9uLnB5)
 | `92.75% <ø> (ø)` | :arrow_up: |
   | 
[airflow/operators/mssql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/6702/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXNzcWxfdG9faGl2ZS5weQ==)
 | `100% <ø> (ø)` | :arrow_up: |
   | 
[airflow/kubernetes/kube\_client.py](https://codecov.io/gh/apache/airflow/pull/6702/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL2t1YmVfY2xpZW50LnB5)
 | `85.71% <ø> (ø)` | :arrow_up: |
   | 
[airflow/cli/commands/flower\_command.py](https://codecov.io/gh/apache/airflow/pull/6702/diff?src=pr=tree#diff-YWlyZmxvdy9jbGkvY29tbWFuZHMvZmxvd2VyX2NvbW1hbmQucHk=)
 | `0% <0%> (ø)` | :arrow_up: |
   | 
[airflow/cli/commands/serve\_logs\_command.py](https://codecov.io/gh/apache/airflow/pull/6702/diff?src=pr=tree#diff-YWlyZmxvdy9jbGkvY29tbWFuZHMvc2VydmVfbG9nc19jb21tYW5kLnB5)
 | `0% <0%> (ø)` | :arrow_up: |
   | 
[airflow/jobs/base\_job.py](https://codecov.io/gh/apache/airflow/pull/6702/diff?src=pr=tree#diff-YWlyZmxvdy9qb2JzL2Jhc2Vfam9iLnB5)
 | `88.81% <100%> (+0.07%)` | :arrow_up: |
   | 
[airflow/settings.py](https://codecov.io/gh/apache/airflow/pull/6702/diff?src=pr=tree#diff-YWlyZmxvdy9zZXR0aW5ncy5weQ==)
 | `88.81% <100%> (ø)` | :arrow_up: |
   | 
[airflow/models/dagbag.py](https://codecov.io/gh/apache/airflow/pull/6702/diff?src=pr=tree#diff-YWlyZmxvdy9tb2RlbHMvZGFnYmFnLnB5)
 | `86.58% <100%> (+0.1%)` | :arrow_up: |
   | 
[airflow/models/kubernetes.py](https://codecov.io/gh/apache/airflow/pull/6702/diff?src=pr=tree#diff-YWlyZmxvdy9tb2RlbHMva3ViZXJuZXRlcy5weQ==)
 | `100% <100%> (ø)` | :arrow_up: |
   | 
[airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/6702/diff?src=pr=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==)
 | `91.48% <100%> (+0.05%)` | :arrow_up: |
   | ... and [34 
more](https://codecov.io/gh/apache/airflow/pull/6702/diff?src=pr=tree-more) 
| |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/airflow/pull/6702?src=pr=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/airflow/pull/6702?src=pr=footer). 
Last update 
[91eb871...3e30d44](https://codecov.io/gh/apache/airflow/pull/6702?src=pr=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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] mik-laj opened a new pull request #6717: [AIRFLOW-6160] Move _check_and_change_state_before_execution to LocalTaskJob

2019-12-02 Thread GitBox
mik-laj opened a new pull request #6717: [AIRFLOW-6160] Move 
_check_and_change_state_before_execution to LocalTaskJob
URL: https://github.com/apache/airflow/pull/6717
 
 
   Hello
   
   I think that LocaclTaskJob should control which tasks are to be executed. 
SchedulerJob and BackfillingJob behave similarly.  Otherwise, we have 
inconsistencies in the way these methods are placed, and the execution rules 
from Scheduler and Backfilling should be moved to TaskInstance. However, I 
think it's better when Jobs implements specific logic.
   
   Best regards
   
   ---
   
   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-6160
 - 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:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Commented] (AIRFLOW-6160) Move _check_and_change_state_before_execution to LocalTaskJob

2019-12-02 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/AIRFLOW-6160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16986485#comment-16986485
 ] 

ASF GitHub Bot commented on AIRFLOW-6160:
-

mik-laj commented on pull request #6717: [AIRFLOW-6160] Move 
_check_and_change_state_before_execution to LocalTaskJob
URL: https://github.com/apache/airflow/pull/6717
 
 
   Hello
   
   I think that LocaclTaskJob should control which tasks are to be executed. 
SchedulerJob and BackfillingJob behave similarly.  Otherwise, we have 
inconsistencies in the way these methods are placed, and the execution rules 
from Scheduler and Backfilling should be moved to TaskInstance. However, I 
think it's better when Jobs implements specific logic.
   
   Best regards
   
   ---
   
   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-6160
 - 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:
us...@infra.apache.org


> Move _check_and_change_state_before_execution to LocalTaskJob
> -
>
> Key: AIRFLOW-6160
> URL: https://issues.apache.org/jira/browse/AIRFLOW-6160
> Project: Apache Airflow
>  Issue Type: Bug
>  Components: scheduler
>Affects Versions: 1.10.6
>Reporter: Kamil Bregula
>Priority: Major
>
> Hello
> I think that LocaclTaskJob should control which tasks are to be executed. 
> SchedulerJob and BackfillingJob behave similarly.
> Best regards



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [airflow] codecov-io edited a comment on issue #6696: [AIRFLOW-6128] Simplify plugins_manager and webserver plugin code. Based on [AIRFLOW-6004] [AIRFLOW-6140]

2019-12-02 Thread GitBox
codecov-io edited a comment on issue #6696: [AIRFLOW-6128] Simplify 
plugins_manager and webserver plugin code. Based on [AIRFLOW-6004] 
[AIRFLOW-6140]
URL: https://github.com/apache/airflow/pull/6696#issuecomment-560183548
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6696?src=pr=h1) 
Report
   > Merging 
[#6696](https://codecov.io/gh/apache/airflow/pull/6696?src=pr=desc) into 
[master](https://codecov.io/gh/apache/airflow/commit/91eb87112b343ce4c18a994a41703bf793125dca?src=pr=desc)
 will **decrease** coverage by `75.04%`.
   > The diff coverage is `41.5%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/airflow/pull/6696/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/6696?src=pr=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master   #6696   +/-   ##
   ==
   - Coverage   83.82%   8.78%   -75.05% 
   ==
 Files 668 664-4 
 Lines   37569   37431  -138 
   ==
   - Hits314943288-28206 
   - Misses   6075   34143+28068
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/airflow/pull/6696?src=pr=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[airflow/configuration.py](https://codecov.io/gh/apache/airflow/pull/6696/diff?src=pr=tree#diff-YWlyZmxvdy9jb25maWd1cmF0aW9uLnB5)
 | `58.33% <ø> (-34.43%)` | :arrow_down: |
   | 
[airflow/operators/mssql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/6696/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXNzcWxfdG9faGl2ZS5weQ==)
 | `0% <ø> (-100%)` | :arrow_down: |
   | 
[airflow/kubernetes/kube\_client.py](https://codecov.io/gh/apache/airflow/pull/6696/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL2t1YmVfY2xpZW50LnB5)
 | `77.14% <ø> (-8.58%)` | :arrow_down: |
   | 
[airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/6696/diff?src=pr=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5)
 | `0% <0%> (-76.12%)` | :arrow_down: |
   | 
[airflow/cli/commands/worker\_command.py](https://codecov.io/gh/apache/airflow/pull/6696/diff?src=pr=tree#diff-YWlyZmxvdy9jbGkvY29tbWFuZHMvd29ya2VyX2NvbW1hbmQucHk=)
 | `0% <0%> (-38.1%)` | :arrow_down: |
   | 
[airflow/cli/commands/flower\_command.py](https://codecov.io/gh/apache/airflow/pull/6696/diff?src=pr=tree#diff-YWlyZmxvdy9jbGkvY29tbWFuZHMvZmxvd2VyX2NvbW1hbmQucHk=)
 | `0% <0%> (ø)` | :arrow_up: |
   | 
[airflow/executors/dask\_executor.py](https://codecov.io/gh/apache/airflow/pull/6696/diff?src=pr=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvZGFza19leGVjdXRvci5weQ==)
 | `0% <0%> (-6%)` | :arrow_down: |
   | 
[airflow/executors/kubernetes\_executor.py](https://codecov.io/gh/apache/airflow/pull/6696/diff?src=pr=tree#diff-YWlyZmxvdy9leGVjdXRvcnMva3ViZXJuZXRlc19leGVjdXRvci5weQ==)
 | `0% <0%> (-59%)` | :arrow_down: |
   | 
[airflow/cli/commands/task\_command.py](https://codecov.io/gh/apache/airflow/pull/6696/diff?src=pr=tree#diff-YWlyZmxvdy9jbGkvY29tbWFuZHMvdGFza19jb21tYW5kLnB5)
 | `0% <0%> (-63.24%)` | :arrow_down: |
   | 
[airflow/macros/\_\_init\_\_.py](https://codecov.io/gh/apache/airflow/pull/6696/diff?src=pr=tree#diff-YWlyZmxvdy9tYWNyb3MvX19pbml0X18ucHk=)
 | `0% <0%> (-86.37%)` | :arrow_down: |
   | ... and [626 
more](https://codecov.io/gh/apache/airflow/pull/6696/diff?src=pr=tree-more) 
| |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/airflow/pull/6696?src=pr=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/airflow/pull/6696?src=pr=footer). 
Last update 
[91eb871...d400eb1](https://codecov.io/gh/apache/airflow/pull/6696?src=pr=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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] codecov-io edited a comment on issue #6599: [AIRFLOW-6007] Check providers instead of provider package. Depends on [AIRFLOW-6004]

2019-12-02 Thread GitBox
codecov-io edited a comment on issue #6599: [AIRFLOW-6007] Check providers 
instead of provider package. Depends on [AIRFLOW-6004]
URL: https://github.com/apache/airflow/pull/6599#issuecomment-560146249
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6599?src=pr=h1) 
Report
   > Merging 
[#6599](https://codecov.io/gh/apache/airflow/pull/6599?src=pr=desc) into 
[master](https://codecov.io/gh/apache/airflow/commit/91eb87112b343ce4c18a994a41703bf793125dca?src=pr=desc)
 will **increase** coverage by `0.02%`.
   > The diff coverage is `84.2%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/airflow/pull/6599/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/6599?src=pr=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master#6599  +/-   ##
   ==
   + Coverage   83.82%   83.85%   +0.02% 
   ==
 Files 668  669   +1 
 Lines   3756937692 +123 
   ==
   + Hits3149431606 +112 
   - Misses   6075 6086  +11
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/airflow/pull/6599?src=pr=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[airflow/operators/mssql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/6599/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXNzcWxfdG9faGl2ZS5weQ==)
 | `100% <ø> (ø)` | :arrow_up: |
   | 
[airflow/kubernetes/kube\_client.py](https://codecov.io/gh/apache/airflow/pull/6599/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL2t1YmVfY2xpZW50LnB5)
 | `85.71% <ø> (ø)` | :arrow_up: |
   | 
[airflow/macros/\_\_init\_\_.py](https://codecov.io/gh/apache/airflow/pull/6599/diff?src=pr=tree#diff-YWlyZmxvdy9tYWNyb3MvX19pbml0X18ucHk=)
 | `81.25% <ø> (-5.12%)` | :arrow_down: |
   | 
[airflow/cli/commands/flower\_command.py](https://codecov.io/gh/apache/airflow/pull/6599/diff?src=pr=tree#diff-YWlyZmxvdy9jbGkvY29tbWFuZHMvZmxvd2VyX2NvbW1hbmQucHk=)
 | `0% <0%> (ø)` | :arrow_up: |
   | 
[airflow/cli/commands/serve\_logs\_command.py](https://codecov.io/gh/apache/airflow/pull/6599/diff?src=pr=tree#diff-YWlyZmxvdy9jbGkvY29tbWFuZHMvc2VydmVfbG9nc19jb21tYW5kLnB5)
 | `0% <0%> (ø)` | :arrow_up: |
   | 
[airflow/jobs/base\_job.py](https://codecov.io/gh/apache/airflow/pull/6599/diff?src=pr=tree#diff-YWlyZmxvdy9qb2JzL2Jhc2Vfam9iLnB5)
 | `88.81% <100%> (+0.07%)` | :arrow_up: |
   | 
[airflow/settings.py](https://codecov.io/gh/apache/airflow/pull/6599/diff?src=pr=tree#diff-YWlyZmxvdy9zZXR0aW5ncy5weQ==)
 | `88.81% <100%> (ø)` | :arrow_up: |
   | 
[airflow/models/dagbag.py](https://codecov.io/gh/apache/airflow/pull/6599/diff?src=pr=tree#diff-YWlyZmxvdy9tb2RlbHMvZGFnYmFnLnB5)
 | `86.58% <100%> (+0.1%)` | :arrow_up: |
   | 
[airflow/models/kubernetes.py](https://codecov.io/gh/apache/airflow/pull/6599/diff?src=pr=tree#diff-YWlyZmxvdy9tb2RlbHMva3ViZXJuZXRlcy5weQ==)
 | `100% <100%> (ø)` | :arrow_up: |
   | 
[airflow/models/baseoperator.py](https://codecov.io/gh/apache/airflow/pull/6599/diff?src=pr=tree#diff-YWlyZmxvdy9tb2RlbHMvYmFzZW9wZXJhdG9yLnB5)
 | `96.28% <100%> (ø)` | :arrow_up: |
   | ... and [30 
more](https://codecov.io/gh/apache/airflow/pull/6599/diff?src=pr=tree-more) 
| |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/airflow/pull/6599?src=pr=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/airflow/pull/6599?src=pr=footer). 
Last update 
[91eb871...b014b37](https://codecov.io/gh/apache/airflow/pull/6599?src=pr=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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] stale[bot] closed pull request #6288: AIRFLOW-5540 adding rise exception when the _resolve_connection method fails

2019-12-02 Thread GitBox
stale[bot] closed pull request #6288: AIRFLOW-5540 adding rise exception when 
the _resolve_connection  method fails
URL: https://github.com/apache/airflow/pull/6288
 
 
   


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:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Commented] (AIRFLOW-5540) task with SparkSubmitOperator does not fail if the spark job it executes fails.

2019-12-02 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/AIRFLOW-5540?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16986511#comment-16986511
 ] 

ASF GitHub Bot commented on AIRFLOW-5540:
-

stale[bot] commented on pull request #6288: AIRFLOW-5540 adding rise exception 
when the _resolve_connection  method fails
URL: https://github.com/apache/airflow/pull/6288
 
 
   
 

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:
us...@infra.apache.org


> task with SparkSubmitOperator  does not  fail if the  spark job it executes 
> fails.
> --
>
> Key: AIRFLOW-5540
> URL: https://issues.apache.org/jira/browse/AIRFLOW-5540
> Project: Apache Airflow
>  Issue Type: Wish
>  Components: operators
>Affects Versions: 1.10.3
> Environment: RHEL 7.3 with default airflow  installation.
>Reporter: Juan M George
>Assignee: Adan Christian Rosales Ornelas
>Priority: Major
> Attachments: airfow_issues
>
>
> In  my  test Dag, I have a task that uses SparkSubmitOperator operator to 
> execute a spark job  that  reads from a table in a database and  does some 
> processing and write into a file.   In  a scenario, where source table from 
> where  I read data does not exist, the spark job fails but the  task  is 
> still be shown as  executed successfully.  I don't see any other way to 
> handle this business logic failure from the operator side.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (AIRFLOW-6014) Kubernetes executor - handle preempted deleted pods - queued tasks

2019-12-02 Thread afusr (Jira)


[ 
https://issues.apache.org/jira/browse/AIRFLOW-6014?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16986672#comment-16986672
 ] 

afusr commented on AIRFLOW-6014:


[~dimberman] Do you think this can be merged? This fixed has saved us a number 
of times now, by catching pods which have failed to start, and rescheduling 
them. 

> Kubernetes executor - handle preempted deleted pods - queued tasks
> --
>
> Key: AIRFLOW-6014
> URL: https://issues.apache.org/jira/browse/AIRFLOW-6014
> Project: Apache Airflow
>  Issue Type: Improvement
>  Components: executor-kubernetes
>Affects Versions: 1.10.6
>Reporter: afusr
>Assignee: Daniel Imberman
>Priority: Minor
>
> We have encountered an issue whereby when using the kubernetes executor, and 
> using autoscaling, airflow pods are preempted and airflow never attempts to 
> rerun these pods. 
> This is partly as a result of having the following set on the pod spec:
> restartPolicy: Never
> This makes sense as if a pod fails when running a task, we don't want 
> kubernetes to retry it, as this should be controlled by airflow. 
> What we believe happens is that when a new node is added by autoscaling, 
> kubernetes schedules a number of airflow pods onto the new node, as well as 
> any pods required by k8s/daemon sets. As these are higher priority, the 
> Airflow pods are preempted, and deleted. You see messages such as:
>  
> Preempted by kube-system/ip-masq-agent-xz77q on node 
> gke-some--airflow--node-1ltl
>  
> Within the kubernetes executor, these pods end up in a status of pending and 
> an event of deleted is received but not handled. 
> The end result is tasks remain in a queued state forever. 
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [airflow] mik-laj commented on a change in pull request #6697: [AIRFLOW-6135] Extract DAG processing from SchedulerJob into separate class

2019-12-02 Thread GitBox
mik-laj commented on a change in pull request #6697: [AIRFLOW-6135] Extract DAG 
processing from SchedulerJob into separate class
URL: https://github.com/apache/airflow/pull/6697#discussion_r352892463
 
 

 ##
 File path: airflow/jobs/scheduler_job.py
 ##
 @@ -54,8 +54,9 @@
 from airflow.utils.state import State
 
 
-class DagFileProcessor(AbstractDagFileProcessor, LoggingMixin):
-"""Helps call SchedulerJob.process_file() in a separate process.
+class DagFileProcessorRunner(AbstractDagFileProcessorRunner, LoggingMixin):
 
 Review comment:
   I would like to separate classes that are executed in different threads and 
processes. New process = New class. This will greatly facilitate the 
understanding of the code. If we combine it, there will be a lot of methods 
available, but some intended only for execution from the main process  and 
others from the child 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:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Created] (AIRFLOW-6160) Move _check_and_change_state_before_execution to LocalTaskJob

2019-12-02 Thread Kamil Bregula (Jira)
Kamil Bregula created AIRFLOW-6160:
--

 Summary: Move _check_and_change_state_before_execution to 
LocalTaskJob
 Key: AIRFLOW-6160
 URL: https://issues.apache.org/jira/browse/AIRFLOW-6160
 Project: Apache Airflow
  Issue Type: Bug
  Components: scheduler
Affects Versions: 1.10.6
Reporter: Kamil Bregula


Hello

I think that LocaclTaskJob should control which tasks are to be executed. 
SchedulerJob and BackfillingJob behave similarly.

Best regards



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352942991
 
 

 ##
 File path: airflow/executors/kubernetes_executor.py
 ##
 @@ -256,15 +256,15 @@ class KubernetesJobWatcher(multiprocessing.Process, 
LoggingMixin):
 def __init__(self,
  namespace: str,
  watcher_queue: 'Queue[KubernetesWatchType]',
- resource_version: str,
+ resource_version: Optional[str],
  worker_uuid: Optional[str],
  kube_config: Configuration):
 multiprocessing.Process.__init__(self)
-self.namespace: str = namespace
 
 Review comment:
   Not removing - just removing the types. The types are inferred from the 
__init__ parameters so not needed to be duplicated.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352942604
 
 

 ##
 File path: airflow/__init__.py
 ##
 @@ -44,23 +44,8 @@
 
 settings.initialize()
 
-login = None  # type: Optional[Callable]
+from airflow.plugins_manager import integrate_plugins
 
-from airflow import executors
-from airflow import hooks
-from airflow import macros
-from airflow import operators
-from airflow import sensors
 
 Review comment:
   yeah!


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352942743
 
 

 ##
 File path: airflow/executors/dask_executor.py
 ##
 @@ -66,20 +63,17 @@ def execute_async(self,
   command: CommandType,
   queue: Optional[str] = None,
   executor_config: Optional[Any] = None) -> None:
-if not self.futures:
-raise AirflowException("Executor should be started first.")
+assert self.futures, NOT_STARTED_MESSAGE
 
 Review comment:
   I am happy to discuss it and see other's opinion there.  I was myself 
doubtful about asserts but they are really nicer and in many cases they simply 
make sense (see above) my comments. And if we discourage asserts we should 
remove them everywhere (they were used in other parts of the code).


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:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Resolved] (AIRFLOW-6069) Host Travis build should use always python 3.6

2019-12-02 Thread Kaxil Naik (Jira)


 [ 
https://issues.apache.org/jira/browse/AIRFLOW-6069?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Kaxil Naik resolved AIRFLOW-6069.
-
Resolution: Fixed

> Host Travis build should use always python 3.6
> --
>
> Key: AIRFLOW-6069
> URL: https://issues.apache.org/jira/browse/AIRFLOW-6069
> Project: Apache Airflow
>  Issue Type: Improvement
>  Components: ci
>Affects Versions: 2.0.0, 1.10.6
>Reporter: Jarek Potiuk
>Priority: Major
>
> Host python version for Travis can be standardized to python 3.6 - which will 
> make the pre-commit checks and scripts more robust. PYTHON_VERSION sets the 
> version used inside CI containers to run the tests instead.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [airflow] codecov-io edited a comment on issue #6702: [AIRFLOW-6140] Add missing types for some core classes. Depends on [AIRFLOW-6004]

2019-12-02 Thread GitBox
codecov-io edited a comment on issue #6702: [AIRFLOW-6140] Add missing types 
for some core classes. Depends on [AIRFLOW-6004]
URL: https://github.com/apache/airflow/pull/6702#issuecomment-560257785
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6702?src=pr=h1) 
Report
   > Merging 
[#6702](https://codecov.io/gh/apache/airflow/pull/6702?src=pr=desc) into 
[master](https://codecov.io/gh/apache/airflow/commit/91eb87112b343ce4c18a994a41703bf793125dca?src=pr=desc)
 will **increase** coverage by `0.03%`.
   > The diff coverage is `84.66%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/airflow/pull/6702/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/6702?src=pr=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master#6702  +/-   ##
   ==
   + Coverage   83.82%   83.85%   +0.03% 
   ==
 Files 668  669   +1 
 Lines   3756937726 +157 
   ==
   + Hits3149431637 +143 
   - Misses   6075 6089  +14
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/airflow/pull/6702?src=pr=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[airflow/configuration.py](https://codecov.io/gh/apache/airflow/pull/6702/diff?src=pr=tree#diff-YWlyZmxvdy9jb25maWd1cmF0aW9uLnB5)
 | `92.75% <ø> (ø)` | :arrow_up: |
   | 
[airflow/operators/mssql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/6702/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXNzcWxfdG9faGl2ZS5weQ==)
 | `100% <ø> (ø)` | :arrow_up: |
   | 
[airflow/kubernetes/kube\_client.py](https://codecov.io/gh/apache/airflow/pull/6702/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL2t1YmVfY2xpZW50LnB5)
 | `85.71% <ø> (ø)` | :arrow_up: |
   | 
[airflow/cli/commands/flower\_command.py](https://codecov.io/gh/apache/airflow/pull/6702/diff?src=pr=tree#diff-YWlyZmxvdy9jbGkvY29tbWFuZHMvZmxvd2VyX2NvbW1hbmQucHk=)
 | `0% <0%> (ø)` | :arrow_up: |
   | 
[airflow/cli/commands/serve\_logs\_command.py](https://codecov.io/gh/apache/airflow/pull/6702/diff?src=pr=tree#diff-YWlyZmxvdy9jbGkvY29tbWFuZHMvc2VydmVfbG9nc19jb21tYW5kLnB5)
 | `0% <0%> (ø)` | :arrow_up: |
   | 
[airflow/jobs/base\_job.py](https://codecov.io/gh/apache/airflow/pull/6702/diff?src=pr=tree#diff-YWlyZmxvdy9qb2JzL2Jhc2Vfam9iLnB5)
 | `88.81% <100%> (+0.07%)` | :arrow_up: |
   | 
[airflow/settings.py](https://codecov.io/gh/apache/airflow/pull/6702/diff?src=pr=tree#diff-YWlyZmxvdy9zZXR0aW5ncy5weQ==)
 | `88.81% <100%> (ø)` | :arrow_up: |
   | 
[airflow/models/dagbag.py](https://codecov.io/gh/apache/airflow/pull/6702/diff?src=pr=tree#diff-YWlyZmxvdy9tb2RlbHMvZGFnYmFnLnB5)
 | `86.58% <100%> (+0.1%)` | :arrow_up: |
   | 
[airflow/models/kubernetes.py](https://codecov.io/gh/apache/airflow/pull/6702/diff?src=pr=tree#diff-YWlyZmxvdy9tb2RlbHMva3ViZXJuZXRlcy5weQ==)
 | `100% <100%> (ø)` | :arrow_up: |
   | 
[airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/6702/diff?src=pr=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==)
 | `91.48% <100%> (+0.05%)` | :arrow_up: |
   | ... and [34 
more](https://codecov.io/gh/apache/airflow/pull/6702/diff?src=pr=tree-more) 
| |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/airflow/pull/6702?src=pr=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/airflow/pull/6702?src=pr=footer). 
Last update 
[91eb871...3e30d44](https://codecov.io/gh/apache/airflow/pull/6702?src=pr=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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] kaxil commented on a change in pull request #6702: [AIRFLOW-6140] Add missing types for some core classes. Depends on [AIRFLOW-6004]

2019-12-02 Thread GitBox
kaxil commented on a change in pull request #6702: [AIRFLOW-6140] Add missing 
types for some core classes. Depends on [AIRFLOW-6004]
URL: https://github.com/apache/airflow/pull/6702#discussion_r352921900
 
 

 ##
 File path: airflow/executors/executor_loader.py
 ##
 @@ -25,11 +25,11 @@ class ExecutorLoader:
 Keeps constants for all the currently available executors.
 """
 
-LOCAL_EXECUTOR = "LocalExecutor"
-SEQUENTIAL_EXECUTOR = "SequentialExecutor"
-CELERY_EXECUTOR = "CeleryExecutor"
-DASK_EXECUTOR = "DaskExecutor"
-KUBERNETES_EXECUTOR = "KubernetesExecutor"
+LOCAL_EXECUTOR: str = "LocalExecutor"
+SEQUENTIAL_EXECUTOR: str = "SequentialExecutor"
+CELERY_EXECUTOR: str = "CeleryExecutor"
+DASK_EXECUTOR: str = "DaskExecutor"
+KUBERNETES_EXECUTOR: str = "KubernetesExecutor"
 
 Review comment:
   Do we need to add `str` here? this are static variables that would always 
contain string ??
   
   May be yes, may be no.
   
   Just laying out my thought


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] kaxil commented on a change in pull request #6702: [AIRFLOW-6140] Add missing types for some core classes. Depends on [AIRFLOW-6004]

2019-12-02 Thread GitBox
kaxil commented on a change in pull request #6702: [AIRFLOW-6140] Add missing 
types for some core classes. Depends on [AIRFLOW-6004]
URL: https://github.com/apache/airflow/pull/6702#discussion_r352922159
 
 

 ##
 File path: airflow/models/baseoperator.py
 ##
 @@ -869,11 +877,16 @@ def get_task_instances(self, start_date=None, 
end_date=None, session=None):
 .order_by(TaskInstance.execution_date)\
 .all()
 
-def get_flat_relative_ids(self, upstream=False, found_descendants=None):
+def get_flat_relative_ids(self,
+  upstream: bool = False,
+  found_descendants: Optional[Set[str]] = None) -> 
Set[str]:
 """
-Get a flat list of relatives' ids, either upstream or downstream.
+Get a flat set of relatives' ids, either upstream or downstream.
 """
 
+if not self._dag:
+return set()
 
 Review comment:
   When is this 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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352941360
 
 

 ##
 File path: airflow/executors/dask_executor.py
 ##
 @@ -36,9 +35,7 @@ def __init__(self, cluster_address=None):
 super().__init__(parallelism=0)
 if cluster_address is None:
 cluster_address = conf.get('dask', 'cluster_address')
-if not cluster_address:
-raise ValueError(
-'Please provide a Dask cluster address in airflow.cfg')
+assert cluster_address, 'Please provide a Dask cluster address in 
airflow.cfg'
 
 Review comment:
   If we do not want to use asserts we should (now we can with pre-commits) 
fail if someone adds it, not only remove them. 
   
   However I would not dismiss the asserts immediately and easily. They are so 
much nicer to read in the code.
   
   Indeed "-O" option removes the asserts but this is pretty much everything it 
does (from Python docs):
   
   > Remove assert statements and any code conditional on the value of 
__debug__. Augment the filename for compiled (bytecode) files by adding .opt-1 
before the .pyc extension (see PEP 488). See also PYTHONOPTIMIZE.
   
   I am not sure if we plan to use -O. However even if we use it, I think 
pretty much all those asserts are really of "This should never happen" case. 
Those places I removed the asserts were really the "impossible" cases that we 
should never get to. Even if we do, we will fail rather sooner than later when 
trying to execute a method of a None field. I am happy to review all the assert 
code and see if that's the case (it was like that in my places as it was really 
mypy complaining that Optional[type] has no some method when it's None. So the 
worse case we will just get slightly less meaningful message requiring to take 
a look at the source code.
   
   One comment:
   
   We do not use __debug__ anywhere in our code - I also checked all the 
libraries and there are just a few places where __debug__ is used:
   
   ```
   ./site-packages/_pytest/assertion/rewrite.py:PYC_EXT
   ./site-packages/bleach/_vendor/html5lib/_inputstream.py:
   ./site-packages/click/_unicodefun.py:
   ./site-packages/click/core.py:
   ./site-packages/click/types.py:
   ./site-packages/coverage/parser.py:
   ./site-packages/flask_openid.py:
   ./site-packages/future/backports/test/support.py:
   ./site-packages/jinja2/nodes.py:if
   ./site-packages/jinja2/runtime.py:
   ./site-packages/mypy/newsemanal/semanal.py:
   ./site-packages/mypy/semanal_pass1.py:
   ./site-packages/pandas/core/reshape/merge.py:if
   ./site-packages/parso/python/errors.py:
   ./site-packages/pip/_vendor/distlib/_backport/misc.py:
   ./site-packages/pip/_vendor/distlib/compat.py:
   ./site-packages/pip/_vendor/html5lib/_inputstream.py:
   ./site-packages/py2app/util.py:
   ./site-packages/pygments/lexers/modula2.py:
   ./site-packages/pygments/lexers/nit.py:
   ./site-packages/werkzeug/wrappers/base_response.py:
   ```
   
   
   
   
   


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352941360
 
 

 ##
 File path: airflow/executors/dask_executor.py
 ##
 @@ -36,9 +35,7 @@ def __init__(self, cluster_address=None):
 super().__init__(parallelism=0)
 if cluster_address is None:
 cluster_address = conf.get('dask', 'cluster_address')
-if not cluster_address:
-raise ValueError(
-'Please provide a Dask cluster address in airflow.cfg')
+assert cluster_address, 'Please provide a Dask cluster address in 
airflow.cfg'
 
 Review comment:
   If we do not want to use asserts we should (now we can with pre-commits) 
fail if someone adds it, not only remove them. 
   
   However I would not dismiss the asserts immediately and easily. They are so 
much nicer to read in the code.
   
   Indeed "-O" option removes the asserts but this is pretty much everything it 
does (from Python docs):
   
   > Remove assert statements and any code conditional on the value of _ 
_debug_ _. Augment the filename for compiled (bytecode) files by adding .opt-1 
before the .pyc extension (see PEP 488). See also PYTHONOPTIMIZE.
   
   I am not sure if we plan to use -O. However even if we use it, I think 
pretty much all those asserts are really of "This should never happen" case. 
Those places I removed the asserts were really the "impossible" cases that we 
should never get to. Even if we do, we will fail rather sooner than later when 
trying to execute a method of a None field. I am happy to review all the assert 
code and see if that's the case (it was like that in my places as it was really 
mypy complaining that Optional[type] has no some method when it's None. So the 
worse case we will just get slightly less meaningful message requiring to take 
a look at the source code.
   
   One comment:
   
   We do not use _ _debug_ _ anywhere in our code - I also checked all the 
libraries and there are just a few places where _ _debug_ _ is used:
   
   ```
   ./site-packages/_pytest/assertion/rewrite.py:PYC_EXT
   ./site-packages/bleach/_vendor/html5lib/_inputstream.py:
   ./site-packages/click/_unicodefun.py:
   ./site-packages/click/core.py:
   ./site-packages/click/types.py:
   ./site-packages/coverage/parser.py:
   ./site-packages/flask_openid.py:
   ./site-packages/future/backports/test/support.py:
   ./site-packages/jinja2/nodes.py:if
   ./site-packages/jinja2/runtime.py:
   ./site-packages/mypy/newsemanal/semanal.py:
   ./site-packages/mypy/semanal_pass1.py:
   ./site-packages/pandas/core/reshape/merge.py:if
   ./site-packages/parso/python/errors.py:
   ./site-packages/pip/_vendor/distlib/_backport/misc.py:
   ./site-packages/pip/_vendor/distlib/compat.py:
   ./site-packages/pip/_vendor/html5lib/_inputstream.py:
   ./site-packages/py2app/util.py:
   ./site-packages/pygments/lexers/modula2.py:
   ./site-packages/pygments/lexers/nit.py:
   ./site-packages/werkzeug/wrappers/base_response.py:
   ```
   
   
   
   
   


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352941360
 
 

 ##
 File path: airflow/executors/dask_executor.py
 ##
 @@ -36,9 +35,7 @@ def __init__(self, cluster_address=None):
 super().__init__(parallelism=0)
 if cluster_address is None:
 cluster_address = conf.get('dask', 'cluster_address')
-if not cluster_address:
-raise ValueError(
-'Please provide a Dask cluster address in airflow.cfg')
+assert cluster_address, 'Please provide a Dask cluster address in 
airflow.cfg'
 
 Review comment:
   If we do not want to use asserts we should (now we can with pre-commits) 
fail if someone adds it, not only remove them. 
   
   However I would not dismiss the asserts immediately and easily. They are so 
much nicer to read in the code.
   
   Indeed "-O" option removes the asserts but this is pretty much everything it 
does (from Python docs):
   
   > Remove assert statements and any code conditional on the value of _ _ 
debug _ _. Augment the filename for compiled (bytecode) files by adding .opt-1 
before the .pyc extension (see PEP 488). See also PYTHONOPTIMIZE.
   
   I am not sure if we plan to use -O. However even if we use it, I think 
pretty much all those asserts are really of "This should never happen" case. 
Those places I removed the asserts were really the "impossible" cases that we 
should never get to. Even if we do, we will fail rather sooner than later when 
trying to execute a method of a None field. I am happy to review all the assert 
code and see if that's the case (it was like that in my places as it was really 
mypy complaining that Optional[type] has no some method when it's None. So the 
worse case we will just get slightly less meaningful message requiring to take 
a look at the source code.
   
   One comment:
   
   We do not use _ _ debug _ _ anywhere in our code - I also checked all the 
libraries and there are just a few places where _ _ debug _ _ is used:
   
   ```
   ./site-packages/_pytest/assertion/rewrite.py:PYC_EXT
   ./site-packages/bleach/_vendor/html5lib/_inputstream.py:
   ./site-packages/click/_unicodefun.py:
   ./site-packages/click/core.py:
   ./site-packages/click/types.py:
   ./site-packages/coverage/parser.py:
   ./site-packages/flask_openid.py:
   ./site-packages/future/backports/test/support.py:
   ./site-packages/jinja2/nodes.py:if
   ./site-packages/jinja2/runtime.py:
   ./site-packages/mypy/newsemanal/semanal.py:
   ./site-packages/mypy/semanal_pass1.py:
   ./site-packages/pandas/core/reshape/merge.py:if
   ./site-packages/parso/python/errors.py:
   ./site-packages/pip/_vendor/distlib/_backport/misc.py:
   ./site-packages/pip/_vendor/distlib/compat.py:
   ./site-packages/pip/_vendor/html5lib/_inputstream.py:
   ./site-packages/py2app/util.py:
   ./site-packages/pygments/lexers/modula2.py:
   ./site-packages/pygments/lexers/nit.py:
   ./site-packages/werkzeug/wrappers/base_response.py:
   ```
   
   
   
   
   


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] codecov-io edited a comment on issue #6715: [AIRFLOW-5945] Make inbuilt OperatorLinks work when using Serialization

2019-12-02 Thread GitBox
codecov-io edited a comment on issue #6715: [AIRFLOW-5945] Make inbuilt 
OperatorLinks work when using Serialization
URL: https://github.com/apache/airflow/pull/6715#issuecomment-560962980
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6715?src=pr=h1) 
Report
   > Merging 
[#6715](https://codecov.io/gh/apache/airflow/pull/6715?src=pr=desc) into 
[master](https://codecov.io/gh/apache/airflow/commit/91eb87112b343ce4c18a994a41703bf793125dca?src=pr=desc)
 will **decrease** coverage by `0.3%`.
   > The diff coverage is `91.07%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/airflow/pull/6715/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/6715?src=pr=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master#6715  +/-   ##
   ==
   - Coverage   83.82%   83.52%   -0.31% 
   ==
 Files 668  668  
 Lines   3756937612  +43 
   ==
   - Hits3149431414  -80 
   - Misses   6075 6198 +123
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/airflow/pull/6715?src=pr=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[airflow/gcp/operators/bigquery.py](https://codecov.io/gh/apache/airflow/pull/6715/diff?src=pr=tree#diff-YWlyZmxvdy9nY3Avb3BlcmF0b3JzL2JpZ3F1ZXJ5LnB5)
 | `84.3% <100%> (+0.12%)` | :arrow_up: |
   | 
[airflow/utils/tests.py](https://codecov.io/gh/apache/airflow/pull/6715/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy90ZXN0cy5weQ==)
 | `83.33% <100%> (+1.93%)` | :arrow_up: |
   | 
[airflow/contrib/operators/qubole\_operator.py](https://codecov.io/gh/apache/airflow/pull/6715/diff?src=pr=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9xdWJvbGVfb3BlcmF0b3IucHk=)
 | `88.23% <100%> (+0.54%)` | :arrow_up: |
   | 
[airflow/models/baseoperator.py](https://codecov.io/gh/apache/airflow/pull/6715/diff?src=pr=tree#diff-YWlyZmxvdy9tb2RlbHMvYmFzZW9wZXJhdG9yLnB5)
 | `96.3% <100%> (+0.01%)` | :arrow_up: |
   | 
[airflow/serialization/base\_serialization.py](https://codecov.io/gh/apache/airflow/pull/6715/diff?src=pr=tree#diff-YWlyZmxvdy9zZXJpYWxpemF0aW9uL2Jhc2Vfc2VyaWFsaXphdGlvbi5weQ==)
 | `85.43% <40%> (-1.65%)` | :arrow_down: |
   | 
[airflow/serialization/serialized\_baseoperator.py](https://codecov.io/gh/apache/airflow/pull/6715/diff?src=pr=tree#diff-YWlyZmxvdy9zZXJpYWxpemF0aW9uL3NlcmlhbGl6ZWRfYmFzZW9wZXJhdG9yLnB5)
 | `96.55% <92.85%> (-1.81%)` | :arrow_down: |
   | 
[airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6715/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==)
 | `44.44% <0%> (-55.56%)` | :arrow_down: |
   | 
[airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6715/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==)
 | `52.94% <0%> (-47.06%)` | :arrow_down: |
   | 
[airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6715/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==)
 | `45.25% <0%> (-46.72%)` | :arrow_down: |
   | 
[airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6715/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5)
 | `50.98% <0%> (-23.53%)` | :arrow_down: |
   | ... and [5 
more](https://codecov.io/gh/apache/airflow/pull/6715/diff?src=pr=tree-more) 
| |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/airflow/pull/6715?src=pr=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/airflow/pull/6715?src=pr=footer). 
Last update 
[91eb871...92078da](https://codecov.io/gh/apache/airflow/pull/6715?src=pr=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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] codecov-io commented on issue #6715: [AIRFLOW-5945] Make inbuilt OperatorLinks work when using Serialization

2019-12-02 Thread GitBox
codecov-io commented on issue #6715: [AIRFLOW-5945] Make inbuilt OperatorLinks 
work when using Serialization
URL: https://github.com/apache/airflow/pull/6715#issuecomment-560962980
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6715?src=pr=h1) 
Report
   > Merging 
[#6715](https://codecov.io/gh/apache/airflow/pull/6715?src=pr=desc) into 
[master](https://codecov.io/gh/apache/airflow/commit/91eb87112b343ce4c18a994a41703bf793125dca?src=pr=desc)
 will **decrease** coverage by `0.54%`.
   > The diff coverage is `91.07%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/airflow/pull/6715/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/6715?src=pr=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master#6715  +/-   ##
   ==
   - Coverage   83.82%   83.28%   -0.55% 
   ==
 Files 668  668  
 Lines   3756937612  +43 
   ==
   - Hits3149431326 -168 
   - Misses   6075 6286 +211
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/airflow/pull/6715?src=pr=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[airflow/gcp/operators/bigquery.py](https://codecov.io/gh/apache/airflow/pull/6715/diff?src=pr=tree#diff-YWlyZmxvdy9nY3Avb3BlcmF0b3JzL2JpZ3F1ZXJ5LnB5)
 | `84.3% <100%> (+0.12%)` | :arrow_up: |
   | 
[airflow/utils/tests.py](https://codecov.io/gh/apache/airflow/pull/6715/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy90ZXN0cy5weQ==)
 | `83.33% <100%> (+1.93%)` | :arrow_up: |
   | 
[airflow/contrib/operators/qubole\_operator.py](https://codecov.io/gh/apache/airflow/pull/6715/diff?src=pr=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9xdWJvbGVfb3BlcmF0b3IucHk=)
 | `88.23% <100%> (+0.54%)` | :arrow_up: |
   | 
[airflow/models/baseoperator.py](https://codecov.io/gh/apache/airflow/pull/6715/diff?src=pr=tree#diff-YWlyZmxvdy9tb2RlbHMvYmFzZW9wZXJhdG9yLnB5)
 | `96.3% <100%> (+0.01%)` | :arrow_up: |
   | 
[airflow/serialization/base\_serialization.py](https://codecov.io/gh/apache/airflow/pull/6715/diff?src=pr=tree#diff-YWlyZmxvdy9zZXJpYWxpemF0aW9uL2Jhc2Vfc2VyaWFsaXphdGlvbi5weQ==)
 | `85.43% <40%> (-1.65%)` | :arrow_down: |
   | 
[airflow/serialization/serialized\_baseoperator.py](https://codecov.io/gh/apache/airflow/pull/6715/diff?src=pr=tree#diff-YWlyZmxvdy9zZXJpYWxpemF0aW9uL3NlcmlhbGl6ZWRfYmFzZW9wZXJhdG9yLnB5)
 | `96.55% <92.85%> (-1.81%)` | :arrow_down: |
   | 
[airflow/operators/mysql\_operator.py](https://codecov.io/gh/apache/airflow/pull/6715/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXlzcWxfb3BlcmF0b3IucHk=)
 | `0% <0%> (-100%)` | :arrow_down: |
   | 
[airflow/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/6715/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXlzcWxfdG9faGl2ZS5weQ==)
 | `0% <0%> (-100%)` | :arrow_down: |
   | 
[airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6715/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==)
 | `44.44% <0%> (-55.56%)` | :arrow_down: |
   | 
[airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6715/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==)
 | `52.94% <0%> (-47.06%)` | :arrow_down: |
   | ... and [11 
more](https://codecov.io/gh/apache/airflow/pull/6715/diff?src=pr=tree-more) 
| |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/airflow/pull/6715?src=pr=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/airflow/pull/6715?src=pr=footer). 
Last update 
[91eb871...92078da](https://codecov.io/gh/apache/airflow/pull/6715?src=pr=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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] kaxil merged pull request #6716: [AIRFLOW-6159] Change logging level of the heartbeat message to DEBUG

2019-12-02 Thread GitBox
kaxil merged pull request #6716: [AIRFLOW-6159] Change logging level of the 
heartbeat message to DEBUG
URL: https://github.com/apache/airflow/pull/6716
 
 
   


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] ITriangle closed pull request #6707: Fix task get log

2019-12-02 Thread GitBox
ITriangle closed pull request #6707: Fix task get log
URL: https://github.com/apache/airflow/pull/6707
 
 
   


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:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Resolved] (AIRFLOW-6159) Change logging level of the heartbeat message to DEBUG

2019-12-02 Thread Kaxil Naik (Jira)


 [ 
https://issues.apache.org/jira/browse/AIRFLOW-6159?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Kaxil Naik resolved AIRFLOW-6159.
-
Fix Version/s: 1.10.7
   Resolution: Fixed

> Change logging level of the heartbeat message to DEBUG 
> ---
>
> Key: AIRFLOW-6159
> URL: https://issues.apache.org/jira/browse/AIRFLOW-6159
> Project: Apache Airflow
>  Issue Type: Improvement
>  Components: logging
>Affects Versions: 1.10.6
>Reporter: Kaxil Naik
>Assignee: Kaxil Naik
>Priority: Critical
> Fix For: 1.10.7
>
>
> As mentioned in 
> https://stackoverflow.com/questions/59142508/airflow-disable-heartbeat-logs , 
> the logs containing heartbeat details can become too noisy. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (AIRFLOW-6159) Change logging level of the heartbeat message to DEBUG

2019-12-02 Thread ASF subversion and git services (Jira)


[ 
https://issues.apache.org/jira/browse/AIRFLOW-6159?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16986525#comment-16986525
 ] 

ASF subversion and git services commented on AIRFLOW-6159:
--

Commit 9fed459441ab94015f79b7b8ed6ee9f42a89cca8 in airflow's branch 
refs/heads/master from Kaxil Naik
[ https://gitbox.apache.org/repos/asf?p=airflow.git;h=9fed459 ]

[AIRFLOW-6159] Change logging level of the heartbeat message to DEBUG (#6716)



> Change logging level of the heartbeat message to DEBUG 
> ---
>
> Key: AIRFLOW-6159
> URL: https://issues.apache.org/jira/browse/AIRFLOW-6159
> Project: Apache Airflow
>  Issue Type: Improvement
>  Components: logging
>Affects Versions: 1.10.6
>Reporter: Kaxil Naik
>Assignee: Kaxil Naik
>Priority: Critical
> Fix For: 1.10.7
>
>
> As mentioned in 
> https://stackoverflow.com/questions/59142508/airflow-disable-heartbeat-logs , 
> the logs containing heartbeat details can become too noisy. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (AIRFLOW-6159) Change logging level of the heartbeat message to DEBUG

2019-12-02 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/AIRFLOW-6159?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16986524#comment-16986524
 ] 

ASF GitHub Bot commented on AIRFLOW-6159:
-

kaxil commented on pull request #6716: [AIRFLOW-6159] Change logging level of 
the heartbeat message to DEBUG
URL: https://github.com/apache/airflow/pull/6716
 
 
   
 

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:
us...@infra.apache.org


> Change logging level of the heartbeat message to DEBUG 
> ---
>
> Key: AIRFLOW-6159
> URL: https://issues.apache.org/jira/browse/AIRFLOW-6159
> Project: Apache Airflow
>  Issue Type: Improvement
>  Components: logging
>Affects Versions: 1.10.6
>Reporter: Kaxil Naik
>Assignee: Kaxil Naik
>Priority: Critical
>
> As mentioned in 
> https://stackoverflow.com/questions/59142508/airflow-disable-heartbeat-logs , 
> the logs containing heartbeat details can become too noisy. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Created] (AIRFLOW-6161) GoogleCloudBaseHook delegate_to doesn't work with GCE service account

2019-12-02 Thread Guillaume Poulin (Jira)
Guillaume Poulin created AIRFLOW-6161:
-

 Summary: GoogleCloudBaseHook delegate_to doesn't work with GCE 
service account
 Key: AIRFLOW-6161
 URL: https://issues.apache.org/jira/browse/AIRFLOW-6161
 Project: Apache Airflow
  Issue Type: Bug
  Components: gcp
Affects Versions: 1.10.3, 2.0.0
 Environment: cloud composer
Reporter: Guillaume Poulin


Currently passing `delegate_to` to GoogleCloudBaseHook will cause a 
AttributeError if the credentials come from the GCE metadata since 
`google.auth.compute_engine.credentials.Credentials` doesn't have the method 
`with_subject`. This seems related to 
[https://github.com/googleapis/google-auth-library-python/issues/310]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [airflow] OmerJog commented on issue #6261: [AIRFLOW-5141] Added optional resource usage logging to KubernetesPod…

2019-12-02 Thread GitBox
OmerJog commented on issue #6261: [AIRFLOW-5141] Added optional resource usage 
logging to KubernetesPod…
URL: https://github.com/apache/airflow/pull/6261#issuecomment-561041583
 
 
   @Sinsin1367 there is a conflict to resolve


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:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Closed] (AIRFLOW-6097) AIP-21: Rename services

2019-12-02 Thread Kaxil Naik (Jira)


 [ 
https://issues.apache.org/jira/browse/AIRFLOW-6097?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Kaxil Naik closed AIRFLOW-6097.
---
Resolution: Implemented

Some of these are already implemented /changed. Have you checked the list of 
the ones that are already updated?

> AIP-21: Rename services
> ---
>
> Key: AIRFLOW-6097
> URL: https://issues.apache.org/jira/browse/AIRFLOW-6097
> Project: Apache Airflow
>  Issue Type: Improvement
>  Components: gcp
>Affects Versions: 1.10.5
>Reporter: Michał Słowikowski
>Assignee: Michał Słowikowski
>Priority: Minor
>
> Whole description about AIP-21 can be found here
> [https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-21%3A+Changes+in+import+paths]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (AIRFLOW-6097) AIP-21: Rename services

2019-12-02 Thread Jira


[ 
https://issues.apache.org/jira/browse/AIRFLOW-6097?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16986678#comment-16986678
 ] 

Michał Słowikowski commented on AIRFLOW-6097:
-

[~kaxilnaik] Yes, I have a list of modules that have to be changed and I am in 
doing it with [~kamil.bregula]. I will double check.

> AIP-21: Rename services
> ---
>
> Key: AIRFLOW-6097
> URL: https://issues.apache.org/jira/browse/AIRFLOW-6097
> Project: Apache Airflow
>  Issue Type: Improvement
>  Components: gcp
>Affects Versions: 1.10.5
>Reporter: Michał Słowikowski
>Assignee: Michał Słowikowski
>Priority: Minor
>
> Whole description about AIP-21 can be found here
> [https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-21%3A+Changes+in+import+paths]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [airflow] codecov-io edited a comment on issue #6599: [AIRFLOW-6007] Check providers instead of provider package. Depends on [AIRFLOW-6004]

2019-12-02 Thread GitBox
codecov-io edited a comment on issue #6599: [AIRFLOW-6007] Check providers 
instead of provider package. Depends on [AIRFLOW-6004]
URL: https://github.com/apache/airflow/pull/6599#issuecomment-560146249
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6599?src=pr=h1) 
Report
   > Merging 
[#6599](https://codecov.io/gh/apache/airflow/pull/6599?src=pr=desc) into 
[master](https://codecov.io/gh/apache/airflow/commit/91eb87112b343ce4c18a994a41703bf793125dca?src=pr=desc)
 will **decrease** coverage by `0.2%`.
   > The diff coverage is `84.2%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/airflow/pull/6599/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/6599?src=pr=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master#6599  +/-   ##
   ==
   - Coverage   83.82%   83.61%   -0.21% 
   ==
 Files 668  669   +1 
 Lines   3756937692 +123 
   ==
   + Hits3149431518  +24 
   - Misses   6075 6174  +99
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/airflow/pull/6599?src=pr=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[airflow/operators/mssql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/6599/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXNzcWxfdG9faGl2ZS5weQ==)
 | `100% <ø> (ø)` | :arrow_up: |
   | 
[airflow/kubernetes/kube\_client.py](https://codecov.io/gh/apache/airflow/pull/6599/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL2t1YmVfY2xpZW50LnB5)
 | `85.71% <ø> (ø)` | :arrow_up: |
   | 
[airflow/macros/\_\_init\_\_.py](https://codecov.io/gh/apache/airflow/pull/6599/diff?src=pr=tree#diff-YWlyZmxvdy9tYWNyb3MvX19pbml0X18ucHk=)
 | `81.25% <ø> (-5.12%)` | :arrow_down: |
   | 
[airflow/cli/commands/flower\_command.py](https://codecov.io/gh/apache/airflow/pull/6599/diff?src=pr=tree#diff-YWlyZmxvdy9jbGkvY29tbWFuZHMvZmxvd2VyX2NvbW1hbmQucHk=)
 | `0% <0%> (ø)` | :arrow_up: |
   | 
[airflow/cli/commands/serve\_logs\_command.py](https://codecov.io/gh/apache/airflow/pull/6599/diff?src=pr=tree#diff-YWlyZmxvdy9jbGkvY29tbWFuZHMvc2VydmVfbG9nc19jb21tYW5kLnB5)
 | `0% <0%> (ø)` | :arrow_up: |
   | 
[airflow/jobs/base\_job.py](https://codecov.io/gh/apache/airflow/pull/6599/diff?src=pr=tree#diff-YWlyZmxvdy9qb2JzL2Jhc2Vfam9iLnB5)
 | `88.81% <100%> (+0.07%)` | :arrow_up: |
   | 
[airflow/settings.py](https://codecov.io/gh/apache/airflow/pull/6599/diff?src=pr=tree#diff-YWlyZmxvdy9zZXR0aW5ncy5weQ==)
 | `88.81% <100%> (ø)` | :arrow_up: |
   | 
[airflow/models/dagbag.py](https://codecov.io/gh/apache/airflow/pull/6599/diff?src=pr=tree#diff-YWlyZmxvdy9tb2RlbHMvZGFnYmFnLnB5)
 | `86.58% <100%> (+0.1%)` | :arrow_up: |
   | 
[airflow/models/kubernetes.py](https://codecov.io/gh/apache/airflow/pull/6599/diff?src=pr=tree#diff-YWlyZmxvdy9tb2RlbHMva3ViZXJuZXRlcy5weQ==)
 | `100% <100%> (ø)` | :arrow_up: |
   | 
[airflow/models/baseoperator.py](https://codecov.io/gh/apache/airflow/pull/6599/diff?src=pr=tree#diff-YWlyZmxvdy9tb2RlbHMvYmFzZW9wZXJhdG9yLnB5)
 | `96.28% <100%> (ø)` | :arrow_up: |
   | ... and [36 
more](https://codecov.io/gh/apache/airflow/pull/6599/diff?src=pr=tree-more) 
| |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/airflow/pull/6599?src=pr=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/airflow/pull/6599?src=pr=footer). 
Last update 
[91eb871...b014b37](https://codecov.io/gh/apache/airflow/pull/6599?src=pr=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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352943542
 
 

 ##
 File path: airflow/utils/dag_processing.py
 ##
 @@ -550,8 +549,8 @@ def __init__(self,
 self._parallelism = conf.getint('scheduler', 'max_threads')
 if 'sqlite' in conf.get('core', 'sql_alchemy_conn') and 
self._parallelism > 1:
 self.log.warning(
-f"Because we cannot use more than 1 thread (max_threads = 
{self._parallelism}) "
-"when using sqlite. So we set parallelism to 1."
+"Because we cannot use more than 1 thread (max_threads = "
 
 Review comment:
   It's not me. It's pylint :).


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352943496
 
 

 ##
 File path: airflow/logging_config.py
 ##
 @@ -1,3 +1,4 @@
+# -*- coding: utf-8 -*-
 
 Review comment:
   Nope. but I added it back as it was the only line changed in the original 
commit. It's a fixup only to revert the main change.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] shaikshakeel commented on issue #6469: [AIRFLOW-5816] S3 to snowflake operator

2019-12-02 Thread GitBox
shaikshakeel commented on issue #6469: [AIRFLOW-5816] S3 to snowflake operator
URL: https://github.com/apache/airflow/pull/6469#issuecomment-560502774
 
 
   @mik-laj @potiuk  can you confirm this. I will change accordingly 


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352740146
 
 

 ##
 File path: airflow/executors/celery_executor.py
 ##
 @@ -264,30 +284,45 @@ def sync(self):
 )
 continue
 key, state = key_and_state
-try:
-if self.last_state[key] != state:
-if state == celery_states.SUCCESS:
-self.success(key)
-del self.tasks[key]
-del self.last_state[key]
-elif state == celery_states.FAILURE:
-self.fail(key)
-del self.tasks[key]
-del self.last_state[key]
-elif state == celery_states.REVOKED:
-self.fail(key)
-del self.tasks[key]
-del self.last_state[key]
-else:
-self.log.info("Unexpected state: %s", state)
-self.last_state[key] = state
-except Exception:
-self.log.exception("Error syncing the Celery executor, 
ignoring it.")
-
-def end(self, synchronous=False):
+self.update_task_state(key, state)
+
+# noinspection PyUnreachableCode
 
 Review comment:
   I removed it. The problem was that PyCharm thinks that "fail()" a few lines 
below is something that will throw an exception. ¯\_(ツ)_/¯ . Just by the method 
name. I do not want to change the BaseExecutor API because of that. But I 
removed the comments as it is indeed misleading.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352741740
 
 

 ##
 File path: airflow/executors/dask_executor.py
 ##
 @@ -55,23 +58,28 @@ def start(self):
 else:
 security = None
 
-self.client = distributed.Client(self.cluster_address, 
security=security)
+self.client = Client(self.cluster_address, security=security)
 self.futures = {}
 
-def execute_async(self, key, command, queue=None, executor_config=None):
-if queue is not None:
-warnings.warn(
-'DaskExecutor does not support queues. '
-'All tasks will be run in the same cluster'
-)
+def execute_async(self,
+  key: TaskInstanceKeyType,
+  command: CommandType,
+  queue: Optional[str] = None,
+  executor_config: Optional[Any] = None) -> None:
+if not self.futures:
+raise AirflowException("Executor should be started first.")
 
 def airflow_run():
 return subprocess.check_call(command, close_fds=True)
 
+if not self.client:
+raise AirflowException("The Dask executor has not been started 
yet!")
 future = self.client.submit(airflow_run, pure=False)
 self.futures[future] = key
 
-def _process_future(self, future):
+def _process_future(self, future: Future) -> None:
+if not self.futures:
 
 Review comment:
   Ah yeah. Good catch. I replaced it in in all cases where I used the 
NOT_STARTED message but I missed a few places where I left the original message.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352742152
 
 

 ##
 File path: airflow/executors/kubernetes_executor.py
 ##
 @@ -601,18 +629,19 @@ def terminate(self):
 
 class KubernetesExecutor(BaseExecutor, LoggingMixin):
 """Executor for Kubernetes"""
+
 def __init__(self):
-self.kube_config = KubeConfig()
-self.task_queue = None
-self.result_queue = None
-self.kube_scheduler = None
-self.kube_client = None
-self.worker_uuid = None
+self.kube_config: KubeConfig = KubeConfig()
 
 Review comment:
   Yeah.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352743665
 
 

 ##
 File path: airflow/executors/kubernetes_executor.py
 ##
 @@ -241,17 +253,23 @@ def _validate(self):
 
 class KubernetesJobWatcher(multiprocessing.Process, LoggingMixin):
 """Watches for Kubernetes jobs"""
-def __init__(self, namespace, watcher_queue, resource_version, 
worker_uuid, kube_config):
+def __init__(self,
+ namespace: str,
+ watcher_queue: 'Queue[KubernetesWatchType]',
+ resource_version: str,
+ worker_uuid: Optional[str],
+ kube_config: Configuration):
 multiprocessing.Process.__init__(self)
-self.namespace = namespace
-self.worker_uuid = worker_uuid
-self.watcher_queue = watcher_queue
-self.resource_version = resource_version
-self.kube_config = kube_config
+self.namespace: str = namespace
 
 Review comment:
   True, I was a bit over-protective here :)


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] eladkal closed pull request #6703: [AIRFLOW-XXX] add notice for Mesos Executor deprecation in docs

2019-12-02 Thread GitBox
eladkal closed pull request #6703: [AIRFLOW-XXX] add notice for Mesos Executor 
deprecation in docs
URL: https://github.com/apache/airflow/pull/6703
 
 
   


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] eladkal opened a new pull request #6712: [AIRFLOW-XXX] add notice for Mesos Executor deprecation in docs

2019-12-02 Thread GitBox
eladkal opened a new pull request #6712:  [AIRFLOW-XXX] add notice for Mesos 
Executor deprecation in docs
URL: https://github.com/apache/airflow/pull/6712
 
 
   ### Description
   
   MesosExecutor is deprecated from Master 
https://github.com/apache/airflow/pull/5115 but there is no notice in docs of 
v1.10.X for that deprecation. Fresh users normally find information in docs 
(there is no need for them to read change log as they don't perform any 
upgrade).


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] codecov-io edited a comment on issue #6652: [AIRFLOW-5548] [AIRFLOW-5550] REST API enhancement - dag info, task …

2019-12-02 Thread GitBox
codecov-io edited a comment on issue #6652: [AIRFLOW-5548] [AIRFLOW-5550] REST 
API enhancement - dag info, task …
URL: https://github.com/apache/airflow/pull/6652#issuecomment-558277657
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6652?src=pr=h1) 
Report
   > Merging 
[#6652](https://codecov.io/gh/apache/airflow/pull/6652?src=pr=desc) into 
[master](https://codecov.io/gh/apache/airflow/commit/a86b6ac01166298d0d138512c3cb0c7dc68cba79?src=pr=desc)
 will **decrease** coverage by `0.53%`.
   > The diff coverage is `32.35%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/airflow/pull/6652/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/6652?src=pr=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master#6652  +/-   ##
   ==
   - Coverage   83.84%   83.31%   -0.54% 
   ==
 Files 668  671   +3 
 Lines   3756937670 +101 
   ==
   - Hits3149931384 -115 
   - Misses   6070 6286 +216
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/airflow/pull/6652?src=pr=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[airflow/www/api/experimental/endpoints.py](https://codecov.io/gh/apache/airflow/pull/6652/diff?src=pr=tree#diff-YWlyZmxvdy93d3cvYXBpL2V4cGVyaW1lbnRhbC9lbmRwb2ludHMucHk=)
 | `73.09% <23.72%> (-15.39%)` | :arrow_down: |
   | 
[airflow/api/common/experimental/get\_task.py](https://codecov.io/gh/apache/airflow/pull/6652/diff?src=pr=tree#diff-YWlyZmxvdy9hcGkvY29tbW9uL2V4cGVyaW1lbnRhbC9nZXRfdGFzay5weQ==)
 | `50% <28.57%> (-50%)` | :arrow_down: |
   | 
[airflow/api/common/experimental/\_\_init\_\_.py](https://codecov.io/gh/apache/airflow/pull/6652/diff?src=pr=tree#diff-YWlyZmxvdy9hcGkvY29tbW9uL2V4cGVyaW1lbnRhbC9fX2luaXRfXy5weQ==)
 | `91.66% <33.33%> (-8.34%)` | :arrow_down: |
   | 
[airflow/api/common/experimental/get\_dags.py](https://codecov.io/gh/apache/airflow/pull/6652/diff?src=pr=tree#diff-YWlyZmxvdy9hcGkvY29tbW9uL2V4cGVyaW1lbnRhbC9nZXRfZGFncy5weQ==)
 | `50% <50%> (ø)` | |
   | 
[airflow/api/common/experimental/get\_tasks.py](https://codecov.io/gh/apache/airflow/pull/6652/diff?src=pr=tree#diff-YWlyZmxvdy9hcGkvY29tbW9uL2V4cGVyaW1lbnRhbC9nZXRfdGFza3MucHk=)
 | `50% <50%> (ø)` | |
   | 
[airflow/api/common/experimental/get\_dag.py](https://codecov.io/gh/apache/airflow/pull/6652/diff?src=pr=tree#diff-YWlyZmxvdy9hcGkvY29tbW9uL2V4cGVyaW1lbnRhbC9nZXRfZGFnLnB5)
 | `66.66% <66.66%> (ø)` | |
   | 
[airflow/operators/postgres\_operator.py](https://codecov.io/gh/apache/airflow/pull/6652/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvcG9zdGdyZXNfb3BlcmF0b3IucHk=)
 | `0% <0%> (-100%)` | :arrow_down: |
   | 
[airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6652/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==)
 | `44.44% <0%> (-55.56%)` | :arrow_down: |
   | 
[airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6652/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==)
 | `52.94% <0%> (-47.06%)` | :arrow_down: |
   | 
[airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6652/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==)
 | `45.25% <0%> (-46.72%)` | :arrow_down: |
   | ... and [10 
more](https://codecov.io/gh/apache/airflow/pull/6652/diff?src=pr=tree-more) 
| |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/airflow/pull/6652?src=pr=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/airflow/pull/6652?src=pr=footer). 
Last update 
[a86b6ac...6d4ae98](https://codecov.io/gh/apache/airflow/pull/6652?src=pr=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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352747261
 
 

 ##
 File path: airflow/executors/local_executor.py
 ##
 @@ -43,40 +42,44 @@
 This option could lead to the unification of the executor implementations, 
running
 locally, into just one `LocalExecutor` with multiple modes.
 """
-
-import multiprocessing
 import subprocess
-from queue import Empty
-
-from airflow.executors.base_executor import BaseExecutor
+from multiprocessing import Manager, Process
+from multiprocessing.managers import SyncManager
+from queue import Empty, Queue  # pylint: disable=unused-import  # noqa: F401
+from typing import Any, List, Optional, Tuple, Union  # pylint: 
disable=unused-import # noqa: F401
+
+from airflow import AirflowException
 
 Review comment:
   I don't think it matters that much any more. for sure it does not matter for 
the "python" cyclic imports (as you explained) but it matters for Pylint ones 
(those are detected differently - using different algorithms). I think that 
Pylint cycles are slightly less forgiving but still valid and they are working 
in a slightly different way (still have not figured all the details). But I 
think it does not matter any more for most of the imports from airflow (now 
that we removed operators/hooks/ etc. It still matters for 'models' because we 
are importing all models there so I prefer to keep 'from 
airflow.models.baseoperator import BaseOperator' in the core rather than 'from 
airflow.models import BaseOperator'. We might revisit that in the future 
however.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352747865
 
 

 ##
 File path: airflow/executors/local_executor.py
 ##
 @@ -43,40 +42,44 @@
 This option could lead to the unification of the executor implementations, 
running
 locally, into just one `LocalExecutor` with multiple modes.
 """
-
-import multiprocessing
 import subprocess
-from queue import Empty
-
-from airflow.executors.base_executor import BaseExecutor
+from multiprocessing import Manager, Process
+from multiprocessing.managers import SyncManager
+from queue import Empty, Queue  # pylint: disable=unused-import  # noqa: F401
+from typing import Any, List, Optional, Tuple, Union  # pylint: 
disable=unused-import # noqa: F401
+
+from airflow import AirflowException
+from airflow.executors.base_executor import NOT_STARTED_MESSAGE, PARALLELISM, 
BaseExecutor, CommandType
+from airflow.models.taskinstance import (  # pylint: disable=unused-import # 
noqa: F401
+TaskInstanceKeyType, TaskInstanceStateType,
+)
 from airflow.utils.log.logging_mixin import LoggingMixin
 from airflow.utils.state import State
 
+# This is a work to be executed by a worker.
+# It can Key and Command - but it can also be None, None which is actually a
+# "Poison Pill" - worker seeing Poison Pill should take the pill and ... die 
instantly.
+ExecutorWorkTodo = Tuple[Optional[TaskInstanceKeyType], Optional[CommandType]]
 
 Review comment:
   Interesting TYP(E)O. Too much to do recently maybe :).


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352748235
 
 

 ##
 File path: airflow/logging_config.py
 ##
 @@ -1,4 +1,3 @@
-# -*- coding: utf-8 -*-
 
 Review comment:
   Sure. I think that one was left after I removed some of the changes from 
this file.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352748854
 
 

 ##
 File path: airflow/models/dag.py
 ##
 @@ -1254,9 +1253,11 @@ def run(
 """
 from airflow.jobs import BackfillJob
 if not executor and local:
+from airflow.executors.local_executor import LocalExecutor
 executor = LocalExecutor()
 elif not executor:
-executor = get_default_executor()
+from airflow.executors.executor_loader import ExecutorLoader
 
 Review comment:
   I also do not feel confident about having pluginnable executors. I was quite 
surprised to see that it was possible. 


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] houqp commented on a change in pull request #6553: [AIRFLOW-5902] avoid unnecessary sleep to maintain local task job heart rate

2019-12-02 Thread GitBox
houqp commented on a change in pull request #6553: [AIRFLOW-5902] avoid 
unnecessary sleep to maintain local task job heart rate
URL: https://github.com/apache/airflow/pull/6553#discussion_r352748672
 
 

 ##
 File path: airflow/jobs/base_job.py
 ##
 @@ -171,17 +171,14 @@ def heartbeat(self):
 if self.state == State.SHUTDOWN:
 self.kill()
 
-is_unit_test = conf.getboolean('core', 'unit_test_mode')
 
 Review comment:
   This is still used in other tests unfortunately. But this PR should setup 
the example on how to remove or avoid using it in other places going forward.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352749876
 
 

 ##
 File path: airflow/settings.py
 ##
 @@ -68,13 +67,13 @@
 LOG_FORMAT = conf.get('core', 'log_format')
 SIMPLE_LOG_FORMAT = conf.get('core', 'simple_log_format')
 
-SQL_ALCHEMY_CONN = None  # type: Optional[str]
-DAGS_FOLDER = None  # type: Optional[str]
-PLUGINS_FOLDER = None  # type: Optional[str]
-LOGGING_CLASS_PATH = None  # type: Optional[str]
+SQL_ALCHEMY_CONN: Optional[str] = None
 
 Review comment:
   I changed it because I was touching and moving PLUGINS_FOLDER and Session in 
other files and it felt right to correct also the other neighbouring variables. 
I feel that changing it to be consistent was important.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] houqp commented on a change in pull request #6553: [AIRFLOW-5902] avoid unnecessary sleep to maintain local task job heart rate

2019-12-02 Thread GitBox
houqp commented on a change in pull request #6553: [AIRFLOW-5902] avoid 
unnecessary sleep to maintain local task job heart rate
URL: https://github.com/apache/airflow/pull/6553#discussion_r352749458
 
 

 ##
 File path: airflow/jobs/local_task_job.py
 ##
 @@ -112,13 +111,6 @@ def signal_handler(signum, frame):
"exceeded limit ({}s)."
.format(time_since_last_heartbeat,
heartbeat_time_limit))
-
-if time_since_last_heartbeat < self.heartrate:
 
 Review comment:
   heartbeat already has this check to maintain the heart rate, so this logic 
is redundant for all `heartbeat()` callers. It's safe to just remove it :) The 
test covers this to make sure heart rate is still maintained without this check.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6585: More GSOD improvements

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6585: More GSOD improvements
URL: https://github.com/apache/airflow/pull/6585#discussion_r352681948
 
 

 ##
 File path: CONTRIBUTING.rst
 ##
 @@ -742,8 +495,56 @@ Resources & links
 
   - Airflow users mailing list: ``_
 
-- `Issues on Apache’s Jira `__
+- `Issues on Apache’s JIRA `__
 
 - `Slack (chat) `__
 
+Step 4: Prepare PR
+--
+
+1. Update the local sources to address the JIRA ticket.
+
+   For example, to address this example JIRA ticket, do the following:
+
+   * Read about `email configuration in Airflow 
`__.
+
+   * Find the class you should modify. For the example ticket, this is 
`email.py 
`__.
+
+   * Find the test class where you should add tests. For the example ticket, 
this is `test_email.py 
`__.
+
+   * Modify the class and add necessary code and unit tests.
+
+   * Run the unit tests from the `IDE 
`__ or `local virtualenv 
`__ as you see fit.
+
+   * Run the tests in `Breeze 
`__.
+
+   * Run and fix all the `static checks `__. If you have
+ `pre-commits installed `__,
+ this step is automatically run while you are committing your code. If 
not, you can do it manually
+ via ``git add`` and then ``pre-commit run``.
+
+2. Rebase your fork, squash commits, and resolve all conflicts.
+
+3. Re-run static code checks again.
+
+4. Create a pull request with the following title for the sample ticket:
+   [AIRFLOW-5934: Added extra CC: field to the Airflow emails].
 
 Review comment:
   We have this further down the list: 
   
   > Adhere to guidelines for commit messages described in this `article 
`__.
   > This makes the lives of those who come after you a lot easier.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] ashb commented on a change in pull request #6585: More GSOD improvements

2019-12-02 Thread GitBox
ashb commented on a change in pull request #6585: More GSOD improvements
URL: https://github.com/apache/airflow/pull/6585#discussion_r352688262
 
 

 ##
 File path: CONTRIBUTING.rst
 ##
 @@ -742,8 +495,56 @@ Resources & links
 
   - Airflow users mailing list: ``_
 
-- `Issues on Apache’s Jira `__
+- `Issues on Apache’s JIRA `__
 
 - `Slack (chat) `__
 
+Step 4: Prepare PR
+--
+
+1. Update the local sources to address the JIRA ticket.
+
+   For example, to address this example JIRA ticket, do the following:
+
+   * Read about `email configuration in Airflow 
`__.
+
+   * Find the class you should modify. For the example ticket, this is 
`email.py 
`__.
+
+   * Find the test class where you should add tests. For the example ticket, 
this is `test_email.py 
`__.
+
+   * Modify the class and add necessary code and unit tests.
+
+   * Run the unit tests from the `IDE 
`__ or `local virtualenv 
`__ as you see fit.
+
+   * Run the tests in `Breeze 
`__.
+
+   * Run and fix all the `static checks `__. If you have
+ `pre-commits installed `__,
+ this step is automatically run while you are committing your code. If 
not, you can do it manually
+ via ``git add`` and then ``pre-commit run``.
+
+2. Rebase your fork, squash commits, and resolve all conflicts.
+
+3. Re-run static code checks again.
+
+4. Create a pull request with the following title for the sample ticket:
+   [AIRFLOW-5934: Added extra CC: field to the Airflow emails].
 
 Review comment:
   Not in this document is it? Oh earlier on, but just not changed so didn't 
see it in the diff.
   
   Also the suggestion is for correct balancing of `[]` :)


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352705135
 
 

 ##
 File path: airflow/plugins_manager.py
 ##
 @@ -187,26 +192,29 @@ def make_module(name, objects):
 macros_modules = []
 
 # Plugin components to integrate directly
-admin_views = []  # type: List[Any]
-flask_blueprints = []  # type: List[Any]
-menu_links = []  # type: List[Any]
-flask_appbuilder_views = []  # type: List[Any]
-flask_appbuilder_menu_links = []  # type: List[Any]
-stat_name_handler = None  # type: Any
-global_operator_extra_links = []  # type: List[BaseOperatorLink]
-operator_extra_links = []  # type: List[BaseOperatorLink]
+admin_views: List[Any] = []
+flask_blueprints: List[Any] = []
+menu_links: List[Any] = []
+flask_appbuilder_views: List[Any] = []
+flask_appbuilder_menu_links: List[Any] = []
+stat_name_handler: Any = None
 
 Review comment:
   Yeah. I left all the typing changes in plugins_manager to the follow up 
plugins-manager specific commit that follows up.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352704490
 
 

 ##
 File path: airflow/plugins_manager.py
 ##
 @@ -16,17 +15,18 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-
-import imp
+"""Manages all plugins."""
+# noinspection PyDeprecation
+import imp  # pylint: disable=deprecated-module
 import inspect
 import os
 import re
-from typing import Any, List
+import sys
+from typing import Any, Callable, List, Optional
 
 import pkg_resources
 
 from airflow import settings
-from airflow.models.baseoperator import BaseOperatorLink
 
 Review comment:
   I am strongly agains using those kind of hacks - they are usually a symptom 
of an entangled architecture and not best choice of packages/files to split the 
code. It's like a "broken window" policy - once we let them in, then it's 
easier to add another exception. So I would like to avoid it at all cost.
   
   However in this case I have an idea that maybe we will be able to implement 
in the better version of plugins_manager unentangling. I will try it shortly.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352694783
 
 

 ##
 File path: airflow/__init__.py
 ##
 @@ -44,23 +44,8 @@
 
 settings.initialize()
 
-login = None  # type: Optional[Callable]
+from airflow.plugins_manager import integrate_plugins
 
 Review comment:
   Yep. I think I have done it in the follow-up change where i add a lot of 
typing and restructured plugins_manager.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352698312
 
 

 ##
 File path: airflow/__init__.py
 ##
 @@ -44,23 +44,8 @@
 
 settings.initialize()
 
-login = None  # type: Optional[Callable]
+from airflow.plugins_manager import integrate_plugins
 
-from airflow import executors
-from airflow import hooks
-from airflow import macros
-from airflow import operators
-from airflow import sensors
+login: Optional[Callable] = None
 
-
-class AirflowMacroPlugin:
 
 Review comment:
   Added note in UPDATING.md


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352697033
 
 

 ##
 File path: airflow/__init__.py
 ##
 @@ -44,23 +44,8 @@
 
 settings.initialize()
 
-login = None  # type: Optional[Callable]
+from airflow.plugins_manager import integrate_plugins
 
-from airflow import executors
-from airflow import hooks
-from airflow import macros
-from airflow import operators
-from airflow import sensors
+login: Optional[Callable] = None
 
-
-class AirflowMacroPlugin:
-# pylint: disable=missing-docstring
-def __init__(self, namespace):
-self.namespace = namespace
-
-
-operators._integrate_plugins()  # pylint: disable=protected-access
-sensors._integrate_plugins()  # pylint: disable=protected-access
-hooks._integrate_plugins()  # pylint: disable=protected-access
-executors._integrate_plugins()  # pylint: disable=protected-access
-macros._integrate_plugins()  # pylint: disable=protected-access
+integrate_plugins()
 
 Review comment:
   Yep. Thought so. Added a note in UPDATING.md


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] codecov-io edited a comment on issue #6472: [AIRFLOW-6058] Running tests with pytest

2019-12-02 Thread GitBox
codecov-io edited a comment on issue #6472: [AIRFLOW-6058] Running tests with 
pytest
URL: https://github.com/apache/airflow/pull/6472#issuecomment-558580149
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=h1) 
Report
   > Merging 
[#6472](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=desc) into 
[master](https://codecov.io/gh/apache/airflow/commit/0e2c09d3f337c6b9659cbdd4c71a0c616ec3a35d?src=pr=desc)
 will **increase** coverage by `0.62%`.
   > The diff coverage is `57.14%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/airflow/pull/6472/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master#6472  +/-   ##
   ==
   + Coverage   83.84%   84.46%   +0.62% 
   ==
 Files 668  669   +1 
 Lines   3756737585  +18 
   ==
   + Hits3149831747 +249 
   + Misses   6069 5838 -231
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=)
 | `74.91% <ø> (-1.69%)` | :arrow_down: |
   | 
[airflow/utils/log/colored\_log.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy9sb2cvY29sb3JlZF9sb2cucHk=)
 | `89.58% <57.14%> (-3.6%)` | :arrow_down: |
   | 
[airflow/operators/postgres\_operator.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvcG9zdGdyZXNfb3BlcmF0b3IucHk=)
 | `0% <0%> (-100%)` | :arrow_down: |
   | 
[airflow/executors/sequential\_executor.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvc2VxdWVudGlhbF9leGVjdXRvci5weQ==)
 | `47.61% <0%> (-52.39%)` | :arrow_down: |
   | 
[...low/ti\_deps/deps/exec\_date\_after\_start\_date\_dep.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy90aV9kZXBzL2RlcHMvZXhlY19kYXRlX2FmdGVyX3N0YXJ0X2RhdGVfZGVwLnB5)
 | `80% <0%> (-10%)` | :arrow_down: |
   | 
[airflow/utils/sqlalchemy.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy9zcWxhbGNoZW15LnB5)
 | `88.13% <0%> (-5.09%)` | :arrow_down: |
   | 
[airflow/executors/\_\_init\_\_.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvX19pbml0X18ucHk=)
 | `63.26% <0%> (-4.09%)` | :arrow_down: |
   | 
[airflow/utils/log/es\_task\_handler.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy9sb2cvZXNfdGFza19oYW5kbGVyLnB5)
 | `88.07% <0%> (-3.67%)` | :arrow_down: |
   | 
[airflow/utils/cli\_action\_loggers.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy9jbGlfYWN0aW9uX2xvZ2dlcnMucHk=)
 | `69.69% <0%> (-3.64%)` | :arrow_down: |
   | 
[airflow/hooks/dbapi\_hook.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9ob29rcy9kYmFwaV9ob29rLnB5)
 | `89.83% <0%> (-1.7%)` | :arrow_down: |
   | ... and [28 
more](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree-more) 
| |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=footer). 
Last update 
[0e2c09d...43b2b1f](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] codecov-io edited a comment on issue #6472: [AIRFLOW-6058] Running tests with pytest

2019-12-02 Thread GitBox
codecov-io edited a comment on issue #6472: [AIRFLOW-6058] Running tests with 
pytest
URL: https://github.com/apache/airflow/pull/6472#issuecomment-558580149
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=h1) 
Report
   > Merging 
[#6472](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=desc) into 
[master](https://codecov.io/gh/apache/airflow/commit/0e2c09d3f337c6b9659cbdd4c71a0c616ec3a35d?src=pr=desc)
 will **increase** coverage by `0.62%`.
   > The diff coverage is `57.14%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/airflow/pull/6472/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master#6472  +/-   ##
   ==
   + Coverage   83.84%   84.46%   +0.62% 
   ==
 Files 668  669   +1 
 Lines   3756737585  +18 
   ==
   + Hits3149831747 +249 
   + Misses   6069 5838 -231
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=)
 | `74.91% <ø> (-1.69%)` | :arrow_down: |
   | 
[airflow/utils/log/colored\_log.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy9sb2cvY29sb3JlZF9sb2cucHk=)
 | `89.58% <57.14%> (-3.6%)` | :arrow_down: |
   | 
[airflow/operators/postgres\_operator.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvcG9zdGdyZXNfb3BlcmF0b3IucHk=)
 | `0% <0%> (-100%)` | :arrow_down: |
   | 
[airflow/executors/sequential\_executor.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvc2VxdWVudGlhbF9leGVjdXRvci5weQ==)
 | `47.61% <0%> (-52.39%)` | :arrow_down: |
   | 
[...low/ti\_deps/deps/exec\_date\_after\_start\_date\_dep.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy90aV9kZXBzL2RlcHMvZXhlY19kYXRlX2FmdGVyX3N0YXJ0X2RhdGVfZGVwLnB5)
 | `80% <0%> (-10%)` | :arrow_down: |
   | 
[airflow/utils/sqlalchemy.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy9zcWxhbGNoZW15LnB5)
 | `88.13% <0%> (-5.09%)` | :arrow_down: |
   | 
[airflow/executors/\_\_init\_\_.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvX19pbml0X18ucHk=)
 | `63.26% <0%> (-4.09%)` | :arrow_down: |
   | 
[airflow/utils/log/es\_task\_handler.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy9sb2cvZXNfdGFza19oYW5kbGVyLnB5)
 | `88.07% <0%> (-3.67%)` | :arrow_down: |
   | 
[airflow/utils/cli\_action\_loggers.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy9jbGlfYWN0aW9uX2xvZ2dlcnMucHk=)
 | `69.69% <0%> (-3.64%)` | :arrow_down: |
   | 
[airflow/hooks/dbapi\_hook.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9ob29rcy9kYmFwaV9ob29rLnB5)
 | `89.83% <0%> (-1.7%)` | :arrow_down: |
   | ... and [28 
more](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree-more) 
| |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=footer). 
Last update 
[0e2c09d...43b2b1f](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] codecov-io edited a comment on issue #6472: [AIRFLOW-6058] Running tests with pytest

2019-12-02 Thread GitBox
codecov-io edited a comment on issue #6472: [AIRFLOW-6058] Running tests with 
pytest
URL: https://github.com/apache/airflow/pull/6472#issuecomment-558580149
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=h1) 
Report
   > Merging 
[#6472](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=desc) into 
[master](https://codecov.io/gh/apache/airflow/commit/0e2c09d3f337c6b9659cbdd4c71a0c616ec3a35d?src=pr=desc)
 will **increase** coverage by `0.62%`.
   > The diff coverage is `57.14%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/airflow/pull/6472/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master#6472  +/-   ##
   ==
   + Coverage   83.84%   84.46%   +0.62% 
   ==
 Files 668  669   +1 
 Lines   3756737585  +18 
   ==
   + Hits3149831747 +249 
   + Misses   6069 5838 -231
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=)
 | `74.91% <ø> (-1.69%)` | :arrow_down: |
   | 
[airflow/utils/log/colored\_log.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy9sb2cvY29sb3JlZF9sb2cucHk=)
 | `89.58% <57.14%> (-3.6%)` | :arrow_down: |
   | 
[airflow/operators/postgres\_operator.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvcG9zdGdyZXNfb3BlcmF0b3IucHk=)
 | `0% <0%> (-100%)` | :arrow_down: |
   | 
[airflow/executors/sequential\_executor.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvc2VxdWVudGlhbF9leGVjdXRvci5weQ==)
 | `47.61% <0%> (-52.39%)` | :arrow_down: |
   | 
[...low/ti\_deps/deps/exec\_date\_after\_start\_date\_dep.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy90aV9kZXBzL2RlcHMvZXhlY19kYXRlX2FmdGVyX3N0YXJ0X2RhdGVfZGVwLnB5)
 | `80% <0%> (-10%)` | :arrow_down: |
   | 
[airflow/utils/sqlalchemy.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy9zcWxhbGNoZW15LnB5)
 | `88.13% <0%> (-5.09%)` | :arrow_down: |
   | 
[airflow/executors/\_\_init\_\_.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvX19pbml0X18ucHk=)
 | `63.26% <0%> (-4.09%)` | :arrow_down: |
   | 
[airflow/utils/log/es\_task\_handler.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy9sb2cvZXNfdGFza19oYW5kbGVyLnB5)
 | `88.07% <0%> (-3.67%)` | :arrow_down: |
   | 
[airflow/utils/cli\_action\_loggers.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy9jbGlfYWN0aW9uX2xvZ2dlcnMucHk=)
 | `69.69% <0%> (-3.64%)` | :arrow_down: |
   | 
[airflow/hooks/dbapi\_hook.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9ob29rcy9kYmFwaV9ob29rLnB5)
 | `89.83% <0%> (-1.7%)` | :arrow_down: |
   | ... and [28 
more](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree-more) 
| |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=footer). 
Last update 
[0e2c09d...43b2b1f](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352731417
 
 

 ##
 File path: airflow/executors/base_executor.py
 ##
 @@ -16,67 +14,86 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-
+"""
+Base executor - this is the base class for all the implemented executors.
+"""
 from collections import OrderedDict
+from typing import Any, Dict, List, Optional, Set, Tuple, Union
 
-# To avoid circular imports
-import airflow.utils.dag_processing
-from airflow.configuration import conf
 
 Review comment:
   I think it makes no real difference as we discussed before. And i actually 
(in the further commit) made a check to use it consistently 'from airflow 
import conf' and 'LoggingMixin' in as many places as possible (there are just a 
few exception where it matters - mainly around configuration itself and plugin 
manager.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] houqp commented on a change in pull request #6553: [AIRFLOW-5902] avoid unnecessary sleep to maintain local task job heart rate

2019-12-02 Thread GitBox
houqp commented on a change in pull request #6553: [AIRFLOW-5902] avoid 
unnecessary sleep to maintain local task job heart rate
URL: https://github.com/apache/airflow/pull/6553#discussion_r352750606
 
 

 ##
 File path: tests/jobs/test_local_task_job.py
 ##
 @@ -43,6 +43,9 @@
 class TestLocalTaskJob(unittest.TestCase):
 def setUp(self):
 clear_db_runs()
+patcher = patch('airflow.jobs.base_job.sleep')
 
 Review comment:
   yeah, this test doesn't cover the code path that does the actual sleep call 
in those two classes. we should definitely patch them if a test runs through 
those code path going forward.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6585: More GSOD improvements

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6585: More GSOD improvements
URL: https://github.com/apache/airflow/pull/6585#discussion_r352680690
 
 

 ##
 File path: CONTRIBUTING.rst
 ##
 @@ -742,8 +495,56 @@ Resources & links
 
 
 Review comment:
   Added info on that


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] ashb commented on a change in pull request #6585: More GSOD improvements

2019-12-02 Thread GitBox
ashb commented on a change in pull request #6585: More GSOD improvements
URL: https://github.com/apache/airflow/pull/6585#discussion_r352688262
 
 

 ##
 File path: CONTRIBUTING.rst
 ##
 @@ -742,8 +495,56 @@ Resources & links
 
   - Airflow users mailing list: ``_
 
-- `Issues on Apache’s Jira `__
+- `Issues on Apache’s JIRA `__
 
 - `Slack (chat) `__
 
+Step 4: Prepare PR
+--
+
+1. Update the local sources to address the JIRA ticket.
+
+   For example, to address this example JIRA ticket, do the following:
+
+   * Read about `email configuration in Airflow 
`__.
+
+   * Find the class you should modify. For the example ticket, this is 
`email.py 
`__.
+
+   * Find the test class where you should add tests. For the example ticket, 
this is `test_email.py 
`__.
+
+   * Modify the class and add necessary code and unit tests.
+
+   * Run the unit tests from the `IDE 
`__ or `local virtualenv 
`__ as you see fit.
+
+   * Run the tests in `Breeze 
`__.
+
+   * Run and fix all the `static checks `__. If you have
+ `pre-commits installed `__,
+ this step is automatically run while you are committing your code. If 
not, you can do it manually
+ via ``git add`` and then ``pre-commit run``.
+
+2. Rebase your fork, squash commits, and resolve all conflicts.
+
+3. Re-run static code checks again.
+
+4. Create a pull request with the following title for the sample ticket:
+   [AIRFLOW-5934: Added extra CC: field to the Airflow emails].
 
 Review comment:
   Not in this document is it?
   
   Also the suggestion is for correct balancing of `[]` :)


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on issue #6585: More GSOD improvements

2019-12-02 Thread GitBox
potiuk commented on issue #6585: More GSOD improvements
URL: https://github.com/apache/airflow/pull/6585#issuecomment-560466337
 
 
   I think it's best to simply refer to CONTRIBUTING.rst in github from the 
Airflow Website :)


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] codecov-io edited a comment on issue #6472: [AIRFLOW-6058] Running tests with pytest

2019-12-02 Thread GitBox
codecov-io edited a comment on issue #6472: [AIRFLOW-6058] Running tests with 
pytest
URL: https://github.com/apache/airflow/pull/6472#issuecomment-558580149
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=h1) 
Report
   > Merging 
[#6472](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=desc) into 
[master](https://codecov.io/gh/apache/airflow/commit/0e2c09d3f337c6b9659cbdd4c71a0c616ec3a35d?src=pr=desc)
 will **increase** coverage by `0.02%`.
   > The diff coverage is `57.14%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/airflow/pull/6472/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master#6472  +/-   ##
   ==
   + Coverage   83.84%   83.86%   +0.02% 
   ==
 Files 668  669   +1 
 Lines   3756737585  +18 
   ==
   + Hits3149831522  +24 
   + Misses   6069 6063   -6
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=)
 | `75.74% <ø> (-0.86%)` | :arrow_down: |
   | 
[airflow/utils/log/colored\_log.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy9sb2cvY29sb3JlZF9sb2cucHk=)
 | `89.58% <57.14%> (-3.6%)` | :arrow_down: |
   | 
[airflow/operators/postgres\_operator.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvcG9zdGdyZXNfb3BlcmF0b3IucHk=)
 | `0% <0%> (-100%)` | :arrow_down: |
   | 
[airflow/operators/mysql\_operator.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXlzcWxfb3BlcmF0b3IucHk=)
 | `0% <0%> (-100%)` | :arrow_down: |
   | 
[airflow/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXlzcWxfdG9faGl2ZS5weQ==)
 | `0% <0%> (-100%)` | :arrow_down: |
   | 
[airflow/operators/generic\_transfer.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvZ2VuZXJpY190cmFuc2Zlci5weQ==)
 | `0% <0%> (-100%)` | :arrow_down: |
   | 
[airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==)
 | `44.44% <0%> (-55.56%)` | :arrow_down: |
   | 
[airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==)
 | `52.94% <0%> (-47.06%)` | :arrow_down: |
   | 
[airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==)
 | `45.25% <0%> (-46.72%)` | :arrow_down: |
   | 
[airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5)
 | `47.01% <0%> (-29.11%)` | :arrow_down: |
   | ... and [37 
more](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree-more) 
| |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=footer). 
Last update 
[0e2c09d...43b2b1f](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] codecov-io commented on issue #6708: [AIRFLOW-XXX] Fix docstring minor issues in airflow/kubernetes/

2019-12-02 Thread GitBox
codecov-io commented on issue #6708: [AIRFLOW-XXX] Fix docstring minor issues 
in airflow/kubernetes/
URL: https://github.com/apache/airflow/pull/6708#issuecomment-560476641
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6708?src=pr=h1) 
Report
   > Merging 
[#6708](https://codecov.io/gh/apache/airflow/pull/6708?src=pr=desc) into 
[master](https://codecov.io/gh/apache/airflow/commit/0e2c09d3f337c6b9659cbdd4c71a0c616ec3a35d?src=pr=desc)
 will **decrease** coverage by `75.19%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/airflow/pull/6708/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/6708?src=pr=tree)
   
   ```diff
   @@Coverage Diff@@
   ##   master   #6708  +/-   ##
   =
   - Coverage   83.84%   8.64%   -75.2% 
   =
 Files 668 663   -5 
 Lines   37567   37258 -309 
   =
   - Hits314983222   -28276 
   - Misses   6069   34036   +27967
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/airflow/pull/6708?src=pr=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[airflow/kubernetes/kube\_client.py](https://codecov.io/gh/apache/airflow/pull/6708/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL2t1YmVfY2xpZW50LnB5)
 | `77.14% <ø> (-8.58%)` | :arrow_down: |
   | 
[airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6708/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==)
 | `89.05% <ø> (-2.92%)` | :arrow_down: |
   | 
[airflow/kubernetes/pod\_generator.py](https://codecov.io/gh/apache/airflow/pull/6708/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9nZW5lcmF0b3IucHk=)
 | `56.06% <ø> (-38.64%)` | :arrow_down: |
   | 
[...low/contrib/operators/wasb\_delete\_blob\_operator.py](https://codecov.io/gh/apache/airflow/pull/6708/diff?src=pr=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy93YXNiX2RlbGV0ZV9ibG9iX29wZXJhdG9yLnB5)
 | `0% <0%> (-100%)` | :arrow_down: |
   | 
[...flow/contrib/example\_dags/example\_qubole\_sensor.py](https://codecov.io/gh/apache/airflow/pull/6708/diff?src=pr=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9zZW5zb3IucHk=)
 | `0% <0%> (-100%)` | :arrow_down: |
   | 
[airflow/example\_dags/subdags/subdag.py](https://codecov.io/gh/apache/airflow/pull/6708/diff?src=pr=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3Mvc3ViZGFncy9zdWJkYWcucHk=)
 | `0% <0%> (-100%)` | :arrow_down: |
   | 
[airflow/gcp/sensors/bigquery\_dts.py](https://codecov.io/gh/apache/airflow/pull/6708/diff?src=pr=tree#diff-YWlyZmxvdy9nY3Avc2Vuc29ycy9iaWdxdWVyeV9kdHMucHk=)
 | `0% <0%> (-100%)` | :arrow_down: |
   | 
[airflow/gcp/operators/text\_to\_speech.py](https://codecov.io/gh/apache/airflow/pull/6708/diff?src=pr=tree#diff-YWlyZmxvdy9nY3Avb3BlcmF0b3JzL3RleHRfdG9fc3BlZWNoLnB5)
 | `0% <0%> (-100%)` | :arrow_down: |
   | 
[...ample\_dags/example\_emr\_job\_flow\_automatic\_steps.py](https://codecov.io/gh/apache/airflow/pull/6708/diff?src=pr=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2Vtcl9qb2JfZmxvd19hdXRvbWF0aWNfc3RlcHMucHk=)
 | `0% <0%> (-100%)` | :arrow_down: |
   | 
[...irflow/providers/apache/cassandra/sensors/table.py](https://codecov.io/gh/apache/airflow/pull/6708/diff?src=pr=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9zZW5zb3JzL3RhYmxlLnB5)
 | `0% <0%> (-100%)` | :arrow_down: |
   | ... and [594 
more](https://codecov.io/gh/apache/airflow/pull/6708/diff?src=pr=tree-more) 
| |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/airflow/pull/6708?src=pr=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/airflow/pull/6708?src=pr=footer). 
Last update 
[0e2c09d...6c8b8d3](https://codecov.io/gh/apache/airflow/pull/6708?src=pr=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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] aoen commented on a change in pull request #6709: [AIRFLOW-5834] Option to skip serve_logs process with workers

2019-12-02 Thread GitBox
aoen commented on a change in pull request #6709: [AIRFLOW-5834] Option to skip 
serve_logs process with workers
URL: https://github.com/apache/airflow/pull/6709#discussion_r352726026
 
 

 ##
 File path: pylintrc
 ##
 @@ -154,6 +154,8 @@ disable=print-statement,
 deprecated-sys-function,
 exception-escape,
 comprehension-escape,
+too-many-arguments,
+too-many-instance-attributes,
 
 Review comment:
   Is master CI failing in this case? Probably makes sense to fix the 
regression or revert the breaking commits rather than drop coverage on these 
lint rules.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on issue #6585: More GSOD improvements

2019-12-02 Thread GitBox
potiuk commented on issue #6585: More GSOD improvements
URL: https://github.com/apache/airflow/pull/6585#issuecomment-560467762
 
 
   Review comments applied.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] codecov-io edited a comment on issue #6472: [AIRFLOW-6058] Running tests with pytest

2019-12-02 Thread GitBox
codecov-io edited a comment on issue #6472: [AIRFLOW-6058] Running tests with 
pytest
URL: https://github.com/apache/airflow/pull/6472#issuecomment-558580149
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=h1) 
Report
   > Merging 
[#6472](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=desc) into 
[master](https://codecov.io/gh/apache/airflow/commit/0e2c09d3f337c6b9659cbdd4c71a0c616ec3a35d?src=pr=desc)
 will **increase** coverage by `0.37%`.
   > The diff coverage is `57.14%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/airflow/pull/6472/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master#6472  +/-   ##
   ==
   + Coverage   83.84%   84.21%   +0.37% 
   ==
 Files 668  669   +1 
 Lines   3756737585  +18 
   ==
   + Hits3149831653 +155 
   + Misses   6069 5932 -137
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=)
 | `75.74% <ø> (-0.86%)` | :arrow_down: |
   | 
[airflow/utils/log/colored\_log.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy9sb2cvY29sb3JlZF9sb2cucHk=)
 | `89.58% <57.14%> (-3.6%)` | :arrow_down: |
   | 
[airflow/operators/mysql\_operator.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXlzcWxfb3BlcmF0b3IucHk=)
 | `0% <0%> (-100%)` | :arrow_down: |
   | 
[airflow/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXlzcWxfdG9faGl2ZS5weQ==)
 | `0% <0%> (-100%)` | :arrow_down: |
   | 
[airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==)
 | `44.44% <0%> (-55.56%)` | :arrow_down: |
   | 
[airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==)
 | `52.94% <0%> (-47.06%)` | :arrow_down: |
   | 
[airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==)
 | `45.25% <0%> (-46.72%)` | :arrow_down: |
   | 
[airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5)
 | `50.98% <0%> (-23.53%)` | :arrow_down: |
   | 
[...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==)
 | `78.2% <0%> (-20.52%)` | :arrow_down: |
   | 
[airflow/executors/sequential\_executor.py](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvc2VxdWVudGlhbF9leGVjdXRvci5weQ==)
 | `85.71% <0%> (-14.29%)` | :arrow_down: |
   | ... and [34 
more](https://codecov.io/gh/apache/airflow/pull/6472/diff?src=pr=tree-more) 
| |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=footer). 
Last update 
[0e2c09d...43b2b1f](https://codecov.io/gh/apache/airflow/pull/6472?src=pr=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:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Created] (AIRFLOW-6154) dag_processor_manager log path ignores log directory config

2019-12-02 Thread Ronald Fenner (Jira)
Ronald Fenner created AIRFLOW-6154:
--

 Summary: dag_processor_manager log path ignores log directory 
config
 Key: AIRFLOW-6154
 URL: https://issues.apache.org/jira/browse/AIRFLOW-6154
 Project: Apache Airflow
  Issue Type: Bug
  Components: core
Affects Versions: 1.10.6
 Environment: amazon linux 2
Reporter: Ronald Fenner


The dag_processor_manager log is hard coded in the default_airflow.cfg to be 
\{AIRFLOW_HOME}/logs/dag_processor_manager/dag_processor_manager.log

This ignores the configuration when one changes the log directory via 
base_log_folder.

 

This should allow for the fact if the base_log_folder is set then the dag 
process manager should place it's logs there and not in the airflow home 
directory.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [airflow] msumit commented on a change in pull request #6709: [AIRFLOW-5834] Option to skip serve_logs process with workers

2019-12-02 Thread GitBox
msumit commented on a change in pull request #6709: [AIRFLOW-5834] Option to 
skip serve_logs process with workers
URL: https://github.com/apache/airflow/pull/6709#discussion_r352719535
 
 

 ##
 File path: pylintrc
 ##
 @@ -154,6 +154,8 @@ disable=print-statement,
 deprecated-sys-function,
 exception-escape,
 comprehension-escape,
+too-many-arguments,
+too-many-instance-attributes,
 
 Review comment:
   not related to this PR, but was getting pylint errors 


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] aoen commented on a change in pull request #6709: [AIRFLOW-5834] Option to skip serve_logs process with workers

2019-12-02 Thread GitBox
aoen commented on a change in pull request #6709: [AIRFLOW-5834] Option to skip 
serve_logs process with workers
URL: https://github.com/apache/airflow/pull/6709#discussion_r352718565
 
 

 ##
 File path: airflow/cli/commands/worker_command.py
 ##
 @@ -59,6 +62,12 @@ def worker(args):
 if conf.has_option("celery", "pool"):
 options["pool"] = conf.get("celery", "pool")
 
+if skip_serve_logs is False:
+print(
+"Starting serve logs process within worker is going to be 
deprecated, "
 
 Review comment:
   Are we sure we want to deprecate this in 2.0? One of the current principles 
of Airflow is to make it work out of the box so this log serving may still be a 
good default to have.
   
   If we want to keep the warning should probably use log. syntax instead of 
print()


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] aoen commented on a change in pull request #6709: [AIRFLOW-5834] Option to skip serve_logs process with workers

2019-12-02 Thread GitBox
aoen commented on a change in pull request #6709: [AIRFLOW-5834] Option to skip 
serve_logs process with workers
URL: https://github.com/apache/airflow/pull/6709#discussion_r352723411
 
 

 ##
 File path: airflow/cli/commands/worker_command.py
 ##
 @@ -76,17 +85,19 @@ def worker(args):
 stderr=stderr,
 )
 with ctx:
-sub_proc = subprocess.Popen(['airflow', 'serve_logs'], env=env, 
close_fds=True)
+if skip_serve_logs is False:
 
 Review comment:
   Nit: Let's factor this out into a private function _serve_logs() to dedup (I 
know the previous logic didn't have it but we are slightly increasing 
complexity with this change anyways so I think we should fix this).


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] aoen commented on a change in pull request #6709: [AIRFLOW-5834] Option to skip serve_logs process with workers

2019-12-02 Thread GitBox
aoen commented on a change in pull request #6709: [AIRFLOW-5834] Option to skip 
serve_logs process with workers
URL: https://github.com/apache/airflow/pull/6709#discussion_r352721673
 
 

 ##
 File path: pylintrc
 ##
 @@ -154,6 +154,8 @@ disable=print-statement,
 deprecated-sys-function,
 exception-escape,
 comprehension-escape,
+too-many-arguments,
 
 Review comment:
   I think we should make a one-time exception in the command arguments rather 
than remove coverage everywhere (e.g. 
https://stackoverflow.com/questions/28829236/is-it-possible-to-ignore-one-single-specific-line-with-pylint).
 This is kind of a special case since it's an API that's exposed to users.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] aoen commented on a change in pull request #6709: [AIRFLOW-5834] Option to skip serve_logs process with workers

2019-12-02 Thread GitBox
aoen commented on a change in pull request #6709: [AIRFLOW-5834] Option to skip 
serve_logs process with workers
URL: https://github.com/apache/airflow/pull/6709#discussion_r352720846
 
 

 ##
 File path: airflow/cli/commands/worker_command.py
 ##
 @@ -76,17 +85,19 @@ def worker(args):
 stderr=stderr,
 )
 with ctx:
-sub_proc = subprocess.Popen(['airflow', 'serve_logs'], env=env, 
close_fds=True)
+if skip_serve_logs is False:
+sub_proc = subprocess.Popen(['airflow', 'serve_logs'], 
env=env, close_fds=True)
 worker.run(**options)
-sub_proc.kill()
 
 Review comment:
   Don't you need to keep this line and have an "if skip_serve_logs is False:" 
line above it?


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on issue #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on issue #6596: [AIRFLOW-6004] Untangle Executors class to 
avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#issuecomment-560518806
 
 
   > Still a few unrelated files changed.
   > 
   > Also we've got inconsistent imports airflow.configuration import conf vs 
airflow import conf and airflow import AirflowException vs airflow.exceptions 
import AirflowException.
   
   I fixed few of those in the fixup that's coming but I left the gist of it 
for a follow-up commit where I implemented also validation in pre-commit.
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on issue #6697: [AIRFLOW-6135] Extract DAG processing from SchedulerJob into separate class

2019-12-02 Thread GitBox
potiuk commented on issue #6697: [AIRFLOW-6135] Extract DAG processing from 
SchedulerJob into separate class
URL: https://github.com/apache/airflow/pull/6697#issuecomment-560519623
 
 
   > (Will likely conflict with #6596)
   
   Yep. That's why I think it's better to review this one after it is rebased 
by @mik-laj after my change gets merged.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Commented] (AIRFLOW-6142) Different results for pylint locally and in Travis CI

2019-12-02 Thread ASF subversion and git services (Jira)


[ 
https://issues.apache.org/jira/browse/AIRFLOW-6142?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16986256#comment-16986256
 ] 

ASF subversion and git services commented on AIRFLOW-6142:
--

Commit 91eb87112b343ce4c18a994a41703bf793125dca in airflow's branch 
refs/heads/master from Jarek Potiuk
[ https://gitbox.apache.org/repos/asf?p=airflow.git;h=91eb871 ]

[AIRFLOW-6142] Fix different local/Travis pylint results (#6705)

* [AIRFLOW-6142] Fix different local/Travis pylint results

Sometimes Pylint on Travis CI gives still different results than the one run
locally. This was happening because we were using the
AIRFLOW_MOUNT_SOURCE_DIR_FOR_STATIC_CHECKS="true" for static checks. This is
needed for checklicence check only - just to make sure that all source files
(including scripts etc.) are mounted to the container.

However this makes it slightly different when it comes to pylint checks. We
would like to have it exactly identical when run locally and in CI so in case
of static checks we should rather use
AIRFLOW_MOUNT_HOST_VOLUMES_FOR_STATIC_CHECKS="true" for all checks but the
Checklicence one - same as used locally.

This way running:

pre-commit run pylint --all-files

Should always give the same results locally and in Travis.

* Update scripts/ci/_utils.sh

Co-Authored-By: Felix Uellendall 


> Different results for pylint locally and in Travis CI
> -
>
> Key: AIRFLOW-6142
> URL: https://issues.apache.org/jira/browse/AIRFLOW-6142
> Project: Apache Airflow
>  Issue Type: Sub-task
>  Components: ci
>Affects Versions: 2.0.0
>Reporter: Jarek Potiuk
>Assignee: Jarek Potiuk
>Priority: Major
> Fix For: 2.0.0
>
>
> Sometimes Pylint on Travis CI gives still different results than the one run 
> locally. This seems to be happening because we are using the 
> AIRFLOW_MOUNT_SOURCE_DIR_FOR_STATIC_CHECKS="true" for static checks. This is 
> needed for checklicence check only - just to make sure that all source files 
> (including scripts etc.) are mounted to the container. However this makes it 
> slightly different when it comes to pylint checks. We would like to have it 
> exactly identical when run locally and in CI so in case of static checks we 
> should rather use AIRFLOW_MOUNT_HOST_VOLUMES_FOR_STATIC_CHECKS="true" for all 
> checks but the Checklicence one . - same as used locally.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [airflow] potiuk merged pull request #6705: [AIRFLOW-6142] Fix different local/Travis pylint results

2019-12-02 Thread GitBox
potiuk merged pull request #6705: [AIRFLOW-6142] Fix different local/Travis 
pylint results
URL: https://github.com/apache/airflow/pull/6705
 
 
   


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:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Commented] (AIRFLOW-6142) Different results for pylint locally and in Travis CI

2019-12-02 Thread ASF subversion and git services (Jira)


[ 
https://issues.apache.org/jira/browse/AIRFLOW-6142?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16986255#comment-16986255
 ] 

ASF subversion and git services commented on AIRFLOW-6142:
--

Commit 91eb87112b343ce4c18a994a41703bf793125dca in airflow's branch 
refs/heads/master from Jarek Potiuk
[ https://gitbox.apache.org/repos/asf?p=airflow.git;h=91eb871 ]

[AIRFLOW-6142] Fix different local/Travis pylint results (#6705)

* [AIRFLOW-6142] Fix different local/Travis pylint results

Sometimes Pylint on Travis CI gives still different results than the one run
locally. This was happening because we were using the
AIRFLOW_MOUNT_SOURCE_DIR_FOR_STATIC_CHECKS="true" for static checks. This is
needed for checklicence check only - just to make sure that all source files
(including scripts etc.) are mounted to the container.

However this makes it slightly different when it comes to pylint checks. We
would like to have it exactly identical when run locally and in CI so in case
of static checks we should rather use
AIRFLOW_MOUNT_HOST_VOLUMES_FOR_STATIC_CHECKS="true" for all checks but the
Checklicence one - same as used locally.

This way running:

pre-commit run pylint --all-files

Should always give the same results locally and in Travis.

* Update scripts/ci/_utils.sh

Co-Authored-By: Felix Uellendall 


> Different results for pylint locally and in Travis CI
> -
>
> Key: AIRFLOW-6142
> URL: https://issues.apache.org/jira/browse/AIRFLOW-6142
> Project: Apache Airflow
>  Issue Type: Sub-task
>  Components: ci
>Affects Versions: 2.0.0
>Reporter: Jarek Potiuk
>Assignee: Jarek Potiuk
>Priority: Major
> Fix For: 2.0.0
>
>
> Sometimes Pylint on Travis CI gives still different results than the one run 
> locally. This seems to be happening because we are using the 
> AIRFLOW_MOUNT_SOURCE_DIR_FOR_STATIC_CHECKS="true" for static checks. This is 
> needed for checklicence check only - just to make sure that all source files 
> (including scripts etc.) are mounted to the container. However this makes it 
> slightly different when it comes to pylint checks. We would like to have it 
> exactly identical when run locally and in CI so in case of static checks we 
> should rather use AIRFLOW_MOUNT_HOST_VOLUMES_FOR_STATIC_CHECKS="true" for all 
> checks but the Checklicence one . - same as used locally.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Resolved] (AIRFLOW-6142) Different results for pylint locally and in Travis CI

2019-12-02 Thread Jarek Potiuk (Jira)


 [ 
https://issues.apache.org/jira/browse/AIRFLOW-6142?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jarek Potiuk resolved AIRFLOW-6142.
---
Fix Version/s: 2.0.0
   Resolution: Fixed

> Different results for pylint locally and in Travis CI
> -
>
> Key: AIRFLOW-6142
> URL: https://issues.apache.org/jira/browse/AIRFLOW-6142
> Project: Apache Airflow
>  Issue Type: Sub-task
>  Components: ci
>Affects Versions: 2.0.0
>Reporter: Jarek Potiuk
>Assignee: Jarek Potiuk
>Priority: Major
> Fix For: 2.0.0
>
>
> Sometimes Pylint on Travis CI gives still different results than the one run 
> locally. This seems to be happening because we are using the 
> AIRFLOW_MOUNT_SOURCE_DIR_FOR_STATIC_CHECKS="true" for static checks. This is 
> needed for checklicence check only - just to make sure that all source files 
> (including scripts etc.) are mounted to the container. However this makes it 
> slightly different when it comes to pylint checks. We would like to have it 
> exactly identical when run locally and in CI so in case of static checks we 
> should rather use AIRFLOW_MOUNT_HOST_VOLUMES_FOR_STATIC_CHECKS="true" for all 
> checks but the Checklicence one . - same as used locally.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (AIRFLOW-6142) Different results for pylint locally and in Travis CI

2019-12-02 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/AIRFLOW-6142?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16986254#comment-16986254
 ] 

ASF GitHub Bot commented on AIRFLOW-6142:
-

potiuk commented on pull request #6705: [AIRFLOW-6142] Fix different 
local/Travis pylint results
URL: https://github.com/apache/airflow/pull/6705
 
 
   
 

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:
us...@infra.apache.org


> Different results for pylint locally and in Travis CI
> -
>
> Key: AIRFLOW-6142
> URL: https://issues.apache.org/jira/browse/AIRFLOW-6142
> Project: Apache Airflow
>  Issue Type: Sub-task
>  Components: ci
>Affects Versions: 2.0.0
>Reporter: Jarek Potiuk
>Assignee: Jarek Potiuk
>Priority: Major
>
> Sometimes Pylint on Travis CI gives still different results than the one run 
> locally. This seems to be happening because we are using the 
> AIRFLOW_MOUNT_SOURCE_DIR_FOR_STATIC_CHECKS="true" for static checks. This is 
> needed for checklicence check only - just to make sure that all source files 
> (including scripts etc.) are mounted to the container. However this makes it 
> slightly different when it comes to pylint checks. We would like to have it 
> exactly identical when run locally and in CI so in case of static checks we 
> should rather use AIRFLOW_MOUNT_HOST_VOLUMES_FOR_STATIC_CHECKS="true" for all 
> checks but the Checklicence one . - same as used locally.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [airflow] potiuk commented on a change in pull request #6585: More GSOD improvements

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6585: More GSOD improvements
URL: https://github.com/apache/airflow/pull/6585#discussion_r352762271
 
 

 ##
 File path: CONTRIBUTING.rst
 ##
 @@ -742,8 +495,56 @@ Resources & links
 
   - Airflow users mailing list: ``_
 
-- `Issues on Apache’s Jira `__
+- `Issues on Apache’s JIRA `__
 
 - `Slack (chat) `__
 
+Step 4: Prepare PR
+--
+
+1. Update the local sources to address the JIRA ticket.
+
+   For example, to address this example JIRA ticket, do the following:
+
+   * Read about `email configuration in Airflow 
`__.
+
+   * Find the class you should modify. For the example ticket, this is 
`email.py 
`__.
+
+   * Find the test class where you should add tests. For the example ticket, 
this is `test_email.py 
`__.
+
+   * Modify the class and add necessary code and unit tests.
+
+   * Run the unit tests from the `IDE 
`__ or `local virtualenv 
`__ as you see fit.
+
+   * Run the tests in `Breeze 
`__.
+
+   * Run and fix all the `static checks `__. If you have
+ `pre-commits installed `__,
+ this step is automatically run while you are committing your code. If 
not, you can do it manually
+ via ``git add`` and then ``pre-commit run``.
+
+2. Rebase your fork, squash commits, and resolve all conflicts.
+
+3. Re-run static code checks again.
+
+4. Create a pull request with the following title for the sample ticket:
+   [AIRFLOW-5934: Added extra CC: field to the Airflow emails].
 
 Review comment:
   Right :)


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #6585: More GSOD improvements

2019-12-02 Thread GitBox
potiuk commented on a change in pull request #6585: More GSOD improvements
URL: https://github.com/apache/airflow/pull/6585#discussion_r352763339
 
 

 ##
 File path: CONTRIBUTING.rst
 ##
 @@ -742,8 +495,56 @@ Resources & links
 
   - Airflow users mailing list: ``_
 
-- `Issues on Apache’s Jira `__
+- `Issues on Apache’s JIRA `__
 
 - `Slack (chat) `__
 
+Step 4: Prepare PR
+--
+
+1. Update the local sources to address the JIRA ticket.
+
+   For example, to address this example JIRA ticket, do the following:
+
+   * Read about `email configuration in Airflow 
`__.
+
+   * Find the class you should modify. For the example ticket, this is 
`email.py 
`__.
+
+   * Find the test class where you should add tests. For the example ticket, 
this is `test_email.py 
`__.
+
+   * Modify the class and add necessary code and unit tests.
+
+   * Run the unit tests from the `IDE 
`__ or `local virtualenv 
`__ as you see fit.
+
+   * Run the tests in `Breeze 
`__.
+
+   * Run and fix all the `static checks `__. If you have
+ `pre-commits installed `__,
+ this step is automatically run while you are committing your code. If 
not, you can do it manually
+ via ``git add`` and then ``pre-commit run``.
+
+2. Rebase your fork, squash commits, and resolve all conflicts.
+
+3. Re-run static code checks again.
+
+4. Create a pull request with the following title for the sample ticket:
+   [AIRFLOW-5934: Added extra CC: field to the Airflow emails].
 
 Review comment:
   @ashb:  It is in this document but in the lines that have not changed :). 
It's been there since I remember.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on issue #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
potiuk commented on issue #6596: [AIRFLOW-6004] Untangle Executors class to 
avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#issuecomment-560527624
 
 
   I think NOW I resolved everything that needs to be resolved in this commit 
(some more commits are following), I let Travis CI to take a look now. I also 
made the commit message much shorter - we do not need the full context there, I 
left more of it in JIRA. @ashb -> let me know if you think it's OK.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] codecov-io commented on issue #6712: [AIRFLOW-XXX] add notice for Mesos Executor deprecation in docs

2019-12-02 Thread GitBox
codecov-io commented on issue #6712:  [AIRFLOW-XXX] add notice for Mesos 
Executor deprecation in docs
URL: https://github.com/apache/airflow/pull/6712#issuecomment-560535226
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6712?src=pr=h1) 
Report
   > Merging 
[#6712](https://codecov.io/gh/apache/airflow/pull/6712?src=pr=desc) into 
[v1-10-test](https://codecov.io/gh/apache/airflow/commit/9caf060d140b4dcad29ceb6158808765df3f7c18?src=pr=desc)
 will **decrease** coverage by `<.01%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/airflow/pull/6712/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/6712?src=pr=tree)
   
   ```diff
   @@  Coverage Diff   @@
   ##   v1-10-test#6712  +/-   ##
   ==
   - Coverage   79.62%   79.62%   -0.01% 
   ==
 Files 518  518  
 Lines   3528235282  
   ==
   - Hits2809328092   -1 
   - Misses   7189 7190   +1
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/airflow/pull/6712?src=pr=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[airflow/utils/dag\_processing.py](https://codecov.io/gh/apache/airflow/pull/6712/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy9kYWdfcHJvY2Vzc2luZy5weQ==)
 | `58.69% <0%> (-0.17%)` | :arrow_down: |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/airflow/pull/6712?src=pr=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/airflow/pull/6712?src=pr=footer). 
Last update 
[9caf060...4f2bfae](https://codecov.io/gh/apache/airflow/pull/6712?src=pr=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:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Created] (AIRFLOW-6155) Add some reading and writing functionality to the GCP Bigtable hook

2019-12-02 Thread Emily Mazo (Jira)
Emily Mazo created AIRFLOW-6155:
---

 Summary: Add some reading and writing functionality to the GCP 
Bigtable hook
 Key: AIRFLOW-6155
 URL: https://issues.apache.org/jira/browse/AIRFLOW-6155
 Project: Apache Airflow
  Issue Type: Improvement
  Components: gcp
Affects Versions: 1.10.6
Reporter: Emily Mazo


Airflow's GCP Bigtable Hook contains functionality for administrative actions 
like the creation and destruction of resources in GCP. I would like to add the 
ability to read and write data in Bigtable from an airflow operator, and have 
submitted a PR to add very limited ability to (atomically) mutate rows, delete 
rows, and search for rows by key. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Created] (AIRFLOW-6149) Rename postgres_to_gcs. service

2019-12-02 Thread Jira
Michał Słowikowski created AIRFLOW-6149:
---

 Summary: Rename postgres_to_gcs. service
 Key: AIRFLOW-6149
 URL: https://issues.apache.org/jira/browse/AIRFLOW-6149
 Project: Apache Airflow
  Issue Type: Sub-task
  Components: gcp
Affects Versions: 1.10.6
Reporter: Michał Słowikowski
Assignee: Michał Słowikowski


Added these classes:



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Created] (AIRFLOW-6150) Rename cassandra_to_gcs. service

2019-12-02 Thread Jira
Michał Słowikowski created AIRFLOW-6150:
---

 Summary: Rename cassandra_to_gcs. service
 Key: AIRFLOW-6150
 URL: https://issues.apache.org/jira/browse/AIRFLOW-6150
 Project: Apache Airflow
  Issue Type: Sub-task
  Components: gcp
Affects Versions: 1.10.6
Reporter: Michał Słowikowski
Assignee: Michał Słowikowski


Added these classes:



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [airflow] mik-laj commented on issue #6697: [AIRFLOW-6135] Extract DAG processing from SchedulerJob into separate class

2019-12-02 Thread GitBox
mik-laj commented on issue #6697: [AIRFLOW-6135] Extract DAG processing from 
SchedulerJob into separate class
URL: https://github.com/apache/airflow/pull/6697#issuecomment-560291492
 
 
   @ashb @potiuk @kaxil @nuclearpinguin It works for me. Can you look at it?


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] ashb commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
ashb commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352492591
 
 

 ##
 File path: airflow/__init__.py
 ##
 @@ -44,23 +44,8 @@
 
 settings.initialize()
 
-login = None  # type: Optional[Callable]
+from airflow.plugins_manager import integrate_plugins
 
-from airflow import executors
-from airflow import hooks
-from airflow import macros
-from airflow import operators
-from airflow import sensors
+login: Optional[Callable] = None
 
-
-class AirflowMacroPlugin:
 
 Review comment:
   I think this is probably fine -- this class hasn't done anything since 
mid-2015, so I can't see why someone might be using it.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] nuclearpinguin commented on a change in pull request #6585: More GSOD improvements

2019-12-02 Thread GitBox
nuclearpinguin commented on a change in pull request #6585: More GSOD 
improvements
URL: https://github.com/apache/airflow/pull/6585#discussion_r352510424
 
 

 ##
 File path: LOCAL_VIRTUALENV.rst
 ##
 @@ -19,15 +19,15 @@
 .. contents:: :local:
 
 Local Virtual Environment (virtualenv)
-
+==
 
-Use the local virtualenv development option in the combination with the 
_`Breeze `_
+Use the local virtualenv development option in the combination with the 
`Breeze `_
 development environment. This option helps you benefit from the infrastructure 
provided
-by your IDE (for example, IntelliJ's PyCharm/Idea) and work in the enviroment 
where all necessary dependencies and tests are 
+by your IDE (for example, IntelliJ's PyCharm/Idea) and work in the enviroment 
where all necessary dependencies and tests are
 
 Review comment:
   ```suggestion
   by your IDE (for example, IntelliJ Idea / PyCharm) and work in the 
enviroment where all necessary dependencies and tests are
   ```
   Those are two different products of JetBrains.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] nuclearpinguin commented on a change in pull request #6585: More GSOD improvements

2019-12-02 Thread GitBox
nuclearpinguin commented on a change in pull request #6585: More GSOD 
improvements
URL: https://github.com/apache/airflow/pull/6585#discussion_r352510570
 
 

 ##
 File path: LOCAL_VIRTUALENV.rst
 ##
 @@ -19,15 +19,15 @@
 .. contents:: :local:
 
 Local Virtual Environment (virtualenv)
-
+==
 
-Use the local virtualenv development option in the combination with the 
_`Breeze `_
+Use the local virtualenv development option in the combination with the 
`Breeze `_
 development environment. This option helps you benefit from the infrastructure 
provided
-by your IDE (for example, IntelliJ's PyCharm/Idea) and work in the enviroment 
where all necessary dependencies and tests are 
+by your IDE (for example, IntelliJ's PyCharm/Idea) and work in the enviroment 
where all necessary dependencies and tests are
 
 Review comment:
   ```suggestion
   by your IDE (for example, IntelliJ's PyCharm/Idea) and work in the 
environment where all necessary dependencies and tests are
   ```


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on issue #6706: [AIRFLOW-6143] Remove master-failing pylint:disables

2019-12-02 Thread GitBox
potiuk commented on issue #6706: [AIRFLOW-6143] Remove master-failing 
pylint:disables
URL: https://github.com/apache/airflow/pull/6706#issuecomment-560284151
 
 
   Thanks @Fokko !


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:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Created] (AIRFLOW-6146) Rename gcs_to_bq service

2019-12-02 Thread Jira
Michał Słowikowski created AIRFLOW-6146:
---

 Summary: Rename gcs_to_bq service
 Key: AIRFLOW-6146
 URL: https://issues.apache.org/jira/browse/AIRFLOW-6146
 Project: Apache Airflow
  Issue Type: Sub-task
  Components: gcp
Affects Versions: 1.10.6
Reporter: Michał Słowikowski
Assignee: Michał Słowikowski


Added these classes:



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [airflow] mik-laj commented on issue #6359: [AIRFLOW-XXX] Change instances "Google cloud storage" to "Google Cloud Storage".

2019-12-02 Thread GitBox
mik-laj commented on issue #6359: [AIRFLOW-XXX] Change instances "Google cloud 
storage" to "Google Cloud Storage".
URL: https://github.com/apache/airflow/pull/6359#issuecomment-560289355
 
 
   @dgorelik Can you do rebase? Travis is sad, because master branch was broken.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Created] (AIRFLOW-6152) Rename bigquery_to_gcs service

2019-12-02 Thread Jira
Michał Słowikowski created AIRFLOW-6152:
---

 Summary: Rename bigquery_to_gcs service
 Key: AIRFLOW-6152
 URL: https://issues.apache.org/jira/browse/AIRFLOW-6152
 Project: Apache Airflow
  Issue Type: Sub-task
  Components: gcp
Affects Versions: 1.10.6
Reporter: Michał Słowikowski
Assignee: Michał Słowikowski


Added these classes:



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[GitHub] [airflow] potiuk commented on issue #6697: [AIRFLOW-6135] Extract DAG processing from SchedulerJob into separate class

2019-12-02 Thread GitBox
potiuk commented on issue #6697: [AIRFLOW-6135] Extract DAG processing from 
SchedulerJob into separate class
URL: https://github.com/apache/airflow/pull/6697#issuecomment-560293372
 
 
   Yeah. Seems we have a lot of big changes to the code recently. I will take a 
look shortly.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [airflow] ashb commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
ashb commented on a change in pull request #6596: [AIRFLOW-6004] Untangle 
Executors class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#discussion_r352491482
 
 

 ##
 File path: airflow/__init__.py
 ##
 @@ -44,23 +44,8 @@
 
 settings.initialize()
 
-login = None  # type: Optional[Callable]
+from airflow.plugins_manager import integrate_plugins
 
-from airflow import executors
-from airflow import hooks
-from airflow import macros
-from airflow import operators
-from airflow import sensors
+login: Optional[Callable] = None
 
-
-class AirflowMacroPlugin:
 
 Review comment:
   Hmmm might be worth mentioning? It's not something we mention in our docs 
anywhere but...?


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:
us...@infra.apache.org


With regards,
Apache Git Services


  1   2   3   >