HyunWooZZ commented on code in PR #60689:
URL: https://github.com/apache/airflow/pull/60689#discussion_r2701014921
##########
providers/snowflake/src/airflow/providers/snowflake/hooks/snowflake_sql_api.py:
##########
@@ -78,6 +79,7 @@ class SnowflakeSqlApiHook(SnowflakeHook):
:param token_renewal_delta: Renewal time of the JWT Token in timedelta
:param deferrable: Run operator in the deferrable mode.
:param api_retry_args: An optional dictionary with arguments passed to
``tenacity.Retrying`` & ``tenacity.AsyncRetrying`` classes.
+ :param http_timeout_seconds: Optional timeout for http requests.
Review Comment:
> think we should avoid it.
This is just 1 parameter out of so many that can be passed to request.
I rather have request_kwargs then each user can pass any parameter accepted
by requests.
First i understood change :param http_timeout_seconds: -> :param
request_kwargs:.
The main issue with a single request_kwargs is that we have two different
underlying clients (requests for sync and aiohttp for async) with non-identical
kwargs and timeout semantics.
For example, sync requests accepts timeout as a number or (connect, read)
tuple, while aiohttp uses ClientTimeout(...) (and the keys/types could be
differ).
Because above reason, I just wanted to hide complexity, and focuse on how to
use hook not struggling with other options.
Also we need to guard against users overriding hook-controlled fields (like:
method/url/headers/params), so we can’t blindly forward everything without
validation.
That’s why I initially kept an explicit http_timeout_seconds to provide
consistent behavior across sync and async both with minimal surface area.
If the project prefers a kwargs-based passing, I’m happy to refactor into
separate kwargs (e.g., http_request_kwargs, aiohttp_session_kwargs,
aiohttp_request_kwargs) or another pattern you recommend.
>I think that is how we do it in other http related operators?
I reference dbt hook :)
https://airflow.apache.org/docs/apache-airflow-providers-dbt-cloud/stable/_modules/airflow/providers/dbt/cloud/hooks/dbt.html
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
