New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Log warning message when response_check
attribute is passed in HttpSensorAsync
#780
Conversation
Codecov ReportBase: 98.50% // Head: 98.50% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #780 +/- ##
=======================================
Coverage 98.50% 98.50%
=======================================
Files 87 87
Lines 4745 4746 +1
=======================================
+ Hits 4674 4675 +1
Misses 71 71
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
@@ -42,7 +24,8 @@ def response_check(response, task_instance): | |||
:type request_params: a dictionary of string key/value pairs | |||
:param headers: The HTTP headers to be added to the GET request | |||
:type headers: a dictionary of string key/value pairs | |||
:param response_check: A check against the 'requests' response object. | |||
:param response_check: This param is currently not supported. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove this param from the docstring here altogether since we're not supporting it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, agreed with @pankajkoti can we remove this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why remove this? Removing docstring won't mean that the param isn't supported for Async.
Instead just add a log.warn
line in the Operator's init or execute method to say it will use Synchronous operation since you passed response_check
Yes sure will add the log message. |
b60587a
to
d688ef3
Compare
HttpSensorAsync
response_check
attribute is passed in HttpSensorAsync
Update the docstrings for HttpSensorAsync as discussed in https://astronomer.slack.com/archives/C0387PMM2E7/p1665393838125069..
closes #706