[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] [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)


[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)


[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)


[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)


[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] [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)


[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] [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)


[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


[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] 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 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] 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] 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_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


[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_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


[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] 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


[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


[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] 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_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] 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] 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


[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] 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-560909396
 
 
   @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.


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 #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
kaxil 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_r352916680
 
 

 ##
 File path: airflow/logging_config.py
 ##
 @@ -1,3 +1,4 @@
+# -*- coding: utf-8 -*-
 
 Review comment:
   Do we need 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] kaxil commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
kaxil 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_r352916873
 
 

 ##
 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:
   Good catch


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 #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
kaxil 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_r352916555
 
 

 ##
 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:
   Why are we removing 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] kaxil commented on a change in pull request #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
kaxil 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_r352916409
 
 

 ##
 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 would probably leave the `if` conditions instead of asserts


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 #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
kaxil 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_r352915970
 
 

 ##
 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:
   I think I had added 1 in Serialization code too, I will remove it soon


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 #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
kaxil 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_r352915706
 
 

 ##
 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:
   Found it: https://github.com/apache/airflow/pull/3690


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 #6714: [AIRFLOW-6153] Allow adding Custom Fields to Serialized Operators

2019-12-02 Thread GitBox
codecov-io edited a comment on issue #6714: [AIRFLOW-6153] Allow adding Custom 
Fields to Serialized Operators
URL: https://github.com/apache/airflow/pull/6714#issuecomment-560887984
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6714?src=pr=h1) 
Report
   > Merging 
[#6714](https://codecov.io/gh/apache/airflow/pull/6714?src=pr=desc) into 
[master](https://codecov.io/gh/apache/airflow/commit/91eb87112b343ce4c18a994a41703bf793125dca?src=pr=desc)
 will **decrease** coverage by `0.29%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/airflow/pull/6714/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/6714?src=pr=tree)
   
   ```diff
   @@Coverage Diff@@
   ##   master#6714 +/-   ##
   =
   - Coverage   83.82%   83.53%   -0.3% 
   =
 Files 668  668 
 Lines   3756937580 +11 
   =
   - Hits3149431391-103 
   - Misses   6075 6189+114
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/airflow/pull/6714?src=pr=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[airflow/models/baseoperator.py](https://codecov.io/gh/apache/airflow/pull/6714/diff?src=pr=tree#diff-YWlyZmxvdy9tb2RlbHMvYmFzZW9wZXJhdG9yLnB5)
 | `96.28% <ø> (ø)` | :arrow_up: |
   | 
[airflow/gcp/operators/bigquery.py](https://codecov.io/gh/apache/airflow/pull/6714/diff?src=pr=tree#diff-YWlyZmxvdy9nY3Avb3BlcmF0b3JzL2JpZ3F1ZXJ5LnB5)
 | `84.3% <100%> (+0.12%)` | :arrow_up: |
   | 
[airflow/utils/tests.py](https://codecov.io/gh/apache/airflow/pull/6714/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/6714/diff?src=pr=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9xdWJvbGVfb3BlcmF0b3IucHk=)
 | `88.23% <100%> (+0.54%)` | :arrow_up: |
   | 
[airflow/operators/mysql\_operator.py](https://codecov.io/gh/apache/airflow/pull/6714/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXlzcWxfb3BlcmF0b3IucHk=)
 | `100% <0%> (ø)` | :arrow_up: |
   | 
[airflow/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/6714/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXlzcWxfdG9faGl2ZS5weQ==)
 | `100% <0%> (ø)` | :arrow_up: |
   | 
[airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6714/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==)
 | `44.44% <0%> (-55.56%)` | :arrow_down: |
   | 
[airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6714/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/6714/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/6714/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5)
 | `50.98% <0%> (-23.53%)` | :arrow_down: |
   | ... and [9 
more](https://codecov.io/gh/apache/airflow/pull/6714/diff?src=pr=tree-more) 
| |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/airflow/pull/6714?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/6714?src=pr=footer). 
Last update 
[91eb871...0c95ef0](https://codecov.io/gh/apache/airflow/pull/6714?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 #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
kaxil 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_r352914727
 
 

 ##
 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:
   I remember the discussion (in one of the old PRs) where adding assert in the 
code was highly discouraged. One of the reason I think was 
https://stackoverflow.com/a/1838411/5691525 
   
   >"assert" statements are removed when the compilation is optimized.


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 #6714: [AIRFLOW-6153] Allow adding Custom Fields to Serialized Operators

2019-12-02 Thread GitBox
codecov-io commented on issue #6714: [AIRFLOW-6153] Allow adding Custom Fields 
to Serialized Operators
URL: https://github.com/apache/airflow/pull/6714#issuecomment-560887984
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6714?src=pr=h1) 
Report
   > Merging 
[#6714](https://codecov.io/gh/apache/airflow/pull/6714?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 `100%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/airflow/pull/6714/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/6714?src=pr=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master#6714  +/-   ##
   ==
   - Coverage   83.82%   83.28%   -0.55% 
   ==
 Files 668  668  
 Lines   3756937580  +11 
   ==
   - Hits3149431300 -194 
   - Misses   6075 6280 +205
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/airflow/pull/6714?src=pr=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[airflow/models/baseoperator.py](https://codecov.io/gh/apache/airflow/pull/6714/diff?src=pr=tree#diff-YWlyZmxvdy9tb2RlbHMvYmFzZW9wZXJhdG9yLnB5)
 | `96.28% <ø> (ø)` | :arrow_up: |
   | 
[airflow/gcp/operators/bigquery.py](https://codecov.io/gh/apache/airflow/pull/6714/diff?src=pr=tree#diff-YWlyZmxvdy9nY3Avb3BlcmF0b3JzL2JpZ3F1ZXJ5LnB5)
 | `84.3% <100%> (+0.12%)` | :arrow_up: |
   | 
[airflow/utils/tests.py](https://codecov.io/gh/apache/airflow/pull/6714/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/6714/diff?src=pr=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9xdWJvbGVfb3BlcmF0b3IucHk=)
 | `88.23% <100%> (+0.54%)` | :arrow_up: |
   | 
[airflow/operators/mysql\_operator.py](https://codecov.io/gh/apache/airflow/pull/6714/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXlzcWxfb3BlcmF0b3IucHk=)
 | `0% <0%> (-100%)` | :arrow_down: |
   | 
[airflow/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/6714/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXlzcWxfdG9faGl2ZS5weQ==)
 | `0% <0%> (-100%)` | :arrow_down: |
   | 
[airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6714/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==)
 | `44.44% <0%> (-55.56%)` | :arrow_down: |
   | 
[airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6714/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/6714/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/6714/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5)
 | `50.98% <0%> (-23.53%)` | :arrow_down: |
   | ... and [9 
more](https://codecov.io/gh/apache/airflow/pull/6714/diff?src=pr=tree-more) 
| |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/airflow/pull/6714?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/6714?src=pr=footer). 
Last update 
[91eb871...0c95ef0](https://codecov.io/gh/apache/airflow/pull/6714?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 #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
codecov-io edited a comment on issue #6596: [AIRFLOW-6004] Untangle Executors 
class to avoid cyclic imports
URL: https://github.com/apache/airflow/pull/6596#issuecomment-559224555
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6596?src=pr=h1) 
Report
   > Merging 
[#6596](https://codecov.io/gh/apache/airflow/pull/6596?src=pr=desc) into 
[master](https://codecov.io/gh/apache/airflow/commit/91eb87112b343ce4c18a994a41703bf793125dca?src=pr=desc)
 will **increase** coverage by `0.01%`.
   > The diff coverage is `84.2%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/airflow/pull/6596/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/6596?src=pr=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master#6596  +/-   ##
   ==
   + Coverage   83.82%   83.84%   +0.01% 
   ==
 Files 668  669   +1 
 Lines   3756937692 +123 
   ==
   + Hits3149431604 +110 
   - Misses   6075 6088  +13
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/airflow/pull/6596?src=pr=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[airflow/operators/mssql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/6596/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXNzcWxfdG9faGl2ZS5weQ==)
 | `100% <ø> (ø)` | :arrow_up: |
   | 
[airflow/kubernetes/kube\_client.py](https://codecov.io/gh/apache/airflow/pull/6596/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL2t1YmVfY2xpZW50LnB5)
 | `85.71% <ø> (ø)` | :arrow_up: |
   | 
[airflow/macros/\_\_init\_\_.py](https://codecov.io/gh/apache/airflow/pull/6596/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/6596/diff?src=pr=tree#diff-YWlyZmxvdy9jbGkvY29tbWFuZHMvZmxvd2VyX2NvbW1hbmQucHk=)
 | `0% <0%> (ø)` | :arrow_up: |
   | 
[airflow/cli/commands/serve\_logs\_command.py](https://codecov.io/gh/apache/airflow/pull/6596/diff?src=pr=tree#diff-YWlyZmxvdy9jbGkvY29tbWFuZHMvc2VydmVfbG9nc19jb21tYW5kLnB5)
 | `0% <0%> (ø)` | :arrow_up: |
   | 
[airflow/jobs/base\_job.py](https://codecov.io/gh/apache/airflow/pull/6596/diff?src=pr=tree#diff-YWlyZmxvdy9qb2JzL2Jhc2Vfam9iLnB5)
 | `88.81% <100%> (+0.07%)` | :arrow_up: |
   | 
[airflow/settings.py](https://codecov.io/gh/apache/airflow/pull/6596/diff?src=pr=tree#diff-YWlyZmxvdy9zZXR0aW5ncy5weQ==)
 | `88.81% <100%> (ø)` | :arrow_up: |
   | 
[airflow/models/dagbag.py](https://codecov.io/gh/apache/airflow/pull/6596/diff?src=pr=tree#diff-YWlyZmxvdy9tb2RlbHMvZGFnYmFnLnB5)
 | `86.58% <100%> (+0.1%)` | :arrow_up: |
   | 
[airflow/models/kubernetes.py](https://codecov.io/gh/apache/airflow/pull/6596/diff?src=pr=tree#diff-YWlyZmxvdy9tb2RlbHMva3ViZXJuZXRlcy5weQ==)
 | `100% <100%> (ø)` | :arrow_up: |
   | 
[airflow/models/baseoperator.py](https://codecov.io/gh/apache/airflow/pull/6596/diff?src=pr=tree#diff-YWlyZmxvdy9tb2RlbHMvYmFzZW9wZXJhdG9yLnB5)
 | `96.28% <100%> (ø)` | :arrow_up: |
   | ... and [30 
more](https://codecov.io/gh/apache/airflow/pull/6596/diff?src=pr=tree-more) 
| |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/airflow/pull/6596?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/6596?src=pr=footer). 
Last update 
[91eb871...29d4061](https://codecov.io/gh/apache/airflow/pull/6596?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 #6596: [AIRFLOW-6004] Untangle Executors class to avoid cyclic imports

2019-12-02 Thread GitBox
kaxil 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_r352911855
 
 

 ##
 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:
   Finally removed  


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 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-560857645
 
 
   Just read Ash's comment and I think it might be good to remove Abstract* 
versions  !
   
   >I think that the DagFileProcessor etc classes (how ever we decide to split 
them) would be better placed in utils.dag_processing -- that way we could 
probably also then remove the Abstract* versions -- there is only a single 
implementation of them anyway.
   
   


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 #6697: [AIRFLOW-6135] Extract DAG processing from SchedulerJob into separate class

2019-12-02 Thread GitBox
kaxil 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_r352907259
 
 

 ##
 File path: airflow/jobs/scheduler_job.py
 ##
 @@ -290,105 +292,28 @@ def start_time(self):
 return self._start_time
 
 
-class SchedulerJob(BaseJob):
-"""
-This SchedulerJob runs for a specific time interval and schedules the jobs
-that are ready to run. It figures out the latest runs for each
-task and sees if the dependencies for the next schedules are met.
-If so, it creates appropriate TaskInstances and sends run commands to the
-executor. It does this for each task in each DAG and repeats.
-
-:param dag_id: if specified, only schedule tasks with this DAG ID
-:type dag_id: unicode
-:param dag_ids: if specified, only schedule tasks with these DAG IDs
-:type dag_ids: list[unicode]
-:param subdir: directory containing Python files with Airflow DAG
-definitions, or a specific path to a file
-:type subdir: unicode
-:param num_runs: The number of times to try to schedule each DAG file.
--1 for unlimited times.
-:type num_runs: int
-:param processor_poll_interval: The number of seconds to wait between
-polls of running processors
-:type processor_poll_interval: int
-:param do_pickle: once a DAG object is obtained by executing the Python
-file, whether to serialize the DAG object to the DB
-:type do_pickle: bool
+class DagFileProcessor:
 """
+Process a Python file containing Airflow DAGs.
 
-__mapper_args__ = {
-'polymorphic_identity': 'SchedulerJob'
-}
-heartrate = conf.getint('scheduler', 'SCHEDULER_HEARTBEAT_SEC')
-
-def __init__(
-self,
-dag_id=None,
-dag_ids=None,
-subdir=settings.DAGS_FOLDER,
-num_runs=conf.getint('scheduler', 'num_runs'),
-processor_poll_interval=conf.getfloat('scheduler', 
'processor_poll_interval'),
-do_pickle=False,
-log=None,
-*args, **kwargs):
-# for BaseJob compatibility
-self.dag_id = dag_id
-self.dag_ids = [dag_id] if dag_id else []
-if dag_ids:
-self.dag_ids.extend(dag_ids)
-
-self.subdir = subdir
-
-self.num_runs = num_runs
-self._processor_poll_interval = processor_poll_interval
-
-self.do_pickle = do_pickle
-super().__init__(*args, **kwargs)
-
-self.max_threads = conf.getint('scheduler', 'max_threads')
-
-if log:
-self._log = log
-
-self.using_sqlite = False
-if 'sqlite' in conf.get('core', 'sql_alchemy_conn'):
-self.using_sqlite = True
-
-self.max_tis_per_query = conf.getint('scheduler', 'max_tis_per_query')
-self.processor_agent = None
-
-signal.signal(signal.SIGINT, self._exit_gracefully)
-signal.signal(signal.SIGTERM, self._exit_gracefully)
-
-def _exit_gracefully(self, signum, frame):
-"""
-Helper method to clean up processor_agent to avoid leaving orphan 
processes.
-"""
-self.log.info("Exiting gracefully upon receiving signal %s", signum)
-if self.processor_agent:
-self.processor_agent.end()
-sys.exit(os.EX_OK)
+This includes:
 
-def is_alive(self, grace_multiplier=None):
-"""
-Is this SchedulerJob alive?
+1. Execute the file and look for DAG objects in the namespace.
+2. Pickle the DAG and save it to the DB (if necessary).
+3. For each DAG, see what tasks should run and create appropriate task
+instances in the DB.
+4. Record any errors importing the file into ORM
+5. Kill (in ORM) any task instances belonging to the DAGs that haven't
+issued a heartbeat in a while.
 
-We define alive as in a state of running and a heartbeat within the
-threshold defined in the ``scheduler_health_check_threshold`` config
-setting.
-
-``grace_multiplier`` is accepted for compatibility with the parent 
class.
-
-:rtype: boolean
-"""
-if grace_multiplier is not None:
-# Accept the same behaviour as superclass
-return super().is_alive(grace_multiplier=grace_multiplier)
-scheduler_health_check_threshold = conf.getint('scheduler', 
'scheduler_health_check_threshold')
-return (
-self.state == State.RUNNING and
-(timezone.utcnow() - self.latest_heartbeat).seconds < 
scheduler_health_check_threshold
-)
+:param dag_ids: If specified, only look at these DAG ID's
 
 Review comment:
   ```suggestion
   
   :param dag_ids: If specified, only look at these DAG ID's
   ```


This is an automated message from the Apache Git Service.
To respond to the 

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

2019-12-02 Thread GitBox
kaxil 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_r352907442
 
 

 ##
 File path: airflow/jobs/scheduler_job.py
 ##
 @@ -290,105 +292,28 @@ def start_time(self):
 return self._start_time
 
 
-class SchedulerJob(BaseJob):
-"""
-This SchedulerJob runs for a specific time interval and schedules the jobs
-that are ready to run. It figures out the latest runs for each
-task and sees if the dependencies for the next schedules are met.
-If so, it creates appropriate TaskInstances and sends run commands to the
-executor. It does this for each task in each DAG and repeats.
-
-:param dag_id: if specified, only schedule tasks with this DAG ID
-:type dag_id: unicode
-:param dag_ids: if specified, only schedule tasks with these DAG IDs
-:type dag_ids: list[unicode]
-:param subdir: directory containing Python files with Airflow DAG
-definitions, or a specific path to a file
-:type subdir: unicode
-:param num_runs: The number of times to try to schedule each DAG file.
--1 for unlimited times.
-:type num_runs: int
-:param processor_poll_interval: The number of seconds to wait between
-polls of running processors
-:type processor_poll_interval: int
-:param do_pickle: once a DAG object is obtained by executing the Python
-file, whether to serialize the DAG object to the DB
-:type do_pickle: bool
+class DagFileProcessor:
 """
+Process a Python file containing Airflow DAGs.
 
-__mapper_args__ = {
-'polymorphic_identity': 'SchedulerJob'
-}
-heartrate = conf.getint('scheduler', 'SCHEDULER_HEARTBEAT_SEC')
-
-def __init__(
-self,
-dag_id=None,
-dag_ids=None,
-subdir=settings.DAGS_FOLDER,
-num_runs=conf.getint('scheduler', 'num_runs'),
-processor_poll_interval=conf.getfloat('scheduler', 
'processor_poll_interval'),
-do_pickle=False,
-log=None,
-*args, **kwargs):
-# for BaseJob compatibility
-self.dag_id = dag_id
-self.dag_ids = [dag_id] if dag_id else []
-if dag_ids:
-self.dag_ids.extend(dag_ids)
-
-self.subdir = subdir
-
-self.num_runs = num_runs
-self._processor_poll_interval = processor_poll_interval
-
-self.do_pickle = do_pickle
-super().__init__(*args, **kwargs)
-
-self.max_threads = conf.getint('scheduler', 'max_threads')
-
-if log:
-self._log = log
-
-self.using_sqlite = False
-if 'sqlite' in conf.get('core', 'sql_alchemy_conn'):
-self.using_sqlite = True
-
-self.max_tis_per_query = conf.getint('scheduler', 'max_tis_per_query')
-self.processor_agent = None
-
-signal.signal(signal.SIGINT, self._exit_gracefully)
-signal.signal(signal.SIGTERM, self._exit_gracefully)
-
-def _exit_gracefully(self, signum, frame):
-"""
-Helper method to clean up processor_agent to avoid leaving orphan 
processes.
-"""
-self.log.info("Exiting gracefully upon receiving signal %s", signum)
-if self.processor_agent:
-self.processor_agent.end()
-sys.exit(os.EX_OK)
+This includes:
 
-def is_alive(self, grace_multiplier=None):
-"""
-Is this SchedulerJob alive?
+1. Execute the file and look for DAG objects in the namespace.
+2. Pickle the DAG and save it to the DB (if necessary).
+3. For each DAG, see what tasks should run and create appropriate task
+instances in the DB.
+4. Record any errors importing the file into ORM
+5. Kill (in ORM) any task instances belonging to the DAGs that haven't
+issued a heartbeat in a while.
 
-We define alive as in a state of running and a heartbeat within the
-threshold defined in the ``scheduler_health_check_threshold`` config
-setting.
-
-``grace_multiplier`` is accepted for compatibility with the parent 
class.
-
-:rtype: boolean
-"""
-if grace_multiplier is not None:
-# Accept the same behaviour as superclass
-return super().is_alive(grace_multiplier=grace_multiplier)
-scheduler_health_check_threshold = conf.getint('scheduler', 
'scheduler_health_check_threshold')
-return (
-self.state == State.RUNNING and
-(timezone.utcnow() - self.latest_heartbeat).seconds < 
scheduler_health_check_threshold
-)
+:param dag_ids: If specified, only look at these DAG ID's
+:type dag_ids: list[unicode]
 
 Review comment:
   ```suggestion
   :type dag_ids: list[str]
   ```
   
   now that we no longer support Py2


This is an automated message from the 

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

2019-12-02 Thread GitBox
kaxil 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_r352906748
 
 

 ##
 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 like the idea: _**New process = New class**_


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] tfindlay-au commented on a change in pull request #6604: [AIRFLOW-5920] Neo4j operator and hook

2019-12-02 Thread GitBox
tfindlay-au commented on a change in pull request #6604: [AIRFLOW-5920] Neo4j 
operator and hook
URL: https://github.com/apache/airflow/pull/6604#discussion_r352903155
 
 

 ##
 File path: airflow/contrib/hooks/neo4j_hook.py
 ##
 @@ -0,0 +1,101 @@
+# -*- coding: utf-8 -*-
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""This hook provides minimal thin wrapper around the neo4j python library to 
provide query execution"""
+from neo4j import BoltStatementResult, Driver, GraphDatabase, Session
+
+from airflow.hooks.base_hook import BaseHook
+
+
+class Neo4JHook(BaseHook):
+"""This class enables the neo4j operator to execute queries against a 
configured neo4j server.
+It requires the configuration name as set in Airflow -> Connections ->
+:param n4j_conn_id:
+:type str:
+"""
+n4j_conn_id: str
+
+template_fields = ['n4j_conn_id']
+
+def __init__(self, n4j_conn_id: str = 'n4j_default', *args, **kwargs):
+self.n4j_conn_id = n4j_conn_id
+
+@staticmethod
+def get_config(n4j_conn_id: str) -> dict:
+"""
+Obtain the Username + Password from the Airflow connection definition
+Store them in _config dictionary as:
+*credentials* -- a tuple of username/password eg. ("username", 
"password")
+*host* -- String for Neo4J URI eg. "bolt://1.1.1.1:7687"
+:param n4j_conn_id: Name of connection configured in Airflow
+:type n4j_conn_id: str
+:return: dictionary with configuration values
+:rtype dict
+"""
+# Initialize with empty dictionary
+config: dict = {}
+connection_object = Neo4JHook.get_connection(n4j_conn_id)
+if connection_object.login and connection_object.host:
+config['credentials'] = connection_object.login, 
connection_object.password
+config['host'] = "bolt://{0}:{1}".format(connection_object.host, 
connection_object.port)
+
+return config
+
+@staticmethod
+def get_driver(config: dict) -> Driver:
+"""
+Establish a TCP connection to the server
+:param config: Dictionary containing the host and credentials needed 
to connect
+:return Driver connection
+:rtype neo4j.Driver
+"""
+# Check if we already have a driver we can re-use before creating a 
new one
+return GraphDatabase.driver(
+uri=config['host'],
+auth=config['credentials']
+)
+
+@staticmethod
+def get_session(driver: Driver) -> Session:
+"""
+Get a neo4j.session from the driver.
+:param driver Neo4J Driver (established connection to a server)
+:return Session Neo4J session which may contain many transactions
+:rtype neo4j.Session
+"""
+# Check if we already have a session we can re-use before creating a 
new one
 
 Review comment:
   Thanks - It originally worked this way, but not any more. I should have 
removed the comment when refactoring.


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] tfindlay-au commented on a change in pull request #6604: [AIRFLOW-5920] Neo4j operator and hook

2019-12-02 Thread GitBox
tfindlay-au commented on a change in pull request #6604: [AIRFLOW-5920] Neo4j 
operator and hook
URL: https://github.com/apache/airflow/pull/6604#discussion_r352903155
 
 

 ##
 File path: airflow/contrib/hooks/neo4j_hook.py
 ##
 @@ -0,0 +1,101 @@
+# -*- coding: utf-8 -*-
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""This hook provides minimal thin wrapper around the neo4j python library to 
provide query execution"""
+from neo4j import BoltStatementResult, Driver, GraphDatabase, Session
+
+from airflow.hooks.base_hook import BaseHook
+
+
+class Neo4JHook(BaseHook):
+"""This class enables the neo4j operator to execute queries against a 
configured neo4j server.
+It requires the configuration name as set in Airflow -> Connections ->
+:param n4j_conn_id:
+:type str:
+"""
+n4j_conn_id: str
+
+template_fields = ['n4j_conn_id']
+
+def __init__(self, n4j_conn_id: str = 'n4j_default', *args, **kwargs):
+self.n4j_conn_id = n4j_conn_id
+
+@staticmethod
+def get_config(n4j_conn_id: str) -> dict:
+"""
+Obtain the Username + Password from the Airflow connection definition
+Store them in _config dictionary as:
+*credentials* -- a tuple of username/password eg. ("username", 
"password")
+*host* -- String for Neo4J URI eg. "bolt://1.1.1.1:7687"
+:param n4j_conn_id: Name of connection configured in Airflow
+:type n4j_conn_id: str
+:return: dictionary with configuration values
+:rtype dict
+"""
+# Initialize with empty dictionary
+config: dict = {}
+connection_object = Neo4JHook.get_connection(n4j_conn_id)
+if connection_object.login and connection_object.host:
+config['credentials'] = connection_object.login, 
connection_object.password
+config['host'] = "bolt://{0}:{1}".format(connection_object.host, 
connection_object.port)
+
+return config
+
+@staticmethod
+def get_driver(config: dict) -> Driver:
+"""
+Establish a TCP connection to the server
+:param config: Dictionary containing the host and credentials needed 
to connect
+:return Driver connection
+:rtype neo4j.Driver
+"""
+# Check if we already have a driver we can re-use before creating a 
new one
+return GraphDatabase.driver(
+uri=config['host'],
+auth=config['credentials']
+)
+
+@staticmethod
+def get_session(driver: Driver) -> Session:
+"""
+Get a neo4j.session from the driver.
+:param driver Neo4J Driver (established connection to a server)
+:return Session Neo4J session which may contain many transactions
+:rtype neo4j.Session
+"""
+# Check if we already have a session we can re-use before creating a 
new one
 
 Review comment:
   Thanks - It was originally worked this way, but not any more. I should have 
removed the comment when refactoring.


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] tfindlay-au commented on a change in pull request #6604: [AIRFLOW-5920] Neo4j operator and hook

2019-12-02 Thread GitBox
tfindlay-au commented on a change in pull request #6604: [AIRFLOW-5920] Neo4j 
operator and hook
URL: https://github.com/apache/airflow/pull/6604#discussion_r352908365
 
 

 ##
 File path: airflow/contrib/hooks/neo4j_hook.py
 ##
 @@ -0,0 +1,101 @@
+# -*- coding: utf-8 -*-
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""This hook provides minimal thin wrapper around the neo4j python library to 
provide query execution"""
+from neo4j import BoltStatementResult, Driver, GraphDatabase, Session
+
+from airflow.hooks.base_hook import BaseHook
+
+
+class Neo4JHook(BaseHook):
+"""This class enables the neo4j operator to execute queries against a 
configured neo4j server.
 
 Review comment:
   Thanks, have updated to reflect 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] tfindlay-au commented on a change in pull request #6604: [AIRFLOW-5920] Neo4j operator and hook

2019-12-02 Thread GitBox
tfindlay-au commented on a change in pull request #6604: [AIRFLOW-5920] Neo4j 
operator and hook
URL: https://github.com/apache/airflow/pull/6604#discussion_r352908491
 
 

 ##
 File path: airflow/contrib/hooks/neo4j_hook.py
 ##
 @@ -0,0 +1,101 @@
+# -*- coding: utf-8 -*-
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""This hook provides minimal thin wrapper around the neo4j python library to 
provide query execution"""
+from neo4j import BoltStatementResult, Driver, GraphDatabase, Session
+
+from airflow.hooks.base_hook import BaseHook
+
+
+class Neo4JHook(BaseHook):
+"""This class enables the neo4j operator to execute queries against a 
configured neo4j server.
+It requires the configuration name as set in Airflow -> Connections ->
 
 Review comment:
   Thanks, should be fixed now.


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


With regards,
Apache Git Services


[GitHub] [airflow] tfindlay-au commented on a change in pull request #6604: [AIRFLOW-5920] Neo4j operator and hook

2019-12-02 Thread GitBox
tfindlay-au commented on a change in pull request #6604: [AIRFLOW-5920] Neo4j 
operator and hook
URL: https://github.com/apache/airflow/pull/6604#discussion_r352908300
 
 

 ##
 File path: airflow/contrib/hooks/neo4j_hook.py
 ##
 @@ -0,0 +1,101 @@
+# -*- coding: utf-8 -*-
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""This hook provides minimal thin wrapper around the neo4j python library to 
provide query execution"""
+from neo4j import BoltStatementResult, Driver, GraphDatabase, Session
+
+from airflow.hooks.base_hook import BaseHook
+
+
+class Neo4JHook(BaseHook):
+"""This class enables the neo4j operator to execute queries against a 
configured neo4j server.
+It requires the configuration name as set in Airflow -> Connections ->
+:param n4j_conn_id:
 
 Review comment:
   Done.


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] XD-DENG merged pull request #6708: [AIRFLOW-XXX] Fix docstring minor issues in airflow/kubernetes/

2019-12-02 Thread GitBox
XD-DENG merged pull request #6708: [AIRFLOW-XXX] Fix docstring minor issues in 
airflow/kubernetes/
URL: https://github.com/apache/airflow/pull/6708
 
 
   


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 opened a new pull request #6716: [AIRFLOW-6159] Change logging level of the heartbeat message to DEBUG

2019-12-02 Thread GitBox
kaxil opened a new pull request #6716: [AIRFLOW-6159] Change logging level of 
the heartbeat message to DEBUG
URL: https://github.com/apache/airflow/pull/6716
 
 
   Make sure you have checked _all_ steps below.
   
   ### Jira
   
   - [x] 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-6159
   
   
   ### Description
   
   - [x] Here are some details about my PR, including screenshots of any UI 
changes:
   As mentioned in 
https://stackoverflow.com/questions/59142508/airflow-disable-heartbeat-logs , 
the logs containing heartbeat details can become too noisy.
   
   ### Tests
   
   - [ ] My PR adds the following unit tests __OR__ does not need testing for 
this extremely good reason:
   
   ### Commits
   
   - [x] 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
   
   - [x] 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-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=16986444#comment-16986444
 ] 

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
 
 
   Make sure you have checked _all_ steps below.
   
   ### Jira
   
   - [x] 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-6159
   
   
   ### Description
   
   - [x] Here are some details about my PR, including screenshots of any UI 
changes:
   As mentioned in 
https://stackoverflow.com/questions/59142508/airflow-disable-heartbeat-logs , 
the logs containing heartbeat details can become too noisy.
   
   ### Tests
   
   - [ ] My PR adds the following unit tests __OR__ does not need testing for 
this extremely good reason:
   
   ### Commits
   
   - [x] 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
   
   - [x] 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


> 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] [Updated] (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 updated AIRFLOW-6159:

Description: As mentioned in 
https://stackoverflow.com/questions/59142508/airflow-disable-heartbeat-logs , 
the logs containing heartbeat details can become too noisy. 

> 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-6159) Change logging level of the heartbeat message to DEBUG

2019-12-02 Thread Kaxil Naik (Jira)
Kaxil Naik created AIRFLOW-6159:
---

 Summary: 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






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


[GitHub] [airflow] tfindlay-au commented on a change in pull request #6604: [AIRFLOW-5920] Neo4j operator and hook

2019-12-02 Thread GitBox
tfindlay-au commented on a change in pull request #6604: [AIRFLOW-5920] Neo4j 
operator and hook
URL: https://github.com/apache/airflow/pull/6604#discussion_r352903466
 
 

 ##
 File path: airflow/contrib/hooks/neo4j_hook.py
 ##
 @@ -0,0 +1,101 @@
+# -*- coding: utf-8 -*-
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""This hook provides minimal thin wrapper around the neo4j python library to 
provide query execution"""
+from neo4j import BoltStatementResult, Driver, GraphDatabase, Session
+
+from airflow.hooks.base_hook import BaseHook
+
+
+class Neo4JHook(BaseHook):
+"""This class enables the neo4j operator to execute queries against a 
configured neo4j server.
+It requires the configuration name as set in Airflow -> Connections ->
+:param n4j_conn_id:
+:type str:
+"""
+n4j_conn_id: str
+
+template_fields = ['n4j_conn_id']
+
+def __init__(self, n4j_conn_id: str = 'n4j_default', *args, **kwargs):
+self.n4j_conn_id = n4j_conn_id
+
+@staticmethod
+def get_config(n4j_conn_id: str) -> dict:
+"""
+Obtain the Username + Password from the Airflow connection definition
+Store them in _config dictionary as:
+*credentials* -- a tuple of username/password eg. ("username", 
"password")
+*host* -- String for Neo4J URI eg. "bolt://1.1.1.1:7687"
+:param n4j_conn_id: Name of connection configured in Airflow
+:type n4j_conn_id: str
+:return: dictionary with configuration values
+:rtype dict
+"""
+# Initialize with empty dictionary
+config: dict = {}
+connection_object = Neo4JHook.get_connection(n4j_conn_id)
+if connection_object.login and connection_object.host:
+config['credentials'] = connection_object.login, 
connection_object.password
+config['host'] = "bolt://{0}:{1}".format(connection_object.host, 
connection_object.port)
+
+return config
+
+@staticmethod
+def get_driver(config: dict) -> Driver:
+"""
+Establish a TCP connection to the server
+:param config: Dictionary containing the host and credentials needed 
to connect
+:return Driver connection
+:rtype neo4j.Driver
+"""
+# Check if we already have a driver we can re-use before creating a 
new one
 
 Review comment:
   Thanks - It originally worked this way, but not any more. I should have 
removed the comment when refactoring.


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] tfindlay-au commented on a change in pull request #6604: [AIRFLOW-5920] Neo4j operator and hook

2019-12-02 Thread GitBox
tfindlay-au commented on a change in pull request #6604: [AIRFLOW-5920] Neo4j 
operator and hook
URL: https://github.com/apache/airflow/pull/6604#discussion_r352903155
 
 

 ##
 File path: airflow/contrib/hooks/neo4j_hook.py
 ##
 @@ -0,0 +1,101 @@
+# -*- coding: utf-8 -*-
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""This hook provides minimal thin wrapper around the neo4j python library to 
provide query execution"""
+from neo4j import BoltStatementResult, Driver, GraphDatabase, Session
+
+from airflow.hooks.base_hook import BaseHook
+
+
+class Neo4JHook(BaseHook):
+"""This class enables the neo4j operator to execute queries against a 
configured neo4j server.
+It requires the configuration name as set in Airflow -> Connections ->
+:param n4j_conn_id:
+:type str:
+"""
+n4j_conn_id: str
+
+template_fields = ['n4j_conn_id']
+
+def __init__(self, n4j_conn_id: str = 'n4j_default', *args, **kwargs):
+self.n4j_conn_id = n4j_conn_id
+
+@staticmethod
+def get_config(n4j_conn_id: str) -> dict:
+"""
+Obtain the Username + Password from the Airflow connection definition
+Store them in _config dictionary as:
+*credentials* -- a tuple of username/password eg. ("username", 
"password")
+*host* -- String for Neo4J URI eg. "bolt://1.1.1.1:7687"
+:param n4j_conn_id: Name of connection configured in Airflow
+:type n4j_conn_id: str
+:return: dictionary with configuration values
+:rtype dict
+"""
+# Initialize with empty dictionary
+config: dict = {}
+connection_object = Neo4JHook.get_connection(n4j_conn_id)
+if connection_object.login and connection_object.host:
+config['credentials'] = connection_object.login, 
connection_object.password
+config['host'] = "bolt://{0}:{1}".format(connection_object.host, 
connection_object.port)
+
+return config
+
+@staticmethod
+def get_driver(config: dict) -> Driver:
+"""
+Establish a TCP connection to the server
+:param config: Dictionary containing the host and credentials needed 
to connect
+:return Driver connection
+:rtype neo4j.Driver
+"""
+# Check if we already have a driver we can re-use before creating a 
new one
+return GraphDatabase.driver(
+uri=config['host'],
+auth=config['credentials']
+)
+
+@staticmethod
+def get_session(driver: Driver) -> Session:
+"""
+Get a neo4j.session from the driver.
+:param driver Neo4J Driver (established connection to a server)
+:return Session Neo4J session which may contain many transactions
+:rtype neo4j.Session
+"""
+# Check if we already have a session we can re-use before creating a 
new one
 
 Review comment:
   Thanks - It originally worked this way, but not any more. I should have 
removed the comment when refactoring.


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-6158) Upgrade sendgrid dependency

2019-12-02 Thread Marcin Szymanski (Jira)
Marcin Szymanski created AIRFLOW-6158:
-

 Summary: Upgrade sendgrid dependency
 Key: AIRFLOW-6158
 URL: https://issues.apache.org/jira/browse/AIRFLOW-6158
 Project: Apache Airflow
  Issue Type: Improvement
  Components: dependencies
Affects Versions: 1.10.6
Reporter: Marcin Szymanski
Assignee: Marcin Szymanski


Parameter naming changed in Sendgrid client 6.0.0



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


[GitHub] [airflow] tfindlay-au commented on a change in pull request #6604: [AIRFLOW-5920] Neo4j operator and hook

2019-12-02 Thread GitBox
tfindlay-au commented on a change in pull request #6604: [AIRFLOW-5920] Neo4j 
operator and hook
URL: https://github.com/apache/airflow/pull/6604#discussion_r352901973
 
 

 ##
 File path: airflow/contrib/hooks/neo4j_hook.py
 ##
 @@ -0,0 +1,101 @@
+# -*- coding: utf-8 -*-
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""This hook provides minimal thin wrapper around the neo4j python library to 
provide query execution"""
+from neo4j import BoltStatementResult, Driver, GraphDatabase, Session
+
+from airflow.hooks.base_hook import BaseHook
+
+
+class Neo4JHook(BaseHook):
+"""This class enables the neo4j operator to execute queries against a 
configured neo4j server.
+It requires the configuration name as set in Airflow -> Connections ->
+:param n4j_conn_id:
+:type str:
+"""
+n4j_conn_id: str
 
 Review comment:
   I defined this class variable as it was set when the class was instantiated 
via __init__(). It needs to be held in the class object so it can be consumed 
later.
   
   If there is a better way to store / retrieve this value, I am happy to 
change 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] kaxil commented on issue #6715: [AIRFLOW-5945] Make inbuilt OperatorLinks work when using Serialization

2019-12-02 Thread GitBox
kaxil commented on issue #6715: [AIRFLOW-5945] Make inbuilt OperatorLinks work 
when using Serialization
URL: https://github.com/apache/airflow/pull/6715#issuecomment-560795290
 
 
   > What do you think to save the generated links in a separate column in the 
database after Task execution? I think it will be nicer because it will not be 
storing additional partial data, but the final data that interests us. This 
could be realized independent of DAG serialization. In addition, it will 
simplify the logic of handling these links. Now it is not possible to download 
all links for a given task, but we download one link in one HTTP request. 
https://github.com/apache/airflow/blob/master/airflow/www/templates/airflow/dag.html#L492-L518
   
   This is actually the 1st solution I mentioned to Ash and I thought it was 
the way to go :) Unfortunately Ash mentioned a use-case that ruled it out which 
was I think "S3 signed URL can have an expiration date of 15 mins and change 
after that". @ashb can probably explain that in more detail but due to that 
use-case storing the links in DB wouldn't 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] 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_r352895446
 
 

 ##
 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 think the name DagFileProcessorProcess will be better. This better 
describes its roles.


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 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-560761473
 
 
   I would love to wait for Jarek's changes. I am pleased with the warm words 
about this change.


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


With regards,
Apache Git Services


[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_r352893158
 
 

 ##
 File path: airflow/jobs/__init__.py
 ##
 @@ -23,4 +23,4 @@
 from airflow.jobs.backfill_job import BackfillJob  # noqa: F401
 from airflow.jobs.base_job import BaseJob  # noqa: F401
 from airflow.jobs.local_task_job import LocalTaskJob  # noqa: F401
-from airflow.jobs.scheduler_job import DagFileProcessor, SchedulerJob  # noqa: 
F401
+from airflow.jobs.scheduler_job import DagFileProcessor, 
DagFileProcessorRunner, SchedulerJob  # noqa: F401
 
 Review comment:
   Ok. Agree. I will remove 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] 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 level 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


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

2019-12-02 Thread GitBox
mik-laj commented on issue #6715: [AIRFLOW-5945] Make inbuilt OperatorLinks 
work when using Serialization
URL: https://github.com/apache/airflow/pull/6715#issuecomment-560746534
 
 
   What do you think to save the generated links in a separate column in the 
database after Task execution?  I think it will be nicer because it will not be 
storing additional partial data, but the final data that interests us.  This 
could be realized independent of DAG serialization. In addition, it will 
simplify the logic of handling these links. Now it is not possible to download 
all links for a given task, but we download one link in one HTTP request. 
https://github.com/apache/airflow/blob/master/airflow/www/templates/airflow/dag.html#L492-L518


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 edited a comment on issue #6714: [AIRFLOW-6153] Allow adding Custom Fields to Serialized Operators

2019-12-02 Thread GitBox
kaxil edited a comment on issue #6714: [AIRFLOW-6153] Allow adding Custom 
Fields to Serialized Operators
URL: https://github.com/apache/airflow/pull/6714#issuecomment-560726423
 
 
   > @potiuk We once talked that DAG serialization can be done by serializing 
class arguments in the database. This PR brings us closer to implementing this 
idea.
   
   We discussed this initially, however that can easily become a pain as the 
Serialized Blob grows in sizes rapidly. We don't need all the information (of a 
task) for the Webserver and the Scheduler. Hence, the 1st task of DAG 
Serialization was to figure out what information is actually required by the 
Webserver.
   
   Increased JSON Blob size caused performance issues when we benchmarked it in 
#5743 so we reduced the blob size to as small as we could.


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 issue #6714: [AIRFLOW-6153] Allow adding Custom Fields to Serialized Operators

2019-12-02 Thread GitBox
kaxil commented on issue #6714: [AIRFLOW-6153] Allow adding Custom Fields to 
Serialized Operators
URL: https://github.com/apache/airflow/pull/6714#issuecomment-560726423
 
 
   > @potiuk We once talked that DAG serialization can be done by serializing 
class arguments in the database. This PR brings us closer to implementing this 
idea.
   
   We discussed this initially, however that can easily become a pain as the 
Serialized Blob grows in sizes rapidly. We don't need all the information (of a 
task) for the Webserver and the Scheduler. Hence, the 1st task of DAG 
Serialization was to figure out what information is actually required by the 
Webserver.
   
   Increased JSON Blob size caused performance issues when we benchmarked it 
#5743 so we reduced the blob size to as small as we could.


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 commented on issue #6714: [AIRFLOW-6153] Allow adding Custom Fields to Serialized Operators

2019-12-02 Thread GitBox
mik-laj commented on issue #6714: [AIRFLOW-6153] Allow adding Custom Fields to 
Serialized Operators
URL: https://github.com/apache/airflow/pull/6714#issuecomment-560715143
 
 
   @potiuk We once talked that DAG serialization can be done by serializing 
class arguments in the database. This PR brings us closer to implementing this 
idea.


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-5945) Fix limitation with OperatorLinks when using DAG Serialization

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


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

ASF GitHub Bot commented on AIRFLOW-5945:
-

kaxil commented on pull request #6715: [AIRFLOW-5945] Make inbuilt 
OperatorLinks work when using Serialization
URL: https://github.com/apache/airflow/pull/6715
 
 
   Follow Up PR of https://github.com/apache/airflow/pull/6714
   
   Make sure you have checked _all_ steps below.
   
   ### Jira
   
   - [x] 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-5945
   
   ### Description
   
   - [x] Here are some details about my PR, including screenshots of any UI 
changes:
   The inbuilt Operator links were not working when using DAG Serialization 
(which was 1 of the limitations of DAG Serialization). The work-around 
previously was adding those OperatorLinks using Airflow Plugin.
   
   This PR fixes it so we inbuilt links work out of the box. This PR is 
dependent on https://github.com/apache/airflow/pull/6714
   
   ### Tests
   
   - [x] My PR adds the following unit tests __OR__ does not need testing for 
this extremely good reason:
   Added
   
   ### Commits
   
   - [x] 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
   
   - [x] 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


> Fix limitation with OperatorLinks when using DAG Serialization
> --
>
> Key: AIRFLOW-5945
> URL: https://issues.apache.org/jira/browse/AIRFLOW-5945
> Project: Apache Airflow
>  Issue Type: Improvement
>  Components: core, webserver
>Affects Versions: 2.0.0, 1.10.7
>Reporter: Kaxil Naik
>Assignee: Kaxil Naik
>Priority: Major
>
> Operator links (Re-write the Quoble link to not need an operator of the right 
> class and add this to the docs)
> or store the info in a new column in Task Instance or Serialized DAG table.



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


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

2019-12-02 Thread GitBox
kaxil commented on issue #6715: [AIRFLOW-5945] Make inbuilt OperatorLinks work 
when using Serialization
URL: https://github.com/apache/airflow/pull/6715#issuecomment-560711709
 
 
   This PR is dependent on #6714 so please only review 
https://github.com/apache/airflow/pull/6715/commits/92078dab7c201c1fdcd39c6b4850973839fd8d54
 for this PR.


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


With regards,
Apache Git Services


[GitHub] [airflow] kaxil opened a new pull request #6715: [AIRFLOW-5945] Make inbuilt OperatorLinks work when using Serialization

2019-12-02 Thread GitBox
kaxil opened a new pull request #6715: [AIRFLOW-5945] Make inbuilt 
OperatorLinks work when using Serialization
URL: https://github.com/apache/airflow/pull/6715
 
 
   Follow Up PR of https://github.com/apache/airflow/pull/6714
   
   Make sure you have checked _all_ steps below.
   
   ### Jira
   
   - [x] 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-5945
   
   ### Description
   
   - [x] Here are some details about my PR, including screenshots of any UI 
changes:
   The inbuilt Operator links were not working when using DAG Serialization 
(which was 1 of the limitations of DAG Serialization). The work-around 
previously was adding those OperatorLinks using Airflow Plugin.
   
   This PR fixes it so we inbuilt links work out of the box. This PR is 
dependent on https://github.com/apache/airflow/pull/6714
   
   ### Tests
   
   - [x] My PR adds the following unit tests __OR__ does not need testing for 
this extremely good reason:
   Added
   
   ### Commits
   
   - [x] 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
   
   - [x] 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] [Created] (AIRFLOW-6157) Run multiple executors

2019-12-02 Thread Jarek Potiuk (Jira)
Jarek Potiuk created AIRFLOW-6157:
-

 Summary: Run multiple executors
 Key: AIRFLOW-6157
 URL: https://issues.apache.org/jira/browse/AIRFLOW-6157
 Project: Apache Airflow
  Issue Type: Improvement
  Components: core
Affects Versions: 2.0.0
Reporter: Jarek Potiuk


Proof of concept to support multiple executors

 



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


[GitHub] [airflow] dougblack closed pull request #6673: Pod aliases

2019-12-02 Thread GitBox
dougblack closed pull request #6673: Pod aliases
URL: https://github.com/apache/airflow/pull/6673
 
 
   


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-6153) Allow adding Custom Fields to Serialized Operators

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


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

ASF GitHub Bot commented on AIRFLOW-6153:
-

kaxil commented on pull request #6714: [AIRFLOW-6153] Allow adding Custom 
Fields to Serialized Operators
URL: https://github.com/apache/airflow/pull/6714
 
 
   Make sure you have checked _all_ steps below.
   
   ### Jira
   
   - [x] My PR addresses the following [Airflow 
Jira](https://issues.apache.org/jira/browse/AIRFLOW/) issues and references 
them in the PR title. 
 - https://issues.apache.org/jira/browse/AIRFLOW-6153
   
   
   ### Description
   
   - [x] Here are some details about my PR, including screenshots of any UI 
changes:
   We would want to allow fields like `sql` (in BigQueryOperator) and 
`qubole_conn_id` (in QuboleOperator) to be serialized and saved in DB. This is 
needed for OperatorLinks to work for both the respective operators as they rely 
on this field.
   
   
   
   ### Tests
   
   - [x] My PR adds the following unit tests __OR__ does not need testing for 
this extremely good reason:
   Added
   
   ### Commits
   
   - [x] 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
   
   - [x] 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


> Allow adding Custom Fields to Serialized Operators
> --
>
> Key: AIRFLOW-6153
> URL: https://issues.apache.org/jira/browse/AIRFLOW-6153
> Project: Apache Airflow
>  Issue Type: Sub-task
>  Components: core
>Affects Versions: 2.0.0
>Reporter: Kaxil Naik
>Assignee: Kaxil Naik
>Priority: Major
> Fix For: 2.0.0
>
>
> We would want to allow fields like `sql` (in BigQueryOperator) and 
> `qubole_conn_id` (in QuboleOperator) to be serialized and saved in DB. This 
> is needed for OperatorLinks to work for both the respective operators as they 
> rely on this field.



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


[GitHub] [airflow] kaxil opened a new pull request #6714: [AIRFLOW-6153] Allow adding Custom Fields to Serialized Operators

2019-12-02 Thread GitBox
kaxil opened a new pull request #6714: [AIRFLOW-6153] Allow adding Custom 
Fields to Serialized Operators
URL: https://github.com/apache/airflow/pull/6714
 
 
   Make sure you have checked _all_ steps below.
   
   ### Jira
   
   - [x] My PR addresses the following [Airflow 
Jira](https://issues.apache.org/jira/browse/AIRFLOW/) issues and references 
them in the PR title. 
 - https://issues.apache.org/jira/browse/AIRFLOW-6153
   
   
   ### Description
   
   - [x] Here are some details about my PR, including screenshots of any UI 
changes:
   We would want to allow fields like `sql` (in BigQueryOperator) and 
`qubole_conn_id` (in QuboleOperator) to be serialized and saved in DB. This is 
needed for OperatorLinks to work for both the respective operators as they rely 
on this field.
   
   
   
   ### Tests
   
   - [x] My PR adds the following unit tests __OR__ does not need testing for 
this extremely good reason:
   Added
   
   ### Commits
   
   - [x] 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
   
   - [x] 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


[GitHub] [airflow] codecov-io commented on issue #6683: [AIRFLOW-6019] Add flushing in execute method for BigQueryCursor

2019-12-02 Thread GitBox
codecov-io commented on issue #6683: [AIRFLOW-6019] Add flushing in execute 
method for BigQueryCursor
URL: https://github.com/apache/airflow/pull/6683#issuecomment-560622552
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6683?src=pr=h1) 
Report
   > Merging 
[#6683](https://codecov.io/gh/apache/airflow/pull/6683?src=pr=desc) into 
[master](https://codecov.io/gh/apache/airflow/commit/e82008d96c8aa78a08fd5f832615feed427d8ea6?src=pr=desc)
 will **decrease** coverage by `0.48%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/airflow/pull/6683/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/6683?src=pr=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master#6683  +/-   ##
   ==
   - Coverage   83.82%   83.34%   -0.49% 
   ==
 Files 668  668  
 Lines   3754338013 +470 
   ==
   + Hits3147231681 +209 
   - Misses   6071 6332 +261
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/airflow/pull/6683?src=pr=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[airflow/gcp/hooks/bigquery.py](https://codecov.io/gh/apache/airflow/pull/6683/diff?src=pr=tree#diff-YWlyZmxvdy9nY3AvaG9va3MvYmlncXVlcnkucHk=)
 | `70.38% <100%> (+0.18%)` | :arrow_up: |
   | 
[airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6683/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==)
 | `44.44% <0%> (-55.56%)` | :arrow_down: |
   | 
[airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6683/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/6683/diff?src=pr=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==)
 | `45.25% <0%> (-46.72%)` | :arrow_down: |
   | 
[airflow/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/6683/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXlzcWxfdG9faGl2ZS5weQ==)
 | `68% <0%> (-32%)` | :arrow_down: |
   | 
[airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6683/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/6683/diff?src=pr=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==)
 | `78.2% <0%> (-20.52%)` | :arrow_down: |
   | 
[airflow/utils/dag\_processing.py](https://codecov.io/gh/apache/airflow/pull/6683/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy9kYWdfcHJvY2Vzc2luZy5weQ==)
 | `54.48% <0%> (-3.68%)` | :arrow_down: |
   | 
[airflow/configuration.py](https://codecov.io/gh/apache/airflow/pull/6683/diff?src=pr=tree#diff-YWlyZmxvdy9jb25maWd1cmF0aW9uLnB5)
 | `89.13% <0%> (-3.63%)` | :arrow_down: |
   | 
[airflow/models/dagbag.py](https://codecov.io/gh/apache/airflow/pull/6683/diff?src=pr=tree#diff-YWlyZmxvdy9tb2RlbHMvZGFnYmFnLnB5)
 | `85.99% <0%> (-0.54%)` | :arrow_down: |
   | ... and [17 
more](https://codecov.io/gh/apache/airflow/pull/6683/diff?src=pr=tree-more) 
| |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/airflow/pull/6683?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/6683?src=pr=footer). 
Last update 
[e82008d...fcf0c63](https://codecov.io/gh/apache/airflow/pull/6683?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] jomach commented on issue #6694: AIRFLOW-6132 - Allow to pass in tags

2019-12-02 Thread GitBox
jomach commented on issue #6694: AIRFLOW-6132 - Allow to pass in tags
URL: https://github.com/apache/airflow/pull/6694#issuecomment-560612984
 
 
   @potiuk can  we merge ?


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 #6683: [AIRFLOW-6019] Add flushing in execute method for BigQueryCursor

2019-12-02 Thread GitBox
potiuk commented on issue #6683: [AIRFLOW-6019] Add flushing in execute method 
for BigQueryCursor
URL: https://github.com/apache/airflow/pull/6683#issuecomment-560583506
 
 
   I restarted the failing job. It's a flaky "run_on_kill" test that 
@nuclearpinguin is solving with the upcoming #6472 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] zuku1985 commented on issue #6683: [AIRFLOW-6019] Add flushing in execute method for BigQueryCursor

2019-12-02 Thread GitBox
zuku1985 commented on issue #6683: [AIRFLOW-6019] Add flushing in execute 
method for BigQueryCursor
URL: https://github.com/apache/airflow/pull/6683#issuecomment-560581631
 
 
   @nuclearpinguin any hints what should I do to fix failing tests? I ran out 
of ideas :/


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-6156) sla_miss_callback on baseoperator for task level

2019-12-02 Thread Chengzhi Zhao (Jira)
Chengzhi Zhao created AIRFLOW-6156:
--

 Summary: sla_miss_callback on baseoperator for task level
 Key: AIRFLOW-6156
 URL: https://issues.apache.org/jira/browse/AIRFLOW-6156
 Project: Apache Airflow
  Issue Type: Improvement
  Components: operators
Affects Versions: 1.10.6
Reporter: Chengzhi Zhao


It looks like the sla_miss_callback is currently at DAG level, any interests to 
bring it to task level so users can override it when the different operator 
requires the different callbacks, just like `on_failure_callback`? Happy to 
work on if this is necessary.  

 



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


[GitHub] [airflow] nuclearpinguin commented on a change in pull request #6713: Add some r/w functionality to GCP Bigtable Hook

2019-12-02 Thread GitBox
nuclearpinguin commented on a change in pull request #6713: Add some r/w 
functionality to GCP Bigtable Hook
URL: https://github.com/apache/airflow/pull/6713#discussion_r352818753
 
 

 ##
 File path: airflow/gcp/hooks/bigtable.py
 ##
 @@ -276,3 +278,58 @@ def get_cluster_states_for_table(instance: Instance, 
table_id: str) -> Dict[str,
 
 table = Table(table_id, instance)
 return table.get_cluster_states()
+
+def create_rowkey_regex_filter(self, regex):
 
 Review comment:
   Where is the helper used?


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 #6713: Add some r/w functionality to GCP Bigtable Hook

2019-12-02 Thread GitBox
nuclearpinguin commented on a change in pull request #6713: Add some r/w 
functionality to GCP Bigtable Hook
URL: https://github.com/apache/airflow/pull/6713#discussion_r352819063
 
 

 ##
 File path: airflow/gcp/hooks/bigtable.py
 ##
 @@ -276,3 +278,58 @@ def get_cluster_states_for_table(instance: Instance, 
table_id: str) -> Dict[str,
 
 table = Table(table_id, instance)
 return table.get_cluster_states()
+
+def create_rowkey_regex_filter(self, regex):
+"""
+A helper method to create a RowKeyRegexFilter subclass of RowFilter, 
which
+will only match rows with keys that exactly match the regex string.
+Returns a RowFilter.
+
+:type regex: str
+:param regex: The exact match regex string for the value we wish to 
find
+"""
+return RowKeyRegexFilter(regex)
+
+
+def delete_row(self, instance, table_id, row_key):
 
 Review comment:
   Please add type annotations.
   ```suggestion
   def delete_row(self, instance: Instance, table_id: str, row_key: str) -> 
None:
   ```


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-6155) Add some reading and writing functionality to the GCP Bigtable hook

2019-12-02 Thread Emily Mazo (Jira)


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

Emily Mazo commented on AIRFLOW-6155:
-

PR: [https://github.com/apache/airflow/pull/6713]

> 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
>Priority: Major
>
> 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)


[GitHub] [airflow] EmilyMazo opened a new pull request #6713: Add some r/w functionality to GCP Bigtable Hook

2019-12-02 Thread GitBox
EmilyMazo opened a new pull request #6713: Add some r/w functionality to GCP 
Bigtable Hook
URL: https://github.com/apache/airflow/pull/6713
 
 
   Make sure you have checked _all_ steps below.
   
   ### Jira
   
   - [ X] 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-6155
   
   ### Description
   
   - [ X] Here are some details about my PR, including screenshots of any UI 
changes:
   The GCP Bigtable Hook, a wrapper about the GCP Client functionality for 
Bigtable, has only a few methods currently, all addressing administrative 
tasks. I would like to add the ability to read and write Bigtable data in rows 
using the Hook.
   
   ### Tests
   
   - [X ] My PR adds the following unit tests __OR__ does not need testing for 
this extremely good reason:
   tests/gcp/hooks/test_bigtable/test_create_rowkey_regex_filter
   tests/gcp/hooks/test_bigtable/test_delete_row
   tests/gcp/hooks/test_bigtable/test_check_and_mutate_row
   
   ### Commits
   
   - [ X] 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
   
   - [ X] 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] [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)


[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


[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] 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 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


[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)


[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 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


  1   2   3   >