You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/04/27 16:35:50 UTC

[GitHub] [airflow] feluelle opened a new pull request #8591: Add http system test

feluelle opened a new pull request #8591:
URL: https://github.com/apache/airflow/pull/8591


   ---
   Make sure to mark the boxes below before creating PR: [x]
   
   - [x] Description above provides context of the change
   - [x] Unit tests coverage for changes (not needed for documentation changes)
   - [x] Target Github ISSUE in description if exists
   - [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
   - [x] Relevant documentation is updated including usage instructions.
   - [x] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
   
   ---
   In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
   Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] feluelle commented on a change in pull request #8591: Add system test and doc to http

Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #8591:
URL: https://github.com/apache/airflow/pull/8591#discussion_r416519560



##########
File path: docs/exts/exampleinclude.py
##########
@@ -252,5 +252,5 @@ def setup(app):
     app.add_config_value("exampleinclude_sourceroot", None, "env")
     if not airflow_theme_is_available:
         # Sphinx airflow theme has its own styles.
-        app.add_stylesheet('exampleinclude.css')
+        app.add_css_file('exampleinclude.css')

Review comment:
       I added the info to the commit msg.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] feluelle commented on a change in pull request #8591: Add system test and doc to http

Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #8591:
URL: https://github.com/apache/airflow/pull/8591#discussion_r416519289



##########
File path: docs/exts/exampleinclude.py
##########
@@ -252,5 +252,5 @@ def setup(app):
     app.add_config_value("exampleinclude_sourceroot", None, "env")
     if not airflow_theme_is_available:
         # Sphinx airflow theme has its own styles.
-        app.add_stylesheet('exampleinclude.css')
+        app.add_css_file('exampleinclude.css')

Review comment:
       BTW this fixes a deprecation warning.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] mik-laj commented on a change in pull request #8591: Add system test and doc to http

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #8591:
URL: https://github.com/apache/airflow/pull/8591#discussion_r416572011



##########
File path: docs/exts/exampleinclude.py
##########
@@ -252,5 +252,5 @@ def setup(app):
     app.add_config_value("exampleinclude_sourceroot", None, "env")
     if not airflow_theme_is_available:
         # Sphinx airflow theme has its own styles.
-        app.add_stylesheet('exampleinclude.css')
+        app.add_css_file('exampleinclude.css')

Review comment:
       Can you do it in a separate PR? We would like to have these changes in Airflow 1.10 too.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] turbaszek commented on a change in pull request #8591: Add http system test

Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #8591:
URL: https://github.com/apache/airflow/pull/8591#discussion_r415973240



##########
File path: airflow/providers/http/example_dags/example_http.py
##########
@@ -42,6 +42,7 @@
 dag.doc_md = __doc__
 
 # t1, t2 and t3 are examples of tasks created by instantiating operators
+# [START howto_operator_http_t1]
 t1 = SimpleHttpOperator(
     task_id='post_op',

Review comment:
       ```suggestion
   post_op = SimpleHttpOperator(
       task_id='post_op',
   ```
   I like it when task instance and task_id are inline.WDYT?

##########
File path: tests/providers/http/operators/test_http_system.py
##########
@@ -0,0 +1,35 @@
+# 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 os
+
+import pytest
+
+from tests.test_utils import AIRFLOW_MAIN_FOLDER
+from tests.test_utils.system_tests_class import SystemTest
+
+HTTP_DAG_FOLDER = os.path.join(
+    AIRFLOW_MAIN_FOLDER, "airflow", "providers", "http", "example_dags"
+)
+
+
+@pytest.mark.backend("mysql", "postgres")
+@pytest.mark.system("http")

Review comment:
       I'm wondering if we should use more generic system type like "core" or something. WDYT? @potiuk ?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] feluelle commented on a change in pull request #8591: Add http system test

Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #8591:
URL: https://github.com/apache/airflow/pull/8591#discussion_r416022682



##########
File path: airflow/providers/http/example_dags/example_http.py
##########
@@ -42,6 +42,7 @@
 dag.doc_md = __doc__
 
 # t1, t2 and t3 are examples of tasks created by instantiating operators
+# [START howto_operator_http_t1]
 t1 = SimpleHttpOperator(
     task_id='post_op',

Review comment:
       me too - will change it :)




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] turbaszek commented on pull request #8591: Add http system test

Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #8591:
URL: https://github.com/apache/airflow/pull/8591#issuecomment-620103045


   > @turbaszek Is this fine or should I expose the params via env variables? When and which params should we make available to env variables?
   
   
   
   > @turbaszek Is this fine or should I expose the params via env variables? When and which params should we make available to env variables?
   
   All ids should be env variables (for security reasons). In case of GCP buckets names have to be unique (and constant during dag run so no way to generate them in code) that's why we use env variables. 


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] kaxil commented on a change in pull request #8591: Add system test and doc to http

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #8591:
URL: https://github.com/apache/airflow/pull/8591#discussion_r416577705



##########
File path: docs/exts/exampleinclude.py
##########
@@ -252,5 +252,5 @@ def setup(app):
     app.add_config_value("exampleinclude_sourceroot", None, "env")
     if not airflow_theme_is_available:
         # Sphinx airflow theme has its own styles.
-        app.add_stylesheet('exampleinclude.css')
+        app.add_css_file('exampleinclude.css')

Review comment:
       yeah would be good to have it in a separate PR




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] zhongjiajie commented on pull request #8591: Add system test and doc to http

Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on pull request #8591:
URL: https://github.com/apache/airflow/pull/8591#issuecomment-620525605


   FYI, I change the PR titile


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] feluelle commented on pull request #8591: Add http system test

Posted by GitBox <gi...@apache.org>.
feluelle commented on pull request #8591:
URL: https://github.com/apache/airflow/pull/8591#issuecomment-620099840


   @turbaszek Is this fine or should I expose the params via env variables? 


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] feluelle edited a comment on pull request #8591: Add http system test

Posted by GitBox <gi...@apache.org>.
feluelle edited a comment on pull request #8591:
URL: https://github.com/apache/airflow/pull/8591#issuecomment-620099840


   @turbaszek Is this fine or should I expose the params via env variables? When and which params should we make available to env variables?


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] mik-laj commented on a change in pull request #8591: Add http system test

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #8591:
URL: https://github.com/apache/airflow/pull/8591#discussion_r416109551



##########
File path: tests/providers/http/operators/test_http_system.py
##########
@@ -0,0 +1,35 @@
+# 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 os
+
+import pytest
+
+from tests.test_utils import AIRFLOW_MAIN_FOLDER
+from tests.test_utils.system_tests_class import SystemTest
+
+HTTP_DAG_FOLDER = os.path.join(
+    AIRFLOW_MAIN_FOLDER, "airflow", "providers", "http", "example_dags"
+)
+
+
+@pytest.mark.backend("mysql", "postgres")
+@pytest.mark.system("http")

Review comment:
       It is not the core operator. It is in the provider directory. This allows it to be released independently from Airflow.  It is common and mature, but its use and usage are still developing. It is flexible enough to be extended with new authentication mechanisms and more. Last week we had a valuable and non-doc contribution to this operator.
   https://github.com/apache/airflow/pull/8429
   
   Here is list of all core operators.
   ```
   airflow.operators.bash.BashOperator
   airflow.operators.branch_operator.BaseBranchOperator
   airflow.operators.check_operator.CheckOperator
   airflow.operators.check_operator.IntervalCheckOperator
   airflow.operators.check_operator.ThresholdCheckOperator
   airflow.operators.check_operator.ValueCheckOperator
   airflow.operators.dagrun_operator.TriggerDagRunOperator
   airflow.operators.dummy_operator.DummyOperator
   airflow.operators.generic_transfer.GenericTransfer
   airflow.operators.latest_only_operator.LatestOnlyOperator
   airflow.operators.presto_check_operator.PrestoCheckOperator
   airflow.operators.presto_check_operator.PrestoIntervalCheckOperator
   airflow.operators.presto_check_operator.PrestoValueCheckOperator
   airflow.operators.python.BranchPythonOperator
   airflow.operators.python.PythonOperator
   airflow.operators.python.PythonVirtualenvOperator
   airflow.operators.python.ShortCircuitOperator
   airflow.operators.subdag_operator.SubDagOperator
   ```




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] kaxil commented on a change in pull request #8591: Add http system test

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #8591:
URL: https://github.com/apache/airflow/pull/8591#discussion_r416010413



##########
File path: docs/howto/operator/http/http.rst
##########
@@ -0,0 +1,77 @@
+ .. 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.
+
+HTTP Operators
+==============
+
+In all of the following code examples we are using the ``http_default`` connection which means we calling requests
+against `httpbin <https://www.httpbin.org/>`__ site to perform basic http operations.
+
+.. _howto/operator:HttpSensor:
+
+Use the :class:`~airflow.providers.http.sensors.http.HttpSensor` to poke until the ``response_check`` callable evaluates
+to ``true``.
+
+Here we are poking until httpbin gives us a response text containing ``httpbin``.
+
+.. exampleinclude:: ../../../airflow/providers/http/example_dags/example_http.py
+    :language: python
+    :start-after: [START howto_operator_http_sensor]
+    :end-before: [END howto_operator_http_sensor]
+
+.. _howto/operator:SimpleHttpOperator:
+
+Use the :class:`~airflow.providers.http.operators.http.SimpleHttpOperator` to call HTTP requests and get
+the response text back.
+
+In the first example we are calling a ``POST`` with json data and succeed when we get the same json data back
+otherwise the task will fail.
+
+.. exampleinclude:: ../../../airflow/providers/http/example_dags/example_http.py
+    :language: python
+    :start-after: [START howto_operator_http_t1]
+    :end-before: [END howto_operator_http_t1]
+
+Here we are calling a ``GET`` request and pass params to it. The task will succeed regardless of the response text.
+
+.. exampleinclude:: ../../../airflow/providers/http/example_dags/example_http.py
+    :language: python
+    :start-after: [START howto_operator_http_t2]
+    :end-before: [END howto_operator_http_t2]
+
+In the third example we are performing a ``PUT`` operation to put / set data according to the data that is being
+provided to the request.
+
+.. exampleinclude:: ../../../airflow/providers/http/example_dags/example_http.py
+    :language: python
+    :start-after: [START howto_operator_http_t3]
+    :end-before: [END howto_operator_http_t3]
+
+In this example we calling a ``DELETE`` operation to the ``delete`` endpoint. This time we are passing form data to the

Review comment:
       ```suggestion
   In this example we call a ``DELETE`` operation to the ``delete`` endpoint. This time we are passing form data to the
   ```

##########
File path: docs/howto/operator/http/http.rst
##########
@@ -0,0 +1,77 @@
+ .. 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.
+
+HTTP Operators
+==============
+
+In all of the following code examples we are using the ``http_default`` connection which means we calling requests
+against `httpbin <https://www.httpbin.org/>`__ site to perform basic http operations.

Review comment:
       ```suggestion
   The following code examples use the ``http_default`` connection which means the requests are sent against `httpbin <https://www.httpbin.org/>`__ site to perform basic HTTP operations.
   ```

##########
File path: docs/howto/operator/http/http.rst
##########
@@ -0,0 +1,77 @@
+ .. 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.
+
+HTTP Operators
+==============
+
+In all of the following code examples we are using the ``http_default`` connection which means we calling requests
+against `httpbin <https://www.httpbin.org/>`__ site to perform basic http operations.
+
+.. _howto/operator:HttpSensor:
+
+Use the :class:`~airflow.providers.http.sensors.http.HttpSensor` to poke until the ``response_check`` callable evaluates
+to ``true``.
+
+Here we are poking until httpbin gives us a response text containing ``httpbin``.
+
+.. exampleinclude:: ../../../airflow/providers/http/example_dags/example_http.py
+    :language: python
+    :start-after: [START howto_operator_http_sensor]
+    :end-before: [END howto_operator_http_sensor]
+
+.. _howto/operator:SimpleHttpOperator:
+
+Use the :class:`~airflow.providers.http.operators.http.SimpleHttpOperator` to call HTTP requests and get
+the response text back.
+
+In the first example we are calling a ``POST`` with json data and succeed when we get the same json data back
+otherwise the task will fail.
+
+.. exampleinclude:: ../../../airflow/providers/http/example_dags/example_http.py
+    :language: python
+    :start-after: [START howto_operator_http_t1]
+    :end-before: [END howto_operator_http_t1]
+
+Here we are calling a ``GET`` request and pass params to it. The task will succeed regardless of the response text.
+
+.. exampleinclude:: ../../../airflow/providers/http/example_dags/example_http.py
+    :language: python
+    :start-after: [START howto_operator_http_t2]
+    :end-before: [END howto_operator_http_t2]
+
+In the third example we are performing a ``PUT`` operation to put / set data according to the data that is being
+provided to the request.
+
+.. exampleinclude:: ../../../airflow/providers/http/example_dags/example_http.py
+    :language: python
+    :start-after: [START howto_operator_http_t3]
+    :end-before: [END howto_operator_http_t3]
+
+In this example we calling a ``DELETE`` operation to the ``delete`` endpoint. This time we are passing form data to the
+request.
+
+.. exampleinclude:: ../../../airflow/providers/http/example_dags/example_http.py
+    :language: python
+    :start-after: [START howto_operator_http_t4]
+    :end-before: [END howto_operator_http_t4]
+
+Here we are passing form data to a ``POST`` operation which is equal to a usual form submit.

Review comment:
       ```suggestion
   Here we pass form data to a ``POST`` operation which is equal to a usual form submit.
   ```




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] mik-laj commented on a change in pull request #8591: Add system test and doc to http

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #8591:
URL: https://github.com/apache/airflow/pull/8591#discussion_r416109551



##########
File path: tests/providers/http/operators/test_http_system.py
##########
@@ -0,0 +1,35 @@
+# 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 os
+
+import pytest
+
+from tests.test_utils import AIRFLOW_MAIN_FOLDER
+from tests.test_utils.system_tests_class import SystemTest
+
+HTTP_DAG_FOLDER = os.path.join(
+    AIRFLOW_MAIN_FOLDER, "airflow", "providers", "http", "example_dags"
+)
+
+
+@pytest.mark.backend("mysql", "postgres")
+@pytest.mark.system("http")

Review comment:
       It is not the core operator. It is in the provider directory. This allows it to be released independently from Airflow.  It is common and mature, but its use and usage are still developing. It is flexible enough to be extended with new authentication mechanisms and more. Last week we had a valuable and non-doc contribution to this operator.
   https://github.com/apache/airflow/pull/8429
   
   Here is list of all core operators.
   ```
   airflow.operators.bash.BashOperator
   airflow.operators.branch_operator.BaseBranchOperator
   airflow.operators.check_operator.CheckOperator
   airflow.operators.check_operator.IntervalCheckOperator
   airflow.operators.check_operator.ThresholdCheckOperator
   airflow.operators.check_operator.ValueCheckOperator
   airflow.operators.dagrun_operator.TriggerDagRunOperator
   airflow.operators.dummy_operator.DummyOperator
   airflow.operators.generic_transfer.GenericTransfer
   airflow.operators.latest_only_operator.LatestOnlyOperator
   airflow.operators.python.BranchPythonOperator
   airflow.operators.python.PythonOperator
   airflow.operators.python.PythonVirtualenvOperator
   airflow.operators.python.ShortCircuitOperator
   airflow.operators.subdag_operator.SubDagOperator
   ```




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] zhongjiajie commented on pull request #8591: Add http system test

Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on pull request #8591:
URL: https://github.com/apache/airflow/pull/8591#issuecomment-620321096


   > @kaxil fixed the doc issues but wait for ci before you merge please 😁 ..did not test the docs. I also changed the task var names and I hope I refactored it correctly. (if there are issues I am gonna fix it tomorrow :))
   
   FYI, still failed 😢 


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] feluelle commented on a change in pull request #8591: Add http system test

Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #8591:
URL: https://github.com/apache/airflow/pull/8591#discussion_r416467053



##########
File path: tests/providers/http/operators/test_http_system.py
##########
@@ -0,0 +1,35 @@
+# 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 os
+
+import pytest
+
+from tests.test_utils import AIRFLOW_MAIN_FOLDER
+from tests.test_utils.system_tests_class import SystemTest
+
+HTTP_DAG_FOLDER = os.path.join(
+    AIRFLOW_MAIN_FOLDER, "airflow", "providers", "http", "example_dags"
+)
+
+
+@pytest.mark.backend("mysql", "postgres")
+@pytest.mark.system("http")

Review comment:
       Agree with Kamil 👍 




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] zhongjiajie commented on pull request #8591: Add system test and doc to http

Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on pull request #8591:
URL: https://github.com/apache/airflow/pull/8591#issuecomment-621553711


   Merging it, Thanks @feluelle 


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] turbaszek edited a comment on pull request #8591: Add http system test

Posted by GitBox <gi...@apache.org>.
turbaszek edited a comment on pull request #8591:
URL: https://github.com/apache/airflow/pull/8591#issuecomment-620103045


   > @turbaszek Is this fine or should I expose the params via env variables? When and which params should we make available to env variables?
   
   All ids should be env variables (for security reasons). In case of GCP buckets names have to be unique (and constant during dag run so no way to generate them in code) that's why we use env variables. 


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] zhongjiajie commented on pull request #8591: Add http system test

Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on pull request #8591:
URL: https://github.com/apache/airflow/pull/8591#issuecomment-620322452


   I think it's failed due to
   ```log
   2020-04-27T18:10:16.7233125Z looking for now-outdated files... none found
   2020-04-27T18:10:16.9090056Z pickling environment... done
   2020-04-27T18:10:16.9090564Z checking consistency... /opt/airflow/docs/howto/operator/http/http.rst: WARNING: document isn't included in any toctree
   ```


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] feluelle commented on pull request #8591: Add http system test

Posted by GitBox <gi...@apache.org>.
feluelle commented on pull request #8591:
URL: https://github.com/apache/airflow/pull/8591#issuecomment-620144096


   @kaxil fixed the doc issues but wait for ci before you merge please 😁 ..did not test the docs. I also changed the task var names and I hope I refactored it correctly. (if there are issues I am gonna fix it tomorrow :))


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] kaxil commented on a change in pull request #8591: Add http system test

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #8591:
URL: https://github.com/apache/airflow/pull/8591#discussion_r416013831



##########
File path: docs/howto/operator/http/http.rst
##########
@@ -0,0 +1,77 @@
+ .. 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.
+
+HTTP Operators
+==============
+
+In all of the following code examples we are using the ``http_default`` connection which means we calling requests
+against `httpbin <https://www.httpbin.org/>`__ site to perform basic http operations.
+
+.. _howto/operator:HttpSensor:

Review comment:
       Thanks for adding docs, always love seeing additional docs




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org