Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add sql_hook_params parameter to S3ToSqlOperator #33427

Merged
merged 1 commit into from
Aug 19, 2023

Conversation

alexbegg
Copy link
Contributor

@alexbegg alexbegg commented Aug 16, 2023

Adding sql_hook_params parameter to SqlToS3Operator. This will allow you to pass extra config params to the underlying SQL hook.

This uses the same "sql_hook_params" parameter name as already used in SqlToSlackOperator.

(This is related to #33425 which adds sql_hook_params to the opposite transfer, SqlToS3Operator)


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborg boring-cyborg bot added area:core-operators Operators, Sensors and hooks within Core Airflow area:providers provider:amazon AWS/Amazon - related issues labels Aug 16, 2023
@alexbegg
Copy link
Contributor Author

alexbegg commented Aug 16, 2023

@uranusjr This PR retains how S3ToSqlOperator accesses BaseHook, but adds a way to pass hook_params parameter to BaseHook.get_hook, which then calls Connection.get_hook (which itself already accepts hook_params).

@alexbegg alexbegg force-pushed the S3ToSql-hook_params branch from 2c5f979 to f4b0838 Compare August 16, 2023 06:58
@alexbegg alexbegg marked this pull request as draft August 16, 2023 07:36
@vincbeck vincbeck changed the title Add sql_hook_params parameter to S3ToSqlOperator Add sql_hook_params parameter to S3ToSqlOperator Aug 16, 2023
@alexbegg alexbegg force-pushed the S3ToSql-hook_params branch from f4b0838 to 28d191d Compare August 18, 2023 21:57
Adding `sql_hook_params` parameter to `SqlToS3Operator`. This will allow you to pass extra config params to the underlying SQL hook.

This uses the same "sql_hook_params" parameter name as already used in `SqlToSlackOperator`.
@alexbegg alexbegg force-pushed the S3ToSql-hook_params branch from 28d191d to dbe29a3 Compare August 18, 2023 22:48
@alexbegg alexbegg marked this pull request as ready for review August 18, 2023 22:48
@alexbegg
Copy link
Contributor Author

@uranusjr I corrected the test to access the hook using the changed method of getting it from Connection.get_hook (which supports hook_params) instead of BaseHook.get_hook (which doesn't), the test passes now.

@alexbegg
Copy link
Contributor Author

alexbegg commented Aug 18, 2023

Please remove area:core-operators label from this PR as it no longer changes the core operators

@alexbegg alexbegg requested a review from uranusjr August 18, 2023 22:56
@potiuk potiuk removed the area:core-operators Operators, Sensors and hooks within Core Airflow label Aug 19, 2023
@potiuk potiuk merged commit 1407e27 into apache:main Aug 19, 2023
@alexbegg alexbegg deleted the S3ToSql-hook_params branch August 19, 2023 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers provider:amazon AWS/Amazon - related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants