[GitHub] codecov-io commented on issue #4734: [AIRFLOW-XXXX] Use on_load in plugin example
codecov-io commented on issue #4734: [AIRFLOW-] Use on_load in plugin example URL: https://github.com/apache/airflow/pull/4734#issuecomment-465463699 # [Codecov](https://codecov.io/gh/apache/airflow/pull/4734?src=pr=h1) Report > Merging [#4734](https://codecov.io/gh/apache/airflow/pull/4734?src=pr=desc) into [master](https://codecov.io/gh/apache/airflow/commit/de75b7a2bd7f5bef6a1d09942e0b43c17a3fbb95?src=pr=desc) will **not change** coverage. > The diff coverage is `n/a`. [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/4734/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/4734?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#4734 +/- ## === Coverage 74.61% 74.61% === Files 430 430 Lines 2800228002 === Hits2089320893 Misses 7109 7109 ``` -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/4734?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/4734?src=pr=footer). Last update [de75b7a...b734b29](https://codecov.io/gh/apache/airflow/pull/4734?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 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] codecov-io edited a comment on issue #4703: [AIRFLOW-3885] Improve Travis CI cycle time
codecov-io edited a comment on issue #4703: [AIRFLOW-3885] Improve Travis CI cycle time URL: https://github.com/apache/airflow/pull/4703#issuecomment-463366055 # [Codecov](https://codecov.io/gh/apache/airflow/pull/4703?src=pr=h1) Report > Merging [#4703](https://codecov.io/gh/apache/airflow/pull/4703?src=pr=desc) into [master](https://codecov.io/gh/apache/airflow/commit/de75b7a2bd7f5bef6a1d09942e0b43c17a3fbb95?src=pr=desc) will **decrease** coverage by `0.17%`. > The diff coverage is `n/a`. [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/4703/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/4703?src=pr=tree) ```diff @@Coverage Diff @@ ## master#4703 +/- ## == - Coverage 74.61% 74.43% -0.18% == Files 430 430 Lines 2800227980 -22 == - Hits2089320828 -65 - Misses 7109 7152 +43 ``` | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/4703?src=pr=tree) | Coverage Δ | | |---|---|---| | [airflow/operators/mssql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/4703/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXNzcWxfdG9faGl2ZS5weQ==) | `0% <0%> (-100%)` | :arrow_down: | | [airflow/hooks/mssql\_hook.py](https://codecov.io/gh/apache/airflow/pull/4703/diff?src=pr=tree#diff-YWlyZmxvdy9ob29rcy9tc3NxbF9ob29rLnB5) | `0% <0%> (-58.83%)` | :arrow_down: | | [airflow/contrib/hooks/gcs\_hook.py](https://codecov.io/gh/apache/airflow/pull/4703/diff?src=pr=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL2djc19ob29rLnB5) | `49.13% <0%> (-1.69%)` | :arrow_down: | | [airflow/contrib/hooks/snowflake\_hook.py](https://codecov.io/gh/apache/airflow/pull/4703/diff?src=pr=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL3Nub3dmbGFrZV9ob29rLnB5) | `73.68% <0%> (-1.32%)` | :arrow_down: | | [airflow/models/connection.py](https://codecov.io/gh/apache/airflow/pull/4703/diff?src=pr=tree#diff-YWlyZmxvdy9tb2RlbHMvY29ubmVjdGlvbi5weQ==) | `64.24% <0%> (-1.01%)` | :arrow_down: | | [airflow/utils/dag\_processing.py](https://codecov.io/gh/apache/airflow/pull/4703/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy9kYWdfcHJvY2Vzc2luZy5weQ==) | `59.74% <0%> (-0.15%)` | :arrow_down: | | [airflow/www/app.py](https://codecov.io/gh/apache/airflow/pull/4703/diff?src=pr=tree#diff-YWlyZmxvdy93d3cvYXBwLnB5) | `97.22% <0%> (-0.1%)` | :arrow_down: | | [airflow/hooks/hive\_hooks.py](https://codecov.io/gh/apache/airflow/pull/4703/diff?src=pr=tree#diff-YWlyZmxvdy9ob29rcy9oaXZlX2hvb2tzLnB5) | `75.26% <0%> (-0.07%)` | :arrow_down: | | [airflow/operators/docker\_operator.py](https://codecov.io/gh/apache/airflow/pull/4703/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvZG9ja2VyX29wZXJhdG9yLnB5) | `97.67% <0%> (-0.03%)` | :arrow_down: | | [...low/contrib/operators/datastore\_import\_operator.py](https://codecov.io/gh/apache/airflow/pull/4703/diff?src=pr=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9kYXRhc3RvcmVfaW1wb3J0X29wZXJhdG9yLnB5) | `0% <0%> (ø)` | :arrow_up: | | ... and [8 more](https://codecov.io/gh/apache/airflow/pull/4703/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/4703?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/4703?src=pr=footer). Last update [de75b7a...e9ae7b9](https://codecov.io/gh/apache/airflow/pull/4703?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 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] codecov-io edited a comment on issue #4703: [AIRFLOW-3885] Improve Travis CI cycle time
codecov-io edited a comment on issue #4703: [AIRFLOW-3885] Improve Travis CI cycle time URL: https://github.com/apache/airflow/pull/4703#issuecomment-463366055 # [Codecov](https://codecov.io/gh/apache/airflow/pull/4703?src=pr=h1) Report > Merging [#4703](https://codecov.io/gh/apache/airflow/pull/4703?src=pr=desc) into [master](https://codecov.io/gh/apache/airflow/commit/de75b7a2bd7f5bef6a1d09942e0b43c17a3fbb95?src=pr=desc) will **decrease** coverage by `0.17%`. > The diff coverage is `n/a`. [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/4703/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/4703?src=pr=tree) ```diff @@Coverage Diff @@ ## master#4703 +/- ## == - Coverage 74.61% 74.43% -0.18% == Files 430 430 Lines 2800227980 -22 == - Hits2089320828 -65 - Misses 7109 7152 +43 ``` | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/4703?src=pr=tree) | Coverage Δ | | |---|---|---| | [airflow/operators/mssql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/4703/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXNzcWxfdG9faGl2ZS5weQ==) | `0% <0%> (-100%)` | :arrow_down: | | [airflow/hooks/mssql\_hook.py](https://codecov.io/gh/apache/airflow/pull/4703/diff?src=pr=tree#diff-YWlyZmxvdy9ob29rcy9tc3NxbF9ob29rLnB5) | `0% <0%> (-58.83%)` | :arrow_down: | | [airflow/contrib/hooks/gcs\_hook.py](https://codecov.io/gh/apache/airflow/pull/4703/diff?src=pr=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL2djc19ob29rLnB5) | `49.13% <0%> (-1.69%)` | :arrow_down: | | [airflow/contrib/hooks/snowflake\_hook.py](https://codecov.io/gh/apache/airflow/pull/4703/diff?src=pr=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL3Nub3dmbGFrZV9ob29rLnB5) | `73.68% <0%> (-1.32%)` | :arrow_down: | | [airflow/models/connection.py](https://codecov.io/gh/apache/airflow/pull/4703/diff?src=pr=tree#diff-YWlyZmxvdy9tb2RlbHMvY29ubmVjdGlvbi5weQ==) | `64.24% <0%> (-1.01%)` | :arrow_down: | | [airflow/utils/dag\_processing.py](https://codecov.io/gh/apache/airflow/pull/4703/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy9kYWdfcHJvY2Vzc2luZy5weQ==) | `59.74% <0%> (-0.15%)` | :arrow_down: | | [airflow/www/app.py](https://codecov.io/gh/apache/airflow/pull/4703/diff?src=pr=tree#diff-YWlyZmxvdy93d3cvYXBwLnB5) | `97.22% <0%> (-0.1%)` | :arrow_down: | | [airflow/hooks/hive\_hooks.py](https://codecov.io/gh/apache/airflow/pull/4703/diff?src=pr=tree#diff-YWlyZmxvdy9ob29rcy9oaXZlX2hvb2tzLnB5) | `75.26% <0%> (-0.07%)` | :arrow_down: | | [airflow/operators/docker\_operator.py](https://codecov.io/gh/apache/airflow/pull/4703/diff?src=pr=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvZG9ja2VyX29wZXJhdG9yLnB5) | `97.67% <0%> (-0.03%)` | :arrow_down: | | [...low/contrib/operators/datastore\_import\_operator.py](https://codecov.io/gh/apache/airflow/pull/4703/diff?src=pr=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9kYXRhc3RvcmVfaW1wb3J0X29wZXJhdG9yLnB5) | `0% <0%> (ø)` | :arrow_up: | | ... and [8 more](https://codecov.io/gh/apache/airflow/pull/4703/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/4703?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/4703?src=pr=footer). Last update [de75b7a...e9ae7b9](https://codecov.io/gh/apache/airflow/pull/4703?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 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] mik-laj edited a comment on issue #4740: [AIRFLOW-3925] Don't pull docker-images on pretest
mik-laj edited a comment on issue #4740: [AIRFLOW-3925] Don't pull docker-images on pretest URL: https://github.com/apache/airflow/pull/4740#issuecomment-465436258 I know about it. the purpose of your PR is not to fix the problem presented in this PR, but it does it additionally. Your PR is large and have discussion. I would like this change to come faster, so I made a separate PR. I added you as a co-author of commit. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] mik-laj commented on issue #4740: [AIRFLOW-3925] Don't pull docker-images on pretest
mik-laj commented on issue #4740: [AIRFLOW-3925] Don't pull docker-images on pretest URL: https://github.com/apache/airflow/pull/4740#issuecomment-465436258 I know about it. the purpose of your PR is not to fix the problem presented in this PR, but it does it additionally. I would like this change to come faster, so I made a separate PR. I added you as a co-author of commit. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] jmcarp commented on issue #4685: [AIRFLOW-3862] Check types with mypy.
jmcarp commented on issue #4685: [AIRFLOW-3862] Check types with mypy. URL: https://github.com/apache/airflow/pull/4685#issuecomment-465430554 Conflicts resolved @ashb. This is an automated message from the Apache Git Service. To respond to the message, please log on 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-3924) Email alerts after task instance fails has next try number
[ https://issues.apache.org/jira/browse/AIRFLOW-3924?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16772633#comment-16772633 ] ASF GitHub Bot commented on AIRFLOW-3924: - TheZepto commented on pull request #4741: AIRFLOW-3924 Alert emails have correct try number URL: https://github.com/apache/airflow/pull/4741 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-3924 ### Description - [ ] Here are some details about my PR, including screenshots of any UI changes: Alert emails are being sent after the state of the Task Instance is changed from State.RUNNING. This causes self.try_number to be incremented for the alert email. Subtracting 1 from self.try_number works as long as alert_email() is only called for self.states that are not RUNNING. ### Tests - [x] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason: Does not appear to need testing. ### 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. - When adding new operators/hooks/sensors, the autoclass documentation generation needs to be added. - All the public functions and the classes in the PR contain docstrings that explain what it does ### Code Quality - [x] Passes `flake8` This is an automated message from the Apache Git Service. To respond to the message, please log on 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 > Email alerts after task instance fails has next try number > -- > > Key: AIRFLOW-3924 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3924 > Project: Apache Airflow > Issue Type: Bug >Reporter: Dane Laban >Assignee: Dane Laban >Priority: Minor > > The email alerts that are generated by Airflow when a task instance fails has > the next try number rather than the try that failed. > For example, this is the email from a task failing on the first attempt. > Try 2 out of 2 > Exception: > Shell task failed > Log: Link > Host: x > Log file: x/2019-02-18T10:00:00+00:00.log > Mark success: Link -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] jmcarp commented on issue #4740: [AIRFLOW-3925] Don't pull docker-images on pretest
jmcarp commented on issue #4740: [AIRFLOW-3925] Don't pull docker-images on pretest URL: https://github.com/apache/airflow/pull/4740#issuecomment-465430426 This is already included in #4685. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] zhongjiajie commented on issue #4699: [WIP][AIRFLOW-3881] Correct to_csv row number
zhongjiajie commented on issue #4699: [WIP][AIRFLOW-3881] Correct to_csv row number URL: https://github.com/apache/airflow/pull/4699#issuecomment-465429585 Thinks for your advice. @ashb I have change the unittest test code. @Fokko and rebase on master. I add `unittest.skipIf` in test due to [unittest.TestCase.assertLogs](https://docs.python.org/3.5/library/unittest.html#unittest.TestCase.assertLogs) new in python 3.4, CI test will test in python2. We should discuss if it make sence PTAL, thanks. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] codecov-io edited a comment on issue #4699: [WIP][AIRFLOW-3881] Correct to_csv row number
codecov-io edited a comment on issue #4699: [WIP][AIRFLOW-3881] Correct to_csv row number URL: https://github.com/apache/airflow/pull/4699#issuecomment-463509050 # [Codecov](https://codecov.io/gh/apache/airflow/pull/4699?src=pr=h1) Report > Merging [#4699](https://codecov.io/gh/apache/airflow/pull/4699?src=pr=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6bfa0bab2c68bdc43449b57d0adf9d5fb3c78471?src=pr=desc) will **not change** coverage. > The diff coverage is `100%`. [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/4699/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/4699?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#4699 +/- ## === Coverage 74.61% 74.61% === Files 430 430 Lines 2800228002 === Hits2089320893 Misses 7109 7109 ``` | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/4699?src=pr=tree) | Coverage Δ | | |---|---|---| | [airflow/hooks/hive\_hooks.py](https://codecov.io/gh/apache/airflow/pull/4699/diff?src=pr=tree#diff-YWlyZmxvdy9ob29rcy9oaXZlX2hvb2tzLnB5) | `75.32% <100%> (ø)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/4699?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/4699?src=pr=footer). Last update [6bfa0ba...9c8b391](https://codecov.io/gh/apache/airflow/pull/4699?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 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] TheZepto opened a new pull request #4741: AIRFLOW-3924 Alert emails have correct try number
TheZepto opened a new pull request #4741: AIRFLOW-3924 Alert emails have correct try number URL: https://github.com/apache/airflow/pull/4741 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-3924 ### Description - [ ] Here are some details about my PR, including screenshots of any UI changes: Alert emails are being sent after the state of the Task Instance is changed from State.RUNNING. This causes self.try_number to be incremented for the alert email. Subtracting 1 from self.try_number works as long as alert_email() is only called for self.states that are not RUNNING. ### Tests - [x] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason: Does not appear to need testing. ### 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. - When adding new operators/hooks/sensors, the autoclass documentation generation needs to be added. - All the public functions and the classes in the PR contain docstrings that explain what it does ### Code Quality - [x] Passes `flake8` This is an automated message from the Apache Git Service. To respond to the message, please log on 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] codecov-io edited a comment on issue #4699: [WIP][AIRFLOW-3881] Correct to_csv row number
codecov-io edited a comment on issue #4699: [WIP][AIRFLOW-3881] Correct to_csv row number URL: https://github.com/apache/airflow/pull/4699#issuecomment-463509050 # [Codecov](https://codecov.io/gh/apache/airflow/pull/4699?src=pr=h1) Report > Merging [#4699](https://codecov.io/gh/apache/airflow/pull/4699?src=pr=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6bfa0bab2c68bdc43449b57d0adf9d5fb3c78471?src=pr=desc) will **not change** coverage. > The diff coverage is `100%`. [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/4699/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/4699?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#4699 +/- ## === Coverage 74.61% 74.61% === Files 430 430 Lines 2800228002 === Hits2089320893 Misses 7109 7109 ``` | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/4699?src=pr=tree) | Coverage Δ | | |---|---|---| | [airflow/hooks/hive\_hooks.py](https://codecov.io/gh/apache/airflow/pull/4699/diff?src=pr=tree#diff-YWlyZmxvdy9ob29rcy9oaXZlX2hvb2tzLnB5) | `75.32% <100%> (ø)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/4699?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/4699?src=pr=footer). Last update [6bfa0ba...9c8b391](https://codecov.io/gh/apache/airflow/pull/4699?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 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] [Assigned] (AIRFLOW-3926) Remove flask-admin usage
[ https://issues.apache.org/jira/browse/AIRFLOW-3926?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Tao Feng reassigned AIRFLOW-3926: - Assignee: Andrew Stahlman > Remove flask-admin usage > > > Key: AIRFLOW-3926 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3926 > Project: Apache Airflow > Issue Type: Improvement >Reporter: Tao Feng >Assignee: Andrew Stahlman >Priority: Major > > Given we remove old UI which built up flask-admin, we should remove its usage > as well. > > AirflowViewPlugin - not referenced anywhere in the project: > https://github.com/apache/airflow/blob/master/airflow/__init__.py#L75 > Hive Metastore Browser: > https://github.com/apache/airflow/blame/master/airflow/contrib/plugins/metastore_browser/main.py#L48 > unit test for plugins, uses BaseView and Menu Link: > https://github.com/apache/airflow/blob/master/tests/plugins/test_plugin.py#L101 -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Created] (AIRFLOW-3926) Remove flask-admin usage
Tao Feng created AIRFLOW-3926: - Summary: Remove flask-admin usage Key: AIRFLOW-3926 URL: https://issues.apache.org/jira/browse/AIRFLOW-3926 Project: Apache Airflow Issue Type: Improvement Reporter: Tao Feng Given we remove old UI which built up flask-admin, we should remove its usage as well. AirflowViewPlugin - not referenced anywhere in the project: https://github.com/apache/airflow/blob/master/airflow/__init__.py#L75 Hive Metastore Browser: https://github.com/apache/airflow/blame/master/airflow/contrib/plugins/metastore_browser/main.py#L48 unit test for plugins, uses BaseView and Menu Link: https://github.com/apache/airflow/blob/master/tests/plugins/test_plugin.py#L101 -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (AIRFLOW-3923) Dependency flask-admin==1.5.2 fails safety check
[ https://issues.apache.org/jira/browse/AIRFLOW-3923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16772630#comment-16772630 ] ASF subversion and git services commented on AIRFLOW-3923: -- Commit de75b7a2bd7f5bef6a1d09942e0b43c17a3fbb95 in airflow's branch refs/heads/master from David Smith [ https://gitbox.apache.org/repos/asf?p=airflow.git;h=de75b7a ] [AIRFLOW-3923] Update flask-admin dependency to 1.5.3 to resolve security vulnerabilities from safety (#4739) > Dependency flask-admin==1.5.2 fails safety check > > > Key: AIRFLOW-3923 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3923 > Project: Apache Airflow > Issue Type: Bug >Affects Versions: 1.10.2 >Reporter: David Smith >Assignee: David Smith >Priority: Minor > > Python security vulnerability tool `safety` reports two known security > vulnerabilities with flask-admin==1.5.2 that are resolved in the latest > stable release 1.5.3 > > flask-admin, installed 1.5.2, affected <1.5.3, id 36746 > flask-admin, installed 1.5.2, affected <=1.5.2, id 36437 -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Resolved] (AIRFLOW-3923) Dependency flask-admin==1.5.2 fails safety check
[ https://issues.apache.org/jira/browse/AIRFLOW-3923?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Tao Feng resolved AIRFLOW-3923. --- Resolution: Fixed > Dependency flask-admin==1.5.2 fails safety check > > > Key: AIRFLOW-3923 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3923 > Project: Apache Airflow > Issue Type: Bug >Affects Versions: 1.10.2 >Reporter: David Smith >Assignee: David Smith >Priority: Minor > > Python security vulnerability tool `safety` reports two known security > vulnerabilities with flask-admin==1.5.2 that are resolved in the latest > stable release 1.5.3 > > flask-admin, installed 1.5.2, affected <1.5.3, id 36746 > flask-admin, installed 1.5.2, affected <=1.5.2, id 36437 -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (AIRFLOW-3923) Dependency flask-admin==1.5.2 fails safety check
[ https://issues.apache.org/jira/browse/AIRFLOW-3923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16772629#comment-16772629 ] ASF GitHub Bot commented on AIRFLOW-3923: - feng-tao commented on pull request #4739: [AIRFLOW-3923] Update flask-admin dependency to 1.5.3 URL: https://github.com/apache/airflow/pull/4739 This is an automated message from the Apache Git Service. To respond to the message, please log on 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 > Dependency flask-admin==1.5.2 fails safety check > > > Key: AIRFLOW-3923 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3923 > Project: Apache Airflow > Issue Type: Bug >Affects Versions: 1.10.2 >Reporter: David Smith >Assignee: David Smith >Priority: Minor > > Python security vulnerability tool `safety` reports two known security > vulnerabilities with flask-admin==1.5.2 that are resolved in the latest > stable release 1.5.3 > > flask-admin, installed 1.5.2, affected <1.5.3, id 36746 > flask-admin, installed 1.5.2, affected <=1.5.2, id 36437 -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] feng-tao merged pull request #4739: [AIRFLOW-3923] Update flask-admin dependency to 1.5.3
feng-tao merged pull request #4739: [AIRFLOW-3923] Update flask-admin dependency to 1.5.3 URL: https://github.com/apache/airflow/pull/4739 This is an automated message from the Apache Git Service. To respond to the message, please log on 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] feng-tao commented on issue #4739: [AIRFLOW-3923] Update flask-admin dependency to 1.5.3
feng-tao commented on issue #4739: [AIRFLOW-3923] Update flask-admin dependency to 1.5.3 URL: https://github.com/apache/airflow/pull/4739#issuecomment-465426600 I see. Let's merge your change and then remove the dependency later. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Closed] (AIRFLOW-3903) simplify function get_parser dict comprehension in cli.py
[ https://issues.apache.org/jira/browse/AIRFLOW-3903?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] zhongjiajie closed AIRFLOW-3903. Resolution: Won't Fix Fix Version/s: (was: 1.10.3) 1.10.2 I finally find out the reason, Is not a bug > simplify function get_parser dict comprehension in cli.py > - > > Key: AIRFLOW-3903 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3903 > Project: Apache Airflow > Issue Type: Improvement > Components: cli >Affects Versions: 1.10.2 >Reporter: zhongjiajie >Assignee: zhongjiajie >Priority: Minor > Labels: patch > Fix For: 1.10.2 > > > airflow/cli.py funciton *get_parser* dict comprehension have useless code > {code:java} > and getattr(arg, f){code} > I think, this Jira ticket remove that part -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (AIRFLOW-3903) simplify function get_parser dict comprehension in cli.py
[ https://issues.apache.org/jira/browse/AIRFLOW-3903?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16772624#comment-16772624 ] zhongjiajie commented on AIRFLOW-3903: -- [~kamil.bregula] Done, sorry to close late. > simplify function get_parser dict comprehension in cli.py > - > > Key: AIRFLOW-3903 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3903 > Project: Apache Airflow > Issue Type: Improvement > Components: cli >Affects Versions: 1.10.2 >Reporter: zhongjiajie >Assignee: zhongjiajie >Priority: Minor > Labels: patch > Fix For: 1.10.2 > > > airflow/cli.py funciton *get_parser* dict comprehension have useless code > {code:java} > and getattr(arg, f){code} > I think, this Jira ticket remove that part -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] codecov-io edited a comment on issue #4699: [WIP][AIRFLOW-3881] Correct to_csv row number
codecov-io edited a comment on issue #4699: [WIP][AIRFLOW-3881] Correct to_csv row number URL: https://github.com/apache/airflow/pull/4699#issuecomment-463509050 # [Codecov](https://codecov.io/gh/apache/airflow/pull/4699?src=pr=h1) Report > Merging [#4699](https://codecov.io/gh/apache/airflow/pull/4699?src=pr=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6bfa0bab2c68bdc43449b57d0adf9d5fb3c78471?src=pr=desc) will **not change** coverage. > The diff coverage is `100%`. [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/4699/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/4699?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#4699 +/- ## === Coverage 74.61% 74.61% === Files 430 430 Lines 2800228002 === Hits2089320893 Misses 7109 7109 ``` | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/4699?src=pr=tree) | Coverage Δ | | |---|---|---| | [airflow/hooks/hive\_hooks.py](https://codecov.io/gh/apache/airflow/pull/4699/diff?src=pr=tree#diff-YWlyZmxvdy9ob29rcy9oaXZlX2hvb2tzLnB5) | `75.32% <100%> (ø)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/4699?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/4699?src=pr=footer). Last update [6bfa0ba...53cb74f](https://codecov.io/gh/apache/airflow/pull/4699?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 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] codecov-io commented on issue #4740: [AIRFLOW-3925] Don't pull docker-images on pretest
codecov-io commented on issue #4740: [AIRFLOW-3925] Don't pull docker-images on pretest URL: https://github.com/apache/airflow/pull/4740#issuecomment-465405846 # [Codecov](https://codecov.io/gh/apache/airflow/pull/4740?src=pr=h1) Report > Merging [#4740](https://codecov.io/gh/apache/airflow/pull/4740?src=pr=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6bfa0bab2c68bdc43449b57d0adf9d5fb3c78471?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/4740/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/4740?src=pr=tree) ```diff @@Coverage Diff@@ ## master #4740 +/- ## = - Coverage 74.61% 74.6% -0.01% = Files 430 430 Lines 28002 28002 = - Hits20893 20892 -1 - Misses 71097110 +1 ``` | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/4740?src=pr=tree) | Coverage Δ | | |---|---|---| | [airflow/models/\_\_init\_\_.py](https://codecov.io/gh/apache/airflow/pull/4740/diff?src=pr=tree#diff-YWlyZmxvdy9tb2RlbHMvX19pbml0X18ucHk=) | `92.59% <0%> (-0.06%)` | :arrow_down: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/4740?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/4740?src=pr=footer). Last update [6bfa0ba...16ef81b](https://codecov.io/gh/apache/airflow/pull/4740?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 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] codecov-io edited a comment on issue #4740: [AIRFLOW-3925] Don't pull docker-images on pretest
codecov-io edited a comment on issue #4740: [AIRFLOW-3925] Don't pull docker-images on pretest URL: https://github.com/apache/airflow/pull/4740#issuecomment-465405846 # [Codecov](https://codecov.io/gh/apache/airflow/pull/4740?src=pr=h1) Report > Merging [#4740](https://codecov.io/gh/apache/airflow/pull/4740?src=pr=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6bfa0bab2c68bdc43449b57d0adf9d5fb3c78471?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/4740/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/4740?src=pr=tree) ```diff @@Coverage Diff@@ ## master #4740 +/- ## = - Coverage 74.61% 74.6% -0.01% = Files 430 430 Lines 28002 28002 = - Hits20893 20892 -1 - Misses 71097110 +1 ``` | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/4740?src=pr=tree) | Coverage Δ | | |---|---|---| | [airflow/models/\_\_init\_\_.py](https://codecov.io/gh/apache/airflow/pull/4740/diff?src=pr=tree#diff-YWlyZmxvdy9tb2RlbHMvX19pbml0X18ucHk=) | `92.59% <0%> (-0.06%)` | :arrow_down: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/4740?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/4740?src=pr=footer). Last update [6bfa0ba...16ef81b](https://codecov.io/gh/apache/airflow/pull/4740?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 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] codecov-io commented on issue #4740: [AIRFLOW-3925] Don't pull docker-images on pretest
codecov-io commented on issue #4740: [AIRFLOW-3925] Don't pull docker-images on pretest URL: https://github.com/apache/airflow/pull/4740#issuecomment-465405844 # [Codecov](https://codecov.io/gh/apache/airflow/pull/4740?src=pr=h1) Report > Merging [#4740](https://codecov.io/gh/apache/airflow/pull/4740?src=pr=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6bfa0bab2c68bdc43449b57d0adf9d5fb3c78471?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/4740/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/4740?src=pr=tree) ```diff @@Coverage Diff@@ ## master #4740 +/- ## = - Coverage 74.61% 74.6% -0.01% = Files 430 430 Lines 28002 28002 = - Hits20893 20892 -1 - Misses 71097110 +1 ``` | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/4740?src=pr=tree) | Coverage Δ | | |---|---|---| | [airflow/models/\_\_init\_\_.py](https://codecov.io/gh/apache/airflow/pull/4740/diff?src=pr=tree#diff-YWlyZmxvdy9tb2RlbHMvX19pbml0X18ucHk=) | `92.59% <0%> (-0.06%)` | :arrow_down: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/4740?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/4740?src=pr=footer). Last update [6bfa0ba...16ef81b](https://codecov.io/gh/apache/airflow/pull/4740?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 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] suburbanmtman edited a comment on issue #4739: [AIRFLOW-3923] Update flask-admin dependency to 1.5.3
suburbanmtman edited a comment on issue #4739: [AIRFLOW-3923] Update flask-admin dependency to 1.5.3 URL: https://github.com/apache/airflow/pull/4739#issuecomment-465401112 flask-admin is still referenced in a few areas but I'm not sure if those components are still necessary given the UI rewrite: AirflowViewPlugin - not referenced anywhere in project: https://github.com/apache/airflow/blob/master/airflow/__init__.py#L75 Hive Metastore Browser: https://github.com/apache/airflow/blame/master/airflow/contrib/plugins/metastore_browser/main.py#L48 unit test for plugins, uses BaseView and Menu Link: https://github.com/apache/airflow/blob/master/tests/plugins/test_plugin.py#L101 This is an automated message from the Apache Git Service. To respond to the message, please log on 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] suburbanmtman commented on issue #4739: [AIRFLOW-3923] Update flask-admin dependency to 1.5.3
suburbanmtman commented on issue #4739: [AIRFLOW-3923] Update flask-admin dependency to 1.5.3 URL: https://github.com/apache/airflow/pull/4739#issuecomment-465401112 flask-admin is still referenced in a few areas but I'm not sure if those components are still necessary given the UI rewrite. : AirflowViewPlugin - not referenced anywhere in project: https://github.com/apache/airflow/blob/master/airflow/__init__.py#L75 Hive Metastore Browser: https://github.com/apache/airflow/blame/master/airflow/contrib/plugins/metastore_browser/main.py#L48 unit test for plugins, uses BaseView and Menu Link: https://github.com/apache/airflow/blob/master/tests/plugins/test_plugin.py#L101 This is an automated message from the Apache Git Service. To respond to the message, please log on 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] [Assigned] (AIRFLOW-3924) Email alerts after task instance fails has next try number
[ https://issues.apache.org/jira/browse/AIRFLOW-3924?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Dane Laban reassigned AIRFLOW-3924: --- Assignee: Dane Laban > Email alerts after task instance fails has next try number > -- > > Key: AIRFLOW-3924 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3924 > Project: Apache Airflow > Issue Type: Bug >Reporter: Dane Laban >Assignee: Dane Laban >Priority: Minor > > The email alerts that are generated by Airflow when a task instance fails has > the next try number rather than the try that failed. > For example, this is the email from a task failing on the first attempt. > Try 2 out of 2 > Exception: > Shell task failed > Log: Link > Host: x > Log file: x/2019-02-18T10:00:00+00:00.log > Mark success: Link -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Created] (AIRFLOW-3925) Don't pull docker-images on pretest
Kamil Bregula created AIRFLOW-3925: -- Summary: Don't pull docker-images on pretest Key: AIRFLOW-3925 URL: https://issues.apache.org/jira/browse/AIRFLOW-3925 Project: Apache Airflow Issue Type: Improvement Reporter: Kamil Bregula -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (AIRFLOW-3925) Don't pull docker-images on pretest
[ https://issues.apache.org/jira/browse/AIRFLOW-3925?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16772558#comment-16772558 ] ASF GitHub Bot commented on AIRFLOW-3925: - mik-laj commented on pull request #4740: [AIRFLOW-3925] Don't pull docker-images on pretest URL: https://github.com/apache/airflow/pull/4740 Make sure you have checked _all_ steps below. ### Jira - https://issues.apache.org/jira/browse/AIRFLOW-3925 ### Description We do not need a docker in pretest, so we should not pull images. The problem has previously been discussed: https://github.com/apache/airflow/pull/4685#issuecomment-464285412 ### Tests No applicable ### 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 No applicable ### Code Quality No applicable This is an automated message from the Apache Git Service. To respond to the message, please log on 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 > Don't pull docker-images on pretest > --- > > Key: AIRFLOW-3925 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3925 > Project: Apache Airflow > Issue Type: Improvement >Reporter: Kamil Bregula >Priority: Minor > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] mik-laj opened a new pull request #4740: [AIRFLOW-3925] Don't pull docker-images on pretest
mik-laj opened a new pull request #4740: [AIRFLOW-3925] Don't pull docker-images on pretest URL: https://github.com/apache/airflow/pull/4740 Make sure you have checked _all_ steps below. ### Jira - https://issues.apache.org/jira/browse/AIRFLOW-3925 ### Description We do not need a docker in pretest, so we should not pull images. The problem has previously been discussed: https://github.com/apache/airflow/pull/4685#issuecomment-464285412 ### Tests No applicable ### 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 No applicable ### Code Quality No applicable This is an automated message from the Apache Git Service. To respond to the message, please log on 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-3924) Email alerts after task instance fails has next try number
Dane Laban created AIRFLOW-3924: --- Summary: Email alerts after task instance fails has next try number Key: AIRFLOW-3924 URL: https://issues.apache.org/jira/browse/AIRFLOW-3924 Project: Apache Airflow Issue Type: Bug Reporter: Dane Laban The email alerts that are generated by Airflow when a task instance fails has the next try number rather than the try that failed. For example, this is the email from a task failing on the first attempt. Try 2 out of 2 Exception: Shell task failed Log: Link Host: x Log file: x/2019-02-18T10:00:00+00:00.log Mark success: Link -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] feng-tao edited a comment on issue #4739: [AIRFLOW-3923] Update flask-admin dependency to 1.5.3
feng-tao edited a comment on issue #4739: [AIRFLOW-3923] Update flask-admin dependency to 1.5.3 URL: https://github.com/apache/airflow/pull/4739#issuecomment-465377817 Actually given we remove the old UI which should be the sole place that used flask-admin, do we still need this dependency? This is an automated message from the Apache Git Service. To respond to the message, please log on 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] [Comment Edited] (AIRFLOW-3920) Moving everything out of contrib
[ https://issues.apache.org/jira/browse/AIRFLOW-3920?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16772502#comment-16772502 ] Kamil Bregula edited comment on AIRFLOW-3920 at 2/20/19 1:23 AM: - I think that this is one of the elements that is worth analyzing when imports paths will be changed. I will try to prepare a solution proposal that will allow to introduce my changes in the Airflow 2 version. If this solution is accepted, it will not cause problems for the above-mentioned AIP. I assume that the above-mentioned AIP will be later adopted. My solution will change very much, but I think it is necessary. was (Author: kamil.bregula): I think that this is one of the elements that is worth analyzing when imports paths will be changed. I will try to prepare a solution proposal that will allow to introduce my changes in the Airflow 2 version. If this solution is accepted, it will not cause problems for the above-mentioned AIP. I assume that the above-mentioned AIP will be later adopted > Moving everything out of contrib > > > Key: AIRFLOW-3920 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3920 > Project: Apache Airflow > Issue Type: Improvement >Reporter: Felix Uellendall >Assignee: Kamil Bregula >Priority: Major > > At the moment airflow has a contrib folder that contains a lot of modules > that were contributed by the community. > As this repo grows I think we shouldn't differ committers/maintainers work > from contributors work anymore. We as a community should strive for a unified > code base where everyone has to follow the same rules. > *To drive this change..* > I think we should create multiple PRs to completely move all modules out of > contrib. One PR would probably take too long and be uncomfortable to review. > I suggest that we create a SubTask/PR per subfolder of contrib (or maybe even > smaller ones). -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (AIRFLOW-3920) Moving everything out of contrib
[ https://issues.apache.org/jira/browse/AIRFLOW-3920?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16772502#comment-16772502 ] Kamil Bregula commented on AIRFLOW-3920: I think that this is one of the elements that is worth analyzing when imports paths will be changed. I will try to prepare a solution proposal that will allow to introduce my changes in the Airflow 2 version. If this solution is accepted, it will not cause problems for the above-mentioned AIP. I assume that the above-mentioned AIP will be later adopted > Moving everything out of contrib > > > Key: AIRFLOW-3920 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3920 > Project: Apache Airflow > Issue Type: Improvement >Reporter: Felix Uellendall >Assignee: Kamil Bregula >Priority: Major > > At the moment airflow has a contrib folder that contains a lot of modules > that were contributed by the community. > As this repo grows I think we shouldn't differ committers/maintainers work > from contributors work anymore. We as a community should strive for a unified > code base where everyone has to follow the same rules. > *To drive this change..* > I think we should create multiple PRs to completely move all modules out of > contrib. One PR would probably take too long and be uncomfortable to review. > I suggest that we create a SubTask/PR per subfolder of contrib (or maybe even > smaller ones). -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (AIRFLOW-3920) Moving everything out of contrib
[ https://issues.apache.org/jira/browse/AIRFLOW-3920?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16772498#comment-16772498 ] Tao Feng commented on AIRFLOW-3920: --- How does this ticket work with [https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=100827303] ? Are we going to drop that AIP? > Moving everything out of contrib > > > Key: AIRFLOW-3920 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3920 > Project: Apache Airflow > Issue Type: Improvement >Reporter: Felix Uellendall >Assignee: Kamil Bregula >Priority: Major > > At the moment airflow has a contrib folder that contains a lot of modules > that were contributed by the community. > As this repo grows I think we shouldn't differ committers/maintainers work > from contributors work anymore. We as a community should strive for a unified > code base where everyone has to follow the same rules. > *To drive this change..* > I think we should create multiple PRs to completely move all modules out of > contrib. One PR would probably take too long and be uncomfortable to review. > I suggest that we create a SubTask/PR per subfolder of contrib (or maybe even > smaller ones). -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] feng-tao commented on issue #4739: [AIRFLOW-3923] Update flask-admin dependency to 1.5.3
feng-tao commented on issue #4739: [AIRFLOW-3923] Update flask-admin dependency to 1.5.3 URL: https://github.com/apache/airflow/pull/4739#issuecomment-465377817 Actually given we remove old UI which is based on flask-admin, do we still need this dependency? This is an automated message from the Apache Git Service. To respond to the message, please log on 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] cixuuz edited a comment on issue #4733: [AIRFLOW-3481] Add SFTP in connection type
cixuuz edited a comment on issue #4733: [AIRFLOW-3481] Add SFTP in connection type URL: https://github.com/apache/airflow/pull/4733#issuecomment-465371583 @OmerJog Thank you for bring up these questions. I think you asked a very good question, why we have a field `conn type` which didn't link to anything. Maybe we can ask who designed this. IMO, it could be useful: 1. from user experience perspective, sometimes you cannot tell which is which by reading connection settings. For instance, any hook based on TCP looks similar e.g. HTTP/FTP/SFTP/SSH... Airflow sometimes are tools not only for developers, but also for analysts or data scientist. Those people may not be familiar with those protocols. By having a conn type, they can quickly recognize them. 2. from develop perspective, there is a factory method ```get_conn``` in ```models/connection.py```. The purpose of that one looks like providing a DB conn. In my use case, I made a download/upload interface for any file storage or transfer protocols such as FTP/SFTP/S3/Azure etc. A factory method is needed to get the correct hook based on identifiers. If we have more specific conn_type, it would be great for simplifying my code. Further, I think any existing hook should have its corresponding conn type. Or the conn type could be registered to existing hook. This may related to the topic that AIP-8 split Hooks/Operators to separate repos. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] cixuuz commented on issue #4733: [AIRFLOW-3481] Add SFTP in connection type
cixuuz commented on issue #4733: [AIRFLOW-3481] Add SFTP in connection type URL: https://github.com/apache/airflow/pull/4733#issuecomment-465371583 I think you asked a very good question, why we have a field `conn type` which didn't link to anything. Maybe we can ask who designed this. IMO, it could be useful: 1. from user experience perspective, sometimes you cannot tell which is which by reading connection settings. For instance, any hook based on TCP looks similar e.g. HTTP/FTP/SFTP/SSH... Airflow sometimes are tools not only for developers, but also for analysts or data scientist. Those people may not be familiar with those protocols. By having a conn type, they can quickly recognize them. 2. from develop perspective, there is a factory method ```get_conn``` in ```models/connection.py```. The purpose of that one looks like providing a DB conn. In my use case, I made a download/upload interface for any file storage or transfer protocols such as FTP/SFTP/S3/Azure etc. A factory method is needed to get the correct hook based on identifiers. If we have more specific conn_type, it would be great for simplifying my code. Further, I think any existing hook should have its corresponding conn type. Or the conn type could be registered to existing hook. This may related to the topic that AIP-8 split Hooks/Operators to separate repos. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] codecov-io commented on issue #4739: [AIRFLOW-3923] Update flask-admin dependency to 1.5.3
codecov-io commented on issue #4739: [AIRFLOW-3923] Update flask-admin dependency to 1.5.3 URL: https://github.com/apache/airflow/pull/4739#issuecomment-465371097 # [Codecov](https://codecov.io/gh/apache/airflow/pull/4739?src=pr=h1) Report > Merging [#4739](https://codecov.io/gh/apache/airflow/pull/4739?src=pr=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6bfa0bab2c68bdc43449b57d0adf9d5fb3c78471?src=pr=desc) will **not change** coverage. > The diff coverage is `n/a`. [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/4739/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/4739?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#4739 +/- ## === Coverage 74.61% 74.61% === Files 430 430 Lines 2800228002 === Hits2089320893 Misses 7109 7109 ``` -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/4739?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/4739?src=pr=footer). Last update [6bfa0ba...86c0d7e](https://codecov.io/gh/apache/airflow/pull/4739?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 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] ffinfo commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database
ffinfo commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#discussion_r258290411 ## File path: airflow/www/views.py ## @@ -49,12 +48,13 @@ from wtforms import SelectField, validators import airflow +from airflow import configuration Review comment: fixed This is an automated message from the Apache Git Service. To respond to the message, please log on 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] ffinfo commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database
ffinfo commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#discussion_r258289148 ## File path: airflow/models/__init__.py ## @@ -688,6 +632,57 @@ def __init__(self, task, execution_date, state=None): # Not persisted to the database so only valid for the current process self.raw = False +@staticmethod +@provide_session +def clear_task_instances(tis, + activate_dag_runs=True, + dag=None, + session=None + ): +""" +Clears a set of task instances, but makes sure the running ones +get killed. + +:param tis: a list of task instances +:param session: current session Review comment: fixed This is an automated message from the Apache Git Service. To respond to the message, please log on 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] ffinfo commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database
ffinfo commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#discussion_r258288620 ## File path: airflow/jobs.py ## @@ -2383,9 +2371,8 @@ def _collect_errors(self, ti_status, session=None): return err -@provide_session def _execute_for_run_dates(self, run_dates, ti_status, executor, pickle_id, - start_date, session=None): + start_date): Review comment: fixed This is an automated message from the Apache Git Service. To respond to the message, please log on 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] ffinfo commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database
ffinfo commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#discussion_r258288190 ## File path: airflow/jobs.py ## @@ -39,7 +39,7 @@ from sqlalchemy.exc import OperationalError from sqlalchemy.orm.session import make_transient -from airflow import configuration as conf +from airflow import configuration as conf, configuration Review comment: I think some auto imports from IntelliJ, fixed this This is an automated message from the Apache Git Service. To respond to the message, please log on 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] ffinfo commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database
ffinfo commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#discussion_r258287086 ## File path: airflow/jobs.py ## @@ -1869,7 +1869,7 @@ def __init__( start_date=None, end_date=None, mark_success=False, -donot_pickle=False, +donot_pickle=None, Review comment: This is because of testing. There this class is used directly This is an automated message from the Apache Git Service. To respond to the message, please log on 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-3903) simplify function get_parser dict comprehension in cli.py
[ https://issues.apache.org/jira/browse/AIRFLOW-3903?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16772459#comment-16772459 ] Kamil Bregula commented on AIRFLOW-3903: PR closed. Should we close this issues? > simplify function get_parser dict comprehension in cli.py > - > > Key: AIRFLOW-3903 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3903 > Project: Apache Airflow > Issue Type: Improvement > Components: cli >Affects Versions: 1.10.2 >Reporter: zhongjiajie >Assignee: zhongjiajie >Priority: Minor > Labels: patch > Fix For: 1.10.3 > > > airflow/cli.py funciton *get_parser* dict comprehension have useless code > {code:java} > and getattr(arg, f){code} > I think, this Jira ticket remove that part -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] ffinfo commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database
ffinfo commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#discussion_r258285725 ## File path: airflow/migrations/versions/10aab8d908d0_adding_dag_edges.py ## @@ -0,0 +1,81 @@ +# +# 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. +"""Adding dag edges + +Revision ID: 10aab8d908d0 +Revises: c8ffec048a3b +Create Date: 2018-12-28 09:11:56.730564 + +""" +from alembic import op +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +from airflow.models import DagModel, DagRun + +revision = "10aab8d908d0" +down_revision = ('a56c9515abdc', 'dd4ecb8fbee3') +branch_labels = None +depends_on = None + + +def upgrade(): +connection = op.get_bind() +sessionmaker = sa.orm.sessionmaker() +session = sessionmaker(bind=connection) + +op.create_table( +"dag_edge", +sa.Column("dag_id", sa.String(length=250), nullable=False), +sa.Column("graph_id", sa.Integer, nullable=False), +sa.Column("from_task", sa.String(length=250), nullable=False), +sa.Column("to_task", sa.String(length=250), nullable=False), +sa.PrimaryKeyConstraint("dag_id", "graph_id", "from_task", "to_task"), +) + +op.create_index('idx_dag_edge', 'dag_edge', +['dag_id', 'graph_id'], +unique=False) Review comment: No, this only an extra index, the table has a primary key of 4 columns, this should be unique This is an automated message from the Apache Git Service. To respond to the message, please log on 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] [Assigned] (AIRFLOW-3908) Cloud Vision Operators
[ https://issues.apache.org/jira/browse/AIRFLOW-3908?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Kamil Bregula reassigned AIRFLOW-3908: -- Assignee: Kamil Bregula > Cloud Vision Operators > -- > > Key: AIRFLOW-3908 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3908 > Project: Apache Airflow > Issue Type: Improvement >Reporter: Kamil Bregula >Assignee: Kamil Bregula >Priority: Major > > Hello > I want to create operators for Cloud Vision: > |*Operator name*|*API URL /description*| > |CloudVisionAnnotateImage, > |[Docs|https://googleapis.github.io/google-cloud-python/latest/vision/gapic/v1/api.html#google.cloud.vision_v1.ImageAnnotatorClient.annotate_image]| > |CloudVisionAddProductToProductSetOperator, > |[Docs|https://googleapis.github.io/google-cloud-python/latest/vision/gapic/v1/api.html#google.cloud.vision_v1.ProductSearchClient.add_product_to_product_set]| > |CloudVisionRemoveProductFromProductSet, > |[Docs|https://googleapis.github.io/google-cloud-python/latest/vision/gapic/v1/api.html#google.cloud.vision_v1.ProductSearchClient.remove_product_from_product_set]| > |CloudVisionReferenceImageCreate|[Docs|https://googleapis.github.io/google-cloud-python/latest/vision/gapic/v1/api.html#google.cloud.vision_v1.ProductSearchClient.create_reference_image]| > Greets -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Comment Edited] (AIRFLOW-3920) Moving everything out of contrib
[ https://issues.apache.org/jira/browse/AIRFLOW-3920?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16772453#comment-16772453 ] Kamil Bregula edited comment on AIRFLOW-3920 at 2/20/19 12:00 AM: -- I am currently working on developing a document that discusses many problems with import paths. I will try to finish it soon and share it publicly. was (Author: kamil.bregula): I am currently working on developing a document that discusses many problems with import paths. > Moving everything out of contrib > > > Key: AIRFLOW-3920 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3920 > Project: Apache Airflow > Issue Type: Improvement >Reporter: Felix Uellendall >Priority: Major > > At the moment airflow has a contrib folder that contains a lot of modules > that were contributed by the community. > As this repo grows I think we shouldn't differ committers/maintainers work > from contributors work anymore. We as a community should strive for a unified > code base where everyone has to follow the same rules. > *To drive this change..* > I think we should create multiple PRs to completely move all modules out of > contrib. One PR would probably take too long and be uncomfortable to review. > I suggest that we create a SubTask/PR per subfolder of contrib (or maybe even > smaller ones). -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] feng-tao commented on issue #4396: [AIRFLOW-3585] - Add edges to database
feng-tao commented on issue #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#issuecomment-465363746 sure @Fokko , as I mentioned before I am not against this pr or the idea behind. I understand every solution comes with trade-off which some we could take, some we don't. As long as we are satisfied with the new solution which addresses the previous concern, I am more than happy to give +1. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] feng-tao edited a comment on issue #4396: [AIRFLOW-3585] - Add edges to database
feng-tao edited a comment on issue #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#issuecomment-465339617 thanks @Fokko for your thorough response. To clarify, I am not against the idea of this pr. I am just against if this is the final version. Hence I suggest the author post the testings here instead of every time we find something new issues. And given it is an AIP, it should require 3 PMC +1 based on https://www.apache.org/foundation/voting.html ``` VOTES ON CODE MODIFICATION For code-modification votes, +1 votes are in favour of the proposal, but -1 votes are vetos and kill the proposal dead until all vetoers withdraw their -1 votes. Unless a vote has been declared as using lazy consensus , three +1 votes are required for a code-modification proposal to pass. Whole numbers are recommended for this type of vote, as the opinion being expressed is Boolean: 'I approve/do not approve of this change.' A code-modification proposal may be stopped dead in its tracks by a -1 vote by a qualified voter. ``` CC @bolkedebruin @jghoman This is an automated message from the Apache Git Service. To respond to the message, please log on 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] [Assigned] (AIRFLOW-3920) Moving everything out of contrib
[ https://issues.apache.org/jira/browse/AIRFLOW-3920?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Kamil Bregula reassigned AIRFLOW-3920: -- Assignee: Kamil Bregula > Moving everything out of contrib > > > Key: AIRFLOW-3920 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3920 > Project: Apache Airflow > Issue Type: Improvement >Reporter: Felix Uellendall >Assignee: Kamil Bregula >Priority: Major > > At the moment airflow has a contrib folder that contains a lot of modules > that were contributed by the community. > As this repo grows I think we shouldn't differ committers/maintainers work > from contributors work anymore. We as a community should strive for a unified > code base where everyone has to follow the same rules. > *To drive this change..* > I think we should create multiple PRs to completely move all modules out of > contrib. One PR would probably take too long and be uncomfortable to review. > I suggest that we create a SubTask/PR per subfolder of contrib (or maybe even > smaller ones). -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (AIRFLOW-3920) Moving everything out of contrib
[ https://issues.apache.org/jira/browse/AIRFLOW-3920?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16772453#comment-16772453 ] Kamil Bregula commented on AIRFLOW-3920: I am currently working on developing a document that discusses many problems with import paths. > Moving everything out of contrib > > > Key: AIRFLOW-3920 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3920 > Project: Apache Airflow > Issue Type: Improvement >Reporter: Felix Uellendall >Priority: Major > > At the moment airflow has a contrib folder that contains a lot of modules > that were contributed by the community. > As this repo grows I think we shouldn't differ committers/maintainers work > from contributors work anymore. We as a community should strive for a unified > code base where everyone has to follow the same rules. > *To drive this change..* > I think we should create multiple PRs to completely move all modules out of > contrib. One PR would probably take too long and be uncomfortable to review. > I suggest that we create a SubTask/PR per subfolder of contrib (or maybe even > smaller ones). -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] suburbanmtman opened a new pull request #4739: [AIRFLOW-3923] Update flask-admin dependency to 1.5.3
suburbanmtman opened a new pull request #4739: [AIRFLOW-3923] Update flask-admin dependency to 1.5.3 URL: https://github.com/apache/airflow/pull/4739 ### Jira - [ X] My PR addresses the following [AIRFLOW-3923](https://issues.apache.org/jira/browse/AIRFLOW/3923) issues and references them in the PR title. ### Description - [X ] Here are some details about my PR, including screenshots of any UI changes: - Resolves security vulnerabilities detected in safety for flask-admin 1.5.2: 36746 36437 Release notes for 1.5.3: - Fixed XSS vulnerability - Support nested categories in the navbar menu - SQLAlchemy -- sort on multiple columns with column_default_sort -- sort on related models in column_sortable_list -- fix: inline model forms can now also be used for models with multiple primary keys -- support for using mapped column_property - Upgrade Leaflet and Leaflet.draw plugins, used for geoalchemy integration - Specify minimum_input_length for ajax widget - Peewee: support composite keys - MongoEngine: when searching/filtering the input is now regarded as case-insensitive by default - FileAdmin -- handle special characters in filename -- fix a bug with listing directories on Windows -- avoid raising an exception when unknown sort parameter is encountered -- WTForms 3 support ### Tests - [ X] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason: Airflow usage of flask-admin does not appear to touch any of the changes introduced by 1.5.3 ### 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. - When adding new operators/hooks/sensors, the autoclass documentation generation needs to be added. - All the public functions and the classes in the PR contain docstrings that explain what it does ### Code Quality - [X] Passes `flake8` This is an automated message from the Apache Git Service. To respond to the message, please log on 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-2289) Add additional quick start to INSTALL
[ https://issues.apache.org/jira/browse/AIRFLOW-2289?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16772446#comment-16772446 ] Kamil Bregula commented on AIRFLOW-2289: Related to: https://issues.apache.org/jira/browse/AIRFLOW-3674 in my opition > Add additional quick start to INSTALL > - > > Key: AIRFLOW-2289 > URL: https://issues.apache.org/jira/browse/AIRFLOW-2289 > Project: Apache Airflow > Issue Type: Improvement >Reporter: Bolke de Bruin >Priority: Blocker > Fix For: 1.10.0 > > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Created] (AIRFLOW-3923) Dependency flask-admin==1.5.2 fails safety check
David Smith created AIRFLOW-3923: Summary: Dependency flask-admin==1.5.2 fails safety check Key: AIRFLOW-3923 URL: https://issues.apache.org/jira/browse/AIRFLOW-3923 Project: Apache Airflow Issue Type: Bug Affects Versions: 1.10.2 Reporter: David Smith Assignee: David Smith Python security vulnerability tool `safety` reports two known security vulnerabilities with flask-admin==1.5.2 that are resolved in the latest stable release 1.5.3 flask-admin, installed 1.5.2, affected <1.5.3, id 36746 flask-admin, installed 1.5.2, affected <=1.5.2, id 36437 -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (AIRFLOW-2224) Add support for CSV file exports in mysql_to_gcs contrib operator
[ https://issues.apache.org/jira/browse/AIRFLOW-2224?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16772442#comment-16772442 ] ASF GitHub Bot commented on AIRFLOW-2224: - ttanay commented on pull request #4738: [AIRFLOW-2224] Add support CSV files in MySqlToGoogleCloudStorageOperator URL: https://github.com/apache/airflow/pull/4738 MySqlToGoogleCloudStorageOperator supported export from MySQL in newline-delimited JSON format only. Added support for export in CSV format with the option of specifying a field delimiter. Thanks to Bernardo Najlis(@bnajlis) for the original PR(#3139). I made some changes to the the original PR. 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-2224 - 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. ### Description - [X] Here are some details about my PR, including screenshots of any UI changes: ### Tests - [X] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason: Added tests: * test_init * test_exec_success_json * test_exec_success_csv * test_exec_success_csv_with_delimiter * test_file_splitting * test_schema_file Replaced the pre-existing test with the one @bnajlis wrote as those tests covered that case too. ### 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. - When adding new operators/hooks/sensors, the autoclass documentation generation needs to be added. - All the public functions and the classes in the PR contain docstrings that explain what it does ### Code Quality - [ ] Passes `flake8` This is an automated message from the Apache Git Service. To respond to the message, please log on 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 > Add support for CSV file exports in mysql_to_gcs contrib operator > - > > Key: AIRFLOW-2224 > URL: https://issues.apache.org/jira/browse/AIRFLOW-2224 > Project: Apache Airflow > Issue Type: Improvement > Components: contrib >Affects Versions: 1.9.0 >Reporter: Bernardo Najlis >Assignee: Tanay Tummalapalli >Priority: Major > > Add support to export files into CSV format for the mysql_to_gcs contrib > operator. > Using unicodecsv, this allows to: > # Include column names in first row > # Handle different CSV file types > # Delimiters > # Quoting > # Char escaping > # Line termination > Set default format to still be JSON (current default) and also keep the > multi-file format option available as JSON. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] Fokko commented on issue #4396: [AIRFLOW-3585] - Add edges to database
Fokko commented on issue #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#issuecomment-465353682 @feng-tao I think you mean AIP instead of AIF. The idea is elaborated here: https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-12+Persist+DAG+into+DB But this is more on a conceptual level. I think it is good to also discuss on a code level like we're doing here on this PR, otherwise, it is always difficult to discuss it on the level of the actual code changes. Throwing -1's at it isn't really constructive. I prefer to discuss solutions since this is an issue that is in Airflow since the beginning, and fixing [this](https://www.youtube.com/watch?v=sNrBruPS3r4) is high on my list, and I guess on more people's list. This is an automated message from the Apache Git Service. To respond to the message, please log on 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-3853) Duplicate Logs appearing in S3
[ https://issues.apache.org/jira/browse/AIRFLOW-3853?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16772426#comment-16772426 ] ASF GitHub Bot commented on AIRFLOW-3853: - samuelwbock commented on pull request #4675: [AIRFLOW-3853] Default to delete local logs after remote upload URL: https://github.com/apache/airflow/pull/4675 Make sure you have checked _all_ steps below. ### Jira - [X] My PR addresses the following [Airflow Jira](https://issues.apache.org/jira/browse/AIRFLOW-3853) issues and references them in the PR title. For example, "\[AIRFLOW-3853\] My Airflow PR" - https://issues.apache.org/jira/browse/AIRFLOW-3853 ### Description - [X] We've recently started to see duplicate logs in S3. After digging into it, we discovered that this was due to our use of the new `reschedule` mode on our sensors. Because the same `try_number` is used when a task reschedules, the local log file frequently contains results from previous attempts. Additionally, because the `s3_task_helper.py` always tries to `append` the local log file to the remove log file, this can result in massive logs (we found one that 400 mb). To fix this, we'd like to remove the local log after a successful upload. Because the file is uploaded to S3, no data will be lost. ### Tests - [X] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason: I've modified the following unit tests to cover the change to `s3_write`: `test_write`, `test_write_existing`, `test_write_raises`. ### 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. - When adding new operators/hooks/sensors, the autoclass documentation generation needs to be added. - All the public functions and the classes in the PR contain docstrings that explain what it does ### Code Quality - [X] Passes `flake8` This is an automated message from the Apache Git Service. To respond to the message, please log on 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 > Duplicate Logs appearing in S3 > -- > > Key: AIRFLOW-3853 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3853 > Project: Apache Airflow > Issue Type: Bug > Components: logging >Affects Versions: 1.10.2 >Reporter: Sam Bock >Assignee: Sam Bock >Priority: Major > > We've recently started to see duplicate logs in S3. After digging into it, we > discovered that this was due to our use of the new `reschedule` mode on our > sensors. Because the same `try_number` is used when a task reschedules, the > local log file frequently contains results from previous attempts. > Additionally, because the `s3_task_helper.py` always tries to `append` the > local log file to the remove log file, this can result in massive logs (we > found one that 400 mb). > To fix this, we'd like to remove the local log after a successful upload. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] samuelwbock closed pull request #4675: [AIRFLOW-3853] Default to delete local logs after remote upload
samuelwbock closed pull request #4675: [AIRFLOW-3853] Default to delete local logs after remote upload URL: https://github.com/apache/airflow/pull/4675 This is an automated message from the Apache Git Service. To respond to the message, please log on 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-3853) Duplicate Logs appearing in S3
[ https://issues.apache.org/jira/browse/AIRFLOW-3853?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16772425#comment-16772425 ] ASF GitHub Bot commented on AIRFLOW-3853: - samuelwbock commented on pull request #4675: [AIRFLOW-3853] Default to delete local logs after remote upload URL: https://github.com/apache/airflow/pull/4675 This is an automated message from the Apache Git Service. To respond to the message, please log on 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 > Duplicate Logs appearing in S3 > -- > > Key: AIRFLOW-3853 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3853 > Project: Apache Airflow > Issue Type: Bug > Components: logging >Affects Versions: 1.10.2 >Reporter: Sam Bock >Assignee: Sam Bock >Priority: Major > > We've recently started to see duplicate logs in S3. After digging into it, we > discovered that this was due to our use of the new `reschedule` mode on our > sensors. Because the same `try_number` is used when a task reschedules, the > local log file frequently contains results from previous attempts. > Additionally, because the `s3_task_helper.py` always tries to `append` the > local log file to the remove log file, this can result in massive logs (we > found one that 400 mb). > To fix this, we'd like to remove the local log after a successful upload. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] samuelwbock opened a new pull request #4675: [AIRFLOW-3853] Default to delete local logs after remote upload
samuelwbock opened a new pull request #4675: [AIRFLOW-3853] Default to delete local logs after remote upload URL: https://github.com/apache/airflow/pull/4675 Make sure you have checked _all_ steps below. ### Jira - [X] My PR addresses the following [Airflow Jira](https://issues.apache.org/jira/browse/AIRFLOW-3853) issues and references them in the PR title. For example, "\[AIRFLOW-3853\] My Airflow PR" - https://issues.apache.org/jira/browse/AIRFLOW-3853 ### Description - [X] We've recently started to see duplicate logs in S3. After digging into it, we discovered that this was due to our use of the new `reschedule` mode on our sensors. Because the same `try_number` is used when a task reschedules, the local log file frequently contains results from previous attempts. Additionally, because the `s3_task_helper.py` always tries to `append` the local log file to the remove log file, this can result in massive logs (we found one that 400 mb). To fix this, we'd like to remove the local log after a successful upload. Because the file is uploaded to S3, no data will be lost. ### Tests - [X] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason: I've modified the following unit tests to cover the change to `s3_write`: `test_write`, `test_write_existing`, `test_write_raises`. ### 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. - When adding new operators/hooks/sensors, the autoclass documentation generation needs to be added. - All the public functions and the classes in the PR contain docstrings that explain what it does ### Code Quality - [X] Passes `flake8` This is an automated message from the Apache Git Service. To respond to the message, please log on 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] feng-tao commented on issue #4396: [AIRFLOW-3585] - Add edges to database
feng-tao commented on issue #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#issuecomment-465339617 thanks @Fokko for your thorough response. To clarify, I am not against the idea of this pr. I am just against if this is the final version. Hence I suggest the author post the testings here instead of every time we find something new issues. And given it is an AIF, it should require 3 PMC +1 based on https://www.apache.org/foundation/voting.html ``` VOTES ON CODE MODIFICATION For code-modification votes, +1 votes are in favour of the proposal, but -1 votes are vetos and kill the proposal dead until all vetoers withdraw their -1 votes. Unless a vote has been declared as using lazy consensus , three +1 votes are required for a code-modification proposal to pass. Whole numbers are recommended for this type of vote, as the opinion being expressed is Boolean: 'I approve/do not approve of this change.' A code-modification proposal may be stopped dead in its tracks by a -1 vote by a qualified voter. ``` CC @bolkedebruin @jghoman This is an automated message from the Apache Git Service. To respond to the message, please log on 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] Fokko commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database
Fokko commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#discussion_r258246166 ## File path: airflow/jobs.py ## @@ -39,7 +39,7 @@ from sqlalchemy.exc import OperationalError from sqlalchemy.orm.session import make_transient -from airflow import configuration as conf +from airflow import configuration as conf, configuration Review comment: Something is off here, we do a duplicate import. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] Fokko commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database
Fokko commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#discussion_r258252532 ## File path: tests/sensors/test_base_sensor.py ## @@ -60,12 +61,12 @@ def setUp(self): 'start_date': DEFAULT_DATE } self.dag = DAG(TEST_DAG_ID, default_args=args) +self.dag.sync_to_db() -session = settings.Session() -session.query(TaskReschedule).delete() -session.query(DagRun).delete() -session.query(TaskInstance).delete() -session.commit() +with create_session() as session: +session.query(TaskReschedule).delete() +session.query(DagRun).delete() +session.query(TaskInstance).delete() Review comment: The `provide_session` uses, `create_session` which will `.commit()` after the yield: https://github.com/apache/airflow/blob/6b38649fa6cdf16055c7f5458050c70f39cac8fd/airflow/utils/db.py#L44 This is an automated message from the Apache Git Service. To respond to the message, please log on 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] Fokko commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database
Fokko commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#discussion_r258250123 ## File path: airflow/migrations/versions/10aab8d908d0_adding_dag_edges.py ## @@ -0,0 +1,81 @@ +# +# 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. +"""Adding dag edges + +Revision ID: 10aab8d908d0 +Revises: c8ffec048a3b +Create Date: 2018-12-28 09:11:56.730564 + +""" +from alembic import op +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +from airflow.models import DagModel, DagRun + +revision = "10aab8d908d0" +down_revision = ('a56c9515abdc', 'dd4ecb8fbee3') +branch_labels = None +depends_on = None + + +def upgrade(): +connection = op.get_bind() +sessionmaker = sa.orm.sessionmaker() +session = sessionmaker(bind=connection) + +op.create_table( +"dag_edge", +sa.Column("dag_id", sa.String(length=250), nullable=False), +sa.Column("graph_id", sa.Integer, nullable=False), +sa.Column("from_task", sa.String(length=250), nullable=False), +sa.Column("to_task", sa.String(length=250), nullable=False), +sa.PrimaryKeyConstraint("dag_id", "graph_id", "from_task", "to_task"), Review comment: Nit, since we're using `graph_id`, I think we should also have `task_from`, and `task_to` This is an automated message from the Apache Git Service. To respond to the message, please log on 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] Fokko commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database
Fokko commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#discussion_r258249321 ## File path: airflow/jobs.py ## @@ -2383,9 +2371,8 @@ def _collect_errors(self, ti_status, session=None): return err -@provide_session def _execute_for_run_dates(self, run_dates, ti_status, executor, pickle_id, - start_date, session=None): + start_date): Review comment: Nit, please collapse this into a single line This is an automated message from the Apache Git Service. To respond to the message, please log on 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] Fokko commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database
Fokko commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#discussion_r256349920 ## File path: airflow/jobs.py ## @@ -1869,7 +1869,7 @@ def __init__( start_date=None, end_date=None, mark_success=False, -donot_pickle=False, +donot_pickle=None, Review comment: This change is a bit weird. The `BackfillJob(` is only instantiated here: https://github.com/apache/airflow/blob/master/airflow/models/__init__.py#L3954 And this one gets it also from the config by default: https://github.com/apache/airflow/blob/master/airflow/models/__init__.py#L3910 So we're duplicating logic below. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] Fokko commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database
Fokko commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#discussion_r258250745 ## File path: airflow/models/__init__.py ## @@ -688,6 +632,57 @@ def __init__(self, task, execution_date, state=None): # Not persisted to the database so only valid for the current process self.raw = False +@staticmethod +@provide_session +def clear_task_instances(tis, + activate_dag_runs=True, + dag=None, + session=None + ): +""" +Clears a set of task instances, but makes sure the running ones +get killed. + +:param tis: a list of task instances +:param session: current session Review comment: The docstring ordering isn't correct, can we move the session to the end? This is an automated message from the Apache Git Service. To respond to the message, please log on 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] Fokko commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database
Fokko commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#discussion_r258249613 ## File path: airflow/migrations/versions/10aab8d908d0_adding_dag_edges.py ## @@ -0,0 +1,63 @@ +# +# 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. +"""Adding dag edges + +Revision ID: 10aab8d908d0 +Revises: c8ffec048a3b +Create Date: 2018-12-28 09:11:56.730564 + +""" +from alembic import op +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision = "10aab8d908d0" +down_revision = ('a56c9515abdc', 'dd4ecb8fbee3') +branch_labels = None +depends_on = None + + +def upgrade(): +op.create_table( +"dag_edge", +sa.Column("dag_id", sa.String(length=250), nullable=False), Review comment: Maybe split the FK conversation in a separate ticket? I'm all in favor of FK's. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] Fokko commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database
Fokko commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#discussion_r258249961 ## File path: airflow/migrations/versions/10aab8d908d0_adding_dag_edges.py ## @@ -0,0 +1,81 @@ +# +# 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. +"""Adding dag edges + +Revision ID: 10aab8d908d0 +Revises: c8ffec048a3b +Create Date: 2018-12-28 09:11:56.730564 + +""" +from alembic import op +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +from airflow.models import DagModel, DagRun + +revision = "10aab8d908d0" +down_revision = ('a56c9515abdc', 'dd4ecb8fbee3') +branch_labels = None +depends_on = None + + +def upgrade(): +connection = op.get_bind() +sessionmaker = sa.orm.sessionmaker() +session = sessionmaker(bind=connection) + +op.create_table( +"dag_edge", +sa.Column("dag_id", sa.String(length=250), nullable=False), +sa.Column("graph_id", sa.Integer, nullable=False), +sa.Column("from_task", sa.String(length=250), nullable=False), +sa.Column("to_task", sa.String(length=250), nullable=False), +sa.PrimaryKeyConstraint("dag_id", "graph_id", "from_task", "to_task"), +) + +op.create_index('idx_dag_edge', 'dag_edge', +['dag_id', 'graph_id'], +unique=False) Review comment: This one should be unique, right? This is an automated message from the Apache Git Service. To respond to the message, please log on 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] Fokko commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database
Fokko commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#discussion_r257472310 ## File path: airflow/jobs.py ## @@ -1869,7 +1869,7 @@ def __init__( start_date=None, end_date=None, mark_success=False, -donot_pickle=False, +donot_pickle=None, Review comment: Maybe consolidate this logic, since we're adding logic here that doesn't really make sense. This method is only invoked from here: https://github.com/apache/airflow/blob/master/airflow/models/__init__.py#L3960 Here the `donot_pickle` is also loaded from the config: https://github.com/apache/airflow/blob/master/airflow/models/__init__.py#L3933 Apart from that, it feels like we should set this on an Airflow level, instead of a job level. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] Fokko commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database
Fokko commented on a change in pull request #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#discussion_r258251386 ## File path: airflow/www/views.py ## @@ -49,12 +48,13 @@ from wtforms import SelectField, validators import airflow +from airflow import configuration Review comment: Duplicate import This is an automated message from the Apache Git Service. To respond to the message, please log on 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-3922) Consolidate hostname-reporting behind customizable interface
[ https://issues.apache.org/jira/browse/AIRFLOW-3922?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16772369#comment-16772369 ] Andrew Stahlman commented on AIRFLOW-3922: -- I'm pleasantly surprised to report that was done a long time ago and I missed it: https://issues.apache.org/jira/browse/AIRFLOW-1852 > Consolidate hostname-reporting behind customizable interface > > > Key: AIRFLOW-3922 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3922 > Project: Apache Airflow > Issue Type: Improvement >Reporter: Andrew Stahlman >Assignee: Andrew Stahlman >Priority: Major > > There are several ways of getting a network address in use throughout the > code base, including: > * > [socket.gethostname()|https://github.com/apache/incubator-airflow/blob/daec2ff7db64e979c2d8f15787c0ccfdb433647e/airflow/utils/cli.py#L105] > * > [socket.getfqdn()|https://github.com/apache/incubator-airflow/blob/daec2ff7db64e979c2d8f15787c0ccfdb433647e/airflow/www/views.py#L461] > * > [socket.gethostbyaddr()|https://github.com/apache/incubator-airflow/blob/daec2ff7db64e979c2d8f15787c0ccfdb433647e/airflow/contrib/auth/backends/kerberos_auth.py#L57] > {{gethostname()}} and {{getfqdn()}} will not necessarily return the same > value. It would be nice to consolidate these usages into a single interface > like {{gethostname}} or, perhaps more generic, {{gethostaddress}}, which > returns a consistent value. > Ideally, the implementation would be swappable, too. My company has a > slightly unusual usecase where the hostname in {{/etc/hostname}} does not > actually resolve via DNS. We have a local patch where we replace usages of > gethostname() and getfqdn() with a call to get the private IP address. If > this proposed interface existed, we could swap in our own implemenation by > updating a key in {{~/airflow.cfg}} to specify a custom hostname provider, > i.e., > > {noformat} > [core] > hostname_provider=our.custom.module.gethostname > {noformat} > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Closed] (AIRFLOW-3922) Consolidate hostname-reporting behind customizable interface
[ https://issues.apache.org/jira/browse/AIRFLOW-3922?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andrew Stahlman closed AIRFLOW-3922. Resolution: Duplicate > Consolidate hostname-reporting behind customizable interface > > > Key: AIRFLOW-3922 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3922 > Project: Apache Airflow > Issue Type: Improvement >Reporter: Andrew Stahlman >Assignee: Andrew Stahlman >Priority: Major > > There are several ways of getting a network address in use throughout the > code base, including: > * > [socket.gethostname()|https://github.com/apache/incubator-airflow/blob/daec2ff7db64e979c2d8f15787c0ccfdb433647e/airflow/utils/cli.py#L105] > * > [socket.getfqdn()|https://github.com/apache/incubator-airflow/blob/daec2ff7db64e979c2d8f15787c0ccfdb433647e/airflow/www/views.py#L461] > * > [socket.gethostbyaddr()|https://github.com/apache/incubator-airflow/blob/daec2ff7db64e979c2d8f15787c0ccfdb433647e/airflow/contrib/auth/backends/kerberos_auth.py#L57] > {{gethostname()}} and {{getfqdn()}} will not necessarily return the same > value. It would be nice to consolidate these usages into a single interface > like {{gethostname}} or, perhaps more generic, {{gethostaddress}}, which > returns a consistent value. > Ideally, the implementation would be swappable, too. My company has a > slightly unusual usecase where the hostname in {{/etc/hostname}} does not > actually resolve via DNS. We have a local patch where we replace usages of > gethostname() and getfqdn() with a call to get the private IP address. If > this proposed interface existed, we could swap in our own implemenation by > updating a key in {{~/airflow.cfg}} to specify a custom hostname provider, > i.e., > > {noformat} > [core] > hostname_provider=our.custom.module.gethostname > {noformat} > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] Fokko commented on a change in pull request #4724: [AIRFLOW-3906] Add npm compile to docker file
Fokko commented on a change in pull request #4724: [AIRFLOW-3906] Add npm compile to docker file URL: https://github.com/apache/airflow/pull/4724#discussion_r258242613 ## File path: Dockerfile ## @@ -16,26 +16,39 @@ FROM python:3.6-slim -COPY . /opt/airflow/ - ARG AIRFLOW_HOME=/usr/local/airflow ARG AIRFLOW_DEPS="all" ARG PYTHON_DEPS="" -ARG buildDeps="freetds-dev libkrb5-dev libsasl2-dev libssl-dev libffi-dev libpq-dev git" -ARG APT_DEPS="$buildDeps libsasl2-dev freetds-bin build-essential default-libmysqlclient-dev apt-utils curl rsync netcat locales" +ARG buildDeps="freetds-dev libkrb5-dev libssl-dev libffi-dev libpq-dev git" +ARG APT_DEPS="libsasl2-dev freetds-bin build-essential default-libmysqlclient-dev apt-utils curl rsync netcat locales" + +ENV PATH="$HOME/.npm-packages/bin:$PATH" + +RUN if [ -n "${APT_DEPS}" ]; then apt install -y $APT_DEPS; fi \ +&& apt autoremove -yqq --purge \ +&& apt clean + +COPY . /opt/airflow/ WORKDIR /opt/airflow RUN set -x \ && apt update \ -&& if [ -n "${APT_DEPS}" ]; then apt install -y $APT_DEPS; fi \ +&& if [ -n "${buildDeps}" ]; then apt install -y $buildDeps; fi \ +&& curl -sL https://deb.nodesource.com/setup_11.x | bash - \ +&& apt install -y nodejs \ Review comment: @verdan is the author of the npm stuff, therefore I've tagged him to review. Maybe he can give some guidance on the versions as well. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] [Assigned] (AIRFLOW-3922) Consolidate hostname-reporting behind customizable interface
[ https://issues.apache.org/jira/browse/AIRFLOW-3922?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Andrew Stahlman reassigned AIRFLOW-3922: Assignee: Andrew Stahlman > Consolidate hostname-reporting behind customizable interface > > > Key: AIRFLOW-3922 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3922 > Project: Apache Airflow > Issue Type: Improvement >Reporter: Andrew Stahlman >Assignee: Andrew Stahlman >Priority: Major > > There are several ways of getting a network address in use throughout the > code base, including: > * > [socket.gethostname()|https://github.com/apache/incubator-airflow/blob/daec2ff7db64e979c2d8f15787c0ccfdb433647e/airflow/utils/cli.py#L105] > * > [socket.getfqdn()|https://github.com/apache/incubator-airflow/blob/daec2ff7db64e979c2d8f15787c0ccfdb433647e/airflow/www/views.py#L461] > * > [socket.gethostbyaddr()|https://github.com/apache/incubator-airflow/blob/daec2ff7db64e979c2d8f15787c0ccfdb433647e/airflow/contrib/auth/backends/kerberos_auth.py#L57] > {{gethostname()}} and {{getfqdn()}} will not necessarily return the same > value. It would be nice to consolidate these usages into a single interface > like {{gethostname}} or, perhaps more generic, {{gethostaddress}}, which > returns a consistent value. > Ideally, the implementation would be swappable, too. My company has a > slightly unusual usecase where the hostname in {{/etc/hostname}} does not > actually resolve via DNS. We have a local patch where we replace usages of > gethostname() and getfqdn() with a call to get the private IP address. If > this proposed interface existed, we could swap in our own implemenation by > updating a key in {{~/airflow.cfg}} to specify a custom hostname provider, > i.e., > > {noformat} > [core] > hostname_provider=our.custom.module.gethostname > {noformat} > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Assigned] (AIRFLOW-3153) send dag last_run to statsd
[ https://issues.apache.org/jira/browse/AIRFLOW-3153?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Tao Feng reassigned AIRFLOW-3153: - Assignee: Andrew Stahlman (was: Tao Feng) > send dag last_run to statsd > --- > > Key: AIRFLOW-3153 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3153 > Project: Apache Airflow > Issue Type: Bug >Reporter: Tao Feng >Assignee: Andrew Stahlman >Priority: Minor > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] feng-tao commented on issue #4712: [AIRFLOW-3892] Create Redis pub sub sensor
feng-tao commented on issue #4712: [AIRFLOW-3892] Create Redis pub sub sensor URL: https://github.com/apache/airflow/pull/4712#issuecomment-465296647 we have a celery queue sensor(https://github.com/apache/airflow/blob/master/airflow/contrib/sensors/celery_queue_sensor.py)which could be used to wait wait for celery queue back by Redis. Could you share more on your sensor use case? This is an automated message from the Apache Git Service. To respond to the message, please log on 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-3922) Consolidate hostname-reporting behind customizable interface
Andrew Stahlman created AIRFLOW-3922: Summary: Consolidate hostname-reporting behind customizable interface Key: AIRFLOW-3922 URL: https://issues.apache.org/jira/browse/AIRFLOW-3922 Project: Apache Airflow Issue Type: Improvement Reporter: Andrew Stahlman There are several ways of getting a network address in use throughout the code base, including: * [socket.gethostname()|https://github.com/apache/incubator-airflow/blob/daec2ff7db64e979c2d8f15787c0ccfdb433647e/airflow/utils/cli.py#L105] * [socket.getfqdn()|https://github.com/apache/incubator-airflow/blob/daec2ff7db64e979c2d8f15787c0ccfdb433647e/airflow/www/views.py#L461] * [socket.gethostbyaddr()|https://github.com/apache/incubator-airflow/blob/daec2ff7db64e979c2d8f15787c0ccfdb433647e/airflow/contrib/auth/backends/kerberos_auth.py#L57] {{gethostname()}} and {{getfqdn()}} will not necessarily return the same value. It would be nice to consolidate these usages into a single interface like {{gethostname}} or, perhaps more generic, {{gethostaddress}}, which returns a consistent value. Ideally, the implementation would be swappable, too. My company has a slightly unusual usecase where the hostname in {{/etc/hostname}} does not actually resolve via DNS. We have a local patch where we replace usages of gethostname() and getfqdn() with a call to get the private IP address. If this proposed interface existed, we could swap in our own implemenation by updating a key in {{~/airflow.cfg}} to specify a custom hostname provider, i.e., {noformat} [core] hostname_provider=our.custom.module.gethostname {noformat} -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] briandconnelly commented on a change in pull request #3115: [AIRFLOW-2193] Add ROperator for using R
briandconnelly commented on a change in pull request #3115: [AIRFLOW-2193] Add ROperator for using R URL: https://github.com/apache/airflow/pull/3115#discussion_r258190670 ## File path: airflow/contrib/operators/r_operator.py ## @@ -0,0 +1,79 @@ +# -*- coding: utf-8 -*- +# +# Licensed 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. + +from builtins import bytes +import os +from tempfile import NamedTemporaryFile + +from airflow.models import BaseOperator +from airflow.utils.decorators import apply_defaults +from airflow.utils.file import TemporaryDirectory + +import rpy2.robjects as robjects +from rpy2.rinterface import RRuntimeError + + +class ROperator(BaseOperator): +""" +Execute an R script or command + +:param r_command: The command or a reference to an R script (must have +'.r' extension) to be executed (templated) +:type r_command: string +:param output_encoding: encoding output from R (default: 'utf-8') +:type output_encoding: string + +""" + +template_fields = ('r_command',) +template_ext = ('.r', '.R') +ui_color = '#C8D5E6' + +@apply_defaults +def __init__( +self, +r_command, +output_encoding='utf-8', +*args, **kwargs): + +super(ROperator, self).__init__(*args, **kwargs) +self.r_command = r_command +self.output_encoding = output_encoding + +def execute(self, context): +""" +Execute the R command or script in a temporary directory +""" + +with TemporaryDirectory(prefix='airflowtmp') as tmp_dir: +with NamedTemporaryFile(dir=tmp_dir, prefix=self.task_id) as f: + +f.write(bytes(self.r_command, 'utf_8')) +f.flush() +fname = f.name +script_location = os.path.abspath(fname) + +self.log.info("Temporary script location: %s", script_location) +self.log.info("Running command(s):\n%s", self.r_command) + +try: +res = robjects.r.source(fname, echo=False) Review comment: Re-introduced the `env` parameter for exporting environment variables in https://github.com/apache/airflow/pull/3115/commits/9f997a20a889fa42a77f0d72184d6dd267d15726 This is an automated message from the Apache Git Service. To respond to the message, please log on 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] feng-tao edited a comment on issue #4396: [AIRFLOW-3585] - Add edges to database
feng-tao edited a comment on issue #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#issuecomment-465266733 @ffinfo , the fact is that @astahlman has found many issues on this branch. And what does it mean by `because the dagrun did already exist` ? Are we still going to have issues if there are previous DAGRuns exist? Do we still need to run `airflow resetdb`? And what does it mean by "testing scheduler is not needed"? Now the scheduler(normal or backfill) needs to get the graph edge and push it to db? How could it be not related if that's the case? And I am not comfortable with the UX here( that new giant button `show graph from file`). This brings the confusion to the user IMO. And to be honest, I think your issue does exist but only happens in a very rare case. And during my 1-year involvement of Airflow with production, I have never experienced with the issue or heard our users complain about the issues in our 500 DAGs production cluster. Hence I am not sure if it justified with such complicated change. I am -1(binding) with this branch / AIF if that's the way it is. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] feng-tao commented on issue #4396: [AIRFLOW-3585] - Add edges to database
feng-tao commented on issue #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#issuecomment-465266733 @ffinfo , the fact is that @astahlman has found many issues on this branch. And what does it mean by `because the dagrun did already exist` ? Are we still going to have issues if there are previous DAGRuns exist? Do we still need to run `airflow resetdb`? And what does it mean by "testing scheduler is not needed"? Now the scheduler(normal or backfill) needs to get the graph edge and push it to db? How could it be not related if that's the case? And I am not comfortable with the UX here( that new giant button `show graph from file`). This brings the confusion to the user IMO. And to be honest, I think your issue does exist but only happens in a very rare case. And during my 1-year involvement of Airflow with production, I have never experienced with the issue or heard our users complain about the issues in our 500 DAGs production cluster. Hence I am not sure if it justified with such complicated change. I am -1(binding) with this branch if that's the way it is. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] ffinfo commented on issue #4396: [AIRFLOW-3585] - Add edges to database
ffinfo commented on issue #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#issuecomment-465255343 @feng-tao This change has nothing to do with the scheduler or running tasks, so testing scheduler, backfill is not required here. Only place where this is used is in create_dagrun but in the scheduling of the tasks this new feature is not used. So testing with multiple executors is not needed here. @astahlman Did you also run the scheduler on this branch? if so what is the content of the table dag_edge? I think this mainly because the dagrun did already exist. Only at a new call `create_dagrun()` will create this edges. Backfill will only call this when the DagRun does not exist yet This is an automated message from the Apache Git Service. To respond to the message, please log on 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] feluelle commented on a change in pull request #4724: [AIRFLOW-3906] Add npm compile to docker file
feluelle commented on a change in pull request #4724: [AIRFLOW-3906] Add npm compile to docker file URL: https://github.com/apache/airflow/pull/4724#discussion_r258171889 ## File path: Dockerfile ## @@ -16,26 +16,39 @@ FROM python:3.6-slim -COPY . /opt/airflow/ - ARG AIRFLOW_HOME=/usr/local/airflow ARG AIRFLOW_DEPS="all" ARG PYTHON_DEPS="" -ARG buildDeps="freetds-dev libkrb5-dev libsasl2-dev libssl-dev libffi-dev libpq-dev git" -ARG APT_DEPS="$buildDeps libsasl2-dev freetds-bin build-essential default-libmysqlclient-dev apt-utils curl rsync netcat locales" +ARG buildDeps="freetds-dev libkrb5-dev libssl-dev libffi-dev libpq-dev git" +ARG APT_DEPS="libsasl2-dev freetds-bin build-essential default-libmysqlclient-dev apt-utils curl rsync netcat locales" + +ENV PATH="$HOME/.npm-packages/bin:$PATH" + +RUN if [ -n "${APT_DEPS}" ]; then apt install -y $APT_DEPS; fi \ +&& apt autoremove -yqq --purge \ +&& apt clean + +COPY . /opt/airflow/ WORKDIR /opt/airflow RUN set -x \ && apt update \ -&& if [ -n "${APT_DEPS}" ]; then apt install -y $APT_DEPS; fi \ +&& if [ -n "${buildDeps}" ]; then apt install -y $buildDeps; fi \ +&& curl -sL https://deb.nodesource.com/setup_11.x | bash - \ +&& apt install -y nodejs \ Review comment: By the way the lifetime of v11 is really short [node's release schedule](https://github.com/nodejs/Release#release-schedule). Don't know if we should consider this. Due to this I actually would pin it to the LTS (10) release instead. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] feluelle commented on a change in pull request #4724: [AIRFLOW-3906] Add npm compile to docker file
feluelle commented on a change in pull request #4724: [AIRFLOW-3906] Add npm compile to docker file URL: https://github.com/apache/airflow/pull/4724#discussion_r258164079 ## File path: Dockerfile ## @@ -16,26 +16,39 @@ FROM python:3.6-slim -COPY . /opt/airflow/ - ARG AIRFLOW_HOME=/usr/local/airflow ARG AIRFLOW_DEPS="all" ARG PYTHON_DEPS="" -ARG buildDeps="freetds-dev libkrb5-dev libsasl2-dev libssl-dev libffi-dev libpq-dev git" -ARG APT_DEPS="$buildDeps libsasl2-dev freetds-bin build-essential default-libmysqlclient-dev apt-utils curl rsync netcat locales" +ARG buildDeps="freetds-dev libkrb5-dev libssl-dev libffi-dev libpq-dev git" +ARG APT_DEPS="libsasl2-dev freetds-bin build-essential default-libmysqlclient-dev apt-utils curl rsync netcat locales" + +ENV PATH="$HOME/.npm-packages/bin:$PATH" + +RUN if [ -n "${APT_DEPS}" ]; then apt install -y $APT_DEPS; fi \ +&& apt autoremove -yqq --purge \ +&& apt clean + +COPY . /opt/airflow/ WORKDIR /opt/airflow RUN set -x \ && apt update \ -&& if [ -n "${APT_DEPS}" ]; then apt install -y $APT_DEPS; fi \ +&& if [ -n "${buildDeps}" ]; then apt install -y $buildDeps; fi \ +&& curl -sL https://deb.nodesource.com/setup_11.x | bash - \ +&& apt install -y nodejs \ Review comment: I mean it is kinda pinned (to 11.x). I am not sure if we should pin this to a specific version. This is an automated message from the Apache Git Service. To respond to the message, please log on 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-3885) Improve Travis buildtime
[ https://issues.apache.org/jira/browse/AIRFLOW-3885?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16772186#comment-16772186 ] ASF subversion and git services commented on AIRFLOW-3885: -- Commit 6bfa0bab2c68bdc43449b57d0adf9d5fb3c78471 in airflow's branch refs/heads/master from Andrew Stahlman [ https://gitbox.apache.org/repos/asf?p=airflow.git;h=6bfa0ba ] [AIRFLOW-3885] Fix race condition in scheduler test (#4737) We're hitting this race condition frequently now that we don't sleep() during unit tests. We don't actually need to assert that the task is currently running - it's fine if it has already run successfully. > Improve Travis buildtime > > > Key: AIRFLOW-3885 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3885 > Project: Apache Airflow > Issue Type: Bug > Components: travis >Affects Versions: 1.10.2 >Reporter: Drew Sonne >Assignee: Drew Sonne >Priority: Major > Fix For: 1.10.3 > > > * Remove the "install" action on the "pre-test" stage to avoid performing > lengthy Docker pulls to perform pre-checks > * Set nosetests to return on any test failures > ** Given the lengthy runtime of the airflow CI test suites, if any tests > fail, we should fail immediately and return the failed test locally. Users > can run the tests locally to get full lists of failed tests. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (AIRFLOW-3885) Improve Travis buildtime
[ https://issues.apache.org/jira/browse/AIRFLOW-3885?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16772183#comment-16772183 ] ASF GitHub Bot commented on AIRFLOW-3885: - feng-tao commented on pull request #4737: [AIRFLOW-3885] Fix race condition in scheduler test URL: https://github.com/apache/airflow/pull/4737 This is an automated message from the Apache Git Service. To respond to the message, please log on 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 > Improve Travis buildtime > > > Key: AIRFLOW-3885 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3885 > Project: Apache Airflow > Issue Type: Bug > Components: travis >Affects Versions: 1.10.2 >Reporter: Drew Sonne >Assignee: Drew Sonne >Priority: Major > Fix For: 1.10.3 > > > * Remove the "install" action on the "pre-test" stage to avoid performing > lengthy Docker pulls to perform pre-checks > * Set nosetests to return on any test failures > ** Given the lengthy runtime of the airflow CI test suites, if any tests > fail, we should fail immediately and return the failed test locally. Users > can run the tests locally to get full lists of failed tests. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] feng-tao merged pull request #4737: [AIRFLOW-3885] Fix race condition in scheduler test
feng-tao merged pull request #4737: [AIRFLOW-3885] Fix race condition in scheduler test URL: https://github.com/apache/airflow/pull/4737 This is an automated message from the Apache Git Service. To respond to the message, please log on 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] feng-tao commented on issue #4737: [AIRFLOW-3885] Fix race condition in scheduler test
feng-tao commented on issue #4737: [AIRFLOW-3885] Fix race condition in scheduler test URL: https://github.com/apache/airflow/pull/4737#issuecomment-465248258 thanks @astahlman This is an automated message from the Apache Git Service. To respond to the message, please log on 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] feluelle commented on a change in pull request #4724: [AIRFLOW-3906] Add npm compile to docker file
feluelle commented on a change in pull request #4724: [AIRFLOW-3906] Add npm compile to docker file URL: https://github.com/apache/airflow/pull/4724#discussion_r258164079 ## File path: Dockerfile ## @@ -16,26 +16,39 @@ FROM python:3.6-slim -COPY . /opt/airflow/ - ARG AIRFLOW_HOME=/usr/local/airflow ARG AIRFLOW_DEPS="all" ARG PYTHON_DEPS="" -ARG buildDeps="freetds-dev libkrb5-dev libsasl2-dev libssl-dev libffi-dev libpq-dev git" -ARG APT_DEPS="$buildDeps libsasl2-dev freetds-bin build-essential default-libmysqlclient-dev apt-utils curl rsync netcat locales" +ARG buildDeps="freetds-dev libkrb5-dev libssl-dev libffi-dev libpq-dev git" +ARG APT_DEPS="libsasl2-dev freetds-bin build-essential default-libmysqlclient-dev apt-utils curl rsync netcat locales" + +ENV PATH="$HOME/.npm-packages/bin:$PATH" + +RUN if [ -n "${APT_DEPS}" ]; then apt install -y $APT_DEPS; fi \ +&& apt autoremove -yqq --purge \ +&& apt clean + +COPY . /opt/airflow/ WORKDIR /opt/airflow RUN set -x \ && apt update \ -&& if [ -n "${APT_DEPS}" ]; then apt install -y $APT_DEPS; fi \ +&& if [ -n "${buildDeps}" ]; then apt install -y $buildDeps; fi \ +&& curl -sL https://deb.nodesource.com/setup_11.x | bash - \ +&& apt install -y nodejs \ Review comment: I mean it is kinda pinned (to 11.x). I am not sure if we should pin this to a specific version. What about 11.10.x ? This is an automated message from the Apache Git Service. To respond to the message, please log on 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] codecov-io edited a comment on issue #4737: [AIRFLOW-3885] Fix race condition in scheduler test
codecov-io edited a comment on issue #4737: [AIRFLOW-3885] Fix race condition in scheduler test URL: https://github.com/apache/airflow/pull/4737#issuecomment-465246776 # [Codecov](https://codecov.io/gh/apache/airflow/pull/4737?src=pr=h1) Report > Merging [#4737](https://codecov.io/gh/apache/airflow/pull/4737?src=pr=desc) into [master](https://codecov.io/gh/apache/airflow/commit/daec2ff7db64e979c2d8f15787c0ccfdb433647e?src=pr=desc) will **increase** coverage by `0.11%`. > The diff coverage is `n/a`. [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/4737/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/4737?src=pr=tree) ```diff @@Coverage Diff @@ ## master#4737 +/- ## == + Coverage74.5% 74.61% +0.11% == Files 430 430 Lines 2800228002 == + Hits2086220893 +31 + Misses 7140 7109 -31 ``` | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/4737?src=pr=tree) | Coverage Δ | | |---|---|---| | [airflow/jobs.py](https://codecov.io/gh/apache/airflow/pull/4737/diff?src=pr=tree#diff-YWlyZmxvdy9qb2JzLnB5) | `76.46% <0%> (+0.71%)` | :arrow_up: | | [airflow/utils/dag\_processing.py](https://codecov.io/gh/apache/airflow/pull/4737/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy9kYWdfcHJvY2Vzc2luZy5weQ==) | `59.89% <0%> (+1.06%)` | :arrow_up: | | [airflow/executors/\_\_init\_\_.py](https://codecov.io/gh/apache/airflow/pull/4737/diff?src=pr=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvX19pbml0X18ucHk=) | `63.46% <0%> (+3.84%)` | :arrow_up: | | [airflow/utils/sqlalchemy.py](https://codecov.io/gh/apache/airflow/pull/4737/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy9zcWxhbGNoZW15LnB5) | `81.81% <0%> (+4.54%)` | :arrow_up: | | [airflow/executors/sequential\_executor.py](https://codecov.io/gh/apache/airflow/pull/4737/diff?src=pr=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvc2VxdWVudGlhbF9leGVjdXRvci5weQ==) | `100% <0%> (+50%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/4737?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/4737?src=pr=footer). Last update [daec2ff...4f152cd](https://codecov.io/gh/apache/airflow/pull/4737?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 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] codecov-io commented on issue #4737: [AIRFLOW-3885] Fix race condition in scheduler test
codecov-io commented on issue #4737: [AIRFLOW-3885] Fix race condition in scheduler test URL: https://github.com/apache/airflow/pull/4737#issuecomment-465246776 # [Codecov](https://codecov.io/gh/apache/airflow/pull/4737?src=pr=h1) Report > Merging [#4737](https://codecov.io/gh/apache/airflow/pull/4737?src=pr=desc) into [master](https://codecov.io/gh/apache/airflow/commit/daec2ff7db64e979c2d8f15787c0ccfdb433647e?src=pr=desc) will **increase** coverage by `0.11%`. > The diff coverage is `n/a`. [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/4737/graphs/tree.svg?width=650=WdLKlKHOAU=150=pr)](https://codecov.io/gh/apache/airflow/pull/4737?src=pr=tree) ```diff @@Coverage Diff @@ ## master#4737 +/- ## == + Coverage74.5% 74.61% +0.11% == Files 430 430 Lines 2800228002 == + Hits2086220893 +31 + Misses 7140 7109 -31 ``` | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/4737?src=pr=tree) | Coverage Δ | | |---|---|---| | [airflow/jobs.py](https://codecov.io/gh/apache/airflow/pull/4737/diff?src=pr=tree#diff-YWlyZmxvdy9qb2JzLnB5) | `76.46% <0%> (+0.71%)` | :arrow_up: | | [airflow/utils/dag\_processing.py](https://codecov.io/gh/apache/airflow/pull/4737/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy9kYWdfcHJvY2Vzc2luZy5weQ==) | `59.89% <0%> (+1.06%)` | :arrow_up: | | [airflow/executors/\_\_init\_\_.py](https://codecov.io/gh/apache/airflow/pull/4737/diff?src=pr=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvX19pbml0X18ucHk=) | `63.46% <0%> (+3.84%)` | :arrow_up: | | [airflow/utils/sqlalchemy.py](https://codecov.io/gh/apache/airflow/pull/4737/diff?src=pr=tree#diff-YWlyZmxvdy91dGlscy9zcWxhbGNoZW15LnB5) | `81.81% <0%> (+4.54%)` | :arrow_up: | | [airflow/executors/sequential\_executor.py](https://codecov.io/gh/apache/airflow/pull/4737/diff?src=pr=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvc2VxdWVudGlhbF9leGVjdXRvci5weQ==) | `100% <0%> (+50%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/4737?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/4737?src=pr=footer). Last update [daec2ff...4f152cd](https://codecov.io/gh/apache/airflow/pull/4737?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 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] feluelle commented on a change in pull request #4724: [AIRFLOW-3906] Add npm compile to docker file
feluelle commented on a change in pull request #4724: [AIRFLOW-3906] Add npm compile to docker file URL: https://github.com/apache/airflow/pull/4724#discussion_r258164207 ## File path: Dockerfile ## @@ -16,26 +16,39 @@ FROM python:3.6-slim -COPY . /opt/airflow/ - ARG AIRFLOW_HOME=/usr/local/airflow ARG AIRFLOW_DEPS="all" ARG PYTHON_DEPS="" -ARG buildDeps="freetds-dev libkrb5-dev libsasl2-dev libssl-dev libffi-dev libpq-dev git" -ARG APT_DEPS="$buildDeps libsasl2-dev freetds-bin build-essential default-libmysqlclient-dev apt-utils curl rsync netcat locales" +ARG buildDeps="freetds-dev libkrb5-dev libssl-dev libffi-dev libpq-dev git" +ARG APT_DEPS="libsasl2-dev freetds-bin build-essential default-libmysqlclient-dev apt-utils curl rsync netcat locales" + +ENV PATH="$HOME/.npm-packages/bin:$PATH" + +RUN if [ -n "${APT_DEPS}" ]; then apt install -y $APT_DEPS; fi \ +&& apt autoremove -yqq --purge \ +&& apt clean + +COPY . /opt/airflow/ WORKDIR /opt/airflow RUN set -x \ && apt update \ -&& if [ -n "${APT_DEPS}" ]; then apt install -y $APT_DEPS; fi \ +&& if [ -n "${buildDeps}" ]; then apt install -y $buildDeps; fi \ +&& curl -sL https://deb.nodesource.com/setup_11.x | bash - \ +&& apt install -y nodejs \ && if [ -n "${PYTHON_DEPS}" ]; then pip install --no-cache-dir ${PYTHON_DEPS}; fi \ +&& pip install --no-cache-dir --upgrade pip \ Review comment: +1, I think we could pin this (it is only pip) This is an automated message from the Apache Git Service. To respond to the message, please log on 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] feluelle commented on a change in pull request #4724: [AIRFLOW-3906] Add npm compile to docker file
feluelle commented on a change in pull request #4724: [AIRFLOW-3906] Add npm compile to docker file URL: https://github.com/apache/airflow/pull/4724#discussion_r258164079 ## File path: Dockerfile ## @@ -16,26 +16,39 @@ FROM python:3.6-slim -COPY . /opt/airflow/ - ARG AIRFLOW_HOME=/usr/local/airflow ARG AIRFLOW_DEPS="all" ARG PYTHON_DEPS="" -ARG buildDeps="freetds-dev libkrb5-dev libsasl2-dev libssl-dev libffi-dev libpq-dev git" -ARG APT_DEPS="$buildDeps libsasl2-dev freetds-bin build-essential default-libmysqlclient-dev apt-utils curl rsync netcat locales" +ARG buildDeps="freetds-dev libkrb5-dev libssl-dev libffi-dev libpq-dev git" +ARG APT_DEPS="libsasl2-dev freetds-bin build-essential default-libmysqlclient-dev apt-utils curl rsync netcat locales" + +ENV PATH="$HOME/.npm-packages/bin:$PATH" + +RUN if [ -n "${APT_DEPS}" ]; then apt install -y $APT_DEPS; fi \ +&& apt autoremove -yqq --purge \ +&& apt clean + +COPY . /opt/airflow/ WORKDIR /opt/airflow RUN set -x \ && apt update \ -&& if [ -n "${APT_DEPS}" ]; then apt install -y $APT_DEPS; fi \ +&& if [ -n "${buildDeps}" ]; then apt install -y $buildDeps; fi \ +&& curl -sL https://deb.nodesource.com/setup_11.x | bash - \ +&& apt install -y nodejs \ Review comment: I mean it is kinda pinned (to 11.x). I am not sure if we should pin this to a specific version. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] astahlman commented on issue #4737: [AIRFLOW-3885] Fix race condition in scheduler test
astahlman commented on issue #4737: [AIRFLOW-3885] Fix race condition in scheduler test URL: https://github.com/apache/airflow/pull/4737#issuecomment-465245327 cc @feng-tao I expect this to fix all of those failing builds on master This is an automated message from the Apache Git Service. To respond to the message, please log on 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-3921) Logging bytes fails in Python 2
[ https://issues.apache.org/jira/browse/AIRFLOW-3921?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16772173#comment-16772173 ] Maximilian Roos commented on AIRFLOW-3921: -- That said, it doesn't look like these lines of code have changed since 1.10.0 (the version we're upgrading from), so I'm not sure the new errors we're seeing are caused by airflow. > Logging bytes fails in Python 2 > --- > > Key: AIRFLOW-3921 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3921 > Project: Apache Airflow > Issue Type: Bug > Components: utils >Affects Versions: 1.10.2 >Reporter: Maximilian Roos >Priority: Minor > > We just upgraded to 1.10.2. Thanks for the cadence of releases. > > We've hit one small but critical issue though: when we log a Python2 string > (i.e. bytes) that contain non-ascii characters, airflow raises an error. > > This is because airflow uses a `\n` character that is unicode encoded here: > [https://github.com/apache/airflow/blob/master/airflow/utils/log/logging_mixin.py#L102,] > because `from __future__import unicode_literals` is placed here: > [https://github.com/apache/airflow/blob/master/airflow/utils/log/logging_mixin.py#L23] > (I think this is why, and the repro below supports that, but I'm frequently > hitting unicode issues, so please correct me if I'm mistaken) > > You can see the issue reproduced: > > > {code:java} > # non-ascii character > In [16]: print(u"\u00E9") > é > # non-ascii encoded into bytes > In [11]: u"\u00E9aoeu".encode('utf-8') > Out[11]: '\xc3\xa9aoeu' > # works fine when compared with `b"\n"` > In [18]: u"\u00E9aoeu".encode('utf-8').endswith(b"\n") > Out[18]: False > # fails when compared with `u"\n"` > In [15]: '\xc3\xa9aoeu'.endswith(u"\n") > --- > UnicodeDecodeError Traceback (most recent call last) > in () > > 1 '\xc3\xa9aoeu'.endswith(u"\n") > UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 0: > ordinal not in range(128) > {code} > > I'm not sure there's any workaround without something as drastic as removing > the `from __future__import unicode_literals`, or changing all our logging to > emit unicode (which would break lots of other processes in Python 2). Is > there any temporary workaround? > > Thanks -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] feng-tao edited a comment on issue #4396: [AIRFLOW-3585] - Add edges to database
feng-tao edited a comment on issue #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#issuecomment-465237291 thanks @astahlman for the extensive testing. @ffinfo , I feel at this point, there seems to be more work than we think. The change needs some UX improvement as well IMO. e,g: I don't personally like adding the `read from dag file` button in every graph view. And I don't want @astahlman to keep testing your branch and found more issues. If you want to keep pursuing this pr, I request you should conduct the test with your branch on various scheduler(local, celery, backfill or even k8s) and post us the result with gif or picture on how it goes. Once you feel comfortable with your branch, I would suggest you send an email to your airflow-dev AIF discussion email thread and get the feedback. Once the community aligns, we could do some more tests and see how it goes. WDYT. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] feng-tao commented on issue #4396: [AIRFLOW-3585] - Add edges to database
feng-tao commented on issue #4396: [AIRFLOW-3585] - Add edges to database URL: https://github.com/apache/airflow/pull/4396#issuecomment-465237291 thanks @astahlman for the extensive testing. @ffinfo , I feel at this point, there seems to be more work than we think. The change needs some UX improvement as well IMO. e,g: I don't personally like adding the `read from dag file` button in every graph view. And I don't want @astahlman to keep testing your branch and found more issues. If you want to keep pursuing this pr, I request you should conduct the test with your branch on various scheduler(local, celery, backfill or even k8s) and post us the result with gif and picture on how it goes. Once you feel comfortable with your branch, I would suggest you send an email to your airflow-dev AIF discussion email thread and get the feedback. Once the community aligns, we could do some more tests and see how it goes. WDYT. This is an automated message from the Apache Git Service. To respond to the message, please log on 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-3921) Logging bytes fails in Python 2
Maximilian Roos created AIRFLOW-3921: Summary: Logging bytes fails in Python 2 Key: AIRFLOW-3921 URL: https://issues.apache.org/jira/browse/AIRFLOW-3921 Project: Apache Airflow Issue Type: Bug Components: utils Affects Versions: 1.10.2 Reporter: Maximilian Roos We just upgraded to 1.10.2. Thanks for the cadence of releases. We've hit one small but critical issue though: when we log a Python2 string (i.e. bytes) that contain non-ascii characters, airflow raises an error. This is because airflow uses a `\n` character that is unicode encoded here: [https://github.com/apache/airflow/blob/master/airflow/utils/log/logging_mixin.py#L102,] because `from __future__import unicode_literals` is placed here: [https://github.com/apache/airflow/blob/master/airflow/utils/log/logging_mixin.py#L23] (I think this is why, and the repro below supports that, but I'm frequently hitting unicode issues, so please correct me if I'm mistaken) You can see the issue reproduced: {code:java} # non-ascii character In [16]: print(u"\u00E9") é # non-ascii encoded into bytes In [11]: u"\u00E9aoeu".encode('utf-8') Out[11]: '\xc3\xa9aoeu' # works fine when compared with `b"\n"` In [18]: u"\u00E9aoeu".encode('utf-8').endswith(b"\n") Out[18]: False # fails when compared with `u"\n"` In [15]: '\xc3\xa9aoeu'.endswith(u"\n") --- UnicodeDecodeError Traceback (most recent call last) in () > 1 '\xc3\xa9aoeu'.endswith(u"\n") UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 0: ordinal not in range(128) {code} I'm not sure there's any workaround without something as drastic as removing the `from __future__import unicode_literals`, or changing all our logging to emit unicode (which would break lots of other processes in Python 2). Is there any temporary workaround? Thanks -- This message was sent by Atlassian JIRA (v7.6.3#76005)