Skip to content

Conversation

@kazanzhy
Copy link
Contributor

Method get_db_hook was used only in DatabricksSqlOperator.
These changes might make code cleaner

@dstandish
Copy link
Contributor

no concerns with this but either you should keep the old method with deprecation warning or you must bump major

@kazanzhy
Copy link
Contributor Author

Hmm. But this method is defined within BaseSQLOperator which is not a user-end class.
Therefore I expect that it shouldn't be used by users

@kazanzhy kazanzhy force-pushed the remove_get_db_hook_method branch from 8a6d9b3 to 46962a8 Compare November 18, 2022 22:18
@kazanzhy kazanzhy marked this pull request as ready for review November 18, 2022 22:19
@kazanzhy kazanzhy requested a review from eladkal as a code owner November 18, 2022 22:19
@eladkal
Copy link
Contributor

eladkal commented Nov 18, 2022

Hmm. But this method is defined within BaseSQLOperator which is not a user-end class. Therefore I expect that it shouldn't be used by users

It is user facing as users can build custom operators with it. Any class in providers code that we didnt declare as private should go through deprecation process.

@kazanzhy kazanzhy marked this pull request as draft November 20, 2022 00:30
@kazanzhy kazanzhy force-pushed the remove_get_db_hook_method branch 2 times, most recently from dbd3362 to 08e3f65 Compare November 28, 2022 00:05
@kazanzhy kazanzhy force-pushed the remove_get_db_hook_method branch from 08e3f65 to b1a8b85 Compare November 30, 2022 23:52
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Jan 15, 2023
@github-actions github-actions bot closed this Jan 21, 2023
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 provider:common-sql provider:databricks provider:snowflake Issues related to Snowflake provider stale Stale PRs per the .github/workflows/stale.yml policy file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants