Skip to content

Destinations bigquery+snowflake+s3: Chore: upgrade CDK version - #33263

Merged
Edward Gao (edgao) merged 9 commits into
masterfrom
edgao/destination_cdk_bump
Dec 8, 2023
Merged

Edward Gao (edgao) merged 9 commits into
masterfrom
edgao/destination_cdk_bump

Conversation

@edgao

Copy link
Copy Markdown
Contributor

in #32326 we made some breaking changes to the java destination cdk. Upgrade the cdk version in bigquery+snowflake+s3 and fix their respective builds.

(only snowflake actually needed code changes)

I originally had these as part of #33232, figured it's probably better to merge this stuff separately.

@edgao
Edward Gao (edgao) requested a review from a team as a code owner December 8, 2023 19:13
@vercel

vercel Bot commented Dec 8, 2023 •

Copy link
Copy Markdown

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 Dec 8, 2023 9:41pm

@github-actions

github-actions Bot commented Dec 8, 2023 •

Copy link
Copy Markdown
Contributor

Before Merging a Connector Pull Request

Wow! What a great pull request you have here! 🎉

To merge this PR, ensure the following has been done/considered for each connector added or updated:

  • PR name follows PR naming conventions
  • Breaking changes are considered. If a Breaking Change is being introduced, ensure an Airbyte engineer has created a Breaking Change Plan.
  • Connector version has been incremented in the Dockerfile and metadata.yaml according to our Semantic Versioning for Connectors guidelines
  • You've updated the connector's metadata.yaml file any other relevant changes, including a breakingChanges entry for major version bumps. See metadata.yaml docs
  • Secrets in the connector's spec are annotated with airbyte_secret
  • All documentation files are up to date. (README.md, bootstrap.md, docs.md, etc...)
  • Changelog updated in docs/integrations/<source or destination>/<name>.md with an entry for the new version. See changelog example
  • Migration guide updated in docs/integrations/<source or destination>/<name>-migrations.md with an entry for the new version, if the version is a breaking change. See migration guide example
  • If set, you've ensured the icon is present in the platform-internal repo. (Docs)

If the checklist is complete, but the CI check is failing,

  1. Check for hidden checklists in your PR description

  2. Toggle the github label checklist-action-run on/off to re-run the checklist CI.


@Override
protected JdbcSqlGenerator getSqlGenerator() {
throw new UnsupportedOperationException("Snowflake does not yet use the native JDBC DV2 interface");

@edgao Edward Gao (edgao) Dec 8, 2023 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we probably should do this eventually, but not a high priority imo

}

@Override
protected void insertRecordsInternalV2(final JdbcDatabase jdbcDatabase, final List<PartialAirbyteMessage> list, final String s, final String s1) throws Exception {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure we ever want to do this? Since snowflake only has an internal staging loading method, i.e. standard inserts doesn't exist at all

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So the insertRecordsInternal is also dead code path ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

wasn't sure, did some digging just now. I suspect it's used as part of the check method... which it probably shouldn't be. Specifically this line dispatches to JdbcSqlOperations, which invokes insertRecordsInternal.

but otherwise yes, dead code

@edgao
Edward Gao (edgao) merged commit a9b1e8d into master Dec 8, 2023
@edgao
Edward Gao (edgao) deleted the edgao/destination_cdk_bump branch December 8, 2023 22:24

This branch was successfully deployed

1 active deployment
Preview — 36e9d9f0 Deployed Dec 8, 2023 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants