-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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 databricks: Remove user/pass auth #40712
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
This was referenced Jul 3, 2024
This stack of pull requests is managed by Graphite. Learn more about stacking. |
a7384fd
to
aa5f4d5
Compare
035032d
to
f8228c3
Compare
edgao
commented
Jul 3, 2024
override val imageName: String | ||
get() = "airbyte/destination-databricks:dev" | ||
|
||
companion object { | ||
private var jdbcDatabase: JdbcDatabase = Mockito.mock() | ||
private var connectorConfig: DatabricksConnectorConfig = Mockito.mock() |
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.
we need these variables to be instanced per connector config, so push them down into their respective class' companion object
edgao
commented
Jul 3, 2024
...rbyte/integrations/destination/databricks/typededupe/AbstractDatabricksTypingDedupingTest.kt
Show resolved
Hide resolved
f8228c3
to
06ee74e
Compare
ed23537
to
4b5fff5
Compare
14ca76b
to
19b3cf4
Compare
19b3cf4
to
d537847
Compare
d537847
to
3766180
Compare
gisripa
approved these changes
Aug 12, 2024
3766180
to
dda8229
Compare
dda8229
to
5f06e35
Compare
3d76924
to
8c959c8
Compare
86e4091
to
75ee096
Compare
75ee096
to
b74867f
Compare
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/databricks
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes https://github.com/airbytehq/airbyte-internal-issues/issues/9146
(note: I don't think there's much urgency on this PR, in spite of the ostensible deadline. Databricks is clearly sending comms to all of their users already, so 🤷 hopefully anyone using airbyte+databricks already knows they should switch to oauth)
Databricks will deprecate user/pass auth on july 10
but PATs will continue working
AFAICT we don't actually need the user+pass? I.e. a PAT is sufficient. (relevant docs) So just... stop using the user+pass stuff
Also move oauth to be the first option, since it's the recommended auth type
(I created a databricks PAT + uploaded it into https://console.cloud.google.com/security/secret-manager/secret/SECRET_DESTINATION_DATABRICKS_PERSONAL_ACCESS_TOKEN_CONFIG/versions?project=dataline-integration-testing)
Closes https://github.com/airbytehq/airbyte-internal-issues/issues/9146