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

Destination Snowflake: cleaning up unused code paths #38553

Merged
merged 1 commit into from
May 23, 2024

Conversation

gisripa
Copy link
Contributor

@gisripa gisripa commented May 22, 2024

What

Remove remnants of SwitchingDestination and decouple Snowflake from AbstractJdbcDestination base implementation (nothing is reused except a check related function)

User Impact

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

Copy link

vercel bot commented May 22, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
airbyte-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 23, 2024 2:18am

@octavia-squidington-iii octavia-squidington-iii added area/connectors Connector related issues CDK Connector Development Kit labels May 22, 2024
@gisripa gisripa changed the title snowflake-clean-unused Destination Snowflake: cleaning up unused code paths May 22, 2024
@gisripa gisripa force-pushed the gireesh/05-21-snowflake-clean-unused branch from a3d34bc to 217c373 Compare May 22, 2024 01:46
Copy link
Contributor

@edgao edgao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tiny nits. I only skimmed SnowflakeDestination, since I assume that's mostly just autoconverted code?

// verify if user has permission to make SQL INSERT queries
try {
if (attemptInsert) {
sqlOps.insertRecords(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sigh. snowflake shouldn't even be using this method. fine for now, but eventually we should have snowflake do the actual internal stage + COPY thing for check

@@ -361,4 +362,9 @@ private String toJdbcTypeName(final AirbyteProtocolType airbyteProtocolType) {
};
}

@Override
public void createNamespaces(@NotNull Set<String> schemas) {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: is it safe to throw new UnsupportedOperationException("not yet implemented")? (just to be explicit that this should be implemented, we just haven't gotten around to it yet

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah was planning to add that. just to be sure it doesn't get called erroneously.

@gisripa gisripa force-pushed the gireesh/05-21-snowflake-clean-unused branch 4 times, most recently from e67a86e to 41fd326 Compare May 22, 2024 18:50
@gisripa gisripa changed the base branch from master to gireesh/cdk-cleanup-snowflake May 22, 2024 18:50
@octavia-squidington-iii octavia-squidington-iii removed the CDK Connector Development Kit label May 22, 2024
@gisripa gisripa marked this pull request as ready for review May 22, 2024 18:52
@gisripa gisripa requested a review from a team as a May 22, 2024 18:52
@gisripa gisripa force-pushed the gireesh/05-21-snowflake-clean-unused branch from 41fd326 to ba3d8fc Compare May 22, 2024 18:54
@gisripa gisripa force-pushed the gireesh/cdk-cleanup-snowflake branch from 71b6cba to 3ac8c86 Compare May 22, 2024 22:37
@gisripa gisripa force-pushed the gireesh/05-21-snowflake-clean-unused branch from ba3d8fc to e3601d8 Compare May 22, 2024 22:37
Base automatically changed from gireesh/cdk-cleanup-snowflake to master May 22, 2024 22:56
@gisripa gisripa requested a review from a team as a code owner May 22, 2024 22:56
@gisripa gisripa force-pushed the gireesh/05-21-snowflake-clean-unused branch from e3601d8 to 1eb280e Compare May 22, 2024 23:22
@octavia-squidington-iii octavia-squidington-iii added the area/documentation Improvements or additions to documentation label May 22, 2024
@gisripa gisripa force-pushed the gireesh/05-21-snowflake-clean-unused branch from 1eb280e to 429ce3a Compare May 23, 2024 02:14
@gisripa gisripa merged commit f6fdaf3 into master May 23, 2024
30 checks passed
@gisripa gisripa deleted the gireesh/05-21-snowflake-clean-unused branch May 23, 2024 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/destination/snowflake
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants