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

spark-bigtable-connector: Scala Version Upgrade (2.12 -> 2.13) #52

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ShreeshaS01
Copy link

parent PR - #51

  • scala upgrade (2.12 -> 2.13)
  • module restructure (separate module for scala2.12 and scala2.13)
  • third_party folder is restructured: separate folders for SchemaConverters class are created (scala2.12 and scala2.13)
  • common spark bigtable source code is kept under spark-bigtable-core folder (pom.xml is removed from this folder as only source files will be used in both scala2.12 and scala2.13)
  • all the common dependencies are written in root pom.xml

- module restructure (separate module for scala2.12 and scala2.13)
- third_party folder is restructured: separate folders for SchemaConverters class are created (scala2.12 and scala2.13)
- common spark bigtable source code is kept under spark-bigtable-core folder (pom.xml is removed as this source files will be used both in scala2.12 and scala2.13 with third_party source code)
Copy link

google-cla bot commented Dec 19, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@@ -56,8 +56,12 @@ object WordCount extends App {
)
.drop("frequency_double")

// "bigtable" is not working - throwing Data Source not found
// when full path like below is provided, it works!
val bigtableFormat = "com.google.cloud.spark.bigtable.BigtableDefaultSource"

Choose a reason for hiding this comment

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

Does this work without this PR's changes? If so this would be a breaking change and we should try to understand why it is failing

Copy link
Author

Choose a reason for hiding this comment

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

Resolved. Now it is working with "bigtable"

Choose a reason for hiding this comment

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

Copy link
Author

@ShreeshaS01 ShreeshaS01 Dec 23, 2024

Choose a reason for hiding this comment

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

added

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

added

pom.xml Outdated
@@ -94,6 +94,198 @@
<commons-lang.version>2.6</commons-lang.version>
<openlineage.version>1.22.0</openlineage.version>
</properties>
<dependencies>

Choose a reason for hiding this comment

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

Should some (all?) of these dependencies be on the -core package? Things like google-cloud-bigtable and such are dependencies for core, but since the parent package doesn't really have any source code we can leave these out of here and keep this pom focused on package assembly

Copy link
Author

Choose a reason for hiding this comment

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

core is just to provide all the source file. Created a new module (spark-bigtable). scala2.12 and scala2.13 are placed inside it. common dependencies are part of this new module now.

- created a main module and common dependency are moved from root pom to spark-bigtable/pom
- spark.read.format("bigtable") is working properly now
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants