[GitHub] [airflow] chaimt commented on issue #4633: AIRFLOW-3791: Dataflow

2019-03-25 Thread GitBox
chaimt commented on issue #4633: AIRFLOW-3791: Dataflow
URL: https://github.com/apache/airflow/pull/4633#issuecomment-476494140
 
 
   why os this not being merged to master?


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


With regards,
Apache Git Services


[jira] [Created] (AIRFLOW-4157) FileToGoogleCloudStorageOperator should expose "multipart" uploading supported in GoogleCloudStorageHook.upload()

2019-03-25 Thread Evgeny Podlepaev (JIRA)
Evgeny Podlepaev created AIRFLOW-4157:
-

 Summary: FileToGoogleCloudStorageOperator should expose 
"multipart" uploading supported in GoogleCloudStorageHook.upload()
 Key: AIRFLOW-4157
 URL: https://issues.apache.org/jira/browse/AIRFLOW-4157
 Project: Apache Airflow
  Issue Type: Improvement
  Components: gcp
Affects Versions: 1.10.2
Reporter: Evgeny Podlepaev


FileToGoogleCloudStorageOperator should allow "multipart" flag to be specified 
and passed to GoogleCloudStorageHook.upload(). Uploading large files 
(gigabytes) in a single shot results in an out of memory error.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (AIRFLOW-4150) Modify the docker operator implementation

2019-03-25 Thread aaronluo (JIRA)


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

aaronluo commented on AIRFLOW-4150:
---

[~ash] For docker container tasks that need to run for a long time, is it not 
suitable to run on airflow under the current architecture?

> Modify the docker operator implementation
> -
>
> Key: AIRFLOW-4150
> URL: https://issues.apache.org/jira/browse/AIRFLOW-4150
> Project: Apache Airflow
>  Issue Type: Improvement
>  Components: docker
>Reporter: aaronluo
>Priority: Major
>
> 1.  I create a test python script  testpython.py for docker
> {quote}import time
> time.sleep(1000)
> {quote}
>  
> 2. I create a DAG with a task that calls the script through a docker
> {quote}docker_ls = DockerOperator(
>  task_id='docker_ls',
>  image='python',
>  working_dir = '/data/wf/',
>  command='python testpython.py',
>  docker_url='http://192.168.1.215:2375',
>  start_date=datetime(2015, 6, 1),
>  volumes = ['/data/wf:/data/wf/'],
>  dag=dag
> )
> {quote}
>  
> 3. When I run this DAG, obviously, celery worker will be working for a very 
> long time,
> In addition, docker container will also run for a long time。
> {quote}for line in self.cli.logs(container=self.container['Id'], stream=True):
>  line = line.strip()
>  if hasattr(line, 'decode'):
>  line = line.decode('utf-8')
>  self.log.info(line)
> result = self.cli.wait(self.container['Id'])
> if result['StatusCode'] != 0:
>  raise AirflowException('docker container failed: ' + repr(result)){quote}
>  
> My suggestion is that after submitting the task to docker, celery's 
> corresponding worker should end up monitoring the docker's events, rather 
> than blocking them all the time, because the events take a long time to 
> execute.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] [airflow] cooltoast commented on issue #4939: [AIRFLOW-4120] Modify SchedulerJob.manage_slas to respect zero timedelta SLAs

2019-03-25 Thread GitBox
cooltoast commented on issue #4939: [AIRFLOW-4120] Modify 
SchedulerJob.manage_slas to respect zero timedelta SLAs
URL: https://github.com/apache/airflow/pull/4939#issuecomment-476474485
 
 
   @Fokko I added a couple tests, please take a look. thank you


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


With regards,
Apache Git Services


[GitHub] [airflow] astahlman commented on issue #4974: [AIRFLOW-4057] Fix bug in stat name validation

2019-03-25 Thread GitBox
astahlman commented on issue #4974: [AIRFLOW-4057] Fix bug in stat name 
validation
URL: https://github.com/apache/airflow/pull/4974#issuecomment-476466982
 
 
   Oops, sorry @ashb. Will keep that in mind for next time


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


With regards,
Apache Git Services


[GitHub] [airflow] codecov-io edited a comment on issue #4939: [AIRFLOW-4120] Modify SchedulerJob.manage_slas to respect zero timedelta SLAs

2019-03-25 Thread GitBox
codecov-io edited a comment on issue #4939: [AIRFLOW-4120] Modify 
SchedulerJob.manage_slas to respect zero timedelta SLAs
URL: https://github.com/apache/airflow/pull/4939#issuecomment-475907566
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/4939?src=pr&el=h1) 
Report
   > Merging 
[#4939](https://codecov.io/gh/apache/airflow/pull/4939?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/airflow/commit/dce92a54190155898c75c0f3392d42fb28f1884a?src=pr&el=desc)
 will **increase** coverage by `0.2%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/airflow/pull/4939/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/4939?src=pr&el=tree)
   
   ```diff
   @@   Coverage Diff@@
   ##   master   #4939 +/-   ##
   
   + Coverage   75.59%   75.8%   +0.2% 
   
 Files 454 458  +4 
 Lines   29197   29862+665 
   
   + Hits22071   22636+565 
   - Misses   71267226+100
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/airflow/pull/4939?src=pr&el=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[airflow/jobs.py](https://codecov.io/gh/apache/airflow/pull/4939/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzLnB5)
 | `78.83% <100%> (+1.95%)` | :arrow_up: |
   | 
[...w/contrib/operators/s3\_to\_gcs\_transfer\_operator.py](https://codecov.io/gh/apache/airflow/pull/4939/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9zM190b19nY3NfdHJhbnNmZXJfb3BlcmF0b3IucHk=)
 | `0% <0%> (-100%)` | :arrow_down: |
   | 
[.../contrib/operators/gcs\_to\_gcs\_transfer\_operator.py](https://codecov.io/gh/apache/airflow/pull/4939/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9nY3NfdG9fZ2NzX3RyYW5zZmVyX29wZXJhdG9yLnB5)
 | `0% <0%> (-100%)` | :arrow_down: |
   | 
[airflow/contrib/executors/kubernetes\_executor.py](https://codecov.io/gh/apache/airflow/pull/4939/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4ZWN1dG9ycy9rdWJlcm5ldGVzX2V4ZWN1dG9yLnB5)
 | `62.96% <0%> (-2.27%)` | :arrow_down: |
   | 
[airflow/configuration.py](https://codecov.io/gh/apache/airflow/pull/4939/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb25maWd1cmF0aW9uLnB5)
 | `92.17% <0%> (-2.23%)` | :arrow_down: |
   | 
[airflow/utils/helpers.py](https://codecov.io/gh/apache/airflow/pull/4939/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9oZWxwZXJzLnB5)
 | `82.51% <0%> (-0.36%)` | :arrow_down: |
   | 
[airflow/utils/db.py](https://codecov.io/gh/apache/airflow/pull/4939/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9kYi5weQ==)
 | `90.19% <0%> (-0.19%)` | :arrow_down: |
   | 
[airflow/\_\_init\_\_.py](https://codecov.io/gh/apache/airflow/pull/4939/diff?src=pr&el=tree#diff-YWlyZmxvdy9fX2luaXRfXy5weQ==)
 | `95.83% <0%> (-0.17%)` | :arrow_down: |
   | 
[airflow/utils/dag\_processing.py](https://codecov.io/gh/apache/airflow/pull/4939/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9kYWdfcHJvY2Vzc2luZy5weQ==)
 | `59.4% <0%> (-0.11%)` | :arrow_down: |
   | 
[airflow/logging\_config.py](https://codecov.io/gh/apache/airflow/pull/4939/diff?src=pr&el=tree#diff-YWlyZmxvdy9sb2dnaW5nX2NvbmZpZy5weQ==)
 | `97.5% <0%> (-0.07%)` | :arrow_down: |
   | ... and [26 
more](https://codecov.io/gh/apache/airflow/pull/4939/diff?src=pr&el=tree-more) 
| |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/airflow/pull/4939?src=pr&el=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/4939?src=pr&el=footer). 
Last update 
[dce92a5...e9ea55f](https://codecov.io/gh/apache/airflow/pull/4939?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


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


With regards,
Apache Git Services


[GitHub] [airflow] jmcarp commented on a change in pull request #4926: [AIRFLOW-4104] Add type annotations to common classes.

2019-03-25 Thread GitBox
jmcarp commented on a change in pull request #4926: [AIRFLOW-4104] Add type 
annotations to common classes.
URL: https://github.com/apache/airflow/pull/4926#discussion_r268934073
 
 

 ##
 File path: airflow/models/__init__.py
 ##
 @@ -4290,7 +4291,13 @@ def setdefault(cls, key, default, 
deserialize_json=False):
 
 @classmethod
 @provide_session
-def get(cls, key, default_var=__NO_DEFAULT_SENTINEL, 
deserialize_json=False, session=None):
+def get(
+cls,
+key,  # type: str
+default_var=__NO_DEFAULT_SENTINEL,  # type: Any
+deserialize_json=False,  # type: bool
+session=None
 
 Review comment:
   I think this is a little unwieldy for one line:
   
   ```python
   def get(cls, key, default_var=__NO_DEFAULT_SENTINEL, deserialize=False, 
session=None):  # type: (str, Any, bool, Any) -> Any
   ```
   
   We could split the definition over a few lines, but then we're getting close 
to the original formatting:
   
   ```python
   def get(
   cls, key, default_var=__NO_DEFAULT_SENTINEL, deserialize=False, 
session=None
   ):  # type: (str, Any, bool, Any) -> Any
   ```
   
   I don't have a strong preference between the second example and the current 
code. But I think it would be even better to adopt 
https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-6+Enforce+the+usage+of+a+code+formatter
 and avoid having these conversations.


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


With regards,
Apache Git Services


[GitHub] [airflow] jmcarp commented on issue #4926: [AIRFLOW-4104] Add type annotations to common classes.

2019-03-25 Thread GitBox
jmcarp commented on issue #4926: [AIRFLOW-4104] Add type annotations to common 
classes.
URL: https://github.com/apache/airflow/pull/4926#issuecomment-476463889
 
 
   Conflicts resolved.


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


With regards,
Apache Git Services


[GitHub] [airflow] feng-tao commented on issue #4974: [AIRFLOW-4057] Fix bug in stat name validation

2019-03-25 Thread GitBox
feng-tao commented on issue #4974: [AIRFLOW-4057] Fix bug in stat name 
validation
URL: https://github.com/apache/airflow/pull/4974#issuecomment-476459855
 
 
   @astahlman , didn't catch this before, but it is suggested to have a 
separate jira in this case as it would make @ashb 's life(release manager) a 
bit easier :)


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


With regards,
Apache Git Services


[GitHub] [airflow] feng-tao commented on a change in pull request #4973: [AIRFLOW-4155] Allow Public role access to /home

2019-03-25 Thread GitBox
feng-tao commented on a change in pull request #4973: [AIRFLOW-4155] Allow 
Public role access to /home
URL: https://github.com/apache/airflow/pull/4973#discussion_r268930130
 
 

 ##
 File path: tests/www/test_security.py
 ##
 @@ -308,3 +308,26 @@ def test_override_role_vm(self):
 test_security_manager = TestSecurityManager(appbuilder=self.appbuilder)
 self.assertEqual(len(test_security_manager.VIEWER_VMS), 1)
 self.assertEqual(test_security_manager.VIEWER_VMS, {'Airflow'})
+
+def test_is_user_logged_in_returns_false_if_not_authenticated(self):
+user = mock.MagicMock()
+user.is_authenticated = False
+self.assertFalse(self.security_manager.is_user_logged_in(user))
+
+def test_is_user_logged_in_returns_true_if_authenticated(self):
+user = mock.MagicMock()
+user.is_authenticated = True
+self.assertTrue(self.security_manager.is_user_logged_in(user))
+
+def test_unauthenticated_user_is_public(self):
+user = mock.MagicMock()
+user.is_authenticated = False
+self.assertTrue(self.security_manager.is_public_user(user))
 
 Review comment:
   @ashb, could you elaborate your concern? Currently this should just let the 
user know that they need to register with certain type of roles to view the dag 
instead of blank page. I think this will help to make the onboarding for a new 
user smoother.


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


With regards,
Apache Git Services


[jira] [Commented] (AIRFLOW-4057) airflow should handle invalid stats name

2019-03-25 Thread ASF subversion and git services (JIRA)


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

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

Commit 5eccaf642e3686f8562121d448b0800c0315c8fa in airflow's branch 
refs/heads/master from Andrew Stahlman
[ https://gitbox.apache.org/repos/asf?p=airflow.git;h=5eccaf6 ]

[AIRFLOW-4057] Fix bug in stat name validation (#4974)

* [AIRFLOW-4057] Fix bug in stat name validation

The `@validate_stats` wrapper is wrapping an instance method, so it's first
argument is actually an instance of `SafeStatsdLogger`, not the stat_name.
Here's the backtrace that shows up in the webserver logs without the fix:

  File "/Users/andrewstahlman/src/incubator-airflow/airflow/www/views.py", 
line 86, in 
dagbag = models.DagBag(os.devnull, include_examples=False)
  File 
"/Users/andrewstahlman/src/incubator-airflow/airflow/models/__init__.py", line 
312, in __init__
safe_mode=safe_mode)
  File 
"/Users/andrewstahlman/src/incubator-airflow/airflow/models/__init__.py", line 
594, in collect_dags
'collect_dags', (timezone.utcnow() - start_dttm).total_seconds(), 1)
  File "/Users/andrewstahlman/src/incubator-airflow/airflow/stats.py", line 
85, in wrapper
log.warning('Invalid stat name: {stat}.'.format(stat=stat), err)
Message: 'Invalid stat name: .'
Arguments: (InvalidStatsNameException('The stat_name has to be a string'),)

I've verified the fix by updating the unit tests to exercise the "public"
methods rather than testing the internal validation logic directly.

* Wrap stat_name_handler in staticmethod


> airflow should handle invalid stats name
> 
>
> Key: AIRFLOW-4057
> URL: https://issues.apache.org/jira/browse/AIRFLOW-4057
> Project: Apache Airflow
>  Issue Type: New Feature
>Reporter: Chao-Han Tsai
>Assignee: Chao-Han Tsai
>Priority: Major
> Fix For: 2.0.0
>
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] [airflow] feng-tao merged pull request #4974: [AIRFLOW-4057] Fix bug in stat name validation

2019-03-25 Thread GitBox
feng-tao merged pull request #4974: [AIRFLOW-4057] Fix bug in stat name 
validation
URL: https://github.com/apache/airflow/pull/4974
 
 
   


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


With regards,
Apache Git Services


[GitHub] [airflow] feng-tao commented on issue #4974: [AIRFLOW-4057] Fix bug in stat name validation

2019-03-25 Thread GitBox
feng-tao commented on issue #4974: [AIRFLOW-4057] Fix bug in stat name 
validation
URL: https://github.com/apache/airflow/pull/4974#issuecomment-476458849
 
 
   cc @ashb  FYI, as we have two prs for the same jira ticket.


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


With regards,
Apache Git Services


[jira] [Commented] (AIRFLOW-4057) airflow should handle invalid stats name

2019-03-25 Thread ASF subversion and git services (JIRA)


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

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

Commit 5eccaf642e3686f8562121d448b0800c0315c8fa in airflow's branch 
refs/heads/master from Andrew Stahlman
[ https://gitbox.apache.org/repos/asf?p=airflow.git;h=5eccaf6 ]

[AIRFLOW-4057] Fix bug in stat name validation (#4974)

* [AIRFLOW-4057] Fix bug in stat name validation

The `@validate_stats` wrapper is wrapping an instance method, so it's first
argument is actually an instance of `SafeStatsdLogger`, not the stat_name.
Here's the backtrace that shows up in the webserver logs without the fix:

  File "/Users/andrewstahlman/src/incubator-airflow/airflow/www/views.py", 
line 86, in 
dagbag = models.DagBag(os.devnull, include_examples=False)
  File 
"/Users/andrewstahlman/src/incubator-airflow/airflow/models/__init__.py", line 
312, in __init__
safe_mode=safe_mode)
  File 
"/Users/andrewstahlman/src/incubator-airflow/airflow/models/__init__.py", line 
594, in collect_dags
'collect_dags', (timezone.utcnow() - start_dttm).total_seconds(), 1)
  File "/Users/andrewstahlman/src/incubator-airflow/airflow/stats.py", line 
85, in wrapper
log.warning('Invalid stat name: {stat}.'.format(stat=stat), err)
Message: 'Invalid stat name: .'
Arguments: (InvalidStatsNameException('The stat_name has to be a string'),)

I've verified the fix by updating the unit tests to exercise the "public"
methods rather than testing the internal validation logic directly.

* Wrap stat_name_handler in staticmethod


> airflow should handle invalid stats name
> 
>
> Key: AIRFLOW-4057
> URL: https://issues.apache.org/jira/browse/AIRFLOW-4057
> Project: Apache Airflow
>  Issue Type: New Feature
>Reporter: Chao-Han Tsai
>Assignee: Chao-Han Tsai
>Priority: Major
> Fix For: 2.0.0
>
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (AIRFLOW-4057) airflow should handle invalid stats name

2019-03-25 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on AIRFLOW-4057:
-

feng-tao commented on pull request #4974: [AIRFLOW-4057] Fix bug in stat name 
validation
URL: https://github.com/apache/airflow/pull/4974
 
 
   
 

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


> airflow should handle invalid stats name
> 
>
> Key: AIRFLOW-4057
> URL: https://issues.apache.org/jira/browse/AIRFLOW-4057
> Project: Apache Airflow
>  Issue Type: New Feature
>Reporter: Chao-Han Tsai
>Assignee: Chao-Han Tsai
>Priority: Major
> Fix For: 2.0.0
>
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] [airflow] feng-tao commented on issue #4974: [AIRFLOW-4057] Fix bug in stat name validation

2019-03-25 Thread GitBox
feng-tao commented on issue #4974: [AIRFLOW-4057] Fix bug in stat name 
validation
URL: https://github.com/apache/airflow/pull/4974#issuecomment-476458670
 
 
   @astahlman , thanks for the fix. I think that unit test is flaky. If you 
want, we could fix in a different pr. But let's get this one in.


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


With regards,
Apache Git Services


[GitHub] [airflow] feng-tao commented on a change in pull request #4974: [AIRFLOW-4057] Fix bug in stat name validation

2019-03-25 Thread GitBox
feng-tao commented on a change in pull request #4974: [AIRFLOW-4057] Fix bug in 
stat name validation
URL: https://github.com/apache/airflow/pull/4974#discussion_r268929570
 
 

 ##
 File path: docs/plugins.rst
 ##
 @@ -212,7 +212,7 @@ definitions in Airflow.
 flask_blueprints = [bp]
 appbuilder_views = [v_appbuilder_package]
 appbuilder_menu_items = [appbuilder_mitem]
-stat_name_handler = stat_name_dummy_handler
+stat_name_handler = staticmethod(stat_name_dummy_handler)
 
 Review comment:
   ```>>> def x(arg_that_should_be_str):
   ...return arg_that_should_be_str
   ...
   >>> class Foo:
   ...x = x
   ...
   >>> x_impl = Foo.x
   >>> type(x_impl)
   
   >>> x_impl("a string")
   Traceback (most recent call last):
 File "", line 1, in 
   TypeError: unbound method x() must be called with Foo instance as first 
argument (got str instance instead)
   >>> ^D
   tfeng@tfeng-mbp143 ~/playground/incubator-airflow (tfeng_fix_ci_race) $ 
python3
   Python 3.7.1 (default, Nov 28 2018, 11:51:54)
   [Clang 10.0.0 (clang-1000.11.45.5)] on darwin
   Type "help", "copyright", "credits" or "license" for more information.
   >>> def x(t):
   ...   return t
   ...
   >>> class Foo:
   ...   x = x
   ...
   >>> x_impl = Foo.x
   >>> type(x_impl)
   ```
   
   somehow py2 treats as instance method while py3 treats it as function. Good 
find.  This seems to be related to 
https://stackoverflow.com/questions/3589311/get-defining-class-of-unbound-method-object-in-python-3
 (unbound method in python3 is gone)


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


With regards,
Apache Git Services


[GitHub] [airflow] feng-tao commented on a change in pull request #4974: [AIRFLOW-4057] Fix bug in stat name validation

2019-03-25 Thread GitBox
feng-tao commented on a change in pull request #4974: [AIRFLOW-4057] Fix bug in 
stat name validation
URL: https://github.com/apache/airflow/pull/4974#discussion_r268929570
 
 

 ##
 File path: docs/plugins.rst
 ##
 @@ -212,7 +212,7 @@ definitions in Airflow.
 flask_blueprints = [bp]
 appbuilder_views = [v_appbuilder_package]
 appbuilder_menu_items = [appbuilder_mitem]
-stat_name_handler = stat_name_dummy_handler
+stat_name_handler = staticmethod(stat_name_dummy_handler)
 
 Review comment:
   ```>>> def x(arg_that_should_be_str):
   ...return arg_that_should_be_str
   ...
   >>> class Foo:
   ...x = x
   ...
   >>> x_impl = Foo.x
   >>> type(x_impl)
   
   >>> x_impl("a string")
   Traceback (most recent call last):
 File "", line 1, in 
   TypeError: unbound method x() must be called with Foo instance as first 
argument (got str instance instead)
   >>> ^D
   tfeng@tfeng-mbp143 ~/playground/incubator-airflow (tfeng_fix_ci_race) $ 
python3
   Python 3.7.1 (default, Nov 28 2018, 11:51:54)
   [Clang 10.0.0 (clang-1000.11.45.5)] on darwin
   Type "help", "copyright", "credits" or "license" for more information.
   >>> def x(t):
   ...   return t
   ...
   >>> class Foo:
   ...   x = x
   ...
   >>> x_impl = Foo.x
   >>> type(x_impl)
   
   ```
   
   somehow py2 treats as instance method while py3 treats it as function. Good 
find.  This seems to be related to 
https://stackoverflow.com/questions/3589311/get-defining-class-of-unbound-method-object-in-python-3
 (unbound method in python3 is gone)


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


With regards,
Apache Git Services


[GitHub] [airflow] feng-tao commented on a change in pull request #4974: [AIRFLOW-4057] Fix bug in stat name validation

2019-03-25 Thread GitBox
feng-tao commented on a change in pull request #4974: [AIRFLOW-4057] Fix bug in 
stat name validation
URL: https://github.com/apache/airflow/pull/4974#discussion_r268929613
 
 

 ##
 File path: docs/plugins.rst
 ##
 @@ -212,7 +212,7 @@ definitions in Airflow.
 flask_blueprints = [bp]
 appbuilder_views = [v_appbuilder_package]
 appbuilder_menu_items = [appbuilder_mitem]
-stat_name_handler = stat_name_dummy_handler
+stat_name_handler = staticmethod(stat_name_dummy_handler)
 
 Review comment:
   FYI @astahlman @milton0825 


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


With regards,
Apache Git Services


[GitHub] [airflow] milton0825 commented on a change in pull request #4974: [AIRFLOW-4057] Fix bug in stat name validation

2019-03-25 Thread GitBox
milton0825 commented on a change in pull request #4974: [AIRFLOW-4057] Fix bug 
in stat name validation
URL: https://github.com/apache/airflow/pull/4974#discussion_r268927547
 
 

 ##
 File path: docs/plugins.rst
 ##
 @@ -212,7 +212,7 @@ definitions in Airflow.
 flask_blueprints = [bp]
 appbuilder_views = [v_appbuilder_package]
 appbuilder_menu_items = [appbuilder_mitem]
-stat_name_handler = stat_name_dummy_handler
+stat_name_handler = staticmethod(stat_name_dummy_handler)
 
 Review comment:
   hmm interesting
   


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


With regards,
Apache Git Services


[GitHub] [airflow] gerardo commented on a change in pull request #4938: [AIRFLOW-4117] Multi-staging Image - Travis CI tests [Step 3/3]

2019-03-25 Thread GitBox
gerardo commented on a change in pull request #4938: [AIRFLOW-4117] 
Multi-staging Image - Travis CI tests [Step 3/3]
URL: https://github.com/apache/airflow/pull/4938#discussion_r268924028
 
 

 ##
 File path: scripts/ci/docker-compose.yml
 ##
 @@ -18,61 +18,67 @@ version: "2.2"
 services:
   mysql:
 image: mysql:5.6
-restart: always
+restart: "no"
 environment:
   - MYSQL_ALLOW_EMPTY_PASSWORD=true
   - MYSQL_ROOT_HOST=%
 volumes:
   - ./mysql/conf.d:/etc/mysql/conf.d
-
   postgres:
 image: postgres:9.6
-restart: always
+restart: "no"
 environment:
   - POSTGRES_USER=postgres
   - POSTGRES_PASSWORD=airflow
   - POSTGRES_DB=airflow
-
   mongo:
 image: mongo:3
-restart: always
+restart: "no"
 
   cassandra:
 image: cassandra:3.0
-restart: always
+restart: "no"
 
   rabbitmq:
 image: rabbitmq:3.7
-restart: always
+restart: "no"
 
 Review comment:
   Yeah, we can switch it to no. It shouldn't affect the tests.


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


With regards,
Apache Git Services


[GitHub] [airflow] astahlman commented on issue #4974: [AIRFLOW-4057] Fix bug in stat name validation

2019-03-25 Thread GitBox
astahlman commented on issue #4974: [AIRFLOW-4057] Fix bug in stat name 
validation
URL: https://github.com/apache/airflow/pull/4974#issuecomment-476451175
 
 
   @feng-tao Have you ever seen this failure before? I'm pretty sure it's not 
related to my changes, but I can dig deeper if you think it's worth it.
   
   ```
   test_no_orphan_process_will_be_left (tests.test_jobs.SchedulerJobTest) ...
   
   No output has been received in the last 10m0s, this potentially indicates a 
stalled build or something wrong with the build itself.
   
   Check the details on how to adjust your build configuration on: 
https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received
   
   The build has been terminated
   ```


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


With regards,
Apache Git Services


[GitHub] [airflow] astahlman commented on issue #4974: [AIRFLOW-4057] Fix bug in stat name validation

2019-03-25 Thread GitBox
astahlman commented on issue #4974: [AIRFLOW-4057] Fix bug in stat name 
validation
URL: https://github.com/apache/airflow/pull/4974#issuecomment-476446514
 
 
   @feng-tao Fixed - see 
https://github.com/apache/airflow/pull/4974/files#r268920074 for an explanation.
   
   cc @milton0825 I've updated the docs, as well.


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


With regards,
Apache Git Services


[GitHub] [airflow] astahlman commented on a change in pull request #4974: [AIRFLOW-4057] Fix bug in stat name validation

2019-03-25 Thread GitBox
astahlman commented on a change in pull request #4974: [AIRFLOW-4057] Fix bug 
in stat name validation
URL: https://github.com/apache/airflow/pull/4974#discussion_r268920074
 
 

 ##
 File path: docs/plugins.rst
 ##
 @@ -212,7 +212,7 @@ definitions in Airflow.
 flask_blueprints = [bp]
 appbuilder_views = [v_appbuilder_package]
 appbuilder_menu_items = [appbuilder_mitem]
-stat_name_handler = stat_name_dummy_handler
+stat_name_handler = staticmethod(stat_name_dummy_handler)
 
 Review comment:
   Without this, assigning the function in this way binds it to the 
AirflowPlugin class. Illustration of the difference (on Python2):
   
   ```
   >>> def x(arg_that_should_be_str):
   ...   return arg_that_should_be_str
   ...
   >>> class Foo:
   ...   x = x
   ...
   >>> x_impl = Foo.x
   >>> x_impl("a string")
   Traceback (most recent call last):
 File "", line 1, in 
   TypeError: unbound method x() must be called with Foo instance as first 
argument (got str instance instead)
   >>>
   >>> class Foo:
   ...   x = staticmethod(x)
   ...
   >>> x_impl = Foo.x
   >>> x_impl("a string")
   'a string'
   ```


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


With regards,
Apache Git Services


[GitHub] [airflow] zhongjiajie commented on a change in pull request #4895: [AIRFLOW-1526] Add dingding hook and operator

2019-03-25 Thread GitBox
zhongjiajie commented on a change in pull request #4895: [AIRFLOW-1526] Add 
dingding hook and operator
URL: https://github.com/apache/airflow/pull/4895#discussion_r268915806
 
 

 ##
 File path: airflow/contrib/hooks/dingding_hook.py
 ##
 @@ -0,0 +1,133 @@
+# -*- coding: utf-8 -*-
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import json
+
+import requests
+
+from airflow import AirflowException
+from airflow.hooks.http_hook import HttpHook
+
+
+class DingdingHook(HttpHook):
+"""
+This hook allows you send Dingding message using Dingding custom bot.
+Get Dingding token from conn_id.password. And prefer set domain to
+conn_id.host, if not will use default ``https://oapi.dingtalk.com``.
+
+For more detail message in
+`Dingding custom bot 
`_
+
+:param dingding_conn_id: The name of the Dingding connection to use
+:type dingding_conn_id: str
+:param message_type: Message type you want to send to Dingding, support 
five type so far
+ including text, link, markdown, actionCard, feedCard
+:type message_type: str
+:param message: The message send to Dingding chat group
+:type message: str or dict
+:param at_mobiles: Remind specific users with this message
+:type at_mobiles: list of str
 
 Review comment:
   @mik-laj Thanks your clarification, That help me a lot :+1: 


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


With regards,
Apache Git Services


[GitHub] [airflow] zhongjiajie commented on a change in pull request #4903: [AIRFLOW-4069] Add Opsgenie Alert Hook and Operator

2019-03-25 Thread GitBox
zhongjiajie commented on a change in pull request #4903: [AIRFLOW-4069] Add 
Opsgenie Alert Hook and Operator
URL: https://github.com/apache/airflow/pull/4903#discussion_r268914882
 
 

 ##
 File path: airflow/contrib/hooks/opsgenie_alert_hook.py
 ##
 @@ -0,0 +1,68 @@
+# -*- coding: utf-8 -*-
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+import json
+
+from airflow.hooks.http_hook import HttpHook
+
+
+class OpsgenieAlertHook(HttpHook):
+"""
+This hook allows you to post alerts to Opsgenie.
+Accepts a connection that has an Opsgenie API key as the connection's 
password.
+Each Opsgenie API key can be pre-configured to a team integration.
+You can override these defaults in this hook.
+
+:param http_conn_id: Http connection ID with host as 
"https://api.opsgenie.com/";
+  and Opsgenie API key as the connection's password
+  (e.g. "eb243592-faa2-4ba2-a551q-1afdf565c889")
+:type http_conn_id: str
+:param payload: Opsgenie API Create Alert payload values
+See 
https://docs.opsgenie.com/docs/alert-api#section-create-alert
+:type payload: dict
+:param proxy: Proxy to use to make the Opsgenie Alert API call
+:type proxy: str
+"""
+def __init__(self,
+ http_conn_id,
 
 Review comment:
   Maybe `opsgenie_conn_id` is better see 
https://github.com/apache/airflow/pull/4895#discussion_r268204537
   
   so as operator


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


With regards,
Apache Git Services


[GitHub] [airflow] zhongjiajie commented on a change in pull request #4903: [AIRFLOW-4069] Add Opsgenie Alert Hook and Operator

2019-03-25 Thread GitBox
zhongjiajie commented on a change in pull request #4903: [AIRFLOW-4069] Add 
Opsgenie Alert Hook and Operator
URL: https://github.com/apache/airflow/pull/4903#discussion_r268914120
 
 

 ##
 File path: airflow/contrib/operators/opsgenie_alert_operator.py
 ##
 @@ -0,0 +1,139 @@
+# -*- coding: utf-8 -*-
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+from airflow.contrib.hooks.opsgenie_alert_hook import OpsgenieAlertHook
+from airflow.models import BaseOperator
+from airflow.utils.decorators import apply_defaults
+
+
+class OpsgenieAlertOperator(BaseOperator):
+"""
+This operator allows you to post alerts to Opsgenie.
+Accepts a connection that has an Opsgenie API key as the connection's 
password.
+
+Each Opsgenie API key can be pre-configured to a team integration.
+You can override these defaults in this operator.
+
+:param http_conn_id: Http connection ID, defaults to 'opsgenie_default',
+  with host as "https://api.opsgenie.com/";
+  and Opsgenie API key as the connection's password
+  (e.g. "eb243592-faa2-4ba2-a551q-1afdf565c889")
+:type http_conn_id: str
+:param message: The Message of the Opsgenie alert
+:type message: str
+:param alias: Client-defined identifier of the alert
+:type alias: str
+:param description: Description field of the alert
+:type description: str
+:param responders: Teams, users, escalations and schedules that
+  the alert will be routed to send notifications.
+:type responders: list[dict]
+:param visibleTo: Teams and users that the alert will become visible
+  to without sending any notification.
+:type visibleTo: list[dict]
+:param actions: Custom actions that will be available for the alert.
+:type actions: list[str]
+:param tags: Tags of the alert.
+:type tags: list[str]
+:param details: Map of key-value pairs to use as custom properties of the 
alert.
+:type details: dict
+:param entity: Entity field of the alert that is
+generally used to specify which domain alert is related to.
+:type entity: str
+:param source: Source field of the alert. Default value is
+IP address of the incoming request.
+:type source: str
+:param priority: Priority level of the alert. Default value is P3.
+:type priority: str
+:param user: Display name of the request owner.
+:type user: str
+:param note: Additional note that will be added while creating the alert.
+:type note: str
+:param proxy: Proxy to use to make the Opsgenie Alert API call
+:type proxy: str
+"""
+
+@apply_defaults
+def __init__(self,
+ message,
+ http_conn_id='opsgenie_default',
+ alias=None,
+ description=None,
+ responders=None,
+ visibleTo=None,
+ actions=None,
+ tags=None,
+ details=None,
+ entity=None,
+ source=None,
+ priority=None,
+ user=None,
+ note=None,
+ proxy=None,
+ *args,
+ **kwargs
+ ):
+super(OpsgenieAlertOperator, self).__init__(*args, **kwargs)
+
+self.message = message
+self.http_conn_id = http_conn_id
+self.alias = alias
+self.description = description
+self.responders = responders
+self.visibleTo = visibleTo
+self.actions = actions
+self.tags = tags
+self.details = details
+self.entity = entity
+self.source = source
+self.priority = priority
+self.user = user
+self.note = note
+self.proxy = proxy
+self.hook = None
+
+def _build_opsgenie_payload(self):
 
 Review comment:
   Shoud we move this function to Hook rather than operator?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the spec

[GitHub] [airflow] zhongjiajie commented on a change in pull request #4903: [AIRFLOW-4069] Add Opsgenie Alert Hook and Operator

2019-03-25 Thread GitBox
zhongjiajie commented on a change in pull request #4903: [AIRFLOW-4069] Add 
Opsgenie Alert Hook and Operator
URL: https://github.com/apache/airflow/pull/4903#discussion_r268913989
 
 

 ##
 File path: airflow/contrib/hooks/opsgenie_alert_hook.py
 ##
 @@ -0,0 +1,68 @@
+# -*- coding: utf-8 -*-
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+import json
+
+from airflow.hooks.http_hook import HttpHook
+
+
+class OpsgenieAlertHook(HttpHook):
+"""
+This hook allows you to post alerts to Opsgenie.
+Accepts a connection that has an Opsgenie API key as the connection's 
password.
+Each Opsgenie API key can be pre-configured to a team integration.
+You can override these defaults in this hook.
+
+:param http_conn_id: Http connection ID with host as 
"https://api.opsgenie.com/";
+  and Opsgenie API key as the connection's password
+  (e.g. "eb243592-faa2-4ba2-a551q-1afdf565c889")
+:type http_conn_id: str
+:param payload: Opsgenie API Create Alert payload values
+See 
https://docs.opsgenie.com/docs/alert-api#section-create-alert
+:type payload: dict
+:param proxy: Proxy to use to make the Opsgenie Alert API call
+:type proxy: str
+"""
+def __init__(self,
+ http_conn_id,
+ payload={},
+ proxy=None,
+ *args,
+ **kwargs
+ ):
+super(OpsgenieAlertHook, self).__init__(*args, **kwargs)
+self.http_conn_id = http_conn_id
+self.payload = payload
+self.proxy = proxy
+
+def execute(self):
+"""
+Remote Popen (actually execute the Opsgenie Alert call)
+"""
+proxies = {}
+if self.proxy:
+# we only need https proxy for Opsgenie, as the endpoint is https
+proxies = {'https': self.proxy}
+api_key = self.get_connection(self.http_conn_id).password
+return self.run(endpoint='v2/alerts',
+data=json.dumps(self.payload, sort_keys=True),
 
 Review comment:
   Maybe we should remove `sort_keys`? I personally think is unnecessary.


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


With regards,
Apache Git Services


[GitHub] [airflow] zhongjiajie commented on a change in pull request #4903: [AIRFLOW-4069] Add Opsgenie Alert Hook and Operator

2019-03-25 Thread GitBox
zhongjiajie commented on a change in pull request #4903: [AIRFLOW-4069] Add 
Opsgenie Alert Hook and Operator
URL: https://github.com/apache/airflow/pull/4903#discussion_r268914994
 
 

 ##
 File path: airflow/contrib/hooks/opsgenie_alert_hook.py
 ##
 @@ -0,0 +1,68 @@
+# -*- coding: utf-8 -*-
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+import json
+
+from airflow.hooks.http_hook import HttpHook
+
+
+class OpsgenieAlertHook(HttpHook):
+"""
+This hook allows you to post alerts to Opsgenie.
+Accepts a connection that has an Opsgenie API key as the connection's 
password.
+Each Opsgenie API key can be pre-configured to a team integration.
+You can override these defaults in this hook.
+
+:param http_conn_id: Http connection ID with host as 
"https://api.opsgenie.com/";
+  and Opsgenie API key as the connection's password
+  (e.g. "eb243592-faa2-4ba2-a551q-1afdf565c889")
+:type http_conn_id: str
+:param payload: Opsgenie API Create Alert payload values
+See 
https://docs.opsgenie.com/docs/alert-api#section-create-alert
+:type payload: dict
+:param proxy: Proxy to use to make the Opsgenie Alert API call
+:type proxy: str
+"""
+def __init__(self,
+ http_conn_id,
+ payload={},
+ proxy=None,
+ *args,
+ **kwargs
+ ):
+super(OpsgenieAlertHook, self).__init__(*args, **kwargs)
 
 Review comment:
   Maybe 
   ```suggestion
   super(OpsgenieAlertHook, 
self).__init__(http_conn_id=opsgenie_conn_id, *args, **kwargs)
   ```
   and remove L51 
   https://github.com/apache/airflow/pull/4895#discussion_r268206731


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


With regards,
Apache Git Services


[GitHub] [airflow] XD-DENG merged pull request #4976: adding Beeswax as a company who uses airflow

2019-03-25 Thread GitBox
XD-DENG merged pull request #4976: adding Beeswax as a company who uses airflow
URL: https://github.com/apache/airflow/pull/4976
 
 
   


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


With regards,
Apache Git Services


[GitHub] [airflow] TheOriginalAlex opened a new pull request #4977: [AIRFLOW-XXX] Add another engineer to Bombora Inc's list of engineers

2019-03-25 Thread GitBox
TheOriginalAlex opened a new pull request #4977: [AIRFLOW-XXX] Add another 
engineer to Bombora Inc's list of engineers
URL: https://github.com/apache/airflow/pull/4977
 
 
   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-XXX
 - In case you are fixing a typo in the documentation you can prepend your 
commit with \[AIRFLOW-XXX\], code changes always need a Jira issue.
 - In case you are proposing a fundamental code change, you need to create 
an Airflow Improvement Proposal 
([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)).
 - In case you are adding a dependency, check if the license complies with 
the [ASF 3rd Party License 
Policy](https://www.apache.org/legal/resolved.html#category-x).
   
   ### Description
   
   - [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:
   
   ### 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
 - If you implement backwards incompatible changes, please leave a note in 
the [Updating.md](https://github.com/apache/airflow/blob/master/UPDATING.md) so 
we can assign it to a appropriate release
   
   ### Code Quality
   
   - [x] Passes `flake8`
   


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


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #4938: [AIRFLOW-4117] Multi-staging Image - Travis CI tests [Step 3/3]

2019-03-25 Thread GitBox
potiuk commented on a change in pull request #4938: [AIRFLOW-4117] 
Multi-staging Image - Travis CI tests [Step 3/3]
URL: https://github.com/apache/airflow/pull/4938#discussion_r268896300
 
 

 ##
 File path: setup.py
 ##
 @@ -35,31 +34,34 @@
 
 PY3 = sys.version_info[0] == 3
 
-with io.open('README.md', encoding='utf-8') as f:
-long_description = f.read()
+try:
+FileNotFoundError
+except NameError:
+FileNotFoundError = IOError
 
+try:
+with io.open('README.md', encoding='utf-8') as f:
+long_description = f.read()
+except FileNotFoundError:
+long_description = ''
 
-class Tox(TestCommand):
-user_options = [('tox-args=', None, "Arguments to pass to tox")]
+
+class CleanCommand(Command):
+"""Custom clean command to tidy up the project root."""
+user_options = []
 
 def initialize_options(self):
-TestCommand.initialize_options(self)
-self.tox_args = ''
+pass
 
 def finalize_options(self):
-TestCommand.finalize_options(self)
-self.test_args = []
-self.test_suite = True
+pass
 
-def run_tests(self):
-# import here, cause outside the eggs aren't loaded
-import tox
-errno = tox.cmdline(args=self.tox_args.split())
-sys.exit(errno)
+def run(self):
+os.system('rm -vrf ./build ./dist ./*.pyc ./*.tgz ./*.egg-info')
 
 Review comment:
   To be cleaned-up indeed.


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


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #4938: [AIRFLOW-4117] Multi-staging Image - Travis CI tests [Step 3/3]

2019-03-25 Thread GitBox
potiuk commented on a change in pull request #4938: [AIRFLOW-4117] 
Multi-staging Image - Travis CI tests [Step 3/3]
URL: https://github.com/apache/airflow/pull/4938#discussion_r268896021
 
 

 ##
 File path: scripts/ci/docker-compose.yml
 ##
 @@ -18,61 +18,67 @@ version: "2.2"
 services:
   mysql:
 image: mysql:5.6
-restart: always
+restart: "no"
 environment:
   - MYSQL_ALLOW_EMPTY_PASSWORD=true
   - MYSQL_ROOT_HOST=%
 volumes:
   - ./mysql/conf.d:/etc/mysql/conf.d
-
   postgres:
 image: postgres:9.6
-restart: always
+restart: "no"
 environment:
   - POSTGRES_USER=postgres
   - POSTGRES_PASSWORD=airflow
   - POSTGRES_DB=airflow
-
   mongo:
 image: mongo:3
-restart: always
+restart: "no"
 
   cassandra:
 image: cassandra:3.0
-restart: always
+restart: "no"
 
   rabbitmq:
 image: rabbitmq:3.7
-restart: always
+restart: "no"
 
 Review comment:
   Yep. I think I left it as I wasn't really what will be the final version. I 
think "no" is better now. But that's something to be confirmed by @gerardo 


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


With regards,
Apache Git Services


[GitHub] [airflow] codecov-io commented on issue #4976: adding Beeswax as a company who uses airflow

2019-03-25 Thread GitBox
codecov-io commented on issue #4976: adding Beeswax as a company who uses 
airflow
URL: https://github.com/apache/airflow/pull/4976#issuecomment-476417971
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/4976?src=pr&el=h1) 
Report
   > Merging 
[#4976](https://codecov.io/gh/apache/airflow/pull/4976?src=pr&el=desc) into 
[master](https://codecov.io/gh/apache/airflow/commit/5d2c740e2d8e2ebaa7358e839fa03b981d358f88?src=pr&el=desc)
 will **increase** coverage by `<.01%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/airflow/pull/4976/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/4976?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff @@
   ##   master#4976  +/-   ##
   ==
   + Coverage   75.75%   75.75%   +<.01% 
   ==
 Files 458  458  
 Lines   2986129861  
   ==
   + Hits2262022621   +1 
   + Misses   7241 7240   -1
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/airflow/pull/4976?src=pr&el=tree) | 
Coverage Δ | |
   |---|---|---|
   | 
[airflow/models/\_\_init\_\_.py](https://codecov.io/gh/apache/airflow/pull/4976/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvX19pbml0X18ucHk=)
 | `93% <0%> (+0.04%)` | :arrow_up: |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/airflow/pull/4976?src=pr&el=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/4976?src=pr&el=footer). 
Last update 
[5d2c740...63e09d9](https://codecov.io/gh/apache/airflow/pull/4976?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


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


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #4937: [AIRFLOW-4116] Multi-staging includes CI image [Step 2/3]

2019-03-25 Thread GitBox
potiuk commented on a change in pull request #4937: [AIRFLOW-4116] 
Multi-staging includes CI image [Step 2/3]
URL: https://github.com/apache/airflow/pull/4937#discussion_r268895206
 
 

 ##
 File path: Dockerfile
 ##
 @@ -85,14 +85,134 @@ RUN adduser airflow \
 && echo "airflow ALL=(ALL) NOPASSWD: ALL" > /etc/sudoers.d/airflow \
 && chmod 0440 /etc/sudoers.d/airflow
 
+
+# This is an image with all APT dependencies needed by CI. It is built on top 
of the airlfow APT image
+# Parameters:
+# airflow-apt-deps - this is the base image for CI deps image.
+
+FROM airflow-apt-deps as airflow-ci-apt-deps
+
+SHELL ["/bin/bash", "-o", "pipefail", "-e", "-u", "-x", "-c"]
+
+ENV JAVA_HOME=/usr/lib/jvm/java-8-openjdk-amd64/
+
+ARG APT_DEPS_IMAGE
+ENV APT_DEPS_IMAGE=${APT_DEPS_IMAGE}
+
+RUN echo "${APT_DEPS_IMAGE}"
+
+# Note the ifs below might be removed if Buildkit will become usable. It 
should skip building this
+# image automatically if it is not used. For now we still go through all 
layers below but they are empty
+# Note missing directories on debian-stretch 
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=863199
+RUN if [[ "${APT_DEPS_IMAGE}" == "airflow-ci-apt-deps" ]]; then \
+mkdir -pv /usr/share/man/man1 \
+&& mkdir -pv /usr/share/man/man7 \
+&& apt-get update \
+&& apt-get install --no-install-recommends -y \
+  lsb-release gnupg dirmngr openjdk-8-jdk \
+  vim tmux less unzip net-tools netcat \
+  ldap-utils postgresql-client sqlite3 \
+  krb5-user openssh-client openssh-server \
+  python-selinux \
+&& apt-get autoremove -yqq --purge \
+&& apt-get clean \
+&& rm -rf /var/lib/apt/lists/* \
+;\
+fi
+
+RUN if [[ "${APT_DEPS_IMAGE}" == "airflow-ci-apt-deps" ]]; then \
+KEY="A4A9406876FCBD3C456770C88C718D3B5072E1F5" \
+&& GNUPGHOME="$(mktemp -d)" \
+&& export GNUPGHOME \
+&& for KEYSERVER in $(shuf -e \
+ha.pool.sks-keyservers.net \
+hkp://p80.pool.sks-keyservers.net:80 \
+keyserver.ubuntu.com \
+hkp://keyserver.ubuntu.com:80 \
+pgp.mit.edu) ; do \
+  gpg --keyserver "${KEYSERVER}" --recv-keys "${KEY}" && break || 
true ; \
+   done \
+&& gpg --export "${KEY}" > /etc/apt/trusted.gpg.d/mysql.gpg \
+&& gpgconf --kill all \
+rm -rf "${GNUPGHOME}"; \
+apt-key list > /dev/null \
+&& echo "deb http://repo.mysql.com/apt/ubuntu/ trusty mysql-5.7" | \
+tee -a /etc/apt/sources.list.d/mysql.list \
+&& apt-get update \
+&& MYSQL_PASS="secret" \
+&& debconf-set-selections <<< \
+"mysql-community-server mysql-community-server/data-dir select ''" 
\
+&& debconf-set-selections <<< \
+"mysql-community-server mysql-community-server/root-pass password 
${MYSQL_PASS}" \
+&& debconf-set-selections <<< \
+"mysql-community-server mysql-community-server/re-root-pass 
password ${MYSQL_PASS}" \
 
 Review comment:
   I am not 100% sure now, but I believe I struggled with similar problems when 
implementing CloudSQL operators (with cloudsqlproxy). I spent quite some time 
trying to find the best way to install mysql client on debian-stretch so it 
might be the result of my struggles and trial/error then and when I clean it 
up, it might be not needed. 
   
   I might find other, simpler solution - like stop using root user for mysql 
db at all or enforcing TCP protocol. 
   
   Sorry for being so verbose but I am venting my frustrations with mysql 
connectivity issues :). But for some people it might be even interesting :). 
   
   
   OK. Beginning of rant on MySQL.
   
   The problem is that the original scripts from ci_script use passwordless, 
remote root connection to create airflow database and I probably want to change 
that. This is the most probable reason why Travis tests fail now - some 
authentication option somewhere either on client or server prevent from running 
the query below. We currently got "Host mysql does not exist" but really this 
is a generic error when there is any problem with authentication. Yeah - 
security first - better pretend that you are not there than admit that password 
is wrong.
   
   Here is the offending query:
   
   mysql -h ${MYSQL_HOST} -u root -e 'drop database if exists airflow; 
create database airflow
   
   The main reason is that MySQL 5.7 changed the secure model: now MySQL root 
login requires a sudo. By default you cannot login to root user unless you have 
sudo. And that's where the journey starts.
   
   For mysql "root only with sudo" is checked and enforced in both client

[GitHub] [airflow] feng-tao commented on issue #4974: [AIRFLOW-4057] Fix bug in stat name validation

2019-03-25 Thread GitBox
feng-tao commented on issue #4974: [AIRFLOW-4057] Fix bug in stat name 
validation
URL: https://github.com/apache/airflow/pull/4974#issuecomment-476414687
 
 
   @astahlman , fyi, the py27 fails with the followings:
   
   ```==
   52) ERROR: test_increment_counter_with_valid_name 
(tests.test_stats.TestStats)
   --
  Traceback (most recent call last):
   tests/test_stats.py line 33 in test_increment_counter_with_valid_name
 self.stats.incr('test_stats_run')
   airflow/stats.py line 83 in wrapper
 stat_name = handle_stat_name_func(stat)
  TypeError: unbound method stat_name_dummy_handler() must be called with 
AirflowTestPlugin instance as first argument (got str instance instead)
   ==
   53) ERROR: test_stat_name_must_be_a_string (tests.test_stats.TestStats)
   --
  Traceback (most recent call last):
   tests/test_stats.py line 37 in test_stat_name_must_be_a_string
 self.stats.incr(list())
   airflow/stats.py line 83 in wrapper
 stat_name = handle_stat_name_func(stat)
  TypeError: unbound method stat_name_dummy_handler() must be called with 
AirflowTestPlugin instance as first argument (got list instance instead)
   ==
   54) ERROR: test_stat_name_must_not_exceed_max_length 
(tests.test_stats.TestStats)
   --
  Traceback (most recent call last):
   tests/test_stats.py line 41 in test_stat_name_must_not_exceed_max_length
 self.stats.incr('X' * 300)
   airflow/stats.py line 83 in wrapper
 stat_name = handle_stat_name_func(stat)
  TypeError: unbound method stat_name_dummy_handler() must be called with 
AirflowTestPlugin instance as first argument (got str instance instead)
   ==
   55) ERROR: test_stat_name_must_only_include_whitelisted_characters 
(tests.test_stats.TestStats)
   --
  Traceback (most recent call last):
   tests/test_stats.py line 45 in 
test_stat_name_must_only_include_whitelisted_characters
 self.stats.incr('test/$tats')
   airflow/stats.py line 83 in wrapper
 stat_name = handle_stat_name_func(stat)
  TypeError: unbound method stat_name_dummy_handler() must be called with 
AirflowTestPlugin instance as first argument (got str instance instead)``` 


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


With regards,
Apache Git Services


[GitHub] [airflow] mans2singh commented on a change in pull request #4967: [AIRFLOW-4147] Operator to publish event to Redis

2019-03-25 Thread GitBox
mans2singh commented on a change in pull request #4967: [AIRFLOW-4147] Operator 
to publish event to Redis
URL: https://github.com/apache/airflow/pull/4967#discussion_r268892210
 
 

 ##
 File path: tests/contrib/operators/test_redis_publish_operator.py
 ##
 @@ -0,0 +1,59 @@
+# -*- coding: utf-8 -*-
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+
+import unittest
+from airflow import DAG, configuration
+from airflow.contrib.operators.redis_publish_operator import 
RedisPublishOperator
+from airflow.utils import timezone
+from mock import MagicMock
+
+DEFAULT_DATE = timezone.datetime(2017, 1, 1)
+
+
+class TestRedisPublishOperator(unittest.TestCase):
+
+def setUp(self):
+configuration.load_test_config()
+
+args = {
+'owner': 'airflow',
+'start_date': DEFAULT_DATE
+}
+
+self.dag = DAG('test_redis_dag_id', default_args=args)
+
+self.mock_context = MagicMock()
+
+def test_execute_hello(self):
+operator = RedisPublishOperator(
+task_id='test_task',
+dag=self.dag,
+message='{{message}}',
+channel='test',
+redis_conn_id='redis_default'
+)
+
+operator.execute(self.mock_context)
+context_calls = []
+self.assertTrue(self.mock_context['ti'].method_calls == context_calls, 
"context calls should be same")
+
+
+if __name__ == '__main__':
+unittest.main()
 
 Review comment:
   Removed.


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


With regards,
Apache Git Services


[GitHub] [airflow] mans2singh commented on a change in pull request #4967: [AIRFLOW-4147] Operator to publish event to Redis

2019-03-25 Thread GitBox
mans2singh commented on a change in pull request #4967: [AIRFLOW-4147] Operator 
to publish event to Redis
URL: https://github.com/apache/airflow/pull/4967#discussion_r268892174
 
 

 ##
 File path: tests/contrib/operators/test_redis_publish_operator.py
 ##
 @@ -0,0 +1,59 @@
+# -*- coding: utf-8 -*-
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+
+import unittest
+from airflow import DAG, configuration
+from airflow.contrib.operators.redis_publish_operator import 
RedisPublishOperator
+from airflow.utils import timezone
+from mock import MagicMock
+
+DEFAULT_DATE = timezone.datetime(2017, 1, 1)
+
+
+class TestRedisPublishOperator(unittest.TestCase):
+
+def setUp(self):
+configuration.load_test_config()
+
+args = {
+'owner': 'airflow',
+'start_date': DEFAULT_DATE
+}
+
+self.dag = DAG('test_redis_dag_id', default_args=args)
+
+self.mock_context = MagicMock()
+
+def test_execute_hello(self):
 
 Review comment:
   Added validation.


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


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #4936: [AIRFLOW-4115] Multi-staging Aiflow Docker image [Step 1/3]

2019-03-25 Thread GitBox
potiuk commented on a change in pull request #4936: [AIRFLOW-4115] 
Multi-staging Aiflow Docker image [Step 1/3]
URL: https://github.com/apache/airflow/pull/4936#discussion_r268880220
 
 

 ##
 File path: .dockerignore
 ##
 @@ -0,0 +1,107 @@
+#
+# 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.
+
+# NOTE! This docker ignore uses recommended technique
+# Where everything is excluded by default and you deliberately
+# Add only those directories/files you need. This is very useful
+# To make sure that Docker context is always the same on any machine
+# So that generated files are not accidentally added to the context
+# This allows Docker's `COPY .` to behave in predictable way
+
+# Ignore everything
+**
 
 Review comment:
   Correct with the duplicates. I reviewed and removed it.
   
   There is little worry that we will forget something - the tests will catch 
it (and it should rarely be needed - adding a new folder or file at the top 
level should be really rare event)
   
   And yes - you need those extra specs because they will exclude what should 
not be part of those included dirs above. The pattern I follow is (and I saw it 
suggested by several people):
   
   1) ** -> Exclude everything
   2) !/airflow -> include only those top-level folders/top-level files that 
you need
   3) **/__pycache__/ -> from those folders that you included in 2) exclude all 
the generated content that follows common patterns for generated stuff (like 
*.pyc file or /__pycache__/ folders)
   
   I feel very strong about it - having to debug the context changes (and 
figuring out what the hell changed to trigger copying of the files again) for 
many hours. Example: only after a while I realised that there is a generated 
folder in docs which is _build next to _template (which is not generated, 
thought the name kind of suggest it). And just generating the docs locally 
triggered rebuild of Dockerfile (that's why I did not include the whole docs 
folder :) ). Otherwise I would have to specifically exclude that particular 
folder (if I knew that I have to do it). I really think adding stuff 
deliberately to Docker context makes much more sense. 
   
   Alternative is to move ALL generated content to specific directory, and 
exclude that directory - but this is not always possible (node_modules) and 
from my experience people will not "stick" to it - they will start generating 
random files in different folders. Of course we are not protected if someone 
starts generating "xyz" file/folder inside "airflow" folder. But it's rather 
unlikely someone will do something that crazy.  I think generating something in 
sources is rather bad practice and you do it only if you cannot do it otherwise 
(that's one of the parts I don't like about python - .pyc files are generated 
next to actual source files. It reminds me of times when I used RCS versioning 
system and we had source.c,v next to every file BRRR)
   
   But to be honest - It was much bigger problem and I eventually introduced 
checksums for important files (setup/docker) to check if we need to run docker 
build at all.
   
   But still I think it makes it much more resilient for future changes that 
might produce uncontrollable and unpredictable context "growth". 


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


With regards,
Apache Git Services


[GitHub] [airflow] kalyanaramansanthanam opened a new pull request #4976: adding Beeswax as a company who uses airflow

2019-03-25 Thread GitBox
kalyanaramansanthanam opened a new pull request #4976: adding Beeswax as a 
company who uses airflow
URL: https://github.com/apache/airflow/pull/4976
 
 
   


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


With regards,
Apache Git Services


[GitHub] [airflow] feng-tao commented on issue #4974: [AIRFLOW-4057] Fix bug in stat name validation

2019-03-25 Thread GitBox
feng-tao commented on issue #4974: [AIRFLOW-4057] Fix bug in stat name 
validation
URL: https://github.com/apache/airflow/pull/4974#issuecomment-476399767
 
 
   why does it fail for all the python27 CI?


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


With regards,
Apache Git Services


[GitHub] [airflow] feng-tao commented on a change in pull request #4973: [AIRFLOW-4155] Allow Public role access to /home

2019-03-25 Thread GitBox
feng-tao commented on a change in pull request #4973: [AIRFLOW-4155] Allow 
Public role access to /home
URL: https://github.com/apache/airflow/pull/4973#discussion_r268852064
 
 

 ##
 File path: tests/www/test_security.py
 ##
 @@ -308,3 +308,26 @@ def test_override_role_vm(self):
 test_security_manager = TestSecurityManager(appbuilder=self.appbuilder)
 self.assertEqual(len(test_security_manager.VIEWER_VMS), 1)
 self.assertEqual(test_security_manager.VIEWER_VMS, {'Airflow'})
+
+def test_is_user_logged_in_returns_false_if_not_authenticated(self):
+user = mock.MagicMock()
+user.is_authenticated = False
+self.assertFalse(self.security_manager.is_user_logged_in(user))
+
+def test_is_user_logged_in_returns_true_if_authenticated(self):
+user = mock.MagicMock()
+user.is_authenticated = True
+self.assertTrue(self.security_manager.is_user_logged_in(user))
+
+def test_unauthenticated_user_is_public(self):
+user = mock.MagicMock()
+user.is_authenticated = False
+self.assertTrue(self.security_manager.is_public_user(user))
 
 Review comment:
   @ashb , I don't think we want the viewer as the default role as the viewer 
could view all the dags info unless we change this assumption. I think having 
public role as default while enabling the role to access the default home page 
seems to be better.  The use case we have is that we only want certain dags 
with the source code to be viewed by a certain group user as those are high 
sensitive dags. 
   
   Later if users want to access certain dags, the admin could create the 
respective dag role for those users.  


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


With regards,
Apache Git Services


[GitHub] [airflow] ashb commented on a change in pull request #4973: [AIRFLOW-4155] Allow Public role access to /home

2019-03-25 Thread GitBox
ashb commented on a change in pull request #4973: [AIRFLOW-4155] Allow Public 
role access to /home
URL: https://github.com/apache/airflow/pull/4973#discussion_r268874265
 
 

 ##
 File path: tests/www/test_security.py
 ##
 @@ -308,3 +308,26 @@ def test_override_role_vm(self):
 test_security_manager = TestSecurityManager(appbuilder=self.appbuilder)
 self.assertEqual(len(test_security_manager.VIEWER_VMS), 1)
 self.assertEqual(test_security_manager.VIEWER_VMS, {'Airflow'})
+
+def test_is_user_logged_in_returns_false_if_not_authenticated(self):
+user = mock.MagicMock()
+user.is_authenticated = False
+self.assertFalse(self.security_manager.is_user_logged_in(user))
+
+def test_is_user_logged_in_returns_true_if_authenticated(self):
+user = mock.MagicMock()
+user.is_authenticated = True
+self.assertTrue(self.security_manager.is_user_logged_in(user))
+
+def test_unauthenticated_user_is_public(self):
+user = mock.MagicMock()
+user.is_authenticated = False
+self.assertTrue(self.security_manager.is_public_user(user))
 
 Review comment:
   Right, I guess that makes sense too.
   
   I just feel a little bit uneasy about making the home page/`@has_access` 
accessible to Public role (i.e. anon users), (even if not l no dags are shown) 
in a default install, and would rather it was at least a config option of some 
sort.


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


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #4936: [AIRFLOW-4115] Multi-staging Aiflow Docker image [Step 1/3]

2019-03-25 Thread GitBox
potiuk commented on a change in pull request #4936: [AIRFLOW-4115] 
Multi-staging Aiflow Docker image [Step 1/3]
URL: https://github.com/apache/airflow/pull/4936#discussion_r268874142
 
 

 ##
 File path: Dockerfile
 ##
 @@ -13,46 +13,179 @@
 # 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.
+#
+# WARNING: THIS DOCKERFILE IS NOT INTENDED FOR PRODUCTION USE OR DEPLOYMENT.
+#
+# Arguments of the build
+ARG PYTHON_BASE_IMAGE="python:3.6-slim"
+ARG AIRFLOW_VERSION="2.0.0.dev0"
+# Which image is used as dependency for the main image
+ARG APT_DEPS_IMAGE="airflow-apt-deps"
+# Speeds up building the image - cassandra driver without CYTHON saves around 
10 minutes
+ARG CASS_DRIVER_NO_CYTHON="1"
+# Build cassandra driver on multiple CPUs
+ARG CASS_DRIVER_BUILD_CONCURRENCY="8"
+# By default PIP install is run without cache to make image smaller
+ARG PIP_NO_CACHE_DIR="true"
+# Additional python deps to install
+ARG ADDITIONAL_PYTHON_DEPS=""
+# PIP version used to install dependencies
+ARG PIP_VERSION="19.0.1"
+# By increasing this number we can do force build of all dependencies
+ARG DEPENDENCIES_EPOCH_NUMBER="1"
+
+# This is base image with APT dependencies needed by Airflow. It is based on a 
python slim image
+# Parameters:
+#PYTHON_BASE_IMAGE - base python image (python:x.y-slim)
+
+FROM ${PYTHON_BASE_IMAGE} as airflow-apt-deps
+
+SHELL ["/bin/bash", "-o", "pipefail", "-e", "-u", "-x", "-c"]
+
+ARG PYTHON_BASE_IMAGE
+ARG AIRFLOW_VERSION
+ENV PYTHON_BASE_IMAGE=${PYTHON_BASE_IMAGE}
+ENV AIRFLOW_VERSION=$AIRFLOW_VERSION
+
+# Print versions
+RUN echo "Base image: ${PYTHON_BASE_IMAGE}"
+RUN echo "Airflow version: ${AIRFLOW_VERSION}"
+
+# Make sure noninteractie debian install is used and language variab1les set
+ENV DEBIAN_FRONTEND=noninteractive LANGUAGE=C.UTF-8 LANG=C.UTF-8 
LC_ALL=C.UTF-8 \
+LC_CTYPE=C.UTF-8 LC_MESSAGES=C.UTF-8
+
+# Increase the value below to force renstalling of all dependencies
+ENV DEPENDENCIES_EPOCH_NUMBER=${DEPENDENCIES_EPOCH_NUMBER}
+
+# Install curl and gnupg2 - needed to download nodejs in next step
+RUN apt-get update \
+&& apt-get install -y --no-install-recommends \
+   curl gnupg2 \
+&& apt-get autoremove -yqq --purge \
+&& apt-get clean \
+&& rm -rf /var/lib/apt/lists/*
+
+
+# Install basic apt dependencies
+RUN curl -sL https://deb.nodesource.com/setup_10.x | bash - \
+&& apt-get update \
+&& apt-get install -y --no-install-recommends \
+   # Packages to install \
+   libsasl2-dev freetds-bin build-essential sasl2-bin \
+   libsasl2-2 libsasl2-dev libsasl2-modules \
+   default-libmysqlclient-dev apt-utils curl rsync netcat locales  \
+   freetds-dev libkrb5-dev libssl-dev libffi-dev libpq-dev git \
+   nodejs gosu sudo \
+&& apt-get autoremove -yqq --purge \
+&& apt-get clean \
+&& rm -rf /var/lib/apt/lists/*
+
+RUN adduser airflow \
+&& echo "airflow ALL=(ALL) NOPASSWD: ALL" > /etc/sudoers.d/airflow \
+&& chmod 0440 /etc/sudoers.d/airflow
+
+
+# This is the target image - it installs PIP and NPN dependencies including 
efficient caching
+# mechanisms - it might be used to build the bare airflow build or CI build
+# Parameters:
+#APT_DEPS_IMAGE - image with APT dependencies. It might either be base 
deps image with airflow
+# dependencies or CI deps image that contains also 
CI-required dependencies
+
+FROM airflow-apt-deps as main
+
+SHELL ["/bin/bash", "-o", "pipefail", "-e", "-u", "-x", "-c"]
+
+WORKDIR /opt/airflow
+
+RUN echo "Airflow version: ${AIRFLOW_VERSION}"
+
+ARG AIRFLOW_HOME=/opt/airflow
+ENV AIRFLOW_HOME=${AIRFLOW_HOME}
 
-FROM python:3.6-slim
-SHELL ["/bin/bash", "-xc"]
+RUN mkdir -pv ${AIRFLOW_HOME} \
+&& chown -R airflow.airflow ${AIRFLOW_HOME}
 
-ENV AIRFLOW_HOME=/usr/local/airflow
-ARG AIRFLOW_DEPS="all"
-ARG PYTHON_DEPS=""
-ARG BUILD_DEPS="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"
+# Increase the value here to force reinstalling Apache Airflow pip dependencies
+ENV FORCE_REINSTALL_ALL_PIP_DEPENDENCIES=1
 
-ENV PATH="$HOME/.npm-packages/bin:$PATH"
+# Optimizing installation of Cassandra driver
+ARG CASS_DRIVER_BUILD_CONCURRENCY
+ARG CASS_DRIVER_NO_CYTHON
+ENV CASS_DRIVER_BUILD_CONCURRENCY=${CASS_DRIVER_BUILD_CONCURRENCY}
+ENV C

[jira] [Commented] (AIRFLOW-4156) KubernetesPodOperator does not support set security context

2019-03-25 Thread Ash Berlin-Taylor (JIRA)


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

Ash Berlin-Taylor commented on AIRFLOW-4156:


Some overlap with AIRFLOW-3274 (though it sounds like this is more general)

> KubernetesPodOperator does not support set security context
> ---
>
> Key: AIRFLOW-4156
> URL: https://issues.apache.org/jira/browse/AIRFLOW-4156
> Project: Apache Airflow
>  Issue Type: Improvement
>  Components: kubernetes
>Affects Versions: 1.10.2
> Environment: kubernetes
>Reporter: Magnus Runesson
>Priority: Major
>
> Good praxis running containers is to not run them as root nor run with a 
> writable root-filesystem. To be able to restrict this on pods launched by the 
> KubernetesPodOperator one must be able to set [security 
> context|https://kubernetes.io/docs/tasks/configure-pod-container/security-context/].
>  Many hardened Kubernetes clusters require this to be set.
> WIP patch, currently missing tests:
> https://github.com/mrunesson/airflow/tree/feat-k8s-security-context



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] [airflow] potiuk commented on a change in pull request #4936: [AIRFLOW-4115] Multi-staging Aiflow Docker image [Step 1/3]

2019-03-25 Thread GitBox
potiuk commented on a change in pull request #4936: [AIRFLOW-4115] 
Multi-staging Aiflow Docker image [Step 1/3]
URL: https://github.com/apache/airflow/pull/4936#discussion_r268871848
 
 

 ##
 File path: Dockerfile
 ##
 @@ -13,46 +13,179 @@
 # 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.
+#
+# WARNING: THIS DOCKERFILE IS NOT INTENDED FOR PRODUCTION USE OR DEPLOYMENT.
+#
+# Arguments of the build
+ARG PYTHON_BASE_IMAGE="python:3.6-slim"
+ARG AIRFLOW_VERSION="2.0.0.dev0"
+# Which image is used as dependency for the main image
+ARG APT_DEPS_IMAGE="airflow-apt-deps"
+# Speeds up building the image - cassandra driver without CYTHON saves around 
10 minutes
+ARG CASS_DRIVER_NO_CYTHON="1"
+# Build cassandra driver on multiple CPUs
+ARG CASS_DRIVER_BUILD_CONCURRENCY="8"
+# By default PIP install is run without cache to make image smaller
+ARG PIP_NO_CACHE_DIR="true"
+# Additional python deps to install
+ARG ADDITIONAL_PYTHON_DEPS=""
+# PIP version used to install dependencies
+ARG PIP_VERSION="19.0.1"
+# By increasing this number we can do force build of all dependencies
+ARG DEPENDENCIES_EPOCH_NUMBER="1"
+
+# This is base image with APT dependencies needed by Airflow. It is based on a 
python slim image
+# Parameters:
+#PYTHON_BASE_IMAGE - base python image (python:x.y-slim)
+
+FROM ${PYTHON_BASE_IMAGE} as airflow-apt-deps
+
+SHELL ["/bin/bash", "-o", "pipefail", "-e", "-u", "-x", "-c"]
+
+ARG PYTHON_BASE_IMAGE
+ARG AIRFLOW_VERSION
+ENV PYTHON_BASE_IMAGE=${PYTHON_BASE_IMAGE}
+ENV AIRFLOW_VERSION=$AIRFLOW_VERSION
+
+# Print versions
+RUN echo "Base image: ${PYTHON_BASE_IMAGE}"
+RUN echo "Airflow version: ${AIRFLOW_VERSION}"
+
+# Make sure noninteractie debian install is used and language variab1les set
+ENV DEBIAN_FRONTEND=noninteractive LANGUAGE=C.UTF-8 LANG=C.UTF-8 
LC_ALL=C.UTF-8 \
+LC_CTYPE=C.UTF-8 LC_MESSAGES=C.UTF-8
+
+# Increase the value below to force renstalling of all dependencies
+ENV DEPENDENCIES_EPOCH_NUMBER=${DEPENDENCIES_EPOCH_NUMBER}
+
+# Install curl and gnupg2 - needed to download nodejs in next step
+RUN apt-get update \
+&& apt-get install -y --no-install-recommends \
+   curl gnupg2 \
+&& apt-get autoremove -yqq --purge \
+&& apt-get clean \
+&& rm -rf /var/lib/apt/lists/*
+
+
+# Install basic apt dependencies
+RUN curl -sL https://deb.nodesource.com/setup_10.x | bash - \
+&& apt-get update \
+&& apt-get install -y --no-install-recommends \
+   # Packages to install \
+   libsasl2-dev freetds-bin build-essential sasl2-bin \
+   libsasl2-2 libsasl2-dev libsasl2-modules \
+   default-libmysqlclient-dev apt-utils curl rsync netcat locales  \
+   freetds-dev libkrb5-dev libssl-dev libffi-dev libpq-dev git \
+   nodejs gosu sudo \
+&& apt-get autoremove -yqq --purge \
+&& apt-get clean \
+&& rm -rf /var/lib/apt/lists/*
+
+RUN adduser airflow \
+&& echo "airflow ALL=(ALL) NOPASSWD: ALL" > /etc/sudoers.d/airflow \
+&& chmod 0440 /etc/sudoers.d/airflow
+
+
+# This is the target image - it installs PIP and NPN dependencies including 
efficient caching
+# mechanisms - it might be used to build the bare airflow build or CI build
+# Parameters:
+#APT_DEPS_IMAGE - image with APT dependencies. It might either be base 
deps image with airflow
+# dependencies or CI deps image that contains also 
CI-required dependencies
+
+FROM airflow-apt-deps as main
+
+SHELL ["/bin/bash", "-o", "pipefail", "-e", "-u", "-x", "-c"]
+
+WORKDIR /opt/airflow
+
+RUN echo "Airflow version: ${AIRFLOW_VERSION}"
+
+ARG AIRFLOW_HOME=/opt/airflow
+ENV AIRFLOW_HOME=${AIRFLOW_HOME}
 
-FROM python:3.6-slim
-SHELL ["/bin/bash", "-xc"]
+RUN mkdir -pv ${AIRFLOW_HOME} \
+&& chown -R airflow.airflow ${AIRFLOW_HOME}
 
-ENV AIRFLOW_HOME=/usr/local/airflow
-ARG AIRFLOW_DEPS="all"
-ARG PYTHON_DEPS=""
-ARG BUILD_DEPS="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"
+# Increase the value here to force reinstalling Apache Airflow pip dependencies
+ENV FORCE_REINSTALL_ALL_PIP_DEPENDENCIES=1
 
 Review comment:
   Right. My bad :). Will add.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub an

[GitHub] [airflow] ashb commented on a change in pull request #4936: [AIRFLOW-4115] Multi-staging Aiflow Docker image [Step 1/3]

2019-03-25 Thread GitBox
ashb commented on a change in pull request #4936: [AIRFLOW-4115] Multi-staging 
Aiflow Docker image [Step 1/3]
URL: https://github.com/apache/airflow/pull/4936#discussion_r268870799
 
 

 ##
 File path: setup.py
 ##
 @@ -252,7 +253,7 @@ def write_version(filename=os.path.join(*['airflow',
 'qds-sdk>=1.9.6',
 'rednose',
 'requests_mock',
-'flake8>=3.6.0',
+'virtualenv',
 
 Review comment:
   Oh python3  https://docs.python.org/3/library/venv.html is built  in


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


With regards,
Apache Git Services


[GitHub] [airflow] mik-laj commented on issue #4894: [AIRFLOW-3996] Add view source link to included fragments

2019-03-25 Thread GitBox
mik-laj commented on issue #4894: [AIRFLOW-3996] Add view source link to 
included fragments
URL: https://github.com/apache/airflow/pull/4894#issuecomment-476393030
 
 
   @Fokko I updated a PR: 
http://steadfast-machine.surge.sh/howto/operator/index.html


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


With regards,
Apache Git Services


[GitHub] [airflow] mik-laj removed a comment on issue #4894: [AIRFLOW-3996] Add view source link to included fragments

2019-03-25 Thread GitBox
mik-laj removed a comment on issue #4894: [AIRFLOW-3996] Add view source link 
to included fragments
URL: https://github.com/apache/airflow/pull/4894#issuecomment-475994293
 
 
   I updated a PR: http://steadfast-machine.surge.sh/howto/operator/index.html


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


With regards,
Apache Git Services


[GitHub] [airflow] mik-laj commented on a change in pull request #4823: [AIRFLOW-3999] Remove all inline style

2019-03-25 Thread GitBox
mik-laj commented on a change in pull request #4823: [AIRFLOW-3999] Remove all 
inline style
URL: https://github.com/apache/airflow/pull/4823#discussion_r268868500
 
 

 ##
 File path: airflow/www/static/css/_page_dags.scss
 ##
 @@ -0,0 +1,46 @@
+/**
+* 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.
+*/
+
+.edit-dag {
 
 Review comment:
   I updated a PR.


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


With regards,
Apache Git Services


[GitHub] [airflow] mik-laj commented on a change in pull request #4823: [AIRFLOW-3999] Remove all inline style

2019-03-25 Thread GitBox
mik-laj commented on a change in pull request #4823: [AIRFLOW-3999] Remove all 
inline style
URL: https://github.com/apache/airflow/pull/4823#discussion_r268127817
 
 

 ##
 File path: airflow/www/static/css/_page_dags.scss
 ##
 @@ -0,0 +1,46 @@
+/**
+* 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.
+*/
+
+.edit-dag {
 
 Review comment:
   Thanks for discussion. I will update a PR with `.page_dags .edit_dag ` 
solution.


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


With regards,
Apache Git Services


[GitHub] [airflow] mans2singh commented on issue #4967: [AIRFLOW-4147] Operator to publish event to Redis

2019-03-25 Thread GitBox
mans2singh commented on issue #4967: [AIRFLOW-4147] Operator to publish event 
to Redis
URL: https://github.com/apache/airflow/pull/4967#issuecomment-476391674
 
 
   @XD-DENG - I've updated the test based on your feedback.  Please let me know 
if you have any additional advice.  Thanks


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


With regards,
Apache Git Services


[jira] [Updated] (AIRFLOW-3659) Create Google Cloud Transfer Service Operators

2019-03-25 Thread Jarek Potiuk (JIRA)


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

Jarek Potiuk updated AIRFLOW-3659:
--
Fix Version/s: 2.0.0

> Create Google Cloud Transfer Service Operators
> --
>
> Key: AIRFLOW-3659
> URL: https://issues.apache.org/jira/browse/AIRFLOW-3659
> Project: Apache Airflow
>  Issue Type: New Feature
>Reporter: Jarek Potiuk
>Assignee: Kamil Bregula
>Priority: Major
> Fix For: 2.0.0, 1.10.3
>
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Updated] (AIRFLOW-3659) Create Google Cloud Transfer Service Operators

2019-03-25 Thread Jarek Potiuk (JIRA)


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

Jarek Potiuk updated AIRFLOW-3659:
--
Fix Version/s: (was: 2.0.0)
   1.10.3

> Create Google Cloud Transfer Service Operators
> --
>
> Key: AIRFLOW-3659
> URL: https://issues.apache.org/jira/browse/AIRFLOW-3659
> Project: Apache Airflow
>  Issue Type: New Feature
>Reporter: Jarek Potiuk
>Assignee: Kamil Bregula
>Priority: Major
> Fix For: 1.10.3
>
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] [airflow] potiuk commented on a change in pull request #4936: [AIRFLOW-4115] Multi-staging Aiflow Docker image [Step 1/3]

2019-03-25 Thread GitBox
potiuk commented on a change in pull request #4936: [AIRFLOW-4115] 
Multi-staging Aiflow Docker image [Step 1/3]
URL: https://github.com/apache/airflow/pull/4936#discussion_r268862273
 
 

 ##
 File path: setup.py
 ##
 @@ -252,7 +253,7 @@ def write_version(filename=os.path.join(*['airflow',
 'qds-sdk>=1.9.6',
 'rednose',
 'requests_mock',
-'flake8>=3.6.0',
+'virtualenv',
 
 Review comment:
   But on the other hand - what if someone actually wants to use the 
PythonVirtualenvOperator ? Should not they actually have to be given a chance 
to do pip install -e.[something,something,virtualenv] ? Maybe in this case 
virtualenv deserves its own extra?


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


With regards,
Apache Git Services


[GitHub] [airflow] potiuk commented on a change in pull request #4936: [AIRFLOW-4115] Multi-staging Aiflow Docker image [Step 1/3]

2019-03-25 Thread GitBox
potiuk commented on a change in pull request #4936: [AIRFLOW-4115] 
Multi-staging Aiflow Docker image [Step 1/3]
URL: https://github.com/apache/airflow/pull/4936#discussion_r268861624
 
 

 ##
 File path: setup.py
 ##
 @@ -252,7 +253,7 @@ def write_version(filename=os.path.join(*['airflow',
 'qds-sdk>=1.9.6',
 'rednose',
 'requests_mock',
-'flake8>=3.6.0',
+'virtualenv',
 
 Review comment:
   👍 


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


With regards,
Apache Git Services


[GitHub] [airflow] feng-tao commented on a change in pull request #4973: [AIRFLOW-4155] Allow Public role access to /home

2019-03-25 Thread GitBox
feng-tao commented on a change in pull request #4973: [AIRFLOW-4155] Allow 
Public role access to /home
URL: https://github.com/apache/airflow/pull/4973#discussion_r268853058
 
 

 ##
 File path: airflow/www/security.py
 ##
 @@ -155,6 +154,11 @@ class AirflowSecurityManager(SecurityManager, 
LoggingMixin):
 ###
 
 ROLE_CONFIGS = [
+{
+'role': 'Public',
+'perms': {'can_index'},
 
 Review comment:
   another way to do is to move the public role here and keep the perms / vms 
empty, but override the perms / vm in custom security manager.
   
   cc @astahlman @ashb WDYT?


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


With regards,
Apache Git Services


[GitHub] [airflow] feng-tao commented on a change in pull request #4973: [AIRFLOW-4155] Allow Public role access to /home

2019-03-25 Thread GitBox
feng-tao commented on a change in pull request #4973: [AIRFLOW-4155] Allow 
Public role access to /home
URL: https://github.com/apache/airflow/pull/4973#discussion_r268852064
 
 

 ##
 File path: tests/www/test_security.py
 ##
 @@ -308,3 +308,26 @@ def test_override_role_vm(self):
 test_security_manager = TestSecurityManager(appbuilder=self.appbuilder)
 self.assertEqual(len(test_security_manager.VIEWER_VMS), 1)
 self.assertEqual(test_security_manager.VIEWER_VMS, {'Airflow'})
+
+def test_is_user_logged_in_returns_false_if_not_authenticated(self):
+user = mock.MagicMock()
+user.is_authenticated = False
+self.assertFalse(self.security_manager.is_user_logged_in(user))
+
+def test_is_user_logged_in_returns_true_if_authenticated(self):
+user = mock.MagicMock()
+user.is_authenticated = True
+self.assertTrue(self.security_manager.is_user_logged_in(user))
+
+def test_unauthenticated_user_is_public(self):
+user = mock.MagicMock()
+user.is_authenticated = False
+self.assertTrue(self.security_manager.is_public_user(user))
 
 Review comment:
   @ashb , I don't think we want the viewer as the default role as the viewer 
could view all the dags info unless we change this assumption. I think having 
public role as default while enabling the role to access the default home page 
seems to be better.  The use case we have is that we only want certain dags 
with the source code to be viewer by a certain group user as those are high 
sensitive dags. 
   
   Later if users want to access certain dags, the admin could create the 
respective dag role for those users.  


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


With regards,
Apache Git Services


[GitHub] [airflow] feng-tao commented on a change in pull request #4973: [AIRFLOW-4155] Allow Public role access to /home

2019-03-25 Thread GitBox
feng-tao commented on a change in pull request #4973: [AIRFLOW-4155] Allow 
Public role access to /home
URL: https://github.com/apache/airflow/pull/4973#discussion_r268852064
 
 

 ##
 File path: tests/www/test_security.py
 ##
 @@ -308,3 +308,26 @@ def test_override_role_vm(self):
 test_security_manager = TestSecurityManager(appbuilder=self.appbuilder)
 self.assertEqual(len(test_security_manager.VIEWER_VMS), 1)
 self.assertEqual(test_security_manager.VIEWER_VMS, {'Airflow'})
+
+def test_is_user_logged_in_returns_false_if_not_authenticated(self):
+user = mock.MagicMock()
+user.is_authenticated = False
+self.assertFalse(self.security_manager.is_user_logged_in(user))
+
+def test_is_user_logged_in_returns_true_if_authenticated(self):
+user = mock.MagicMock()
+user.is_authenticated = True
+self.assertTrue(self.security_manager.is_user_logged_in(user))
+
+def test_unauthenticated_user_is_public(self):
+user = mock.MagicMock()
+user.is_authenticated = False
+self.assertTrue(self.security_manager.is_public_user(user))
 
 Review comment:
   @ashb , I don't think we want the viewer as the default role as the viewer 
could view all the dags info unless we change this assumption. I think having 
public role as default while enabling the role to access the default home page 
seems to be better.  The use case we have is that we only want certain dags 
with the source code to be viewer by a certain group user as those are high 
sensitive dags. 
   
   Later if users want to access certain dags, they could create the respective 
dag role for those users.  


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


With regards,
Apache Git Services


[jira] [Created] (AIRFLOW-4156) KubernetesPodOperator does not support set security context

2019-03-25 Thread Magnus Runesson (JIRA)
Magnus Runesson created AIRFLOW-4156:


 Summary: KubernetesPodOperator does not support set security 
context
 Key: AIRFLOW-4156
 URL: https://issues.apache.org/jira/browse/AIRFLOW-4156
 Project: Apache Airflow
  Issue Type: Improvement
  Components: kubernetes
Affects Versions: 1.10.2
 Environment: kubernetes
Reporter: Magnus Runesson


Good praxis running containers is to not run them as root nor run with a 
writable root-filesystem. To be able to restrict this on pods launched by the 
KubernetesPodOperator one must be able to set [security 
context|https://kubernetes.io/docs/tasks/configure-pod-container/security-context/].
 Many hardened Kubernetes clusters require this to be set.

WIP patch, currently missing tests:
https://github.com/mrunesson/airflow/tree/feat-k8s-security-context



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (AIRFLOW-4149) gRPC connection should be added to forms

2019-03-25 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on AIRFLOW-4149:
-

morgendave commented on pull request #4975: AIRFLOW-4149 add extra gRPC fields 
to connections forms
URL: https://github.com/apache/airflow/pull/4975
 
 
   Make sure you have checked _all_ steps below.
   
   ### Jira
   
   - [ ] My PR addresses the following [Airflow 
Jira](https://issues.apache.org/jira/browse/AIRFLOW/) issues and references 
them in the PR title.
 - https://issues.apache.org/jira/browse/AIRFLOW-4149
 
   ### Description
   
   - Added the extra fields for gRPC in the separated forms class for connection
   
   ### Tests
   
   - [ ] My PR adds the following unit tests __OR__ does not need testing for 
this extremely good reason:
   
   ### Commits
   
   - [ ] My commits all reference Jira issues in their subject lines, and I 
have squashed multiple commits if they address the same issue. In addition, my 
commits follow the guidelines from "[How to write a good git commit 
message](http://chris.beams.io/posts/git-commit/)":
 1. Subject is separated from body by a blank line
 1. Subject is limited to 50 characters (not including Jira issue reference)
 1. Subject does not end with a period
 1. Subject uses the imperative mood ("add", not "adding")
 1. Body wraps at 72 characters
 1. Body explains "what" and "why", not "how"
   
   ### Documentation
   
   - [ ] In case of new functionality, my PR adds documentation that describes 
how to use it.
 - 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
 - If you implement backwards incompatible changes, please leave a note in 
the [Updating.md](https://github.com/apache/airflow/blob/master/UPDATING.md) so 
we can assign it to a appropriate release
   
   ### Code Quality
   
   - [ ] Passes `flake8`
   
 

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


> gRPC connection should be added to forms
> 
>
> Key: AIRFLOW-4149
> URL: https://issues.apache.org/jira/browse/AIRFLOW-4149
> Project: Apache Airflow
>  Issue Type: Bug
>Reporter: Zhiwei Zhao
>Assignee: Zhiwei Zhao
>Priority: Major
>
> [https://github.com/apache/airflow/pull/4101]
>  
> The connection was separated so the new gRPC extra fields were not in the 
> forms class. Needs to be fixed



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] [airflow] morgendave opened a new pull request #4975: AIRFLOW-4149 add extra gRPC fields to connections forms

2019-03-25 Thread GitBox
morgendave opened a new pull request #4975: AIRFLOW-4149 add extra gRPC fields 
to connections forms
URL: https://github.com/apache/airflow/pull/4975
 
 
   Make sure you have checked _all_ steps below.
   
   ### Jira
   
   - [ ] My PR addresses the following [Airflow 
Jira](https://issues.apache.org/jira/browse/AIRFLOW/) issues and references 
them in the PR title.
 - https://issues.apache.org/jira/browse/AIRFLOW-4149
 
   ### Description
   
   - Added the extra fields for gRPC in the separated forms class for connection
   
   ### Tests
   
   - [ ] My PR adds the following unit tests __OR__ does not need testing for 
this extremely good reason:
   
   ### Commits
   
   - [ ] My commits all reference Jira issues in their subject lines, and I 
have squashed multiple commits if they address the same issue. In addition, my 
commits follow the guidelines from "[How to write a good git commit 
message](http://chris.beams.io/posts/git-commit/)":
 1. Subject is separated from body by a blank line
 1. Subject is limited to 50 characters (not including Jira issue reference)
 1. Subject does not end with a period
 1. Subject uses the imperative mood ("add", not "adding")
 1. Body wraps at 72 characters
 1. Body explains "what" and "why", not "how"
   
   ### Documentation
   
   - [ ] In case of new functionality, my PR adds documentation that describes 
how to use it.
 - 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
 - If you implement backwards incompatible changes, please leave a note in 
the [Updating.md](https://github.com/apache/airflow/blob/master/UPDATING.md) so 
we can assign it to a appropriate release
   
   ### Code Quality
   
   - [ ] Passes `flake8`
   


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


With regards,
Apache Git Services


[jira] [Commented] (AIRFLOW-3650) Fix flaky test in TestTriggerDag

2019-03-25 Thread ASF subversion and git services (JIRA)


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

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

Commit 641eba0c695629f4a27af90ef0436f9c15a715f2 in airflow's branch 
refs/heads/v1-10-stable from Tao Feng
[ https://gitbox.apache.org/repos/asf?p=airflow.git;h=641eba0 ]

[AIRFLOW-3650] Skip running on mysql for the flaky test (#4457)



> Fix flaky test in TestTriggerDag
> 
>
> Key: AIRFLOW-3650
> URL: https://issues.apache.org/jira/browse/AIRFLOW-3650
> Project: Apache Airflow
>  Issue Type: Bug
>Reporter: Tao Feng
>Assignee: Tao Feng
>Priority: Major
> Fix For: 1.10.3
>
>
> The test_trigger_dag_button test fails for mysql ORM almost everytime



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] [airflow] nritholtz commented on issue #4903: [AIRFLOW-4069] Add Opsgenie Alert Hook and Operator

2019-03-25 Thread GitBox
nritholtz commented on issue #4903: [AIRFLOW-4069] Add Opsgenie Alert Hook and 
Operator
URL: https://github.com/apache/airflow/pull/4903#issuecomment-476340038
 
 
   @ashb @XD-DENG I've made all recommended changes (hopefully). PTAL.


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


With regards,
Apache Git Services


svn commit: r33189 - in /dev/airflow/1.10.3b1: ./ apache-airflow-1.10.3b1-bin.tar.gz apache-airflow-1.10.3b1-source.tar.gz apache_airflow-1.10.3b1-py2.py3-none-any.whl

2019-03-25 Thread ash
Author: ash
Date: Mon Mar 25 19:10:56 2019
New Revision: 33189

Log:
Non-signed 1.10.3b1 snapshot artefacts

Added:
dev/airflow/1.10.3b1/
dev/airflow/1.10.3b1/apache-airflow-1.10.3b1-bin.tar.gz   (with props)
dev/airflow/1.10.3b1/apache-airflow-1.10.3b1-source.tar.gz   (with props)
dev/airflow/1.10.3b1/apache_airflow-1.10.3b1-py2.py3-none-any.whl   (with 
props)

Added: dev/airflow/1.10.3b1/apache-airflow-1.10.3b1-bin.tar.gz
==
Binary file - no diff available.

Propchange: dev/airflow/1.10.3b1/apache-airflow-1.10.3b1-bin.tar.gz
--
svn:mime-type = application/octet-stream

Added: dev/airflow/1.10.3b1/apache-airflow-1.10.3b1-source.tar.gz
==
Binary file - no diff available.

Propchange: dev/airflow/1.10.3b1/apache-airflow-1.10.3b1-source.tar.gz
--
svn:mime-type = application/octet-stream

Added: dev/airflow/1.10.3b1/apache_airflow-1.10.3b1-py2.py3-none-any.whl
==
Binary file - no diff available.

Propchange: dev/airflow/1.10.3b1/apache_airflow-1.10.3b1-py2.py3-none-any.whl
--
svn:mime-type = application/octet-stream




[GitHub] [airflow] ashb commented on a change in pull request #4973: [AIRFLOW-4155] Allow Public role access to /home

2019-03-25 Thread GitBox
ashb commented on a change in pull request #4973: [AIRFLOW-4155] Allow Public 
role access to /home
URL: https://github.com/apache/airflow/pull/4973#discussion_r268804775
 
 

 ##
 File path: tests/www/test_security.py
 ##
 @@ -308,3 +308,26 @@ def test_override_role_vm(self):
 test_security_manager = TestSecurityManager(appbuilder=self.appbuilder)
 self.assertEqual(len(test_security_manager.VIEWER_VMS), 1)
 self.assertEqual(test_security_manager.VIEWER_VMS, {'Airflow'})
+
+def test_is_user_logged_in_returns_false_if_not_authenticated(self):
+user = mock.MagicMock()
+user.is_authenticated = False
+self.assertFalse(self.security_manager.is_user_logged_in(user))
+
+def test_is_user_logged_in_returns_true_if_authenticated(self):
+user = mock.MagicMock()
+user.is_authenticated = True
+self.assertTrue(self.security_manager.is_user_logged_in(user))
+
+def test_unauthenticated_user_is_public(self):
+user = mock.MagicMock()
+user.is_authenticated = False
+self.assertTrue(self.security_manager.is_public_user(user))
 
 Review comment:
   I was suggesting that  instead of allowing Public role access to anything in 
 Airflow that we change the signup role to be Viewer, say.
   
   ```
   AUTH_USER_REGISTRATION_ROLE = 'Public'
   ```


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


With regards,
Apache Git Services


[jira] [Updated] (AIRFLOW-4057) airflow should handle invalid stats name

2019-03-25 Thread Ash Berlin-Taylor (JIRA)


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

Ash Berlin-Taylor updated AIRFLOW-4057:
---
Fix Version/s: 2.0.0

> airflow should handle invalid stats name
> 
>
> Key: AIRFLOW-4057
> URL: https://issues.apache.org/jira/browse/AIRFLOW-4057
> Project: Apache Airflow
>  Issue Type: New Feature
>Reporter: Chao-Han Tsai
>Assignee: Chao-Han Tsai
>Priority: Major
> Fix For: 2.0.0
>
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] [airflow] milton0825 commented on issue #4974: [AIRFLOW-4057] Fix bug in stat name validation

2019-03-25 Thread GitBox
milton0825 commented on issue #4974: [AIRFLOW-4057] Fix bug in stat name 
validation
URL: https://github.com/apache/airflow/pull/4974#issuecomment-476318214
 
 
   👍 thanks for the fix


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


With regards,
Apache Git Services


[GitHub] [airflow] astahlman commented on issue #4974: [AIRFLOW-4057] Fix bug in stat name validation

2019-03-25 Thread GitBox
astahlman commented on issue #4974: [AIRFLOW-4057] Fix bug in stat name 
validation
URL: https://github.com/apache/airflow/pull/4974#issuecomment-476317602
 
 
   cc @feng-tao 


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


With regards,
Apache Git Services


[GitHub] [airflow] astahlman commented on a change in pull request #4973: [AIRFLOW-4155] Allow Public role access to /home

2019-03-25 Thread GitBox
astahlman commented on a change in pull request #4973: [AIRFLOW-4155] Allow 
Public role access to /home
URL: https://github.com/apache/airflow/pull/4973#discussion_r268784234
 
 

 ##
 File path: tests/www/test_security.py
 ##
 @@ -308,3 +308,26 @@ def test_override_role_vm(self):
 test_security_manager = TestSecurityManager(appbuilder=self.appbuilder)
 self.assertEqual(len(test_security_manager.VIEWER_VMS), 1)
 self.assertEqual(test_security_manager.VIEWER_VMS, {'Airflow'})
+
+def test_is_user_logged_in_returns_false_if_not_authenticated(self):
+user = mock.MagicMock()
+user.is_authenticated = False
+self.assertFalse(self.security_manager.is_user_logged_in(user))
+
+def test_is_user_logged_in_returns_true_if_authenticated(self):
+user = mock.MagicMock()
+user.is_authenticated = True
+self.assertTrue(self.security_manager.is_user_logged_in(user))
+
+def test_unauthenticated_user_is_public(self):
+user = mock.MagicMock()
+user.is_authenticated = False
+self.assertTrue(self.security_manager.is_public_user(user))
 
 Review comment:
   Are you suggesting creating some role to which users are added by default on 
sign-up which has permissions only to the (empty) Index page? If so, I think 
that's exactly what this PR proposes to do with `Public`. If we did create that 
separate role, I'm not sure what the point of `Public` would be.


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


With regards,
Apache Git Services


[GitHub] [airflow] astahlman commented on a change in pull request #4973: [AIRFLOW-4155] Allow Public role access to /home

2019-03-25 Thread GitBox
astahlman commented on a change in pull request #4973: [AIRFLOW-4155] Allow 
Public role access to /home
URL: https://github.com/apache/airflow/pull/4973#discussion_r268782862
 
 

 ##
 File path: airflow/www/templates/airflow/dags.html
 ##
 @@ -28,6 +28,17 @@
 {% block content %}
   DAGs
 
+{% if not is_user_logged_in %}
+
+  Note: Some DAGs may not be visible until you log in.
+
+{% elif is_public_user %}
+
+  Note: Some DAGs may not be visible because you do not have sufficient 
privileges.
 
 Review comment:
   Sure, I can change that


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


With regards,
Apache Git Services


[GitHub] [airflow] astahlman commented on a change in pull request #4973: [AIRFLOW-4155] Allow Public role access to /home

2019-03-25 Thread GitBox
astahlman commented on a change in pull request #4973: [AIRFLOW-4155] Allow 
Public role access to /home
URL: https://github.com/apache/airflow/pull/4973#discussion_r268782621
 
 

 ##
 File path: airflow/www/templates/airflow/dags.html
 ##
 @@ -28,6 +28,17 @@
 {% block content %}
   DAGs
 
+{% if not is_user_logged_in %}
+
+  Note: Some DAGs may not be visible until you log in.
 
 Review comment:
   They will be able to, actually.
   
   FAB's BaseSecurityManager's implementation of `has_access` allows 
unauthenticated users to access any views that are accessible by the public 
role (see [1], which calls [2])
   
   [1] 
https://github.com/dpgaspar/Flask-AppBuilder/blob/master/flask_appbuilder/security/manager.py#L894
   [2] 
https://github.com/dpgaspar/Flask-AppBuilder/blob/master/flask_appbuilder/security/manager.py#L868


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


With regards,
Apache Git Services


[GitHub] [airflow] astahlman commented on issue #4974: [AIRFLOW-4057] Fix bug in stat name validation

2019-03-25 Thread GitBox
astahlman commented on issue #4974: [AIRFLOW-4057] Fix bug in stat name 
validation
URL: https://github.com/apache/airflow/pull/4974#issuecomment-476313372
 
 
   cc @milton0825 


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


With regards,
Apache Git Services


[jira] [Commented] (AIRFLOW-4057) airflow should handle invalid stats name

2019-03-25 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on AIRFLOW-4057:
-

astahlman commented on pull request #4974: [AIRFLOW-4057] Fix bug in stat name 
validation
URL: https://github.com/apache/airflow/pull/4974
 
 
   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-4057
 - In case you are fixing a typo in the documentation you can prepend your 
commit with \[AIRFLOW-XXX\], code changes always need a Jira issue.
 - In case you are proposing a fundamental code change, you need to create 
an Airflow Improvement Proposal 
([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)).
 - In case you are adding a dependency, check if the license complies with 
the [ASF 3rd Party License 
Policy](https://www.apache.org/legal/resolved.html#category-x).
   
   ### Description
   
   - [ X ] Here are some details about my PR, including screenshots of any UI 
changes:
   
   The `@validate_stats` wrapper is wrapping an instance method, so it's first
   argument is actually an instance of `SafeStatsdLogger`, not the stat_name.
   Here's the backtrace that shows up in the webserver logs without the fix:
   
 File 
"/Users/andrewstahlman/src/incubator-airflow/airflow/www/views.py", line 86, in 

dagbag = models.DagBag(os.devnull, include_examples=False)
 File 
"/Users/andrewstahlman/src/incubator-airflow/airflow/models/__init__.py", line 
312, in __init__
safe_mode=safe_mode)
 File 
"/Users/andrewstahlman/src/incubator-airflow/airflow/models/__init__.py", line 
594, in collect_dags
'collect_dags', (timezone.utcnow() - start_dttm).total_seconds(), 1)
 File "/Users/andrewstahlman/src/incubator-airflow/airflow/stats.py", 
line 85, in wrapper
log.warning('Invalid stat name: {stat}.'.format(stat=stat), err)
   Message: 'Invalid stat name: .'
   Arguments: (InvalidStatsNameException('The stat_name has to be a 
string'),)
   
   
   ### Tests
   
   - [ X ] My PR adds the following unit tests __OR__ does not need testing for 
this extremely good reason:
   
   I've verified the fix by updating the unit tests to exercise the "public"
   methods rather than testing the internal validation logic directly.
   
   ### 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
   
   - [ ] 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
 - If you implement backwards incompatible changes, please leave a note in 
the [Updating.md](https://github.com/apache/airflow/blob/master/UPDATING.md) so 
we can assign it to a appropriate release
   
   ### Code Quality
   
   - [ X ] Passes `flake8`
   
 

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


> airflow should handle invalid stats name
> 
>
> Key: AIRFLOW-4057
> URL: https://issues.apache.org/jira/browse/AIRFLOW-4057
> Project: Apache Airflow
>  Issue Type: New Feature
>Reporter: Chao-Han Tsai
>Assignee: Chao-Han Tsai
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] [airflow] astahlman opened a new pull request #4974: [AIRFLOW-4057] Fix bug in stat name validation

2019-03-25 Thread GitBox
astahlman opened a new pull request #4974: [AIRFLOW-4057] Fix bug in stat name 
validation
URL: https://github.com/apache/airflow/pull/4974
 
 
   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-4057
 - In case you are fixing a typo in the documentation you can prepend your 
commit with \[AIRFLOW-XXX\], code changes always need a Jira issue.
 - In case you are proposing a fundamental code change, you need to create 
an Airflow Improvement Proposal 
([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)).
 - In case you are adding a dependency, check if the license complies with 
the [ASF 3rd Party License 
Policy](https://www.apache.org/legal/resolved.html#category-x).
   
   ### Description
   
   - [ X ] Here are some details about my PR, including screenshots of any UI 
changes:
   
   The `@validate_stats` wrapper is wrapping an instance method, so it's first
   argument is actually an instance of `SafeStatsdLogger`, not the stat_name.
   Here's the backtrace that shows up in the webserver logs without the fix:
   
 File 
"/Users/andrewstahlman/src/incubator-airflow/airflow/www/views.py", line 86, in 

dagbag = models.DagBag(os.devnull, include_examples=False)
 File 
"/Users/andrewstahlman/src/incubator-airflow/airflow/models/__init__.py", line 
312, in __init__
safe_mode=safe_mode)
 File 
"/Users/andrewstahlman/src/incubator-airflow/airflow/models/__init__.py", line 
594, in collect_dags
'collect_dags', (timezone.utcnow() - start_dttm).total_seconds(), 1)
 File "/Users/andrewstahlman/src/incubator-airflow/airflow/stats.py", 
line 85, in wrapper
log.warning('Invalid stat name: {stat}.'.format(stat=stat), err)
   Message: 'Invalid stat name: .'
   Arguments: (InvalidStatsNameException('The stat_name has to be a 
string'),)
   
   
   ### Tests
   
   - [ X ] My PR adds the following unit tests __OR__ does not need testing for 
this extremely good reason:
   
   I've verified the fix by updating the unit tests to exercise the "public"
   methods rather than testing the internal validation logic directly.
   
   ### 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
   
   - [ ] 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
 - If you implement backwards incompatible changes, please leave a note in 
the [Updating.md](https://github.com/apache/airflow/blob/master/UPDATING.md) so 
we can assign it to a appropriate release
   
   ### Code Quality
   
   - [ X ] Passes `flake8`
   


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


With regards,
Apache Git Services


[jira] [Reopened] (AIRFLOW-3650) Fix flaky test in TestTriggerDag

2019-03-25 Thread Ash Berlin-Taylor (JIRA)


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

Ash Berlin-Taylor reopened AIRFLOW-3650:


> Fix flaky test in TestTriggerDag
> 
>
> Key: AIRFLOW-3650
> URL: https://issues.apache.org/jira/browse/AIRFLOW-3650
> Project: Apache Airflow
>  Issue Type: Bug
>Reporter: Tao Feng
>Assignee: Tao Feng
>Priority: Major
> Fix For: 2.0.0
>
>
> The test_trigger_dag_button test fails for mysql ORM almost everytime



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Resolved] (AIRFLOW-3650) Fix flaky test in TestTriggerDag

2019-03-25 Thread Ash Berlin-Taylor (JIRA)


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

Ash Berlin-Taylor resolved AIRFLOW-3650.

   Resolution: Fixed
Fix Version/s: (was: 2.0.0)
   1.10.3

> Fix flaky test in TestTriggerDag
> 
>
> Key: AIRFLOW-3650
> URL: https://issues.apache.org/jira/browse/AIRFLOW-3650
> Project: Apache Airflow
>  Issue Type: Bug
>Reporter: Tao Feng
>Assignee: Tao Feng
>Priority: Major
> Fix For: 1.10.3
>
>
> The test_trigger_dag_button test fails for mysql ORM almost everytime



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (AIRFLOW-3650) Fix flaky test in TestTriggerDag

2019-03-25 Thread ASF subversion and git services (JIRA)


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

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

Commit 641eba0c695629f4a27af90ef0436f9c15a715f2 in airflow's branch 
refs/heads/v1-10-test from Tao Feng
[ https://gitbox.apache.org/repos/asf?p=airflow.git;h=641eba0 ]

[AIRFLOW-3650] Skip running on mysql for the flaky test (#4457)



> Fix flaky test in TestTriggerDag
> 
>
> Key: AIRFLOW-3650
> URL: https://issues.apache.org/jira/browse/AIRFLOW-3650
> Project: Apache Airflow
>  Issue Type: Bug
>Reporter: Tao Feng
>Assignee: Tao Feng
>Priority: Major
> Fix For: 2.0.0
>
>
> The test_trigger_dag_button test fails for mysql ORM almost everytime



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] [airflow] ashb commented on a change in pull request #4973: [AIRFLOW-4155] Allow Public role access to /home

2019-03-25 Thread GitBox
ashb commented on a change in pull request #4973: [AIRFLOW-4155] Allow Public 
role access to /home
URL: https://github.com/apache/airflow/pull/4973#discussion_r268761260
 
 

 ##
 File path: airflow/www/templates/airflow/dags.html
 ##
 @@ -28,6 +28,17 @@
 {% block content %}
   DAGs
 
+{% if not is_user_logged_in %}
+
+  Note: Some DAGs may not be visible until you log in.
+
+{% elif is_public_user %}
+
+  Note: Some DAGs may not be visible because you do not have sufficient 
privileges.
 
 Review comment:
   This warning could equally apply to every other role except Admin.
   
   How about only showing this if `len(dags) == 0`


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


With regards,
Apache Git Services


[GitHub] [airflow] ashb commented on a change in pull request #4973: [AIRFLOW-4155] Allow Public role access to /home

2019-03-25 Thread GitBox
ashb commented on a change in pull request #4973: [AIRFLOW-4155] Allow Public 
role access to /home
URL: https://github.com/apache/airflow/pull/4973#discussion_r268761916
 
 

 ##
 File path: tests/www/test_security.py
 ##
 @@ -308,3 +308,26 @@ def test_override_role_vm(self):
 test_security_manager = TestSecurityManager(appbuilder=self.appbuilder)
 self.assertEqual(len(test_security_manager.VIEWER_VMS), 1)
 self.assertEqual(test_security_manager.VIEWER_VMS, {'Airflow'})
+
+def test_is_user_logged_in_returns_false_if_not_authenticated(self):
+user = mock.MagicMock()
+user.is_authenticated = False
+self.assertFalse(self.security_manager.is_user_logged_in(user))
+
+def test_is_user_logged_in_returns_true_if_authenticated(self):
+user = mock.MagicMock()
+user.is_authenticated = True
+self.assertTrue(self.security_manager.is_user_logged_in(user))
+
+def test_unauthenticated_user_is_public(self):
+user = mock.MagicMock()
+user.is_authenticated = False
+self.assertTrue(self.security_manager.is_public_user(user))
 
 Review comment:
   I'm not sure I like this behaviour.
   
   I think a better fix would be to change the Sign-up role. Anonymous users 
shouldn't pass any `@has_access` check to my mind.
   
   Or this should be a config setting somewhere.


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


With regards,
Apache Git Services


[GitHub] [airflow] ashb commented on a change in pull request #4973: [AIRFLOW-4155] Allow Public role access to /home

2019-03-25 Thread GitBox
ashb commented on a change in pull request #4973: [AIRFLOW-4155] Allow Public 
role access to /home
URL: https://github.com/apache/airflow/pull/4973#discussion_r268761158
 
 

 ##
 File path: airflow/www/templates/airflow/dags.html
 ##
 @@ -28,6 +28,17 @@
 {% block content %}
   DAGs
 
+{% if not is_user_logged_in %}
+
+  Note: Some DAGs may not be visible until you log in.
 
 Review comment:
   When is this going to get shown? Anon users can't get here can they?


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


With regards,
Apache Git Services


[jira] [Commented] (AIRFLOW-4155) Logging in with Public role triggers infinite redirect loop

2019-03-25 Thread Ash Berlin-Taylor (JIRA)


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

Ash Berlin-Taylor commented on AIRFLOW-4155:


Sounds like doc/config issue - auto-registering probably shouldn't put people 
in to the Public role - Viewer sounds more appropriate?

> Logging in with Public role triggers infinite redirect loop
> ---
>
> Key: AIRFLOW-4155
> URL: https://issues.apache.org/jira/browse/AIRFLOW-4155
> Project: Apache Airflow
>  Issue Type: Improvement
>Reporter: Andrew Stahlman
>Assignee: Andrew Stahlman
>Priority: Major
>
> After a user registers in the PUBLIC_AUTH_ROLE they are redirected to /home. 
> But a user in the Public role still doesn't have view privileges on /home, so 
> they are immediately redirected *back* to the /login page, which creates an 
> endless redirect cycle.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (AIRFLOW-4155) Logging in with Public role triggers infinite redirect loop

2019-03-25 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on AIRFLOW-4155:
-

astahlman commented on pull request #4973: [AIRFLOW-4155] Allow Public role 
access to /home
URL: https://github.com/apache/airflow/pull/4973
 
 
   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-4155
 - In case you are fixing a typo in the documentation you can prepend your 
commit with \[AIRFLOW-XXX\], code changes always need a Jira issue.
 - In case you are proposing a fundamental code change, you need to create 
an Airflow Improvement Proposal 
([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)).
 - In case you are adding a dependency, check if the license complies with 
the [ASF 3rd Party License 
Policy](https://www.apache.org/legal/resolved.html#category-x).
   
   ### Description
   
   - [ X ] Here are some details about my PR, including screenshots of any UI 
changes:
   
   Problem: After a user registers in the PUBLIC_AUTH_ROLE they are
   redirected to /home. But a user in the Public role still doesn't have
   view privileges on /home, so they are immediately
   redirected *back* to the /login page, which creates an endless redirect
   cycle.
   
   This PR change grants the Public role view access to the index page.
   Because the Public user doesn't have any other read permission, they
   won't see any DAGs.
   
   If the user is not logged in, we add a banner at the top of the page
   with a link to the login page. If the user is logged in but belongs only
   to the Public role, then the banner displays a message warning the user
   that they don't have sufficient privileges to view the DAGs.
   
    Before logging in 
   ![Screen Shot 2019-03-25 at 9 38 05 
AM](https://user-images.githubusercontent.com/1173394/54939277-675aec00-4ee5-11e9-8dda-0526e5fe0d18.png)
   
    After logging in as user in Public role
   ![Screen Shot 2019-03-25 at 9 38 30 
AM](https://user-images.githubusercontent.com/1173394/54939275-64f89200-4ee5-11e9-9e6c-347814b33ac6.png)
   
   
   ### Tests
   
   - [ X ] My PR adds the following unit tests __OR__ does not need testing for 
this extremely good reason:
   
   ### Commits
   
   - [ X ] My commits all reference Jira issues in their subject lines, and I 
have squashed multiple commits if they address the same issue. In addition, my 
commits follow the guidelines from "[How to write a good git commit 
message](http://chris.beams.io/posts/git-commit/)":
 1. Subject is separated from body by a blank line
 1. Subject is limited to 50 characters (not including Jira issue reference)
 1. Subject does not end with a period
 1. Subject uses the imperative mood ("add", not "adding")
 1. Body wraps at 72 characters
 1. Body explains "what" and "why", not "how"
   
   ### Documentation
   
   - [ ] 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
 - If you implement backwards incompatible changes, please leave a note in 
the [Updating.md](https://github.com/apache/airflow/blob/master/UPDATING.md) so 
we can assign it to a appropriate release
   
   ### Code Quality
   
   - [ X ] Passes `flake8`
   
 

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


> Logging in with Public role triggers infinite redirect loop
> ---
>
> Key: AIRFLOW-4155
> URL: https://issues.apache.org/jira/browse/AIRFLOW-4155
> Project: Apache Airflow
>  Issue Type: Improvement
>Reporter: Andrew Stahlman
>Assignee: Andrew Stahlman
>Priority: Major
>
> After a user registers in the PUBLIC_AUTH_ROLE they are redirected to /home. 
> But a user in the Public role still doesn't have view privileges on /home, so 
> they are immediately redirected *back* to the /login page, which creates an 
> endless redirect cycle.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] [airflow] astahlman opened a new pull request #4973: [AIRFLOW-4155] Allow Public role access to /home

2019-03-25 Thread GitBox
astahlman opened a new pull request #4973: [AIRFLOW-4155] Allow Public role 
access to /home
URL: https://github.com/apache/airflow/pull/4973
 
 
   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-4155
 - In case you are fixing a typo in the documentation you can prepend your 
commit with \[AIRFLOW-XXX\], code changes always need a Jira issue.
 - In case you are proposing a fundamental code change, you need to create 
an Airflow Improvement Proposal 
([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)).
 - In case you are adding a dependency, check if the license complies with 
the [ASF 3rd Party License 
Policy](https://www.apache.org/legal/resolved.html#category-x).
   
   ### Description
   
   - [ X ] Here are some details about my PR, including screenshots of any UI 
changes:
   
   Problem: After a user registers in the PUBLIC_AUTH_ROLE they are
   redirected to /home. But a user in the Public role still doesn't have
   view privileges on /home, so they are immediately
   redirected *back* to the /login page, which creates an endless redirect
   cycle.
   
   This PR change grants the Public role view access to the index page.
   Because the Public user doesn't have any other read permission, they
   won't see any DAGs.
   
   If the user is not logged in, we add a banner at the top of the page
   with a link to the login page. If the user is logged in but belongs only
   to the Public role, then the banner displays a message warning the user
   that they don't have sufficient privileges to view the DAGs.
   
    Before logging in 
   ![Screen Shot 2019-03-25 at 9 38 05 
AM](https://user-images.githubusercontent.com/1173394/54939277-675aec00-4ee5-11e9-8dda-0526e5fe0d18.png)
   
    After logging in as user in Public role
   ![Screen Shot 2019-03-25 at 9 38 30 
AM](https://user-images.githubusercontent.com/1173394/54939275-64f89200-4ee5-11e9-9e6c-347814b33ac6.png)
   
   
   ### Tests
   
   - [ X ] My PR adds the following unit tests __OR__ does not need testing for 
this extremely good reason:
   
   ### Commits
   
   - [ X ] My commits all reference Jira issues in their subject lines, and I 
have squashed multiple commits if they address the same issue. In addition, my 
commits follow the guidelines from "[How to write a good git commit 
message](http://chris.beams.io/posts/git-commit/)":
 1. Subject is separated from body by a blank line
 1. Subject is limited to 50 characters (not including Jira issue reference)
 1. Subject does not end with a period
 1. Subject uses the imperative mood ("add", not "adding")
 1. Body wraps at 72 characters
 1. Body explains "what" and "why", not "how"
   
   ### Documentation
   
   - [ ] 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
 - If you implement backwards incompatible changes, please leave a note in 
the [Updating.md](https://github.com/apache/airflow/blob/master/UPDATING.md) so 
we can assign it to a appropriate release
   
   ### Code Quality
   
   - [ X ] Passes `flake8`
   


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


With regards,
Apache Git Services


[GitHub] [airflow] ashb commented on a change in pull request #4772: [AIRFLOW-3937] KubernetesPodOperator support for envFrom configMapRef…

2019-03-25 Thread GitBox
ashb commented on a change in pull request #4772: [AIRFLOW-3937] 
KubernetesPodOperator support for envFrom configMapRef…
URL: https://github.com/apache/airflow/pull/4772#discussion_r268751356
 
 

 ##
 File path: 
airflow/contrib/kubernetes/kubernetes_request_factory/kubernetes_request_factory.py
 ##
 @@ -131,15 +131,44 @@ def extract_volume_secrets(pod, req):
 
 @staticmethod
 def extract_env_and_secrets(pod, req):
-env_secrets = [s for s in pod.secrets if s.deploy_type == 'env']
-if len(pod.envs) > 0 or len(env_secrets) > 0:
+envs_from_key_secrets = [
+env for env in pod.secrets if env.deploy_type == 'env' and 
hasattr(env, 'key')
+]
+
+envs_from_secrets = [
+env for env in pod.secrets if env.deploy_type == 'env' and not 
hasattr(env, 'key')
+]
+
+if pod.envs_from_configmaps or envs_from_secrets:
+req['spec']['containers'][0]['envFrom'] = []
+
+if len(pod.envs) > 0 or len(envs_from_key_secrets) > 0:
 env = []
 for k in pod.envs.keys():
 env.append({'name': k, 'value': pod.envs[k]})
-for secret in env_secrets:
+for secret in envs_from_key_secrets:
 KubernetesRequestFactory.add_secret_to_env(env, secret)
+
 req['spec']['containers'][0]['env'] = env
 
+for secret in envs_from_secrets:
 
 Review comment:
   To be clear - this  means making `extract_env_and_secrets` call extra helper 
functions, so no new changes outside of  kube_request_factory.py


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


With regards,
Apache Git Services


[jira] [Created] (AIRFLOW-4155) Logging in with Public role triggers infinite redirect loop

2019-03-25 Thread Andrew Stahlman (JIRA)
Andrew Stahlman created AIRFLOW-4155:


 Summary: Logging in with Public role triggers infinite redirect 
loop
 Key: AIRFLOW-4155
 URL: https://issues.apache.org/jira/browse/AIRFLOW-4155
 Project: Apache Airflow
  Issue Type: Improvement
Reporter: Andrew Stahlman
Assignee: Andrew Stahlman


After a user registers in the PUBLIC_AUTH_ROLE they are redirected to /home. 
But a user in the Public role still doesn't have view privileges on /home, so 
they are immediately redirected *back* to the /login page, which creates an 
endless redirect cycle.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] [airflow] dimberman commented on a change in pull request #4772: [AIRFLOW-3937] KubernetesPodOperator support for envFrom configMapRef…

2019-03-25 Thread GitBox
dimberman commented on a change in pull request #4772: [AIRFLOW-3937] 
KubernetesPodOperator support for envFrom configMapRef…
URL: https://github.com/apache/airflow/pull/4772#discussion_r268747692
 
 

 ##
 File path: 
airflow/contrib/kubernetes/kubernetes_request_factory/kubernetes_request_factory.py
 ##
 @@ -131,15 +131,44 @@ def extract_volume_secrets(pod, req):
 
 @staticmethod
 def extract_env_and_secrets(pod, req):
-env_secrets = [s for s in pod.secrets if s.deploy_type == 'env']
-if len(pod.envs) > 0 or len(env_secrets) > 0:
+envs_from_key_secrets = [
+env for env in pod.secrets if env.deploy_type == 'env' and 
hasattr(env, 'key')
+]
+
+envs_from_secrets = [
+env for env in pod.secrets if env.deploy_type == 'env' and not 
hasattr(env, 'key')
+]
+
+if pod.envs_from_configmaps or envs_from_secrets:
+req['spec']['containers'][0]['envFrom'] = []
+
+if len(pod.envs) > 0 or len(envs_from_key_secrets) > 0:
 env = []
 for k in pod.envs.keys():
 env.append({'name': k, 'value': pod.envs[k]})
-for secret in env_secrets:
+for secret in envs_from_key_secrets:
 KubernetesRequestFactory.add_secret_to_env(env, secret)
+
 req['spec']['containers'][0]['env'] = env
 
+for secret in envs_from_secrets:
 
 Review comment:
   @galuszkak int that case please seperate some of this logic into private 
helper functions. After that should be good to merge :)


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


With regards,
Apache Git Services


[GitHub] [airflow] dimberman commented on a change in pull request #4772: [AIRFLOW-3937] KubernetesPodOperator support for envFrom configMapRef…

2019-03-25 Thread GitBox
dimberman commented on a change in pull request #4772: [AIRFLOW-3937] 
KubernetesPodOperator support for envFrom configMapRef…
URL: https://github.com/apache/airflow/pull/4772#discussion_r268747033
 
 

 ##
 File path: airflow/contrib/kubernetes/secret.py
 ##
 @@ -14,28 +14,41 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
+from airflow.exceptions import AirflowConfigException
 
 
 class Secret:
 """Defines Kubernetes Secret Volume"""
 
-def __init__(self, deploy_type, deploy_target, secret, key):
+def __init__(self, deploy_type, deploy_target, secret, key=None):
 """Initialize a Kubernetes Secret Object. Used to track requested 
secrets from
 the user.
 :param deploy_type: The type of secret deploy in Kubernetes, either 
`env` or
 `volume`
 :type deploy_type: str
 :param deploy_target: The environment variable when `deploy_type` 
`env` or
-file path when `deploy_type` `volume` where expose secret
+file path when `deploy_type` `volume` where expose secret.
+If `key` is not provided deploy target should be None.
 :type deploy_target: str
 :param secret: Name of the secrets object in Kubernetes
 :type secret: str
-:param key: Key of the secret within the Kubernetes Secret
+:param key: (Optional) Key of the secret within the Kubernetes Secret
 
 Review comment:
   SGTM.


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


With regards,
Apache Git Services


[jira] [Updated] (AIRFLOW-4072) enable GKEPodOperator xcom

2019-03-25 Thread Ben Marengo (JIRA)


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

Ben Marengo updated AIRFLOW-4072:
-
Issue Type: Bug  (was: Improvement)

> enable GKEPodOperator xcom
> --
>
> Key: AIRFLOW-4072
> URL: https://issues.apache.org/jira/browse/AIRFLOW-4072
> Project: Apache Airflow
>  Issue Type: Bug
>Reporter: Ben Marengo
>Assignee: Ben Marengo
>Priority: Trivial
>
> this provides consistent functionality with {{KubernetesPodOperator}}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


svn commit: r33185 - in /dev/airflow: 1.10.2beta1/ 1.10.2beta2/ 1.10.2rc1/ 1.10.2rc2/

2019-03-25 Thread ash
Author: ash
Date: Mon Mar 25 16:30:56 2019
New Revision: 33185

Log:
Remove old Airflow dev/RC releases

Removed:
dev/airflow/1.10.2beta1/
dev/airflow/1.10.2beta2/
dev/airflow/1.10.2rc1/
dev/airflow/1.10.2rc2/



[jira] [Commented] (AIRFLOW-4072) enable GKEPodOperator xcom

2019-03-25 Thread jack (JIRA)


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

jack commented on AIRFLOW-4072:
---

Duplicated ticket of https://issues.apache.org/jira/browse/AIRFLOW-3113

> enable GKEPodOperator xcom
> --
>
> Key: AIRFLOW-4072
> URL: https://issues.apache.org/jira/browse/AIRFLOW-4072
> Project: Apache Airflow
>  Issue Type: Improvement
>Reporter: Ben Marengo
>Assignee: Ben Marengo
>Priority: Trivial
>
> this provides consistent functionality with {{KubernetesPodOperator}}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] [airflow] OmerJog commented on issue #4855: [AIRFLOW-2421] - HTTPHook set verify True by default

2019-03-25 Thread GitBox
OmerJog commented on issue #4855: [AIRFLOW-2421] - HTTPHook set verify True by 
default 
URL: https://github.com/apache/airflow/pull/4855#issuecomment-476262309
 
 
   @Fokko  Done


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


With regards,
Apache Git Services


[GitHub] [airflow] zhongjiajie commented on issue #4966: [AIRFLOW-4062] Improve docs on install extra package commands

2019-03-25 Thread GitBox
zhongjiajie commented on issue #4966:  [AIRFLOW-4062] Improve docs on install 
extra package commands
URL: https://github.com/apache/airflow/pull/4966#issuecomment-476259643
 
 
   I got it, thanks @ashb 


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


With regards,
Apache Git Services


[jira] [Commented] (AIRFLOW-1582) Improve logging structure of Airflow

2019-03-25 Thread ASF subversion and git services (JIRA)


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

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

Commit c078bea3e85bbfe407a3b0ba920c523a943a93d9 in airflow's branch 
refs/heads/v1-10-test from Ash Berlin-Taylor
[ https://gitbox.apache.org/repos/asf?p=airflow.git;h=c078bea ]

[AIRFLOW-XXX] Remove old/non-test files that nose ignores (#4930)

This file contains tests for a module that was deleted in
[AIRFLOW-1582] but the code was deleted in a7a518902dc (in 2017)

This file _also_ matches the name of a python module:
tests.test_utils.db etc, so having a file with the same name as the
package is bad/ambigious at best.


> Improve logging structure of Airflow
> 
>
> Key: AIRFLOW-1582
> URL: https://issues.apache.org/jira/browse/AIRFLOW-1582
> Project: Apache Airflow
>  Issue Type: Bug
>Reporter: Fokko Driesprong
>Assignee: Fokko Driesprong
>Priority: Major
> Fix For: 1.9.0
>
>
> Hi,
> I would like to improve the logging within Airflow. Currently the logging is 
> missing some consistency across the project. I would like to:
> - Remove airflow/utils/logging.py and move everything to /airflow/utils/log/*
> - Initialise local loggers with the name of the class
> - Move the settings of the logging to one central place
> - Remove setting explicit logging levels within the code
> Future PR's
> - Remove verbose boolean settings, which make no sense; if you want more 
> verbose logging you should set this by increasing the logging verbosity, and 
> this should not be set by a boolean variable.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] [airflow] feluelle commented on issue #4972: [AIRFLOW-4154] Correct string formatting in jobs.py

2019-03-25 Thread GitBox
feluelle commented on issue #4972: [AIRFLOW-4154] Correct string formatting in 
jobs.py
URL: https://github.com/apache/airflow/pull/4972#issuecomment-476250764
 
 
   LGTM


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


With regards,
Apache Git Services


[jira] [Updated] (AIRFLOW-4154) Correct string formatting in jobs.py

2019-03-25 Thread Xiaodong DENG (JIRA)


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

Xiaodong DENG updated AIRFLOW-4154:
---
Description: 
There are improper string formatting, like

 
 * _*["{}".format(x) for x in tis_to_set_to_scheduled])*_
 * using _*.format*_ in log calls

  was:
There are improper string formatting, like

 

- ["{}".format(x) for x in tis_to_set_to_scheduled])
 - using .format in log calls


> Correct string formatting in jobs.py
> 
>
> Key: AIRFLOW-4154
> URL: https://issues.apache.org/jira/browse/AIRFLOW-4154
> Project: Apache Airflow
>  Issue Type: Task
>Affects Versions: 1.10.2
>Reporter: Xiaodong DENG
>Assignee: Xiaodong DENG
>Priority: Minor
>
> There are improper string formatting, like
>  
>  * _*["{}".format(x) for x in tis_to_set_to_scheduled])*_
>  * using _*.format*_ in log calls



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Updated] (AIRFLOW-4154) Correct string formatting in jobs.py

2019-03-25 Thread Xiaodong DENG (JIRA)


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

Xiaodong DENG updated AIRFLOW-4154:
---
Description: 
There are improper string formatting, like

 

- ["{}".format(x) for x in tis_to_set_to_scheduled])
 - using .format in log calls

  was:
There are improper string formatting, like

 

-  ["{}".format(x) for x in tis_to_set_to_scheduled])

- using .format in log calls


> Correct string formatting in jobs.py
> 
>
> Key: AIRFLOW-4154
> URL: https://issues.apache.org/jira/browse/AIRFLOW-4154
> Project: Apache Airflow
>  Issue Type: Task
>Affects Versions: 1.10.2
>Reporter: Xiaodong DENG
>Assignee: Xiaodong DENG
>Priority: Minor
>
> There are improper string formatting, like
>  
> - ["{}".format(x) for x in tis_to_set_to_scheduled])
>  - using .format in log calls



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Updated] (AIRFLOW-4154) Correct string formatting in jobs.py

2019-03-25 Thread Xiaodong DENG (JIRA)


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

Xiaodong DENG updated AIRFLOW-4154:
---
Description: 
There are improper string formatting, like

 

-  ["{}".format(x) for x in tis_to_set_to_scheduled])

- using .format in log calls

  was:
There are improper string formatting, like

-  ["{}".format(x) for x in tis_to_set_to_scheduled])

- using .format in log calls


> Correct string formatting in jobs.py
> 
>
> Key: AIRFLOW-4154
> URL: https://issues.apache.org/jira/browse/AIRFLOW-4154
> Project: Apache Airflow
>  Issue Type: Task
>Affects Versions: 1.10.2
>Reporter: Xiaodong DENG
>Assignee: Xiaodong DENG
>Priority: Minor
>
> There are improper string formatting, like
>  
> -  ["{}".format(x) for x in tis_to_set_to_scheduled])
> - using .format in log calls



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (AIRFLOW-4154) Correct string formatting in jobs.py

2019-03-25 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on AIRFLOW-4154:
-

XD-DENG commented on pull request #4972: [AIRFLOW-4154] Correct string 
formatting in jobs.py
URL: https://github.com/apache/airflow/pull/4972
 
 
   ### Jira
   
 - https://issues.apache.org/jira/browse/AIRFLOW-4154
   
   ### Description
   
   Fix improper string formatting, like
   
   -  `["{}".format(x) for x in tis_to_set_to_scheduled])`
   - using `.format` in log calls
   
   
   
 

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


> Correct string formatting in jobs.py
> 
>
> Key: AIRFLOW-4154
> URL: https://issues.apache.org/jira/browse/AIRFLOW-4154
> Project: Apache Airflow
>  Issue Type: Task
>Affects Versions: 1.10.2
>Reporter: Xiaodong DENG
>Assignee: Xiaodong DENG
>Priority: Minor
>
> There are improper string formatting, like
> -  ["{}".format(x) for x in tis_to_set_to_scheduled])
> - using .format in log calls



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] [airflow] XD-DENG opened a new pull request #4972: [AIRFLOW-4154] Correct string formatting in jobs.py

2019-03-25 Thread GitBox
XD-DENG opened a new pull request #4972: [AIRFLOW-4154] Correct string 
formatting in jobs.py
URL: https://github.com/apache/airflow/pull/4972
 
 
   ### Jira
   
 - https://issues.apache.org/jira/browse/AIRFLOW-4154
   
   ### Description
   
   Fix improper string formatting, like
   
   -  `["{}".format(x) for x in tis_to_set_to_scheduled])`
   - using `.format` in log calls
   
   
   


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


With regards,
Apache Git Services


  1   2   3   >