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 2019/12/23 18:48:14 UTC
[GitHub] [airflow] baolsen opened a new pull request #6886: [AIRFLOW-6327]
http_hook: Accept json= parameter for payload
baolsen opened a new pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886
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:
Add json parameter to http_hook to allow easier usage when posting Python objects. See JIRA for more info: https://issues.apache.org/jira/browse/AIRFLOW-6327
### Tests
- [X] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason:
Relevant test added
### Commits
- [X] My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
1. Subject is limited to 50 characters (not including Jira issue reference)
1. Subject does not end with a period
1. Subject uses the imperative mood ("add", not "adding")
1. Body wraps at 72 characters
1. Body explains "what" and "why", not "how"
### Documentation
- [X] In case of new functionality, my PR adds documentation that describes how to use it.
- All the public functions and the classes in the PR contain docstrings that explain what it does
- If you implement backwards incompatible changes, please leave a note in the [Updating.md](https://github.com/apache/airflow/blob/master/UPDATING.md) so we can assign it to a appropriate release
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] dstandish commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361814771
##########
File path: tests/hooks/test_http_hook.py
##########
@@ -334,5 +338,35 @@ def test_connection_without_host(self, mock_get_connection):
hook.get_conn({})
self.assertEqual(hook.base_url, 'http://')
+ @requests_mock.mock()
+ def test_post_json_request(self, mock_requests):
+ obj1 = {'a': 1, 'b': 'abc', 'c': [1, 2, {"d": 10}]}
+ obj2 = [1, 2, 3]
+
+ # Ensure that obj1 was encoded to JSON
+ def match_obj1(request):
+ return json.loads(request.text) == obj1
+
+ # Filters to catch posted JSON
+ mock_requests.post(
+ '//test:8080/v1/test',
+ status_code=200,
+ request_headers={'Content-Type': 'application/json'},
+ text='test_post_json_request',
+ additional_matcher=match_obj1
+ )
+
+ with mock.patch(
+ 'airflow.hooks.base_hook.BaseHook.get_connection',
+ side_effect=get_airflow_connection
+ ):
+ # Send obj1 as JSON and verify it matched the mock
+ resp = self.post_hook.run('v1/test', json=obj1)
+ self.assertEqual(resp.status_code, 200)
+ self.assertEqual(resp.text, 'test_post_json_request')
+ # Ensure that obj1 was what was sent
+ with self.assertRaises(requests_mock.exceptions.NoMockAddress):
+ resp = self.post_hook.run('v1/test', json=obj2)
Review comment:
ok i was confused by the fact that you are looking for `NoMockAddress` error it's a little confusing but i now understand that's how requests mock is designed.
but anyway, so you are trying to test the test here. i think you should chop this. just test your feature, don't test the functionality of requests mock. fine to do it locally to satisfy yourself that it's working right, but i don't think it belongs in here permanently.
that said... this test-of-the-test is not working right anyway. on line 368 if you replace `obj2` with `obj1`, the test still passes. this is because you aren't mocking `airflow.hooks.base_hook.BaseHook.get_connection` at this point of the code, so it is finding no match, even when you supply the object it is looking for.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/87acbccb5059416854c61f73668e991efcf06dc2?src=pr&el=desc) will **decrease** coverage by `0.38%`.
> The diff coverage is `97.79%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.48% 84.1% -0.39%
=========================================
Files 680 680
Lines 38392 38501 +109
=========================================
- Hits 32437 32381 -56
- Misses 5955 6120 +165
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ow/contrib/example\_dags/example\_qubole\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9vcGVyYXRvci5weQ==) | `80% <ø> (ø)` | :arrow_up: |
| [airflow/contrib/operators/spark\_submit\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9zcGFya19zdWJtaXRfb3BlcmF0b3IucHk=) | `92.68% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/example\_python\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9weXRob25fb3BlcmF0b3IucHk=) | `63.33% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_branch\_python\_dop\_operator\_3.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9icmFuY2hfcHl0aG9uX2RvcF9vcGVyYXRvcl8zLnB5) | `75% <ø> (ø)` | :arrow_up: |
| [...flow/contrib/example\_dags/example\_qubole\_sensor.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9zZW5zb3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/docker\_copy\_data.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZG9ja2VyX2NvcHlfZGF0YS5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...dags/example\_azure\_container\_instances\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2F6dXJlX2NvbnRhaW5lcl9pbnN0YW5jZXNfb3BlcmF0b3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_emr\_job\_flow\_automatic\_steps.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2Vtcl9qb2JfZmxvd19hdXRvbWF0aWNfc3RlcHMucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...contrib/example\_dags/example\_papermill\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3BhcGVybWlsbF9vcGVyYXRvci5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...low/contrib/example\_dags/example\_winrm\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3dpbnJtX29wZXJhdG9yLnB5) | `100% <ø> (ø)` | :arrow_up: |
| ... and [79 more](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [87acbcc...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] dstandish commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r362168727
##########
File path: tests/hooks/test_http_hook.py
##########
@@ -334,5 +334,42 @@ def test_connection_without_host(self, mock_get_connection):
hook.get_conn({})
self.assertEqual(hook.base_url, 'http://')
+ @requests_mock.mock()
Review comment:
this is may be super nit picky, but you could simplify using parameters if you want to
```suggestion
@parameterized.expand([
'GET',
'POST',
])
@requests_mock.mock()
def test_json_request(self, method, mock_requests):
obj1 = {'a': 1, 'b': 'abc', 'c': [1, 2, {"d": 10}]}
def match_obj1(request):
return request.json() == obj1
mock_requests.request(
method=method,
url='//test:8080/v1/test',
additional_matcher=match_obj1
)
with mock.patch(
'airflow.hooks.base_hook.BaseHook.get_connection',
side_effect=get_airflow_connection
):
# will raise NoMockAddress exception if obj1 != request.json()
HttpHook(method=method).run('v1/test', json=obj1)
```
here i am also suggesting removing those asserts.
they are just checking that your mock is returning what you told it to return -- the fact that no exception is raised in `hook.run` is the actual indicator that the test was passed. the asserts don't test anything beyond 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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/87acbccb5059416854c61f73668e991efcf06dc2?src=pr&el=desc) will **decrease** coverage by `0.38%`.
> The diff coverage is `97.79%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.48% 84.1% -0.39%
=========================================
Files 680 680
Lines 38392 38501 +109
=========================================
- Hits 32437 32381 -56
- Misses 5955 6120 +165
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ow/contrib/example\_dags/example\_qubole\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9vcGVyYXRvci5weQ==) | `80% <ø> (ø)` | :arrow_up: |
| [airflow/contrib/operators/spark\_submit\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9zcGFya19zdWJtaXRfb3BlcmF0b3IucHk=) | `92.68% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/example\_python\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9weXRob25fb3BlcmF0b3IucHk=) | `63.33% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_branch\_python\_dop\_operator\_3.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9icmFuY2hfcHl0aG9uX2RvcF9vcGVyYXRvcl8zLnB5) | `75% <ø> (ø)` | :arrow_up: |
| [...flow/contrib/example\_dags/example\_qubole\_sensor.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9zZW5zb3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/docker\_copy\_data.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZG9ja2VyX2NvcHlfZGF0YS5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...dags/example\_azure\_container\_instances\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2F6dXJlX2NvbnRhaW5lcl9pbnN0YW5jZXNfb3BlcmF0b3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_emr\_job\_flow\_automatic\_steps.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2Vtcl9qb2JfZmxvd19hdXRvbWF0aWNfc3RlcHMucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...contrib/example\_dags/example\_papermill\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3BhcGVybWlsbF9vcGVyYXRvci5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...low/contrib/example\_dags/example\_winrm\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3dpbnJtX29wZXJhdG9yLnB5) | `100% <ø> (ø)` | :arrow_up: |
| ... and [79 more](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [87acbcc...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] dstandish commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361814771
##########
File path: tests/hooks/test_http_hook.py
##########
@@ -334,5 +338,35 @@ def test_connection_without_host(self, mock_get_connection):
hook.get_conn({})
self.assertEqual(hook.base_url, 'http://')
+ @requests_mock.mock()
+ def test_post_json_request(self, mock_requests):
+ obj1 = {'a': 1, 'b': 'abc', 'c': [1, 2, {"d": 10}]}
+ obj2 = [1, 2, 3]
+
+ # Ensure that obj1 was encoded to JSON
+ def match_obj1(request):
+ return json.loads(request.text) == obj1
+
+ # Filters to catch posted JSON
+ mock_requests.post(
+ '//test:8080/v1/test',
+ status_code=200,
+ request_headers={'Content-Type': 'application/json'},
+ text='test_post_json_request',
+ additional_matcher=match_obj1
+ )
+
+ with mock.patch(
+ 'airflow.hooks.base_hook.BaseHook.get_connection',
+ side_effect=get_airflow_connection
+ ):
+ # Send obj1 as JSON and verify it matched the mock
+ resp = self.post_hook.run('v1/test', json=obj1)
+ self.assertEqual(resp.status_code, 200)
+ self.assertEqual(resp.text, 'test_post_json_request')
+ # Ensure that obj1 was what was sent
+ with self.assertRaises(requests_mock.exceptions.NoMockAddress):
+ resp = self.post_hook.run('v1/test', json=obj2)
Review comment:
ok i was confused by the fact that you are looking for `NoMockAddress` error it's a little confusing but i now understand that's how requests mock is designed.
but anyway, i think you should chop this. i'd just test your feature, and not the functionality of requests-mock.
that said... this test-of-the-test is not working right anyway. on line 368 if you replace `obj2` with `obj1`, the test still passes. this is because you aren't mocking `airflow.hooks.base_hook.BaseHook.get_connection` at this point of the code, so it is finding no match, even when you supply the correct object.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/374cee0af30d7233698fd813213b7c732cbe284f?src=pr&el=desc) will **decrease** coverage by `0.03%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
==========================================
- Coverage 84.39% 84.35% -0.04%
==========================================
Files 680 680
Lines 38501 38719 +218
==========================================
+ Hits 32492 32663 +171
- Misses 6009 6056 +47
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <75%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/utils/dag\_processing.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9kYWdfcHJvY2Vzc2luZy5weQ==) | `87.85% <0%> (-0.15%)` | :arrow_down: |
| [airflow/providers/amazon/aws/hooks/s3.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYW1hem9uL2F3cy9ob29rcy9zMy5weQ==) | `95.76% <0%> (+0.02%)` | :arrow_up: |
| [airflow/hooks/dbapi\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9kYmFwaV9ob29rLnB5) | `91.73% <0%> (+0.82%)` | :arrow_up: |
| [airflow/models/connection.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvY29ubmVjdGlvbi5weQ==) | `68.78% <0%> (+0.97%)` | :arrow_up: |
| ... and [6 more](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [374cee0...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] dstandish commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361256287
##########
File path: tests/hooks/test_http_hook.py
##########
@@ -334,5 +338,35 @@ def test_connection_without_host(self, mock_get_connection):
hook.get_conn({})
self.assertEqual(hook.base_url, 'http://')
+ @requests_mock.mock()
+ def test_post_json_request(self, mock_requests):
+ obj1 = {'a': 1, 'b': 'abc', 'c': [1, 2, {"d": 10}]}
+ obj2 = [1, 2, 3]
+
+ # Ensure that obj1 was encoded to JSON
+ def match_obj1(request):
+ return json.loads(request.text) == obj1
+
+ # Filters to catch posted JSON
+ mock_requests.post(
+ '//test:8080/v1/test',
+ status_code=200,
+ request_headers={'Content-Type': 'application/json'},
+ text='test_post_json_request',
+ additional_matcher=match_obj1
+ )
+
+ with mock.patch(
+ 'airflow.hooks.base_hook.BaseHook.get_connection',
+ side_effect=get_airflow_connection
+ ):
+ # Send obj1 as JSON and verify it matched the mock
+ resp = self.post_hook.run('v1/test', json=obj1)
+ self.assertEqual(resp.status_code, 200)
+ self.assertEqual(resp.text, 'test_post_json_request')
+ # Ensure that obj1 was what was sent
+ with self.assertRaises(requests_mock.exceptions.NoMockAddress):
+ resp = self.post_hook.run('v1/test', json=obj2)
Review comment:
seems you are just verifying here that the request mock raises with bad address... does this belong in `test_post_json_request`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/87acbccb5059416854c61f73668e991efcf06dc2?src=pr&el=desc) will **decrease** coverage by `0.38%`.
> The diff coverage is `97.79%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.48% 84.1% -0.39%
=========================================
Files 680 680
Lines 38392 38501 +109
=========================================
- Hits 32437 32381 -56
- Misses 5955 6120 +165
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ow/contrib/example\_dags/example\_qubole\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9vcGVyYXRvci5weQ==) | `80% <ø> (ø)` | :arrow_up: |
| [airflow/contrib/operators/spark\_submit\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9zcGFya19zdWJtaXRfb3BlcmF0b3IucHk=) | `92.68% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/example\_python\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9weXRob25fb3BlcmF0b3IucHk=) | `63.33% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_branch\_python\_dop\_operator\_3.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9icmFuY2hfcHl0aG9uX2RvcF9vcGVyYXRvcl8zLnB5) | `75% <ø> (ø)` | :arrow_up: |
| [...flow/contrib/example\_dags/example\_qubole\_sensor.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9zZW5zb3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/docker\_copy\_data.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZG9ja2VyX2NvcHlfZGF0YS5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...dags/example\_azure\_container\_instances\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2F6dXJlX2NvbnRhaW5lcl9pbnN0YW5jZXNfb3BlcmF0b3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_emr\_job\_flow\_automatic\_steps.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2Vtcl9qb2JfZmxvd19hdXRvbWF0aWNfc3RlcHMucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...contrib/example\_dags/example\_papermill\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3BhcGVybWlsbF9vcGVyYXRvci5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...low/contrib/example\_dags/example\_winrm\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3dpbnJtX29wZXJhdG9yLnB5) | `100% <ø> (ø)` | :arrow_up: |
| ... and [79 more](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [87acbcc...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] baolsen commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
baolsen commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361909912
##########
File path: tests/hooks/test_http_hook.py
##########
@@ -334,5 +338,35 @@ def test_connection_without_host(self, mock_get_connection):
hook.get_conn({})
self.assertEqual(hook.base_url, 'http://')
+ @requests_mock.mock()
+ def test_post_json_request(self, mock_requests):
+ obj1 = {'a': 1, 'b': 'abc', 'c': [1, 2, {"d": 10}]}
+ obj2 = [1, 2, 3]
+
+ # Ensure that obj1 was encoded to JSON
+ def match_obj1(request):
+ return json.loads(request.text) == obj1
+
+ # Filters to catch posted JSON
+ mock_requests.post(
+ '//test:8080/v1/test',
+ status_code=200,
+ request_headers={'Content-Type': 'application/json'},
+ text='test_post_json_request',
+ additional_matcher=match_obj1
+ )
+
+ with mock.patch(
+ 'airflow.hooks.base_hook.BaseHook.get_connection',
+ side_effect=get_airflow_connection
+ ):
+ # Send obj1 as JSON and verify it matched the mock
+ resp = self.post_hook.run('v1/test', json=obj1)
+ self.assertEqual(resp.status_code, 200)
+ self.assertEqual(resp.text, 'test_post_json_request')
+ # Ensure that obj1 was what was sent
+ with self.assertRaises(requests_mock.exceptions.NoMockAddress):
+ resp = self.post_hook.run('v1/test', json=obj2)
Review comment:
Interesting, thanks for testing that. Requests_mock is tricky to get my head around. Anyway the second test is a bit overkill and if it isn't doing what I thought then it's certainly worth removing 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
With regards,
Apache Git Services
[GitHub] [airflow] dstandish commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r362170041
##########
File path: airflow/hooks/http_hook.py
##########
@@ -123,6 +127,7 @@ def run(self, endpoint, data=None, headers=None, extra_options=None):
req = requests.Request(self.method,
url,
data=data,
+ json=json,
Review comment:
hi @baolsen i like your `request_kwargs` approach.
i just offered a suggestion for cleaning up the test. you'll also have to add `from parameterized import parameterized` if you choose to adopt the simplification
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] baolsen commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
baolsen commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361609194
##########
File path: airflow/hooks/http_hook.py
##########
@@ -75,20 +75,24 @@ def get_conn(self, headers=None):
try:
session.headers.update(conn.extra_dejson)
except TypeError:
- self.log.warning('Connection to %s has invalid extra field.', conn.host)
+ self.log.warning(
+ 'Connection to %s has invalid extra field.', conn.host)
if headers:
session.headers.update(headers)
return session
- def run(self, endpoint, data=None, headers=None, extra_options=None):
+ def run(self, endpoint, data=None, json=None, headers=None, extra_options=None):
"""
Performs the request
:param endpoint: the endpoint to be called i.e. resource/v1/query?
:type endpoint: str
:param data: payload to be uploaded or request parameters
:type data: dict
+ :param json: Python object which is encoded and posted as JSON. Usually
+ this is a dict but can be any JSON-encodable Python object.
Review comment:
Thanks, will 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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/374cee0af30d7233698fd813213b7c732cbe284f?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.39% 84.1% -0.29%
=========================================
Files 680 680
Lines 38501 38501
=========================================
- Hits 32492 32381 -111
- Misses 6009 6120 +111
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <75%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `91.59% <0%> (-0.29%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [374cee0...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/374cee0af30d7233698fd813213b7c732cbe284f?src=pr&el=desc) will **decrease** coverage by `0.03%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
==========================================
- Coverage 84.39% 84.35% -0.04%
==========================================
Files 680 680
Lines 38501 38719 +218
==========================================
+ Hits 32492 32663 +171
- Misses 6009 6056 +47
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <75%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/utils/dag\_processing.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9kYWdfcHJvY2Vzc2luZy5weQ==) | `87.85% <0%> (-0.15%)` | :arrow_down: |
| [airflow/providers/amazon/aws/hooks/s3.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYW1hem9uL2F3cy9ob29rcy9zMy5weQ==) | `95.76% <0%> (+0.02%)` | :arrow_up: |
| [airflow/hooks/dbapi\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9kYmFwaV9ob29rLnB5) | `91.73% <0%> (+0.82%)` | :arrow_up: |
| [airflow/models/connection.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvY29ubmVjdGlvbi5weQ==) | `68.78% <0%> (+0.97%)` | :arrow_up: |
| ... and [6 more](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [374cee0...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] dstandish commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361814771
##########
File path: tests/hooks/test_http_hook.py
##########
@@ -334,5 +338,35 @@ def test_connection_without_host(self, mock_get_connection):
hook.get_conn({})
self.assertEqual(hook.base_url, 'http://')
+ @requests_mock.mock()
+ def test_post_json_request(self, mock_requests):
+ obj1 = {'a': 1, 'b': 'abc', 'c': [1, 2, {"d": 10}]}
+ obj2 = [1, 2, 3]
+
+ # Ensure that obj1 was encoded to JSON
+ def match_obj1(request):
+ return json.loads(request.text) == obj1
+
+ # Filters to catch posted JSON
+ mock_requests.post(
+ '//test:8080/v1/test',
+ status_code=200,
+ request_headers={'Content-Type': 'application/json'},
+ text='test_post_json_request',
+ additional_matcher=match_obj1
+ )
+
+ with mock.patch(
+ 'airflow.hooks.base_hook.BaseHook.get_connection',
+ side_effect=get_airflow_connection
+ ):
+ # Send obj1 as JSON and verify it matched the mock
+ resp = self.post_hook.run('v1/test', json=obj1)
+ self.assertEqual(resp.status_code, 200)
+ self.assertEqual(resp.text, 'test_post_json_request')
+ # Ensure that obj1 was what was sent
+ with self.assertRaises(requests_mock.exceptions.NoMockAddress):
+ resp = self.post_hook.run('v1/test', json=obj2)
Review comment:
ok i was confused by the fact that you are looking for `NoMockAddress` error it's a little confusing but i now understand that's how requests mock is designed.
but anyway, so you are trying to test the test here. i think you should chop this. just test your feature, don't test the functionality of requests mock. fine to do it locally to satisfy yourself that it's working right, but i don't think it belongs in here permanently.
that said... this test-of-the-test is working right anyway, because on line 368 if you replace `obj2` with `obj1`, the test still passes. this is because you aren't mocking `airflow.hooks.base_hook.BaseHook.get_connection` at this point of the code, so it is finding no match, irrespective of the object you supply.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] dstandish commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r362168727
##########
File path: tests/hooks/test_http_hook.py
##########
@@ -334,5 +334,42 @@ def test_connection_without_host(self, mock_get_connection):
hook.get_conn({})
self.assertEqual(hook.base_url, 'http://')
+ @requests_mock.mock()
Review comment:
this is may be super nit picky, but you could simplify using parameters if you want to
```suggestion
@parameterized.expand([
'GET',
'POST',
])
@requests_mock.mock()
def test_json_request(self, method, mock_requests):
obj1 = {'a': 1, 'b': 'abc', 'c': [1, 2, {"d": 10}]}
def match_obj1(request):
return request.json() == obj1
mock_requests.request(
method=method,
url='//test:8080/v1/test',
additional_matcher=match_obj1
)
with mock.patch(
'airflow.hooks.base_hook.BaseHook.get_connection',
side_effect=get_airflow_connection
):
# Send obj1 as JSON and verify it matched the mock
HttpHook(method=method).run('v1/test', json=obj1)
```
apart from the parameterization, in my suggestion i chop a few things i'll make some inline comments to explain
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/374cee0af30d7233698fd813213b7c732cbe284f?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.39% 84.1% -0.29%
=========================================
Files 680 680
Lines 38501 38501
=========================================
- Hits 32492 32381 -111
- Misses 6009 6120 +111
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <75%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `91.59% <0%> (-0.29%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [374cee0...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/d7d878142b03604d564d0d2ef0b3e61aec23e0bf?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
==========================================
- Coverage 84.44% 84.43% -0.02%
==========================================
Files 679 679
Lines 38543 38518 -25
==========================================
- Hits 32549 32521 -28
- Misses 5994 5997 +3
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/executors/base\_executor.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvYmFzZV9leGVjdXRvci5weQ==) | `96.05% <100%> (+0.43%)` | :arrow_up: |
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <100%> (ø)` | :arrow_up: |
| [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `88.59% <100%> (ø)` | :arrow_up: |
| [airflow/utils/dag\_processing.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9kYWdfcHJvY2Vzc2luZy5weQ==) | `88% <100%> (ø)` | :arrow_up: |
| [airflow/contrib/hooks/spark\_submit\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL3NwYXJrX3N1Ym1pdF9ob29rLnB5) | `82% <100%> (-0.5%)` | :arrow_down: |
| [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `76.08% <100%> (ø)` | :arrow_up: |
| [airflow/models/\_\_init\_\_.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvX19pbml0X18ucHk=) | `91.3% <0%> (-8.7%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [d7d8781...27651cf](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/87acbccb5059416854c61f73668e991efcf06dc2?src=pr&el=desc) will **decrease** coverage by `0.38%`.
> The diff coverage is `97.79%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.48% 84.1% -0.39%
=========================================
Files 680 680
Lines 38392 38501 +109
=========================================
- Hits 32437 32381 -56
- Misses 5955 6120 +165
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ow/contrib/example\_dags/example\_qubole\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9vcGVyYXRvci5weQ==) | `80% <ø> (ø)` | :arrow_up: |
| [airflow/contrib/operators/spark\_submit\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9zcGFya19zdWJtaXRfb3BlcmF0b3IucHk=) | `92.68% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/example\_python\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9weXRob25fb3BlcmF0b3IucHk=) | `63.33% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_branch\_python\_dop\_operator\_3.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9icmFuY2hfcHl0aG9uX2RvcF9vcGVyYXRvcl8zLnB5) | `75% <ø> (ø)` | :arrow_up: |
| [...flow/contrib/example\_dags/example\_qubole\_sensor.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9zZW5zb3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/docker\_copy\_data.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZG9ja2VyX2NvcHlfZGF0YS5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...dags/example\_azure\_container\_instances\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2F6dXJlX2NvbnRhaW5lcl9pbnN0YW5jZXNfb3BlcmF0b3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_emr\_job\_flow\_automatic\_steps.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2Vtcl9qb2JfZmxvd19hdXRvbWF0aWNfc3RlcHMucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...contrib/example\_dags/example\_papermill\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3BhcGVybWlsbF9vcGVyYXRvci5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...low/contrib/example\_dags/example\_winrm\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3dpbnJtX29wZXJhdG9yLnB5) | `100% <ø> (ø)` | :arrow_up: |
| ... and [79 more](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [87acbcc...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/374cee0af30d7233698fd813213b7c732cbe284f?src=pr&el=desc) will **decrease** coverage by `0.03%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
==========================================
- Coverage 84.39% 84.35% -0.04%
==========================================
Files 680 680
Lines 38501 38719 +218
==========================================
+ Hits 32492 32663 +171
- Misses 6009 6056 +47
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <75%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/utils/dag\_processing.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9kYWdfcHJvY2Vzc2luZy5weQ==) | `87.85% <0%> (-0.15%)` | :arrow_down: |
| [airflow/providers/amazon/aws/hooks/s3.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYW1hem9uL2F3cy9ob29rcy9zMy5weQ==) | `95.76% <0%> (+0.02%)` | :arrow_up: |
| [airflow/hooks/dbapi\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9kYmFwaV9ob29rLnB5) | `91.73% <0%> (+0.82%)` | :arrow_up: |
| [airflow/models/connection.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvY29ubmVjdGlvbi5weQ==) | `68.78% <0%> (+0.97%)` | :arrow_up: |
| ... and [6 more](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [374cee0...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/374cee0af30d7233698fd813213b7c732cbe284f?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.39% 84.1% -0.29%
=========================================
Files 680 680
Lines 38501 38501
=========================================
- Hits 32492 32381 -111
- Misses 6009 6120 +111
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <75%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `91.59% <0%> (-0.29%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [374cee0...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/be58f173481c7651e50aeed485cd762a5b3c5629?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
==========================================
- Coverage 84.71% 84.43% -0.29%
==========================================
Files 679 679
Lines 38518 38518
==========================================
- Hits 32631 32521 -110
- Misses 5887 5997 +110
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <100%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [be58f17...27651cf](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] dstandish commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361256035
##########
File path: tests/hooks/test_http_hook.py
##########
@@ -334,5 +338,35 @@ def test_connection_without_host(self, mock_get_connection):
hook.get_conn({})
self.assertEqual(hook.base_url, 'http://')
+ @requests_mock.mock()
+ def test_post_json_request(self, mock_requests):
+ obj1 = {'a': 1, 'b': 'abc', 'c': [1, 2, {"d": 10}]}
+ obj2 = [1, 2, 3]
+
+ # Ensure that obj1 was encoded to JSON
+ def match_obj1(request):
+ return json.loads(request.text) == obj1
+
+ # Filters to catch posted JSON
+ mock_requests.post(
+ '//test:8080/v1/test',
+ status_code=200,
+ request_headers={'Content-Type': 'application/json'},
+ text='test_post_json_request',
+ additional_matcher=match_obj1
+ )
+
+ with mock.patch(
+ 'airflow.hooks.base_hook.BaseHook.get_connection',
+ side_effect=get_airflow_connection
+ ):
+ # Send obj1 as JSON and verify it matched the mock
+ resp = self.post_hook.run('v1/test', json=obj1)
+ self.assertEqual(resp.status_code, 200)
+ self.assertEqual(resp.text, 'test_post_json_request')
+ # Ensure that obj1 was what was sent
Review comment:
this comment does not seem accurate
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/0f7c4563d2f8885e12312aae72bc70a22f734894?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
==========================================
- Coverage 84.7% 84.41% -0.29%
==========================================
Files 679 679
Lines 38489 38489
==========================================
- Hits 32601 32491 -110
- Misses 5888 5998 +110
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <100%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [0f7c456...d42617f](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/87acbccb5059416854c61f73668e991efcf06dc2?src=pr&el=desc) will **decrease** coverage by `0.38%`.
> The diff coverage is `97.79%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.48% 84.1% -0.39%
=========================================
Files 680 680
Lines 38392 38501 +109
=========================================
- Hits 32437 32381 -56
- Misses 5955 6120 +165
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ow/contrib/example\_dags/example\_qubole\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9vcGVyYXRvci5weQ==) | `80% <ø> (ø)` | :arrow_up: |
| [airflow/contrib/operators/spark\_submit\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9zcGFya19zdWJtaXRfb3BlcmF0b3IucHk=) | `92.68% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/example\_python\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9weXRob25fb3BlcmF0b3IucHk=) | `63.33% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_branch\_python\_dop\_operator\_3.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9icmFuY2hfcHl0aG9uX2RvcF9vcGVyYXRvcl8zLnB5) | `75% <ø> (ø)` | :arrow_up: |
| [...flow/contrib/example\_dags/example\_qubole\_sensor.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9zZW5zb3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/docker\_copy\_data.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZG9ja2VyX2NvcHlfZGF0YS5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...dags/example\_azure\_container\_instances\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2F6dXJlX2NvbnRhaW5lcl9pbnN0YW5jZXNfb3BlcmF0b3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_emr\_job\_flow\_automatic\_steps.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2Vtcl9qb2JfZmxvd19hdXRvbWF0aWNfc3RlcHMucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...contrib/example\_dags/example\_papermill\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3BhcGVybWlsbF9vcGVyYXRvci5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...low/contrib/example\_dags/example\_winrm\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3dpbnJtX29wZXJhdG9yLnB5) | `100% <ø> (ø)` | :arrow_up: |
| ... and [79 more](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [87acbcc...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/87acbccb5059416854c61f73668e991efcf06dc2?src=pr&el=desc) will **decrease** coverage by `0.12%`.
> The diff coverage is `97.79%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
==========================================
- Coverage 84.48% 84.35% -0.13%
==========================================
Files 680 680
Lines 38392 38719 +327
==========================================
+ Hits 32437 32663 +226
- Misses 5955 6056 +101
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ow/contrib/example\_dags/example\_qubole\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9vcGVyYXRvci5weQ==) | `80% <ø> (ø)` | :arrow_up: |
| [airflow/contrib/operators/spark\_submit\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9zcGFya19zdWJtaXRfb3BlcmF0b3IucHk=) | `92.68% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/example\_python\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9weXRob25fb3BlcmF0b3IucHk=) | `63.33% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_branch\_python\_dop\_operator\_3.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9icmFuY2hfcHl0aG9uX2RvcF9vcGVyYXRvcl8zLnB5) | `75% <ø> (ø)` | :arrow_up: |
| [...flow/contrib/example\_dags/example\_qubole\_sensor.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9zZW5zb3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/docker\_copy\_data.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZG9ja2VyX2NvcHlfZGF0YS5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...dags/example\_azure\_container\_instances\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2F6dXJlX2NvbnRhaW5lcl9pbnN0YW5jZXNfb3BlcmF0b3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_emr\_job\_flow\_automatic\_steps.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2Vtcl9qb2JfZmxvd19hdXRvbWF0aWNfc3RlcHMucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...contrib/example\_dags/example\_papermill\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3BhcGVybWlsbF9vcGVyYXRvci5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...low/contrib/example\_dags/example\_winrm\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3dpbnJtX29wZXJhdG9yLnB5) | `100% <ø> (ø)` | :arrow_up: |
| ... and [75 more](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [87acbcc...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] dstandish commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361814771
##########
File path: tests/hooks/test_http_hook.py
##########
@@ -334,5 +338,35 @@ def test_connection_without_host(self, mock_get_connection):
hook.get_conn({})
self.assertEqual(hook.base_url, 'http://')
+ @requests_mock.mock()
+ def test_post_json_request(self, mock_requests):
+ obj1 = {'a': 1, 'b': 'abc', 'c': [1, 2, {"d": 10}]}
+ obj2 = [1, 2, 3]
+
+ # Ensure that obj1 was encoded to JSON
+ def match_obj1(request):
+ return json.loads(request.text) == obj1
+
+ # Filters to catch posted JSON
+ mock_requests.post(
+ '//test:8080/v1/test',
+ status_code=200,
+ request_headers={'Content-Type': 'application/json'},
+ text='test_post_json_request',
+ additional_matcher=match_obj1
+ )
+
+ with mock.patch(
+ 'airflow.hooks.base_hook.BaseHook.get_connection',
+ side_effect=get_airflow_connection
+ ):
+ # Send obj1 as JSON and verify it matched the mock
+ resp = self.post_hook.run('v1/test', json=obj1)
+ self.assertEqual(resp.status_code, 200)
+ self.assertEqual(resp.text, 'test_post_json_request')
+ # Ensure that obj1 was what was sent
+ with self.assertRaises(requests_mock.exceptions.NoMockAddress):
+ resp = self.post_hook.run('v1/test', json=obj2)
Review comment:
ok i was confused by the fact that you are looking for `NoMockAddress` error it's a little misleading but i now understand that's how requests mock is designed.
but anyway, so you are trying to test the test here. i think you should chop this. just test your feature, don't test the functionality of requests mock. fine to do it locally to satisfy yourself that it's working right, but i don't think it belongs in here permanently.
that said... this test-of-the-test is working right anyway, because on line 368 if you replace `obj2` with `obj1`, the test still passes. this is because you aren't mocking `airflow.hooks.base_hook.BaseHook.get_connection` at this point of the code, so it is finding no match, irrespective of the object you supply.
2. i think you should chop this testing of the test. just satisfy yourself that the test is doing
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] baolsen commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
baolsen commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361948409
##########
File path: airflow/hooks/http_hook.py
##########
@@ -123,6 +127,7 @@ def run(self, endpoint, data=None, headers=None, extra_options=None):
req = requests.Request(self.method,
url,
data=data,
+ json=json,
Review comment:
@dstandish I've changed my approach a bit -
Now you'll see "**request_kwargs" which is more future proof. Another benefit is I could remove the changes to the slack_webhook tests.
Would love to hear your thoughts :) I tried to add a test for the other kwargs supported by Requests but didn't manage to get it to work properly, and decided to leave it just testing the json= kwarg.
Edit: Also finally figured out how rebase works, and cleaned up the commit history.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/374cee0af30d7233698fd813213b7c732cbe284f?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.39% 84.1% -0.29%
=========================================
Files 680 680
Lines 38501 38501
=========================================
- Hits 32492 32381 -111
- Misses 6009 6120 +111
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <75%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `91.59% <0%> (-0.29%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [374cee0...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/374cee0af30d7233698fd813213b7c732cbe284f?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.39% 84.1% -0.29%
=========================================
Files 680 680
Lines 38501 38501
=========================================
- Hits 32492 32381 -111
- Misses 6009 6120 +111
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <75%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `91.59% <0%> (-0.29%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [374cee0...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361245459
##########
File path: airflow/hooks/http_hook.py
##########
@@ -75,20 +75,24 @@ def get_conn(self, headers=None):
try:
session.headers.update(conn.extra_dejson)
except TypeError:
- self.log.warning('Connection to %s has invalid extra field.', conn.host)
+ self.log.warning(
+ 'Connection to %s has invalid extra field.', conn.host)
if headers:
session.headers.update(headers)
return session
- def run(self, endpoint, data=None, headers=None, extra_options=None):
+ def run(self, endpoint, data=None, json=None, headers=None, extra_options=None):
"""
Performs the request
:param endpoint: the endpoint to be called i.e. resource/v1/query?
:type endpoint: str
:param data: payload to be uploaded or request parameters
:type data: dict
+ :param json: Python object which is encoded and posted as JSON. Usually
+ this is a dict but can be any JSON-encodable Python object.
Review comment:
Four blank space will be better.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] baolsen commented on issue #6886: [AIRFLOW-6327]
http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
baolsen commented on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-569260655
Hi @zhongjiajie and @dstandish , ready for your review again :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361244812
##########
File path: airflow/hooks/http_hook.py
##########
@@ -75,20 +75,24 @@ def get_conn(self, headers=None):
try:
session.headers.update(conn.extra_dejson)
except TypeError:
- self.log.warning('Connection to %s has invalid extra field.', conn.host)
+ self.log.warning(
+ 'Connection to %s has invalid extra field.', conn.host)
Review comment:
I think this line is unrelated and unnecessary change? So as somewhere else in this PR
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] dstandish commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r362168727
##########
File path: tests/hooks/test_http_hook.py
##########
@@ -334,5 +334,42 @@ def test_connection_without_host(self, mock_get_connection):
hook.get_conn({})
self.assertEqual(hook.base_url, 'http://')
+ @requests_mock.mock()
Review comment:
this is may be super nit picky, but you could simplify using parameters if you want to
```suggestion
@parameterized.expand([
'GET',
'POST',
])
@requests_mock.mock()
def test_json_request(self, method, mock_requests):
obj1 = {'a': 1, 'b': 'abc', 'c': [1, 2, {"d": 10}]}
def match_obj1(request):
return request.json() == obj1
mock_requests.request(
method=method,
url='//test:8080/v1/test',
additional_matcher=match_obj1
)
with mock.patch(
'airflow.hooks.base_hook.BaseHook.get_connection',
side_effect=get_airflow_connection
):
# Send obj1 as JSON and verify it matched the mock
HttpHook(method=method).run('v1/test', json=obj1)
```
here i am also suggesting removing those asserts.
they are just checking that your mock is returning what you told it to return -- the fact that no exception is raised in `hook.run` is the actual indicator that the test was passed. the asserts don't test anything beyond 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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/374cee0af30d7233698fd813213b7c732cbe284f?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.39% 84.1% -0.29%
=========================================
Files 680 680
Lines 38501 38501
=========================================
- Hits 32492 32381 -111
- Misses 6009 6120 +111
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <75%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `91.59% <0%> (-0.29%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [374cee0...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/87acbccb5059416854c61f73668e991efcf06dc2?src=pr&el=desc) will **decrease** coverage by `0.12%`.
> The diff coverage is `97.79%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
==========================================
- Coverage 84.48% 84.35% -0.13%
==========================================
Files 680 680
Lines 38392 38719 +327
==========================================
+ Hits 32437 32663 +226
- Misses 5955 6056 +101
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ow/contrib/example\_dags/example\_qubole\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9vcGVyYXRvci5weQ==) | `80% <ø> (ø)` | :arrow_up: |
| [airflow/contrib/operators/spark\_submit\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9zcGFya19zdWJtaXRfb3BlcmF0b3IucHk=) | `92.68% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/example\_python\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9weXRob25fb3BlcmF0b3IucHk=) | `63.33% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_branch\_python\_dop\_operator\_3.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9icmFuY2hfcHl0aG9uX2RvcF9vcGVyYXRvcl8zLnB5) | `75% <ø> (ø)` | :arrow_up: |
| [...flow/contrib/example\_dags/example\_qubole\_sensor.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9zZW5zb3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/docker\_copy\_data.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZG9ja2VyX2NvcHlfZGF0YS5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...dags/example\_azure\_container\_instances\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2F6dXJlX2NvbnRhaW5lcl9pbnN0YW5jZXNfb3BlcmF0b3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_emr\_job\_flow\_automatic\_steps.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2Vtcl9qb2JfZmxvd19hdXRvbWF0aWNfc3RlcHMucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...contrib/example\_dags/example\_papermill\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3BhcGVybWlsbF9vcGVyYXRvci5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...low/contrib/example\_dags/example\_winrm\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3dpbnJtX29wZXJhdG9yLnB5) | `100% <ø> (ø)` | :arrow_up: |
| ... and [75 more](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [87acbcc...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/87acbccb5059416854c61f73668e991efcf06dc2?src=pr&el=desc) will **decrease** coverage by `0.38%`.
> The diff coverage is `97.79%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.48% 84.1% -0.39%
=========================================
Files 680 680
Lines 38392 38501 +109
=========================================
- Hits 32437 32381 -56
- Misses 5955 6120 +165
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ow/contrib/example\_dags/example\_qubole\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9vcGVyYXRvci5weQ==) | `80% <ø> (ø)` | :arrow_up: |
| [airflow/contrib/operators/spark\_submit\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9zcGFya19zdWJtaXRfb3BlcmF0b3IucHk=) | `92.68% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/example\_python\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9weXRob25fb3BlcmF0b3IucHk=) | `63.33% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_branch\_python\_dop\_operator\_3.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9icmFuY2hfcHl0aG9uX2RvcF9vcGVyYXRvcl8zLnB5) | `75% <ø> (ø)` | :arrow_up: |
| [...flow/contrib/example\_dags/example\_qubole\_sensor.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9zZW5zb3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/docker\_copy\_data.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZG9ja2VyX2NvcHlfZGF0YS5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...dags/example\_azure\_container\_instances\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2F6dXJlX2NvbnRhaW5lcl9pbnN0YW5jZXNfb3BlcmF0b3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_emr\_job\_flow\_automatic\_steps.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2Vtcl9qb2JfZmxvd19hdXRvbWF0aWNfc3RlcHMucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...contrib/example\_dags/example\_papermill\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3BhcGVybWlsbF9vcGVyYXRvci5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...low/contrib/example\_dags/example\_winrm\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3dpbnJtX29wZXJhdG9yLnB5) | `100% <ø> (ø)` | :arrow_up: |
| ... and [79 more](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [87acbcc...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] dstandish commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361814771
##########
File path: tests/hooks/test_http_hook.py
##########
@@ -334,5 +338,35 @@ def test_connection_without_host(self, mock_get_connection):
hook.get_conn({})
self.assertEqual(hook.base_url, 'http://')
+ @requests_mock.mock()
+ def test_post_json_request(self, mock_requests):
+ obj1 = {'a': 1, 'b': 'abc', 'c': [1, 2, {"d": 10}]}
+ obj2 = [1, 2, 3]
+
+ # Ensure that obj1 was encoded to JSON
+ def match_obj1(request):
+ return json.loads(request.text) == obj1
+
+ # Filters to catch posted JSON
+ mock_requests.post(
+ '//test:8080/v1/test',
+ status_code=200,
+ request_headers={'Content-Type': 'application/json'},
+ text='test_post_json_request',
+ additional_matcher=match_obj1
+ )
+
+ with mock.patch(
+ 'airflow.hooks.base_hook.BaseHook.get_connection',
+ side_effect=get_airflow_connection
+ ):
+ # Send obj1 as JSON and verify it matched the mock
+ resp = self.post_hook.run('v1/test', json=obj1)
+ self.assertEqual(resp.status_code, 200)
+ self.assertEqual(resp.text, 'test_post_json_request')
+ # Ensure that obj1 was what was sent
+ with self.assertRaises(requests_mock.exceptions.NoMockAddress):
+ resp = self.post_hook.run('v1/test', json=obj2)
Review comment:
ok i was confused by the fact that you are looking for `NoMockAddress` error it's a little confusing but i now understand that's how requests mock is designed.
but anyway, so you are trying to test the test here. i think you should chop this. just test your feature, don't test the functionality of requests mock. fine to do it locally to satisfy yourself that it's working right, but i don't think it belongs in here permanently.
that said... this test-of-the-test is not working right anyway. on line 368 if you replace `obj2` with `obj1`, the test still passes. this is because you aren't mocking `airflow.hooks.base_hook.BaseHook.get_connection` at this point of the code, so it is finding no match, even when you supply the correct object.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] dstandish commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361254408
##########
File path: tests/hooks/test_http_hook.py
##########
@@ -334,5 +338,35 @@ def test_connection_without_host(self, mock_get_connection):
hook.get_conn({})
self.assertEqual(hook.base_url, 'http://')
+ @requests_mock.mock()
+ def test_post_json_request(self, mock_requests):
+ obj1 = {'a': 1, 'b': 'abc', 'c': [1, 2, {"d": 10}]}
+ obj2 = [1, 2, 3]
+
+ # Ensure that obj1 was encoded to JSON
+ def match_obj1(request):
+ return json.loads(request.text) == obj1
+
+ # Filters to catch posted JSON
+ mock_requests.post(
+ '//test:8080/v1/test',
+ status_code=200,
+ request_headers={'Content-Type': 'application/json'},
+ text='test_post_json_request',
+ additional_matcher=match_obj1
+ )
+
+ with mock.patch(
+ 'airflow.hooks.base_hook.BaseHook.get_connection',
+ side_effect=get_airflow_connection
+ ):
+ # Send obj1 as JSON and verify it matched the mock
+ resp = self.post_hook.run('v1/test', json=obj1)
+ self.assertEqual(resp.status_code, 200)
Review comment:
test guidelines suggest we should use standard asserts now
https://github.com/apache/airflow/blob/master/TESTING.rst#writing-unit-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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/be58f173481c7651e50aeed485cd762a5b3c5629?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
==========================================
- Coverage 84.71% 84.43% -0.29%
==========================================
Files 679 679
Lines 38518 38518
==========================================
- Hits 32631 32521 -110
- Misses 5887 5997 +110
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <100%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [be58f17...27651cf](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] dstandish commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361250944
##########
File path: airflow/hooks/http_hook.py
##########
@@ -175,7 +181,8 @@ def run_and_check(self, session, prepped_request, extra_options):
return response
except requests.exceptions.ConnectionError as ex:
- self.log.warning(str(ex) + ' Tenacity will retry to execute the operation')
+ self.log.warning(
Review comment:
as @zhongjiajie has stated, you are introducing line breaks unnecessarily, which makes the diff unnecessarily noisy (and the lines were not overly long to begin with)
currently flake8 is set at 110 characters, so probably best to set it to 110 in your editor, and these extraneous line breaks.
see https://github.com/apache/airflow/blob/master/.flake8#L2
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/374cee0af30d7233698fd813213b7c732cbe284f?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.39% 84.1% -0.29%
=========================================
Files 680 680
Lines 38501 38501
=========================================
- Hits 32492 32381 -111
- Misses 6009 6120 +111
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <75%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `91.59% <0%> (-0.29%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [374cee0...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/374cee0af30d7233698fd813213b7c732cbe284f?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.39% 84.1% -0.29%
=========================================
Files 680 680
Lines 38501 38501
=========================================
- Hits 32492 32381 -111
- Misses 6009 6120 +111
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <75%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `91.59% <0%> (-0.29%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [374cee0...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] dstandish commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r362168727
##########
File path: tests/hooks/test_http_hook.py
##########
@@ -334,5 +334,42 @@ def test_connection_without_host(self, mock_get_connection):
hook.get_conn({})
self.assertEqual(hook.base_url, 'http://')
+ @requests_mock.mock()
Review comment:
this is may be super nit picky, but you could simplify the test using parameters if you want to
```suggestion
@parameterized.expand([
'GET',
'POST',
])
@requests_mock.mock()
def test_json_request(self, method, mock_requests):
obj1 = {'a': 1, 'b': 'abc', 'c': [1, 2, {"d": 10}]}
def match_obj1(request):
return request.json() == obj1
mock_requests.request(
method=method,
url='//test:8080/v1/test',
additional_matcher=match_obj1
)
with mock.patch(
'airflow.hooks.base_hook.BaseHook.get_connection',
side_effect=get_airflow_connection
):
# will raise NoMockAddress exception if obj1 != request.json()
HttpHook(method=method).run('v1/test', json=obj1)
```
here i am also suggesting removing those asserts.
they are just checking that your mock is returning what you told it to return -- the fact that no exception is raised in `hook.run` is the actual indicator that the test was passed. the asserts don't test anything beyond 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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/374cee0af30d7233698fd813213b7c732cbe284f?src=pr&el=desc) will **decrease** coverage by `0.03%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
==========================================
- Coverage 84.39% 84.35% -0.04%
==========================================
Files 680 680
Lines 38501 38719 +218
==========================================
+ Hits 32492 32663 +171
- Misses 6009 6056 +47
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <75%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/utils/dag\_processing.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9kYWdfcHJvY2Vzc2luZy5weQ==) | `87.85% <0%> (-0.15%)` | :arrow_down: |
| [airflow/providers/amazon/aws/hooks/s3.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYW1hem9uL2F3cy9ob29rcy9zMy5weQ==) | `95.76% <0%> (+0.02%)` | :arrow_up: |
| [airflow/hooks/dbapi\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9kYmFwaV9ob29rLnB5) | `91.73% <0%> (+0.82%)` | :arrow_up: |
| [airflow/models/connection.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvY29ubmVjdGlvbi5weQ==) | `68.78% <0%> (+0.97%)` | :arrow_up: |
| ... and [6 more](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [374cee0...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/be58f173481c7651e50aeed485cd762a5b3c5629?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
==========================================
- Coverage 84.71% 84.43% -0.29%
==========================================
Files 679 679
Lines 38518 38518
==========================================
- Hits 32631 32521 -110
- Misses 5887 5997 +110
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <100%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [be58f17...27651cf](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/87acbccb5059416854c61f73668e991efcf06dc2?src=pr&el=desc) will **decrease** coverage by `0.38%`.
> The diff coverage is `97.79%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.48% 84.1% -0.39%
=========================================
Files 680 680
Lines 38392 38501 +109
=========================================
- Hits 32437 32381 -56
- Misses 5955 6120 +165
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ow/contrib/example\_dags/example\_qubole\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9vcGVyYXRvci5weQ==) | `80% <ø> (ø)` | :arrow_up: |
| [airflow/contrib/operators/spark\_submit\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9zcGFya19zdWJtaXRfb3BlcmF0b3IucHk=) | `92.68% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/example\_python\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9weXRob25fb3BlcmF0b3IucHk=) | `63.33% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_branch\_python\_dop\_operator\_3.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9icmFuY2hfcHl0aG9uX2RvcF9vcGVyYXRvcl8zLnB5) | `75% <ø> (ø)` | :arrow_up: |
| [...flow/contrib/example\_dags/example\_qubole\_sensor.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9zZW5zb3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/docker\_copy\_data.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZG9ja2VyX2NvcHlfZGF0YS5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...dags/example\_azure\_container\_instances\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2F6dXJlX2NvbnRhaW5lcl9pbnN0YW5jZXNfb3BlcmF0b3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_emr\_job\_flow\_automatic\_steps.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2Vtcl9qb2JfZmxvd19hdXRvbWF0aWNfc3RlcHMucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...contrib/example\_dags/example\_papermill\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3BhcGVybWlsbF9vcGVyYXRvci5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...low/contrib/example\_dags/example\_winrm\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3dpbnJtX29wZXJhdG9yLnB5) | `100% <ø> (ø)` | :arrow_up: |
| ... and [79 more](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [87acbcc...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/87acbccb5059416854c61f73668e991efcf06dc2?src=pr&el=desc) will **decrease** coverage by `0.38%`.
> The diff coverage is `97.79%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.48% 84.1% -0.39%
=========================================
Files 680 680
Lines 38392 38501 +109
=========================================
- Hits 32437 32381 -56
- Misses 5955 6120 +165
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ow/contrib/example\_dags/example\_qubole\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9vcGVyYXRvci5weQ==) | `80% <ø> (ø)` | :arrow_up: |
| [airflow/contrib/operators/spark\_submit\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9zcGFya19zdWJtaXRfb3BlcmF0b3IucHk=) | `92.68% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/example\_python\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9weXRob25fb3BlcmF0b3IucHk=) | `63.33% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_branch\_python\_dop\_operator\_3.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9icmFuY2hfcHl0aG9uX2RvcF9vcGVyYXRvcl8zLnB5) | `75% <ø> (ø)` | :arrow_up: |
| [...flow/contrib/example\_dags/example\_qubole\_sensor.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9zZW5zb3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/docker\_copy\_data.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZG9ja2VyX2NvcHlfZGF0YS5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...dags/example\_azure\_container\_instances\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2F6dXJlX2NvbnRhaW5lcl9pbnN0YW5jZXNfb3BlcmF0b3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_emr\_job\_flow\_automatic\_steps.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2Vtcl9qb2JfZmxvd19hdXRvbWF0aWNfc3RlcHMucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...contrib/example\_dags/example\_papermill\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3BhcGVybWlsbF9vcGVyYXRvci5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...low/contrib/example\_dags/example\_winrm\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3dpbnJtX29wZXJhdG9yLnB5) | `100% <ø> (ø)` | :arrow_up: |
| ... and [79 more](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [87acbcc...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] baolsen commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
baolsen commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361615704
##########
File path: tests/hooks/test_http_hook.py
##########
@@ -334,5 +338,35 @@ def test_connection_without_host(self, mock_get_connection):
hook.get_conn({})
self.assertEqual(hook.base_url, 'http://')
+ @requests_mock.mock()
+ def test_post_json_request(self, mock_requests):
+ obj1 = {'a': 1, 'b': 'abc', 'c': [1, 2, {"d": 10}]}
+ obj2 = [1, 2, 3]
+
+ # Ensure that obj1 was encoded to JSON
+ def match_obj1(request):
+ return json.loads(request.text) == obj1
+
+ # Filters to catch posted JSON
+ mock_requests.post(
+ '//test:8080/v1/test',
+ status_code=200,
+ request_headers={'Content-Type': 'application/json'},
+ text='test_post_json_request',
+ additional_matcher=match_obj1
+ )
+
+ with mock.patch(
+ 'airflow.hooks.base_hook.BaseHook.get_connection',
+ side_effect=get_airflow_connection
+ ):
+ # Send obj1 as JSON and verify it matched the mock
+ resp = self.post_hook.run('v1/test', json=obj1)
+ self.assertEqual(resp.status_code, 200)
+ self.assertEqual(resp.text, 'test_post_json_request')
+ # Ensure that obj1 was what was sent
Review comment:
Comment clarified, 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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/374cee0af30d7233698fd813213b7c732cbe284f?src=pr&el=desc) will **decrease** coverage by `0.03%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
==========================================
- Coverage 84.39% 84.35% -0.04%
==========================================
Files 680 680
Lines 38501 38719 +218
==========================================
+ Hits 32492 32663 +171
- Misses 6009 6056 +47
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <75%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/utils/dag\_processing.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9kYWdfcHJvY2Vzc2luZy5weQ==) | `87.85% <0%> (-0.15%)` | :arrow_down: |
| [airflow/providers/amazon/aws/hooks/s3.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYW1hem9uL2F3cy9ob29rcy9zMy5weQ==) | `95.76% <0%> (+0.02%)` | :arrow_up: |
| [airflow/hooks/dbapi\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9kYmFwaV9ob29rLnB5) | `91.73% <0%> (+0.82%)` | :arrow_up: |
| [airflow/models/connection.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvY29ubmVjdGlvbi5weQ==) | `68.78% <0%> (+0.97%)` | :arrow_up: |
| ... and [6 more](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [374cee0...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io commented on issue #6886: [AIRFLOW-6327]
http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/374cee0af30d7233698fd813213b7c732cbe284f?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.39% 84.1% -0.29%
=========================================
Files 680 680
Lines 38501 38501
=========================================
- Hits 32492 32381 -111
- Misses 6009 6120 +111
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <75%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `91.59% <0%> (-0.29%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [374cee0...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/87acbccb5059416854c61f73668e991efcf06dc2?src=pr&el=desc) will **decrease** coverage by `0.38%`.
> The diff coverage is `97.79%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.48% 84.1% -0.39%
=========================================
Files 680 680
Lines 38392 38501 +109
=========================================
- Hits 32437 32381 -56
- Misses 5955 6120 +165
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ow/contrib/example\_dags/example\_qubole\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9vcGVyYXRvci5weQ==) | `80% <ø> (ø)` | :arrow_up: |
| [airflow/contrib/operators/spark\_submit\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9zcGFya19zdWJtaXRfb3BlcmF0b3IucHk=) | `92.68% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/example\_python\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9weXRob25fb3BlcmF0b3IucHk=) | `63.33% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_branch\_python\_dop\_operator\_3.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9icmFuY2hfcHl0aG9uX2RvcF9vcGVyYXRvcl8zLnB5) | `75% <ø> (ø)` | :arrow_up: |
| [...flow/contrib/example\_dags/example\_qubole\_sensor.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9zZW5zb3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/docker\_copy\_data.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZG9ja2VyX2NvcHlfZGF0YS5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...dags/example\_azure\_container\_instances\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2F6dXJlX2NvbnRhaW5lcl9pbnN0YW5jZXNfb3BlcmF0b3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_emr\_job\_flow\_automatic\_steps.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2Vtcl9qb2JfZmxvd19hdXRvbWF0aWNfc3RlcHMucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...contrib/example\_dags/example\_papermill\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3BhcGVybWlsbF9vcGVyYXRvci5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...low/contrib/example\_dags/example\_winrm\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3dpbnJtX29wZXJhdG9yLnB5) | `100% <ø> (ø)` | :arrow_up: |
| ... and [79 more](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [87acbcc...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] nuclearpinguin merged pull request #6886: [AIRFLOW-6327]
http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
nuclearpinguin merged pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] dstandish commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361814771
##########
File path: tests/hooks/test_http_hook.py
##########
@@ -334,5 +338,35 @@ def test_connection_without_host(self, mock_get_connection):
hook.get_conn({})
self.assertEqual(hook.base_url, 'http://')
+ @requests_mock.mock()
+ def test_post_json_request(self, mock_requests):
+ obj1 = {'a': 1, 'b': 'abc', 'c': [1, 2, {"d": 10}]}
+ obj2 = [1, 2, 3]
+
+ # Ensure that obj1 was encoded to JSON
+ def match_obj1(request):
+ return json.loads(request.text) == obj1
+
+ # Filters to catch posted JSON
+ mock_requests.post(
+ '//test:8080/v1/test',
+ status_code=200,
+ request_headers={'Content-Type': 'application/json'},
+ text='test_post_json_request',
+ additional_matcher=match_obj1
+ )
+
+ with mock.patch(
+ 'airflow.hooks.base_hook.BaseHook.get_connection',
+ side_effect=get_airflow_connection
+ ):
+ # Send obj1 as JSON and verify it matched the mock
+ resp = self.post_hook.run('v1/test', json=obj1)
+ self.assertEqual(resp.status_code, 200)
+ self.assertEqual(resp.text, 'test_post_json_request')
+ # Ensure that obj1 was what was sent
+ with self.assertRaises(requests_mock.exceptions.NoMockAddress):
+ resp = self.post_hook.run('v1/test', json=obj2)
Review comment:
ok i was confused by the name `NoMockAddress`. it's a little confusing, but i now understand that's how requests mock is designed.
but anyway, i think you should chop this. i'd just test your feature, and not the functionality of requests-mock.
in any case, i don't think this bit is doing what you want it to do. on line 368 if you replace `obj2` with `obj1`, the test still passes. this is because you aren't mocking `airflow.hooks.base_hook.BaseHook.get_connection` at this point of the code, so it is finding no match, even when you supply the correct object.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/374cee0af30d7233698fd813213b7c732cbe284f?src=pr&el=desc) will **decrease** coverage by `0.03%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
==========================================
- Coverage 84.39% 84.35% -0.04%
==========================================
Files 680 680
Lines 38501 38719 +218
==========================================
+ Hits 32492 32663 +171
- Misses 6009 6056 +47
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <75%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/utils/dag\_processing.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9kYWdfcHJvY2Vzc2luZy5weQ==) | `87.85% <0%> (-0.15%)` | :arrow_down: |
| [airflow/providers/amazon/aws/hooks/s3.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYW1hem9uL2F3cy9ob29rcy9zMy5weQ==) | `95.76% <0%> (+0.02%)` | :arrow_up: |
| [airflow/hooks/dbapi\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9kYmFwaV9ob29rLnB5) | `91.73% <0%> (+0.82%)` | :arrow_up: |
| [airflow/models/connection.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvY29ubmVjdGlvbi5weQ==) | `68.78% <0%> (+0.97%)` | :arrow_up: |
| ... and [6 more](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [374cee0...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] dstandish commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361253921
##########
File path: airflow/hooks/http_hook.py
##########
@@ -123,6 +127,7 @@ def run(self, endpoint, data=None, headers=None, extra_options=None):
req = requests.Request(self.method,
url,
data=data,
+ json=json,
Review comment:
i see you are only adding `json` for requests other than GET or HEAD
i know that presently the hook conditionally forwards the `data` param -- in one case to `params`, in another to `data`. but this seems like a mistake.
since `json` is brand new param, why not pass it on in all cases, no matter the request type?
at least, the docstring should probably disclose that `json` is only forwarded for non-GET and non-HEAD requests, if there is a legitimate reason to forward it selectively. and perhaps raise an error if it is supplied and you will not forward 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
With regards,
Apache Git Services
[GitHub] [airflow] dstandish commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361814771
##########
File path: tests/hooks/test_http_hook.py
##########
@@ -334,5 +338,35 @@ def test_connection_without_host(self, mock_get_connection):
hook.get_conn({})
self.assertEqual(hook.base_url, 'http://')
+ @requests_mock.mock()
+ def test_post_json_request(self, mock_requests):
+ obj1 = {'a': 1, 'b': 'abc', 'c': [1, 2, {"d": 10}]}
+ obj2 = [1, 2, 3]
+
+ # Ensure that obj1 was encoded to JSON
+ def match_obj1(request):
+ return json.loads(request.text) == obj1
+
+ # Filters to catch posted JSON
+ mock_requests.post(
+ '//test:8080/v1/test',
+ status_code=200,
+ request_headers={'Content-Type': 'application/json'},
+ text='test_post_json_request',
+ additional_matcher=match_obj1
+ )
+
+ with mock.patch(
+ 'airflow.hooks.base_hook.BaseHook.get_connection',
+ side_effect=get_airflow_connection
+ ):
+ # Send obj1 as JSON and verify it matched the mock
+ resp = self.post_hook.run('v1/test', json=obj1)
+ self.assertEqual(resp.status_code, 200)
+ self.assertEqual(resp.text, 'test_post_json_request')
+ # Ensure that obj1 was what was sent
+ with self.assertRaises(requests_mock.exceptions.NoMockAddress):
+ resp = self.post_hook.run('v1/test', json=obj2)
Review comment:
ok i was confused by the fact that you are looking for `NoMockAddress` error it's a little confusing but i now understand that's how requests mock is designed.
but anyway, so you are trying to test the test here. i think you should chop this. just test your feature, don't test the functionality of requests mock. fine to do it locally to satisfy yourself that it's working right, but i don't think it belongs in here permanently.
that said... this test-of-the-test is not working right anyway, because on line 368 if you replace `obj2` with `obj1`, the test still passes. this is because you aren't mocking `airflow.hooks.base_hook.BaseHook.get_connection` at this point of the code, so it is finding no match, irrespective of the object you supply.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] baolsen commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
baolsen commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361938813
##########
File path: tests/hooks/test_http_hook.py
##########
@@ -334,5 +335,37 @@ def test_connection_without_host(self, mock_get_connection):
hook.get_conn({})
self.assertEqual(hook.base_url, 'http://')
+ @requests_mock.mock()
+ def test_post_json_request(self, mock_requests):
+ obj1 = {'a': 1, 'b': 'abc', 'c': [1, 2, {"d": 10}]}
+ obj2 = [1, 2, 3]
+
+ # Ensure that obj1 was encoded to JSON
+ def match_obj1(request):
+ return json.loads(request.text) == obj1
Review comment:
Thanks, I've changed per your suggestion.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] dstandish commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361720780
##########
File path: airflow/hooks/http_hook.py
##########
@@ -123,6 +127,7 @@ def run(self, endpoint, data=None, headers=None, extra_options=None):
req = requests.Request(self.method,
url,
data=data,
+ json=json,
Review comment:
Though `json` is not a named argument in `get`, it does work. Kwargs are passed on to `request. See [here](https://github.com/psf/requests/blob/master/requests/api.py#L76).
My inclination would be to just blindly forward all args (param / data / json) in all cases, on the basis that we should not get in the way between user and library, without some reason. But you don't have to agree with me :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/374cee0af30d7233698fd813213b7c732cbe284f?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.39% 84.1% -0.29%
=========================================
Files 680 680
Lines 38501 38501
=========================================
- Hits 32492 32381 -111
- Misses 6009 6120 +111
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <75%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `91.59% <0%> (-0.29%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [374cee0...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] dstandish commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361814856
##########
File path: tests/hooks/test_http_hook.py
##########
@@ -334,5 +335,37 @@ def test_connection_without_host(self, mock_get_connection):
hook.get_conn({})
self.assertEqual(hook.base_url, 'http://')
+ @requests_mock.mock()
+ def test_post_json_request(self, mock_requests):
+ obj1 = {'a': 1, 'b': 'abc', 'c': [1, 2, {"d": 10}]}
+ obj2 = [1, 2, 3]
+
+ # Ensure that obj1 was encoded to JSON
+ def match_obj1(request):
+ return json.loads(request.text) == obj1
Review comment:
```suggestion
return request.json() == obj1
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] baolsen commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
baolsen commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361609831
##########
File path: airflow/hooks/http_hook.py
##########
@@ -175,7 +181,8 @@ def run_and_check(self, session, prepped_request, extra_options):
return response
except requests.exceptions.ConnectionError as ex:
- self.log.warning(str(ex) + ' Tenacity will retry to execute the operation')
+ self.log.warning(
Review comment:
Thanks, I've fixed it in my editor
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] baolsen commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
baolsen commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361609301
##########
File path: airflow/hooks/http_hook.py
##########
@@ -123,6 +127,7 @@ def run(self, endpoint, data=None, headers=None, extra_options=None):
req = requests.Request(self.method,
url,
data=data,
+ json=json,
Review comment:
Thanks, Python requests library only has data= and json= parameters for the requests.post method. I'll update the docstring to be clearer
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] baolsen commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
baolsen commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361616709
##########
File path: tests/hooks/test_http_hook.py
##########
@@ -334,5 +338,35 @@ def test_connection_without_host(self, mock_get_connection):
hook.get_conn({})
self.assertEqual(hook.base_url, 'http://')
+ @requests_mock.mock()
+ def test_post_json_request(self, mock_requests):
+ obj1 = {'a': 1, 'b': 'abc', 'c': [1, 2, {"d": 10}]}
+ obj2 = [1, 2, 3]
+
+ # Ensure that obj1 was encoded to JSON
+ def match_obj1(request):
+ return json.loads(request.text) == obj1
+
+ # Filters to catch posted JSON
+ mock_requests.post(
+ '//test:8080/v1/test',
+ status_code=200,
+ request_headers={'Content-Type': 'application/json'},
+ text='test_post_json_request',
+ additional_matcher=match_obj1
+ )
+
+ with mock.patch(
+ 'airflow.hooks.base_hook.BaseHook.get_connection',
+ side_effect=get_airflow_connection
+ ):
+ # Send obj1 as JSON and verify it matched the mock
+ resp = self.post_hook.run('v1/test', json=obj1)
+ self.assertEqual(resp.status_code, 200)
+ self.assertEqual(resp.text, 'test_post_json_request')
+ # Ensure that obj1 was what was sent
+ with self.assertRaises(requests_mock.exceptions.NoMockAddress):
+ resp = self.post_hook.run('v1/test', json=obj2)
Review comment:
I wanted to be sure that obj1 was encoded & posted correctly, not just that a post was done.
So I needed to check the contents of the request payload - using additional_matcher to ensure that what was posted matched. However since request_mock is a bit confusing to me I wasnt convinced that additional_matcher was going to be used. So I added a check to post obj2 the same way as obj1, which raises an exception
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/374cee0af30d7233698fd813213b7c732cbe284f?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.39% 84.1% -0.29%
=========================================
Files 680 680
Lines 38501 38501
=========================================
- Hits 32492 32381 -111
- Misses 6009 6120 +111
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <75%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `91.59% <0%> (-0.29%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [374cee0...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/374cee0af30d7233698fd813213b7c732cbe284f?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.39% 84.1% -0.29%
=========================================
Files 680 680
Lines 38501 38501
=========================================
- Hits 32492 32381 -111
- Misses 6009 6120 +111
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <75%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `91.59% <0%> (-0.29%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [374cee0...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] baolsen commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
baolsen commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361948409
##########
File path: airflow/hooks/http_hook.py
##########
@@ -123,6 +127,7 @@ def run(self, endpoint, data=None, headers=None, extra_options=None):
req = requests.Request(self.method,
url,
data=data,
+ json=json,
Review comment:
@dstandish I've changed my approach a bit -
Now you'll see "**request_kwargs" which is more future proof. Another benefit is I could remove the changes to the slack_webhook tests.
Would love to hear your thoughts :) I tried to add a test for the other kwargs supported by Requests but didn't manage to get it to work properly, and decided to leave it just testing the json= kwarg.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/374cee0af30d7233698fd813213b7c732cbe284f?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.39% 84.1% -0.29%
=========================================
Files 680 680
Lines 38501 38501
=========================================
- Hits 32492 32381 -111
- Misses 6009 6120 +111
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <75%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `91.59% <0%> (-0.29%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [374cee0...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] dstandish commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361814771
##########
File path: tests/hooks/test_http_hook.py
##########
@@ -334,5 +338,35 @@ def test_connection_without_host(self, mock_get_connection):
hook.get_conn({})
self.assertEqual(hook.base_url, 'http://')
+ @requests_mock.mock()
+ def test_post_json_request(self, mock_requests):
+ obj1 = {'a': 1, 'b': 'abc', 'c': [1, 2, {"d": 10}]}
+ obj2 = [1, 2, 3]
+
+ # Ensure that obj1 was encoded to JSON
+ def match_obj1(request):
+ return json.loads(request.text) == obj1
+
+ # Filters to catch posted JSON
+ mock_requests.post(
+ '//test:8080/v1/test',
+ status_code=200,
+ request_headers={'Content-Type': 'application/json'},
+ text='test_post_json_request',
+ additional_matcher=match_obj1
+ )
+
+ with mock.patch(
+ 'airflow.hooks.base_hook.BaseHook.get_connection',
+ side_effect=get_airflow_connection
+ ):
+ # Send obj1 as JSON and verify it matched the mock
+ resp = self.post_hook.run('v1/test', json=obj1)
+ self.assertEqual(resp.status_code, 200)
+ self.assertEqual(resp.text, 'test_post_json_request')
+ # Ensure that obj1 was what was sent
+ with self.assertRaises(requests_mock.exceptions.NoMockAddress):
+ resp = self.post_hook.run('v1/test', json=obj2)
Review comment:
ok i was confused by the name `NoMockAddress`. it's a little confusing, but i now understand that's how requests mock is designed.
but anyway, i think you should chop this. i'd just test your feature, and not the functionality of requests-mock.
that said... this test-of-the-test is not working right anyway. on line 368 if you replace `obj2` with `obj1`, the test still passes. this is because you aren't mocking `airflow.hooks.base_hook.BaseHook.get_connection` at this point of the code, so it is finding no match, even when you supply the correct object.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] dstandish commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361720780
##########
File path: airflow/hooks/http_hook.py
##########
@@ -123,6 +127,7 @@ def run(self, endpoint, data=None, headers=None, extra_options=None):
req = requests.Request(self.method,
url,
data=data,
+ json=json,
Review comment:
Though `json` is not a named argument in `get`, it does work. Kwargs are passed on to `request.
See [here](https://github.com/psf/requests/blob/master/requests/api.py#L76).
My inclination would be to just blindly forward all args (param / data / json) in all cases, on the basis that we should not get in the way between user and library, without some reason. But you don't have to agree with me :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] baolsen commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
baolsen commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361909553
##########
File path: airflow/hooks/http_hook.py
##########
@@ -123,6 +127,7 @@ def run(self, endpoint, data=None, headers=None, extra_options=None):
req = requests.Request(self.method,
url,
data=data,
+ json=json,
Review comment:
Ah I didn't see that. Thanks for pointing it out.
I'm also in favor of being able to forward any kwargs down to the library, will see what I can come up with.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] baolsen commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
baolsen commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361615390
##########
File path: tests/hooks/test_http_hook.py
##########
@@ -334,5 +338,35 @@ def test_connection_without_host(self, mock_get_connection):
hook.get_conn({})
self.assertEqual(hook.base_url, 'http://')
+ @requests_mock.mock()
+ def test_post_json_request(self, mock_requests):
+ obj1 = {'a': 1, 'b': 'abc', 'c': [1, 2, {"d": 10}]}
+ obj2 = [1, 2, 3]
+
+ # Ensure that obj1 was encoded to JSON
+ def match_obj1(request):
+ return json.loads(request.text) == obj1
+
+ # Filters to catch posted JSON
+ mock_requests.post(
+ '//test:8080/v1/test',
+ status_code=200,
+ request_headers={'Content-Type': 'application/json'},
+ text='test_post_json_request',
+ additional_matcher=match_obj1
+ )
+
+ with mock.patch(
+ 'airflow.hooks.base_hook.BaseHook.get_connection',
+ side_effect=get_airflow_connection
+ ):
+ # Send obj1 as JSON and verify it matched the mock
+ resp = self.post_hook.run('v1/test', json=obj1)
+ self.assertEqual(resp.status_code, 200)
Review comment:
Thanks, will update
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] dstandish commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361814771
##########
File path: tests/hooks/test_http_hook.py
##########
@@ -334,5 +338,35 @@ def test_connection_without_host(self, mock_get_connection):
hook.get_conn({})
self.assertEqual(hook.base_url, 'http://')
+ @requests_mock.mock()
+ def test_post_json_request(self, mock_requests):
+ obj1 = {'a': 1, 'b': 'abc', 'c': [1, 2, {"d": 10}]}
+ obj2 = [1, 2, 3]
+
+ # Ensure that obj1 was encoded to JSON
+ def match_obj1(request):
+ return json.loads(request.text) == obj1
+
+ # Filters to catch posted JSON
+ mock_requests.post(
+ '//test:8080/v1/test',
+ status_code=200,
+ request_headers={'Content-Type': 'application/json'},
+ text='test_post_json_request',
+ additional_matcher=match_obj1
+ )
+
+ with mock.patch(
+ 'airflow.hooks.base_hook.BaseHook.get_connection',
+ side_effect=get_airflow_connection
+ ):
+ # Send obj1 as JSON and verify it matched the mock
+ resp = self.post_hook.run('v1/test', json=obj1)
+ self.assertEqual(resp.status_code, 200)
+ self.assertEqual(resp.text, 'test_post_json_request')
+ # Ensure that obj1 was what was sent
+ with self.assertRaises(requests_mock.exceptions.NoMockAddress):
+ resp = self.post_hook.run('v1/test', json=obj2)
Review comment:
ok i was confused by the fact that you are looking for `NoMockAddress` error it's a little confusing but i now understand that's how requests mock is designed.
but anyway, so you are trying to test the test here. i think you should chop this. just test your feature, don't test the functionality of requests mock. fine to do it locally to satisfy yourself that it's working right, but i don't think it belongs in here permanently.
that said... this test-of-the-test is not working right anyway. on line 368 if you replace `obj2` with `obj1`, the test still passes. this is because you aren't mocking `airflow.hooks.base_hook.BaseHook.get_connection` at this point of the code, so it is finding no match, irrespective of the object you supply.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/87acbccb5059416854c61f73668e991efcf06dc2?src=pr&el=desc) will **decrease** coverage by `0.12%`.
> The diff coverage is `97.79%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
==========================================
- Coverage 84.48% 84.35% -0.13%
==========================================
Files 680 680
Lines 38392 38719 +327
==========================================
+ Hits 32437 32663 +226
- Misses 5955 6056 +101
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ow/contrib/example\_dags/example\_qubole\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9vcGVyYXRvci5weQ==) | `80% <ø> (ø)` | :arrow_up: |
| [airflow/contrib/operators/spark\_submit\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9zcGFya19zdWJtaXRfb3BlcmF0b3IucHk=) | `92.68% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/example\_python\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9weXRob25fb3BlcmF0b3IucHk=) | `63.33% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_branch\_python\_dop\_operator\_3.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9icmFuY2hfcHl0aG9uX2RvcF9vcGVyYXRvcl8zLnB5) | `75% <ø> (ø)` | :arrow_up: |
| [...flow/contrib/example\_dags/example\_qubole\_sensor.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3F1Ym9sZV9zZW5zb3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/docker\_copy\_data.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZG9ja2VyX2NvcHlfZGF0YS5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...dags/example\_azure\_container\_instances\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2F6dXJlX2NvbnRhaW5lcl9pbnN0YW5jZXNfb3BlcmF0b3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...ample\_dags/example\_emr\_job\_flow\_automatic\_steps.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX2Vtcl9qb2JfZmxvd19hdXRvbWF0aWNfc3RlcHMucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...contrib/example\_dags/example\_papermill\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3BhcGVybWlsbF9vcGVyYXRvci5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [...low/contrib/example\_dags/example\_winrm\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2V4YW1wbGVfZGFncy9leGFtcGxlX3dpbnJtX29wZXJhdG9yLnB5) | `100% <ø> (ø)` | :arrow_up: |
| ... and [75 more](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [87acbcc...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/374cee0af30d7233698fd813213b7c732cbe284f?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.39% 84.1% -0.29%
=========================================
Files 680 680
Lines 38501 38501
=========================================
- Hits 32492 32381 -111
- Misses 6009 6120 +111
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <75%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `91.59% <0%> (-0.29%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [374cee0...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] dstandish commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361814771
##########
File path: tests/hooks/test_http_hook.py
##########
@@ -334,5 +338,35 @@ def test_connection_without_host(self, mock_get_connection):
hook.get_conn({})
self.assertEqual(hook.base_url, 'http://')
+ @requests_mock.mock()
+ def test_post_json_request(self, mock_requests):
+ obj1 = {'a': 1, 'b': 'abc', 'c': [1, 2, {"d": 10}]}
+ obj2 = [1, 2, 3]
+
+ # Ensure that obj1 was encoded to JSON
+ def match_obj1(request):
+ return json.loads(request.text) == obj1
+
+ # Filters to catch posted JSON
+ mock_requests.post(
+ '//test:8080/v1/test',
+ status_code=200,
+ request_headers={'Content-Type': 'application/json'},
+ text='test_post_json_request',
+ additional_matcher=match_obj1
+ )
+
+ with mock.patch(
+ 'airflow.hooks.base_hook.BaseHook.get_connection',
+ side_effect=get_airflow_connection
+ ):
+ # Send obj1 as JSON and verify it matched the mock
+ resp = self.post_hook.run('v1/test', json=obj1)
+ self.assertEqual(resp.status_code, 200)
+ self.assertEqual(resp.text, 'test_post_json_request')
+ # Ensure that obj1 was what was sent
+ with self.assertRaises(requests_mock.exceptions.NoMockAddress):
+ resp = self.post_hook.run('v1/test', json=obj2)
Review comment:
ok i was confused by the fact that you are looking for `NoMockAddress` error it's a little misleading but i now understand that's how requests mock is designed.
but anyway, so you are trying to test the test here. i think you should chop this. just test your feature, don't test the functionality of requests mock. fine to do it locally to satisfy yourself that it's working right, but i don't think it belongs in here permanently.
that said... this test-of-the-test is working right anyway, because on line 368 if you replace `obj2` with `obj1`, the test still passes. this is because you aren't mocking `airflow.hooks.base_hook.BaseHook.get_connection` at this point of the code, so it is finding no match, irrespective of the object you supply.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/4bfde026d14441a9fcb26e5e7f465def3f13eaa5?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
==========================================
- Coverage 84.81% 84.52% -0.29%
==========================================
Files 679 679
Lines 38495 38495
==========================================
- Hits 32649 32538 -111
- Misses 5846 5957 +111
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <100%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `91.59% <0%> (-0.29%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [4bfde02...37517d9](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/0f7c4563d2f8885e12312aae72bc70a22f734894?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
==========================================
- Coverage 84.7% 84.41% -0.29%
==========================================
Files 679 679
Lines 38489 38489
==========================================
- Hits 32601 32491 -110
- Misses 5888 5998 +110
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <100%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [0f7c456...d42617f](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/374cee0af30d7233698fd813213b7c732cbe284f?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.39% 84.1% -0.29%
=========================================
Files 680 680
Lines 38501 38501
=========================================
- Hits 32492 32381 -111
- Misses 6009 6120 +111
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <75%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `91.59% <0%> (-0.29%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [374cee0...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#issuecomment-568618871
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=h1) Report
> Merging [#6886](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/374cee0af30d7233698fd813213b7c732cbe284f?src=pr&el=desc) will **decrease** coverage by `0.28%`.
> The diff coverage is `75%`.
[](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6886 +/- ##
=========================================
- Coverage 84.39% 84.1% -0.29%
=========================================
Files 680 680
Lines 38501 38501
=========================================
- Hits 32492 32381 -111
- Misses 6009 6120 +111
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `95.45% <75%> (ø)` | :arrow_up: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
| [...rflow/contrib/operators/kubernetes\_pod\_operator.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZF9vcGVyYXRvci5weQ==) | `78.75% <0%> (-20%)` | :arrow_down: |
| [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/6886/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `91.59% <0%> (-0.29%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6886?src=pr&el=footer). Last update [374cee0...5881853](https://codecov.io/gh/apache/airflow/pull/6886?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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] baolsen commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
baolsen commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r361609168
##########
File path: airflow/hooks/http_hook.py
##########
@@ -75,20 +75,24 @@ def get_conn(self, headers=None):
try:
session.headers.update(conn.extra_dejson)
except TypeError:
- self.log.warning('Connection to %s has invalid extra field.', conn.host)
+ self.log.warning(
+ 'Connection to %s has invalid extra field.', conn.host)
Review comment:
Thanks, I'll remove them
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services
[GitHub] [airflow] baolsen commented on a change in pull request #6886:
[AIRFLOW-6327] http_hook: Accept json= parameter for payload
Posted by GitBox <gi...@apache.org>.
baolsen commented on a change in pull request #6886: [AIRFLOW-6327] http_hook: Accept json= parameter for payload
URL: https://github.com/apache/airflow/pull/6886#discussion_r362205118
##########
File path: airflow/hooks/http_hook.py
##########
@@ -123,6 +127,7 @@ def run(self, endpoint, data=None, headers=None, extra_options=None):
req = requests.Request(self.method,
url,
data=data,
+ json=json,
Review comment:
That's neat, I hadn't seen that before. Thanks :) I've included your suggested changes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please 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
With regards,
Apache Git Services