sidshas03 commented on PR #55694: URL: https://github.com/apache/airflow/pull/55694#issuecomment-3299877045
Thanks for the review, @vincbeck @o-nikolas. I’ve pushed **198c5e1**. **Updates** * Removed issue references from comments/docstrings (took the inline suggestions). * **get\_conn()** now also respects `driver` and `aws_domain` on `self`, in line with `_get_conn_params()` / `get_uri()`. * Added **`test_hook_params_override_extras()`** to confirm `hook_params` override connection `extra` for `s3_staging_dir`, `work_group`, `driver`, and `aws_domain`. * Test clean-ups: fixed imports/patching to match existing patterns. Breeze run shows static checks and the targeted unit tests passing. **On the constructor suggestion** For now I’ve kept the `kwargs.pop(...)` pattern to avoid widening the `__init__` signature and to stay consistent with other AWS hooks. If you’d prefer explicit optional params (e.g., `s3_staging_dir: str | None = None`, `work_group`, `driver`, `aws_domain`), I can change this PR or do a follow-up whichever you recommend. Please let me know if you want any more changes. -- 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]
