:bricks: private preview with Unity catalog and Oauth using service principal by gisripa · Pull Request #37613 · airbytehq/airbyte

Gireesh Sreepathi (gisripa) · 2024-04-26T04:51:51Z

Review guide

Ignore deletes in src/main/java, /src/test/java, /src/test-integration/java. Old code deleted.
Databricks destination using AsyncConsumer, V2 column names, Stripped down version of classes needed for Staging and TypeDedupe.

User Impact

This is a breaking change and a private preview release. Destination only supports CSV file format at the moment and works with Unity catalog for tables and Volumes. Cloud and OSS are pinned to 1.1.0 which was the last known stable release of the community connector.
🥳

Can this PR be safely reverted and rolled back?

NOPEEEE ❌ - 1.1.0 version pinned in Cloud and OSS

vercel · 2024-04-26T04:51:58Z

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 19, 2024 5:55pm

Gireesh Sreepathi (gisripa) · 2024-05-03T21:43:51Z

Working sync with CSV format

Stephane Geneix (stephane-airbyte) · 2024-05-09T18:16:03Z

-    applicationDefaultJvmArgs = ['-XX:+ExitOnOutOfMemoryError', '-XX:MaxRAMPercentage=75.0']
+    mainClass = 'io.airbyte.integrations.destination.databricks.DatabricksDestinationKt'
+    applicationDefaultJvmArgs = ['-XX:+ExitOnOutOfMemoryError', '-XX:MaxRAMPercentage=75.0',
+                                 '-XX:NativeMemoryTracking=detail', '-XX:+UnlockDiagnosticVMOptions',


add some comment as to why this is here

Stephane Geneix (stephane-airbyte) · 2024-05-09T18:22:59Z

+import io.airbyte.integrations.destination.databricks.model.OAuth2Authentication
+import javax.sql.DataSource
+
+object ConnectorClientsFactory {


what do we think about always prefixing our connector-specific classes with the connector name (DatabricksConnectorClientFactory here) ?

Stephane Geneix (stephane-airbyte) · 2024-05-09T18:25:31Z

+                connectorConfig.authentication
+            )
+        val storageOperations =
+            DatabricksStorageOperations(


dumb question: is there any case where we'll use the DatabricksStorageOperations with anything else than those particular types? Should we just create them inside that constructor?

injected as dependency for mocking them, although didn't write unit test in this 😅 . Will do that in CDK PR when doing the bigquery thing now.

I think we'll want to experiment with how we test these components. E.g. do we ever want to test sqlgenerator/destinationhandler/etc in isolation, or are we always just testing the entire StorageOperations object?

(my personal guess is that we'll mostly just run integration tests against FooStorageOperations - e.g. our BaseSqlGeneratorIntegrationTest is also expanding to exercise destinationhandler stuff)

also, up for discovery: is it actually just StagingDestinationStorageOperations, and we want to inject databricks stuff through the connector? Or is there actual databricks-specific stuff directly in the StorageOperations impl? (I don't know the answer, this is something we should figure out)

Yeah, if we can eventually fold our SqlGen tests as StorageOps tests. TBH it needs for injection are ConnectorConfig static info like schema, catalog, jdbcurl and both SqlGen and DestHandler can be internally instantiated.

Edward Gao (edgao)

I only skimmed this, fully expecting us to make a lot of changes while we're working on the bigquery stuff. Just a couple nits/info comments.

Edward Gao (edgao) · 2024-05-09T20:28:14Z

    sl: 100
    ql: 100
  supportLevel: community
+  supportsDbt: true


I don't think we want this? my understanding (80%) is that this triggers platform's custom dbt transform thing, which they recently killed

copy-pasta. I'll remove it.

Edward Gao (edgao) · 2024-05-09T20:34:28Z

+import io.airbyte.commons.jackson.MoreMappers
+
+data class DatabricksConnectorConfig(
+    @JsonProperty("accept_terms") val termsAccepted: Boolean,


nit: our spec.json already has an explicit title field to control what the UI, why not just use camelCase for the object keys?

Edward Gao (edgao) · 2024-05-09T20:58:48Z

+                connectorConfig.authentication
+            )
+        val storageOperations =
+            DatabricksStorageOperations(


I think we'll want to experiment with how we test these components. E.g. do we ever want to test sqlgenerator/destinationhandler/etc in isolation, or are we always just testing the entire StorageOperations object?

(my personal guess is that we'll mostly just run integration tests against FooStorageOperations - e.g. our BaseSqlGeneratorIntegrationTest is also expanding to exercise destinationhandler stuff)

also, up for discovery: is it actually just StagingDestinationStorageOperations, and we want to inject databricks stuff through the connector? Or is there actual databricks-specific stuff directly in the StorageOperations impl? (I don't know the answer, this is something we should figure out)

Evan Tahler (evantahler) · 2024-05-10T20:19:09Z

 plugins {
    id 'application'
    id 'airbyte-java-connector'
-    id "de.undercouch.download" version "5.0.1"


lol, what did this do?

😂 no clue

Evan Tahler (evantahler) · 2024-05-10T20:19:24Z

    sl: 100
    ql: 100
  supportLevel: community
+  supportsDbt: true


Evan Tahler (evantahler) · 2024-05-10T20:20:03Z

@@ -344,7 +344,8 @@ Delta Lake tables are created. You may want to consult the tutorial on
 ## CHANGELOG


There will be a lot more docs to update once we are ready to release - fine to skip that for now

Gireesh Sreepathi (gisripa) added 8 commits April 21, 2024 17:40

nuke it all

cb5ac6d

stash musings

178d752

simplify

3b6d729

add defaults/optionals

d5fe4e7

stash it

481c3f9

Another stash

ac58279

T+D and good stuff stashed

5c46247

Merge branch 'master' into gireesh/masonry

8f8ddb0

Octavia Squidington III (octavia-squidington-iii) added area/connectors Connector related issues CDK Connector Development Kit connectors/destination/databricks labels Apr 26, 2024

Gireesh Sreepathi (gisripa) changed the title ~~Mason at work with bricks~~ AirMason: at work with bricks Apr 26, 2024

Gireesh Sreepathi (gisripa) changed the title ~~AirMason: at work with bricks~~ AirMason: at work with 🧱 Apr 26, 2024

Gireesh Sreepathi (gisripa) added 4 commits April 26, 2024 13:11

stash

c51cd4d

allTypes test passing

bec2705

SqlGen tests green

7305182

Merge branch 'master' into gireesh/masonry

dfa823b

vercel Bot deployed to Preview May 2, 2024 17:02 View deployment

Gireesh Sreepathi (gisripa) added 8 commits May 2, 2024 10:57

Merge branch 'master' into gireesh/masonry

7a49728

placeholder

aa5c453

Merge branch 'master' into gireesh/masonry

ba407e2

T+D test wiring

8925962

T+D tests green

28cb5a1

revert verifyRecords to private

19cfa6a

cleanup

424235b

multi-thread init and finalize

26acc4d

minor stash

aa6e90c

Edward Gao (edgao) reviewed May 7, 2024

View reviewed changes

Comment thread ...ain/kotlin/io/airbyte/integrations/destination/databricks/sync/DatabricksStreamOperations.kt Outdated

Octavia Squidington III (octavia-squidington-iii) added the area/documentation Improvements or additions to documentation label May 9, 2024

vercel Bot deployed to Preview May 9, 2024 18:03 View deployment

Gireesh Sreepathi (gisripa) changed the title ~~AirMason: at work with 🧱~~ 🧱 - private preview with Unity catalog and Oauth using service principal May 9, 2024

Gireesh Sreepathi (gisripa) changed the title ~~🧱 - private preview with Unity catalog and Oauth using service principal~~ 🧱 private preview with Unity catalog and Oauth using service principal May 9, 2024

Gireesh Sreepathi (gisripa) marked this pull request as ready for review May 9, 2024 18:06

Gireesh Sreepathi (gisripa) requested review from a team as code owners May 9, 2024 18:06

Stephane Geneix (stephane-airbyte) reviewed May 9, 2024

View reviewed changes

pr comments

814cf50

Edward Gao (edgao) approved these changes May 9, 2024

View reviewed changes

Gireesh Sreepathi (gisripa) added 2 commits May 9, 2024 16:37

Fix for checks

1a9f07d

Merge branch 'master' into gireesh/masonry

e95d80f

Octavia Squidington III (octavia-squidington-iii) removed the CDK Connector Development Kit label May 10, 2024

useLocalCdk false

b22e57f

Evan Tahler (evantahler) suggested changes May 10, 2024

View reviewed changes

Merge branch 'master' into gireesh/masonry

3b21e1e

vercel Bot deployed to Preview May 17, 2024 21:36 View deployment

Gireesh Sreepathi (gisripa) added 2 commits May 17, 2024 15:22

Upgrade to latest CDK, use new interfaces

56e0e51

formatfix

fb7e05f

Evan Tahler (evantahler) approved these changes May 17, 2024

View reviewed changes

migrations placeholder

2c1684e

vercel Bot deployed to Preview May 18, 2024 00:03 View deployment

fix qa check

ad609dd

vercel Bot deployed to Preview May 19, 2024 17:28 View deployment

yet another qa check

43fd319

vercel Bot deployed to Preview May 19, 2024 17:55 View deployment

Gireesh Sreepathi (gisripa) merged commit c627ce8 into master May 19, 2024

Gireesh Sreepathi (gisripa) deleted the gireesh/masonry branch May 19, 2024 18:08

		@@ -344,7 +344,8 @@ Delta Lake tables are created. You may want to consult the tutorial on
		## CHANGELOG

Uh oh!

Conversation

Gireesh Sreepathi (gisripa) commented Apr 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review guide

User Impact

Can this PR be safely reverted and rolled back?

Uh oh!

vercel Bot commented Apr 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Gireesh Sreepathi (gisripa) commented May 3, 2024

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Edward Gao (edgao) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Gireesh Sreepathi (gisripa) commented Apr 26, 2024 •

edited

Loading

vercel Bot commented Apr 26, 2024 •

edited

Loading