[GitHub] [airflow] AmarEL commented on a change in pull request #13499: Add a new argument for HttpSensor to accept a list of http status code to Continue Poking

[GitBox <gi...@apache.org>]

AmarEL commented on a change in pull request #13499:
URL: https://github.com/apache/airflow/pull/13499#discussion_r553058065



##########
File path: airflow/providers/http/sensors/http.py
##########
@@ -90,7 +96,8 @@ def __init__(
         self.headers = headers or {}
         self.extra_options = extra_options or {}
         self.response_check = response_check
-
+        self.acceptable_status_code = acceptable_status_code or []
+        self.acceptable_status_code.append(404)

Review comment:
       Yep, it's true. I take a deep look and notice the same now. Sorry
   
   But in order for the `response_check` works for other codes than 404, it's necessary to fill the param `extra_option` with the value `{"check_response": False}`.
   
   There is a reference for it in the hook method `run`
   >         :param extra_options: additional options to be used when executing the request
   >            i.e. {'check_response': False} to avoid checking raising exceptions on non
   >            2XX or 3XX status codes
   
   So, my changes are not necessary to reach this goal.
   
   I will rewrite the tests for cases with `extra_option` and try to improve the doc section with this information of `{"check_response": False}` since it's an internal optional behavior in the hook.




----------------------------------------------------------------
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