-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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: support oauth #10953
🎉 Source Snowflake: support oauth #10953
Conversation
1efdefc
to
0bf86fe
Compare
a3bd575
to
5fdadb2
Compare
...flake/src/main/java/io.airbyte.integrations.source.snowflake/SnowflakeAccessTokenLoader.java
Outdated
Show resolved
Hide resolved
...source-snowflake/src/main/java/io.airbyte.integrations.source.snowflake/SnowflakeSource.java
Outdated
Show resolved
Hide resolved
new IntegrationRunner(source).run(args); | ||
isSourceAlive = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this code doesn't look good, can isSourceAlive
and tokenLoader
logic be moved in a SnowflakeAccessTokenLoader or to a separate component?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
105fcda
to
335cdb3
Compare
airbyte-oauth/src/test/java/io/airbyte/oauth/flows/SnowflakeOAuthFlowTest.java
Show resolved
Hide resolved
if (config.has("jdbc_url_params")) { | ||
jdbcUrl.append(config.get("jdbc_url_params").asText()); | ||
} | ||
hikariConfig.setJdbcUrl(jdbcUrl.toString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you move the logic of the URL building into a separate method? also, it would be great to cover it with some unit tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
properties.put("authenticator", "oauth"); | ||
properties.put("token", accessToken); | ||
properties.put("account", config.get("host").asText()); | ||
hikariConfig.setDataSourceProperties(properties); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we move properties populating block into a separate method to make it more readable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
5d58623
to
d84c0cb
Compare
/test connector=connectors/source-snowflake
|
/test connector=connectors/source-snowflake
|
01268e0
to
ff588a2
Compare
/test connector=connectors/source-snowflake
|
ff588a2
to
36af69e
Compare
/test connector=connectors/source-snowflake
|
/publish connector=connectors/source-snowflake
|
* add oauth flow to source-snowflake * Add unit test for oauth flow * add docs to method * format code * change configs * fixed remarks * fixed comments * fixed PR remark * update with master * format code * fix PR remmakrs * add test for backward compatibility * bump version * small fix for test * auto-bump connector version Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
What
Fixes #10002
Implemented OAuth flow to Snowflake source.
Add option to select authorization method:
Authorization method "Username and password":
Authorization method "OAuth2.0":
How
SourceSnowflakeOAuthFlow.class
. Add OAuth flow to make authentication and get access and refresh tokens.spec.json
. Change structure of specification. Added OAuth params. Split authentication methods to provide backward compatibility with username and password authentication.SourceSnowflake.class
. Updated reading configuration due to the new structure of spec.json. OwerridedcreateDatabase()
to useHikariDataSource
which has ability to refresh access token for connection pool in execution time.SnowflakeDataSourceUtils.class
. Utility class that has methods toHikariDataSource
and refresh token every 7 minutes (access token expires in 10 mins).Recommended reading order
spec.json
SourceSnowflakeOAuthFlow.class
SourceSnowflake.class
SnowflakeDataSourceUtils.class
🚨 User Impact 🚨
Added new authorization method. The existing auth method (username and password) moved to the bottom of the setting window. User is able to select which authorization method to use.
Pre-merge Checklist
Expand the relevant checklist and delete the others.
New Connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/SUMMARY.md
docs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampledocs/integrations/README.md
airbyte-integrations/builds.md
Airbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>
command is passing/publish
command described hereUpdating a connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampleAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>
command is passing/publish
command described hereConnector Generator
-scaffold
in their name) have been updated with the latest scaffold by running./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates
then checking in your changesTests
Unit
Put your unit tests output here.
Integration
Put your integration tests output here.
Acceptance
Put your acceptance tests output here.