-
Notifications
You must be signed in to change notification settings - Fork 514
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
Change scio-smb to depend on provided scio io modules #5004
Conversation
build.sbt
Outdated
`scio-test` % "test;it", | ||
`scio-avro` % IntegrationTest | ||
`scio-avro` % Provided, | ||
`scio-google-cloud-platform` % Provided, |
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.
smb json is using BQ TableRow
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.
Wut
build.sbt
Outdated
// runtime | ||
"org.apache.beam" % "beam-sdks-java-io-hadoop-format" % beamVersion % Runtime, | ||
"org.apache.hadoop" % "hadoop-client" % hadoopVersion % Runtime, |
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.
No need to define runtime deps anymore, since the'll be pulled by the provided modules
build.sbt
Outdated
"org.apache.avro" % "avro" % avroVersion % Provided, // scio-avro | ||
"org.apache.beam" % "beam-sdks-java-extensions-avro" % beamVersion % Provided, // scio-avro | ||
"org.apache.beam" % "beam-sdks-java-extensions-protobuf" % beamVersion % Provided, // scio-tensorflow | ||
"org.apache.beam" % "beam-sdks-java-io-google-cloud-platform" % beamVersion % Provided, // scio-gcp | ||
"org.apache.beam" % "beam-sdks-java-io-hadoop-common" % beamVersion % Provided, // scio-parquet | ||
"org.apache.hadoop" % "hadoop-common" % hadoopVersion % Provided, // scio-parquet | ||
"org.apache.parquet" % "parquet-avro" % parquetVersion % Provided excludeAll ("org.apache.avro" % "avro"), // scio-parquet | ||
"org.apache.parquet" % "parquet-column" % parquetVersion % Provided, // scio-parquet | ||
"org.apache.parquet" % "parquet-common" % parquetVersion % Provided, // scio-parquet | ||
"org.apache.parquet" % "parquet-hadoop" % parquetVersion % Provided, // scio-parquet |
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 comment for required compile provided dep, indicating the scio module origin
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5004 +/- ##
=======================================
Coverage 63.34% 63.34%
=======================================
Files 291 291
Lines 10840 10840
Branches 753 753
=======================================
Hits 6867 6867
Misses 3973 3973 ☔ View full report in Codecov by Sentry. |
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.
Looks good, maybe we should update https://spotify.github.io/scio/extras/Sort-Merge-Bucket.html to explain the change/how users should import these libs
640afd8
to
0ac4fff
Compare
@@ -1046,6 +1046,9 @@ lazy val `scio-parquet` = project | |||
"org.slf4j" % "slf4j-api" % slf4jVersion, | |||
// provided | |||
"org.tensorflow" % "tensorflow-core-api" % tensorFlowVersion % Provided, | |||
// runtime | |||
"org.apache.hadoop" % "hadoop-client" % hadoopVersion % Runtime excludeAll (Exclude.metricsCore), |
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.
This dep is declared as provided in beam parquet io here.
Moved the dependency from scio-smb
to scio-parquet
0ac4fff
to
a968ff8
Compare
No description provided.