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

🎉 Source Snowflake : Update JDBC driver for Snowflake to 3.13.22 #16766

Merged
merged 30 commits into from Sep 21, 2022

Conversation

akashkulk
Copy link
Contributor

@akashkulk akashkulk commented Sep 15, 2022

What

🎉 Source Snowflake : Per this build, Airbyte is currently using version 3.13.9 of the JDBC drive for Snowflake. That driver is almost a year old and should be updated to the latest version 3.13.22, as of the creation of this ticket. We should consider updating our driver as well as putting in a mechanism to keep it updated on an ongoing basis.

Link for the repo with all of the drivers:
https://repo1.maven.org/maven2/net/snowflake/snowflake-jdbc/

Resolve #16552

How

Update the driver to the latest version (3.13.22)

Pre-merge Checklist (connector update)

Community member or Airbyter

  • Grant edit access to maintainers (instructions)
  • Secrets in the connector's spec are annotated with airbyte_secret
  • Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
  • Code reviews completed
  • Documentation updated
    • Connector's README.md
    • Connector's bootstrap.md. See description and examples
    • Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
  • PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

  • Create a non-forked branch based on this PR and test the below items on it
  • Build is successful
  • If new credentials are required for use in CI, add them to GSM. Instructions.
  • /test connector=connectors/<name> command is passing
  • New Connector version released on Dockerhub and connector version bumped by running the /publish command described here

@akashkulk akashkulk requested a review from a team as a code owner September 15, 2022 00:16
@CLAassistant
Copy link

CLAassistant commented Sep 15, 2022

CLA assistant check
All committers have signed the CLA.

@github-actions
Copy link
Contributor

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

  • destination-snowflake
  • source-snowflake

@tuliren
Copy link
Contributor

tuliren commented Sep 15, 2022

@akashkulk, don't forget to sign the CLA.

Copy link
Contributor

@tuliren tuliren left a comment

Choose a reason for hiding this comment

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

Please also bump the connector version in the dockerfile.

@tuliren
Copy link
Contributor

tuliren commented Sep 15, 2022

/test connector=connectors/source-snowflake

🕑 connectors/source-snowflake https://github.com/airbytehq/airbyte/actions/runs/3057412352
❌ connectors/source-snowflake https://github.com/airbytehq/airbyte/actions/runs/3057412352
🐛 https://gradle.com/s/rgprbtrm2arsq

Build Failed

Test summary info:

Could not find result summary

@tuliren
Copy link
Contributor

tuliren commented Sep 15, 2022

/test connector=connectors/destination-snowflake

🕑 connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/3057413050
❌ connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/3057413050
🐛 https://gradle.com/s/tfmwancrq4cec

Build Failed

Test summary info:

Could not find result summary

@akashkulk
Copy link
Contributor Author

akashkulk commented Sep 15, 2022

/test connector=connectors/destination-snowflake

🕑 connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/3061470044
❌ connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/3061470044
🐛 https://gradle.com/s/zkrlq64g73ioi

Build Failed

Test summary info:

Could not find result summary

@akashkulk
Copy link
Contributor Author

akashkulk commented Sep 15, 2022

/test connector=connectors/source-snowflake

🕑 connectors/source-snowflake https://github.com/airbytehq/airbyte/actions/runs/3061477663
❌ connectors/source-snowflake https://github.com/airbytehq/airbyte/actions/runs/3061477663
🐛 https://gradle.com/s/k2q6cqjipkdqi

Build Failed

Test summary info:

Could not find result summary

@github-actions
Copy link
Contributor

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

  • source-snowflake
  • destination-snowflake

@github-actions
Copy link
Contributor

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

  • destination-snowflake
  • source-snowflake

@akashkulk
Copy link
Contributor Author

akashkulk commented Sep 15, 2022

/test connector=connectors/source-snowflake

🕑 connectors/source-snowflake https://github.com/airbytehq/airbyte/actions/runs/3062256924
❌ connectors/source-snowflake https://github.com/airbytehq/airbyte/actions/runs/3062256924
🐛 https://gradle.com/s/dyaqz5makpdys

Build Failed

Test summary info:

Could not find result summary

@akashkulk
Copy link
Contributor Author

akashkulk commented Sep 15, 2022

/test connector=connectors/destination-snowflake

🕑 connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/3062258072
❌ connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/3062258072
🐛 https://gradle.com/s/yke74alvxmb6y

Build Failed

Test summary info:

Could not find result summary

@github-actions
Copy link
Contributor

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

  • source-snowflake
  • destination-snowflake

Copy link
Contributor

@tuliren tuliren left a comment

Choose a reason for hiding this comment

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

  • The version bump is correct~~
  • You should also update the changelog in source/snowflake.md and destination/snowflake.md.
  • The test failure seems persistent and related to the version bump. For example, the source always had two failed integration tests. Can you take a look into the root cause? Do they pass locally?

@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Sep 15, 2022
@github-actions
Copy link
Contributor

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

  • destination-snowflake
  • source-snowflake

@akashkulk
Copy link
Contributor Author

akashkulk commented Sep 15, 2022

/test connector=connectors/source-snowflake

🕑 connectors/source-snowflake https://github.com/airbytehq/airbyte/actions/runs/3063044233
❌ connectors/source-snowflake https://github.com/airbytehq/airbyte/actions/runs/3063044233
🐛 https://gradle.com/s/pilrnepownraa

Build Failed

Test summary info:

Could not find result summary

@akashkulk akashkulk changed the title 🎉 Destination & Source Snowflake : Update JDBC driver for Snowflake 🎉 Source Snowflake : Update JDBC driver for Snowflake Sep 19, 2022
@github-actions
Copy link
Contributor

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

  • source-snowflake

@github-actions
Copy link
Contributor

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

  • source-snowflake

@akashkulk akashkulk marked this pull request as ready for review September 21, 2022 17:24
@akashkulk akashkulk changed the title 🎉 Source Snowflake : Update JDBC driver for Snowflake 🎉 Source Snowflake : Update JDBC driver for Snowflake to 3.13.22 Sep 21, 2022
@akashkulk
Copy link
Contributor Author

akashkulk commented Sep 21, 2022

/test connector=connectors/source-snowflake

🕑 connectors/source-snowflake https://github.com/airbytehq/airbyte/actions/runs/3099970026
✅ connectors/source-snowflake https://github.com/airbytehq/airbyte/actions/runs/3099970026
No Python unittests run

Build Passed

Test summary info:

All Passed

Copy link
Contributor

@tuliren tuliren left a comment

Choose a reason for hiding this comment

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

Nice!

Can you go through the pre-merge checklist and check off the items accordingly? For example, you should update the CHANGELOG in snowflake.md to add the information about the new version.

@github-actions
Copy link
Contributor

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

  • source-snowflake

@github-actions
Copy link
Contributor

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

  • source-snowflake

@akashkulk
Copy link
Contributor Author

Nice!

Can you go through the pre-merge checklist and check off the items accordingly? For example, you should update the CHANGELOG in snowflake.md to add the information about the new version.

Thanks. I've checked off the relevant items. Just to confirm - after you approve these changes, I should (i) publish the new docker image and then (ii) merge the PR, correct?

@github-actions
Copy link
Contributor

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

  • source-snowflake

@akashkulk
Copy link
Contributor Author

akashkulk commented Sep 21, 2022

/publish connector=connectors/source-snowflake run-tests=false

🕑 Publishing the following connectors:
connectors/source-snowflake
https://github.com/airbytehq/airbyte/actions/runs/3101628032


Connector Did it publish? Were definitions generated?
connectors/source-snowflake

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@github-actions
Copy link
Contributor

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

  • source-snowflake

@akashkulk akashkulk temporarily deployed to more-secrets September 21, 2022 23:06 Inactive
@akashkulk akashkulk merged commit 44ef1c4 into master Sep 21, 2022
@akashkulk akashkulk deleted the akashkulk/snowflake-jdbc-driver branch September 21, 2022 23:07
robbinhan pushed a commit to robbinhan/airbyte that referenced this pull request Sep 29, 2022
…bytehq#16766)

Update JDBC driver for Snowflake

Co-authored-by: Liren Tu <tuliren.git@outlook.com>
Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
jhammarstedt pushed a commit to jhammarstedt/airbyte that referenced this pull request Oct 31, 2022
…bytehq#16766)

Update JDBC driver for Snowflake

Co-authored-by: Liren Tu <tuliren.git@outlook.com>
Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update JDBC driver for Snowflake source
4 participants