Re: [PR] Deferrable HttpSensor does not move to Triggerer when using response_check [airflow]

["subbota19 (via GitHub)" <gi...@apache.org>]

subbota19 commented on code in PR #47719:
URL: https://github.com/apache/airflow/pull/47719#discussion_r2225989164


##########
providers/http/src/airflow/providers/http/triggers/http.py:
##########
@@ -201,15 +201,21 @@ async def run(self) -> AsyncIterator[TriggerEvent]:
         while True:
             try:
                 async with aiohttp.ClientSession() as session:
-                    await hook.run(
+                    client_response = await hook.run(
                         session=session,
                         endpoint=self.endpoint,
                         data=self.data,
                         headers=self.headers,
                         extra_options=self.extra_options,

Review Comment:
   Hi @dabla , sorry for raising this here, but while testing your update locally, I came across a related issue that’s worth mentioning, so let me explain it, the issue can be reproduced with:
   
   HttpSensor(
       ...
       extra_options={'check_response': False},
       deferrable=True,
       ...
   )
   
   In the HttpAsyncHook.run() method **extra_options** are passed correctly, but later on [line 471](https://github.com/apache/airflow/blob/71b03470da2d104a07a68da2d496fa9ed253f335/providers/http/src/airflow/providers/http/hooks/http.py#L471), when **request_func** is called, it fails with:
   
   TypeError: ClientSession._request() got an unexpected keyword argument 'check_response'
   
   This happens because we're passing **extra_options** directly without filtering, while aiohttp.ClientSession.request() expects valid options (proxies, cert, etc.), so the root cause is that we should be using the result of the **_process_extra_options_from_connection()** which correctly filters out valid parameters. 
   
   So instead of:
    await session.request(..., **extra_options)
   We should use:
    await session.request(..., **extra) on [line](https://github.com/apache/airflow/blob/3.0.3/providers/http/src/airflow/providers/http/hooks/http.py#L478)
    
   Again, this isn't caused by your changes but rather by a previous oversight. If you’d like to include the fix here - please do it; if not I can open PR with a fix and tests for 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.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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