-
Notifications
You must be signed in to change notification settings - Fork 34
Addition of Databricks Plugin #680
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| # Databricks Batch Source | ||
|
|
||
| Description | ||
| ----------- | ||
| Reads data from a Databricks table using a configurable SQL query. | ||
|
|
||
| Properties | ||
| ---------- | ||
| * **Use Connection**: Whether to use an existing Databricks connection. | ||
| * **Host**: Server Hostname of the Databricks cluster or SQL warehouse. | ||
| * **Port**: Database port (default is 443). | ||
| * **HTTP Path**: The HTTP Path for the Databricks cluster or SQL warehouse. | ||
| * **Reference Name**: Name used to identify this source for lineage. | ||
| * **Database / Catalog**: Optional catalog or database name. | ||
| * **Import Query**: SQL query to execute against Databricks. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| # Databricks Database Connector | ||
|
|
||
| Description | ||
| ----------- | ||
| Connects to Databricks database / Lakehouse via JDBC. | ||
|
|
||
| Properties | ||
| ---------- | ||
| * **Host**: Server Hostname of the Databricks cluster or SQL warehouse. | ||
| * **Port**: Database port (default is 443). | ||
| * **HTTP Path**: The HTTP Path for the Databricks cluster or SQL warehouse. | ||
| * **Database / Catalog**: Optional catalog or database name to connect to. | ||
| * **Username**: Username / token user. | ||
| * **Password / Token**: Personal Access Token (PAT) or password. | ||
| * **Connection Arguments**: Arbitrary key-value pairs to pass as connection arguments to the JDBC driver (e.g. `AuthMech=11;Auth_Flow=2`). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <!-- | ||
| Copyright © 2026 CDAP | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); you may not | ||
| use this file except in compliance with the License. You may obtain a copy of | ||
| the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, WITHOUT | ||
| WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the | ||
| License for the specific language governing permissions and limitations under | ||
| the License. | ||
| --> | ||
| <project xmlns="http://maven.apache.org/POM/4.0.0" | ||
| xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
| xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> | ||
| <parent> | ||
| <artifactId>database-plugins-parent</artifactId> | ||
| <groupId>io.cdap.plugin</groupId> | ||
| <version>1.13.0-SNAPSHOT</version> | ||
| </parent> | ||
|
|
||
| <name>Databricks plugin</name> | ||
| <artifactId>databricks-plugin</artifactId> | ||
| <modelVersion>4.0.0</modelVersion> | ||
|
|
||
| <properties> | ||
| <databricks-jdbc.version>3.4.3</databricks-jdbc.version> | ||
| </properties> | ||
|
|
||
| <dependencies> | ||
| <dependency> | ||
| <groupId>io.cdap.cdap</groupId> | ||
| <artifactId>cdap-etl-api</artifactId> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>io.cdap.plugin</groupId> | ||
| <artifactId>database-commons</artifactId> | ||
| <version>${project.version}</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>io.cdap.plugin</groupId> | ||
| <artifactId>hydrator-common</artifactId> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>com.google.guava</groupId> | ||
| <artifactId>guava</artifactId> | ||
| </dependency> | ||
|
|
||
| <!-- test dependencies --> | ||
| <dependency> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This dependency isn't referenced by any test in the PR — nothing loads the driver, so it's currently dead weight in the build. That's really a symptom of the wider gap: there's no DatabricksPluginTestBase, DatabricksPluginTestSuite, DatabricksSourceTestRun, DatabricksFailedConnectionTest or DatabricksDBRecordUnitTest here. Every other plugin in the repo ships that set — see amazon-redshift-plugin/src/test/.../RedshiftPluginTestBase.java (218 lines) as the closest template. The two unit tests in this PR don't touch DatabricksSource, DatabricksDBRecord, or any connection path. For a brand-new plugin I'd like at least a DatabricksPluginTestBase following the Redshift pattern, which would also give this dependency a purpose. Separately, could you attach evidence of a real end-to-end run (browse → get schema → sample → pipeline read) against a SQL warehouse, over a table containing TIMESTAMP, TIMESTAMP_NTZ, DATE, DECIMAL(38,10), ARRAY, MAP, STRUCT, VARIANT and a NULL-only column? That's the set most likely to break and none of it is covered today. |
||
| <groupId>com.databricks</groupId> | ||
| <artifactId>databricks-jdbc</artifactId> | ||
| <version>${databricks-jdbc.version}</version> | ||
| <scope>test</scope> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>io.cdap.plugin</groupId> | ||
| <artifactId>database-commons</artifactId> | ||
| <version>${project.version}</version> | ||
| <type>test-jar</type> | ||
| <scope>test</scope> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>io.cdap.cdap</groupId> | ||
| <artifactId>hydrator-test</artifactId> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>io.cdap.cdap</groupId> | ||
| <artifactId>cdap-data-pipeline3_2.12</artifactId> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>junit</groupId> | ||
| <artifactId>junit</artifactId> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.mockito</groupId> | ||
| <artifactId>mockito-core</artifactId> | ||
| <scope>test</scope> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>io.cdap.cdap</groupId> | ||
| <artifactId>cdap-api</artifactId> | ||
| <scope>provided</scope> | ||
| </dependency> | ||
| </dependencies> | ||
|
|
||
| <build> | ||
| <plugins> | ||
| <plugin> | ||
| <groupId>io.cdap</groupId> | ||
| <artifactId>cdap-maven-plugin</artifactId> | ||
| </plugin> | ||
| <plugin> | ||
| <groupId>org.apache.felix</groupId> | ||
| <artifactId>maven-bundle-plugin</artifactId> | ||
| <version>5.1.2</version> | ||
| <extensions>true</extensions> | ||
| <configuration> | ||
| <instructions> | ||
| <_exportcontents> | ||
| io.cdap.plugin.databricks.*; | ||
| io.cdap.plugin.db.source.*; | ||
| org.apache.commons.lang; | ||
| org.apache.commons.logging.*; | ||
| org.codehaus.jackson.* | ||
| </_exportcontents> | ||
| <Embed-Dependency>*;inline=false;scope=compile</Embed-Dependency> | ||
| <Embed-Transitive>true</Embed-Transitive> | ||
| <Embed-Directory>lib</Embed-Directory> | ||
| </instructions> | ||
| </configuration> | ||
| <executions> | ||
| <execution> | ||
| <phase>package</phase> | ||
| <goals> | ||
| <goal>bundle</goal> | ||
| </goals> | ||
| </execution> | ||
| </executions> | ||
| </plugin> | ||
| </plugins> | ||
| </build> | ||
| </project> | ||
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.
getInputFormatProvider() runs for every DB connector in this repo — MySQL, Postgres, Oracle, SQL Server, Redshift — not just Databricks.
The defaulting looks correct to me (isAutoCommitEnabled() → false reproduces the old hard-coded false, and a null isolation level leaves TransactionIsolationLevel.CONF_KEY unset so getLevel(null) still yields SERIALIZABLE). But nothing asserts that, so a future change to either default would silently alter connection behaviour for every other plugin with no test failing.
Could you add coverage in database-commons for:
an overriding connector setting both.
database-commons/src/test/java/io/cdap/plugin/db/source/DataDrivenETLDBInputFormatTest.java is a reasonable place
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.
Added unit tests in DataDrivenETLDBInputFormatTest to verify the default values of isAutoCommitEnabled() (false) and getTransactionIsolationLevel() (null), as well as getInputFormatProvider() configuration for both default (non-overriding) and overriding connectors.
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.
Added unit tests in DataDrivenETLDBInputFormatTest to verify the default values of isAutoCommitEnabled() (false) and getTransactionIsolationLevel() (null), as well as getInputFormatProvider() configuration for both default (non-overriding) and overriding connectors.