Re: [PR] Disallow create/extend thick-wrapped hooks in Amazon Provider [airflow]

2023-10-30 Thread via GitHub
vincbeck merged PR #35163: URL: https://github.com/apache/airflow/pull/35163 -- 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

Re: [PR] Disallow create/extend thick-wrapped hooks in Amazon Provider [airflow]

2023-10-24 Thread via GitHub
o-nikolas commented on PR #35163: URL: https://github.com/apache/airflow/pull/35163#issuecomment-1778007125 > I have a look in detail on listed hook and some of them created on Airflow 1.10.0, some other migrated from old `boto` to `boto3` in the period of airflow 1.8 - 1.10 (s3/dynamodb)

Re: [PR] Disallow create/extend thick-wrapped hooks in Amazon Provider [airflow]

2023-10-24 Thread via GitHub
Taragolis commented on PR #35163: URL: https://github.com/apache/airflow/pull/35163#issuecomment-1777990166 I have a look in detail on listed hook and some of them created on Airflow 1.10.0, some other migrated from old `boto` to `boto3` in the period of airflow 1.8 - 1.10 (s3/dynamodb)

Re: [PR] Disallow create/extend thick-wrapped hooks in Amazon Provider [airflow]

2023-10-24 Thread via GitHub
Taragolis commented on PR #35163: URL: https://github.com/apache/airflow/pull/35163#issuecomment-1777884728 If you need create boto3 client then is already exists `AwsBaseHook(aws_conn_id="foo", client_type="one-of-355-aws-service").conn` maybe this behaviour not well documented ---

Re: [PR] Disallow create/extend thick-wrapped hooks in Amazon Provider [airflow]

2023-10-24 Thread via GitHub
o-nikolas commented on code in PR #35163: URL: https://github.com/apache/airflow/pull/35163#discussion_r1370676442 ## tests/providers/amazon/aws/hooks/test_hooks_signature.py: ## Review Comment: I think keeping the constructor args clean could be nice, but I'm not sure we

Re: [PR] Disallow create/extend thick-wrapped hooks in Amazon Provider [airflow]

2023-10-24 Thread via GitHub
vincbeck commented on PR #35163: URL: https://github.com/apache/airflow/pull/35163#issuecomment-1777804841 Interesting! I am divided on this one to be honest. 1. I like the fact to try to normalize all hooks within the Amazon provider package. I agree that contributors, who might not

Re: [PR] Disallow create/extend thick-wrapped hooks in Amazon Provider [airflow]

2023-10-24 Thread via GitHub
ferruzzi commented on PR #35163: URL: https://github.com/apache/airflow/pull/35163#issuecomment-151491 I would argue the exact opposite. If a hook is just a thin wrapper around boto, then it might as well be a taskflow boto call. I'd argue that if we were going to make a restriction,

[PR] Disallow create/extend thick-wrapped hooks in Amazon Provider [airflow]

2023-10-24 Thread via GitHub
Taragolis opened a new pull request, #35163: URL: https://github.com/apache/airflow/pull/35163 Add simple test which are validate that hooks based on AwsGenericHook / AwsBaseHook allow only common AWS Hooks parameters, such as: `aws_conn_id`, `region_name`, `verify`, `config