scala version upgrade: 2.12 -> 2.13 #51
Conversation
ShreeshaS01
commented
Dec 17, 2024
•
ShreeshaS01
commented
- Scala upgrade: 2.12 to 2.13
- Third party dependency: Upgrade affected classes are separated
- Change in root pom.xml to provide support for multiple scala and spark version build
|
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. |
ShreeshaS01
changed the title
There was a problem hiding this comment.
Why do we have a README-template and a README?
| @@ -13,11 +13,11 @@ You can access the connector in two different ways: | |||
| 1. From | |||
| our [Maven Central repository](https://repo1.maven.org/maven2/com/google/cloud/spark/bigtable). | |||
| 2. Through a public GCS bucket, located | |||
| at `gs://spark-lib/bigtable/spark-bigtable_2.12-<version>.jar`. | |||
| at `gs://spark-lib/bigtable/spark-bigtable_2.13-<version>.jar`. | |||
There was a problem hiding this comment.
Instead of replacing let's add both links and mention that one is for Scala 2.12 and the other 2.13
There was a problem hiding this comment.
Same comment for other changes in this file
| <groupId>com.google.cloud.spark.bigtable</groupId> | ||
| <artifactId>spark-bigtable-common</artifactId> | ||
| <version>0.2.1</version> | ||
| <relativePath>../../spark-bigtable-common/pom.xml</relativePath> |
There was a problem hiding this comment.
I don't think ../../spark-bigtable-common exists. Also spark-bigtable-core is probably a better name for the core functionality, and then maintain the naming pattern for the child packages (spark-bigtable-scala2.12 and spark-bigtable-scala2.13). This will make it more explicit 1) what 2.12/2.13 refers to and 2) spark-bigtable refers to the packages here in general
There was a problem hiding this comment.
In the same spirit, can we remove the intermediate folder ./bigtable-support and just have folders ./spark-bigtable-core, ./spark-bigtable-scala2.12 and ./spark-bigtable-scala2.13
There was a problem hiding this comment.
I think the license for this source code requires us to have a notice for all modifications. See https://github.com/GoogleCloudDataproc/spark-bigtable-connector/blob/main/third_party/hbase-spark-connector/LICENSE
(b) You must cause any modified files to carry prominent notices stating that You changed the files;
I'll double check with someone and comment here again
| <groupId>com.google.cloud.spark.bigtable</groupId> | ||
| <artifactId>spark-bigtable-connector</artifactId> | ||
| <version>0.2.1</version> | ||
| <relativePath>../../pom.xml</relativePath> |
There was a problem hiding this comment.
This package's parent should likely be -common (or -core as suggested on the other pom file)
There was a problem hiding this comment.
I think this was not intentional?
There was a problem hiding this comment.
Did we removed the renaming of these packages intentionally?
| @@ -220,6 +236,8 @@ | |||
| </transformers> | |||
| <artifactSet> | |||
| <excludes> | |||
| <exclude>com.google.cloud:google-cloud-bigtable</exclude> | |||
There was a problem hiding this comment.
Why the exclusions and relocations?
There was a problem hiding this comment.
I think the license for this source code requires us to have a notice for all modifications. See https://github.com/GoogleCloudDataproc/spark-bigtable-connector/blob/main/third_party/hbase-spark-connector/LICENSE
(b) You must cause any modified files to carry prominent notices stating that You changed the files;
I'll double check with someone and comment here again
There was a problem hiding this comment.
Also, don't we need to have a version for 2.12 of this class as well?
