sidshas03 commented on code in PR #55694:
URL: https://github.com/apache/airflow/pull/55694#discussion_r2356126337
##########
providers/amazon/src/airflow/providers/amazon/aws/hooks/athena_sql.py:
##########
@@ -65,8 +65,41 @@ class AthenaSQLHook(AwsBaseHook, DbApiHook):
hook_name = "Amazon Athena"
supports_autocommit = True
- def __init__(self, athena_conn_id: str = default_conn_name, *args,
**kwargs) -> None:
- super().__init__(*args, **kwargs)
+ def __init__(
+ self,
+ athena_conn_id: str | None = None, # keep positional compatibility
+ *,
+ s3_staging_dir: str | None = None,
+ work_group: str | None = None,
+ driver: str | None = None,
+ aws_domain: str | None = None,
+ session_kwargs: dict | None = None,
+ config_kwargs: dict | None = None,
+ role_arn: str | None = None,
+ assume_role_method: str | None = None,
+ assume_role_kwargs: dict | None = None,
+ aws_session_token: str | None = None,
+ endpoint_url: str | None = None,
+ **kwargs,
+ ) -> None:
+ # prefer explicit arg; fall back to kwargs; finally default
+ if athena_conn_id is None:
+ athena_conn_id = kwargs.pop("athena_conn_id",
self.default_conn_name)
+ else:
+ kwargs.pop("athena_conn_id", None) # avoid conflicts
+ super().__init__(**kwargs)
+ # Store explicit params on self
Review Comment:
> Btw, why did we flip back to setting all of these on the init constructor
vs fetching them from kwargs? Didn't you say in a previous message that you
were going to keep the kwargs pop?
Right, I moved to explicit __init__ params based on the feedback from
@vincbeck and @jroachgolf84 to match other AWS hooks. I’ve removed the
kwargs.pop fallback entirely and kept athena_conn_id: str = default_conn_name
for BC. No functional change otherwise.
--
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-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
