Skip to content

Add sql_hook_params parameter to SqlToS3Operator #33425

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

Merged
merged 1 commit into from
Aug 16, 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 parameter name, "sql_hook_params", as already used in SqlToSlackOperator.

(This is related to #33427 which adds sql_hook_params to the opposite transfer, S3ToSqlOperator, however that one needed a different approach by using BaseHook.get_hook instead of Connection.get_hook)


^ 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:providers provider:amazon AWS/Amazon - related issues labels Aug 16, 2023
@alexbegg alexbegg changed the title Adding sql_hook_params to S3ToSql and SqlToS3 operators Add sql_hook_params to S3ToSql and SqlToS3 operators Aug 16, 2023
@alexbegg alexbegg force-pushed the SqlToS3-hook_params branch 2 times, most recently from e904703 to 9903043 Compare August 16, 2023 00:34
@alexbegg alexbegg marked this pull request as draft August 16, 2023 00:40
@alexbegg alexbegg force-pushed the SqlToS3-hook_params branch from 9903043 to 0ab1663 Compare August 16, 2023 01:16
@alexbegg alexbegg marked this pull request as ready for review August 16, 2023 01:16
@alexbegg
Copy link
Contributor Author

alexbegg commented Aug 16, 2023

FYI, I added a test to tests/providers/amazon/aws/transfers/test_s3_to_sql.py that is passing, but the other 2 tests in that file are failing. The code for those tests has not changed in months, and I have not changed the setup method at all. I don't know what is causing this.

@uranusjr
Copy link
Member

Because you changed how the BaseHook is accessed and it breaks the mock.

@alexbegg alexbegg force-pushed the SqlToS3-hook_params branch from 0ab1663 to c356333 Compare August 16, 2023 05:56
@alexbegg alexbegg changed the title Add sql_hook_params to S3ToSql and SqlToS3 operators Add sql_hook_params to SqlToS3Operator Aug 16, 2023
@alexbegg alexbegg changed the title Add sql_hook_params to SqlToS3Operator Add sql_hook_params parameter to SqlToS3Operator Aug 16, 2023
@alexbegg alexbegg force-pushed the SqlToS3-hook_params branch from c356333 to dc943c0 Compare August 16, 2023 05:59
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
Copy link
Contributor Author

I have now split the S3ToSqlOperator changes into its own PR: #33427, it needed a different approach since it uses BaseHook.get_hook instead of Connection.get_hook. Since they take different approaches, I think they should be reviewed separately.

@vincbeck vincbeck changed the title Add sql_hook_params parameter to SqlToS3Operator Add sql_hook_params parameter to SqlToS3Operator Aug 16, 2023
@vincbeck vincbeck merged commit 45d5f64 into apache:main Aug 16, 2023
@alexbegg alexbegg deleted the SqlToS3-hook_params branch August 16, 2023 18:10
ferruzzi pushed a commit to aws-mwaa/upstream-to-airflow that referenced this pull request Aug 17, 2023
Adding `sql_hook_params` parameter to `SqlToS3Operator`. This will allow you to pass extra config params to the underlying SQL hook.
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