[BEAM-10961] Enable strict dependency analysis on all Java modules #12938

sonam-vend · 2020-09-25T11:41:11Z

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

Choose reviewer(s) and mention them in a comment (R: @username).
Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
Update CHANGES.md with noteworthy changes.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

Post-Commit Tests Status (on master branch)

Lang	Dataflow	Samza	Twister2
Go	---	---	---
Java
Python		---	---
XLang	---	---	---

Pre-Commit Tests Status (on master branch)

---	Java	Python	Go	Website	Whitespace	Typescript
Non-portable
Portable	---		---	---	---	---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

codecov · 2020-10-01T11:10:14Z

Codecov Report

Merging #12938 (7cc00b0) into master (2f205f1) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #12938   +/-   ##
=======================================
  Coverage   82.73%   82.74%           
=======================================
  Files         466      466           
  Lines       57518    57518           
=======================================
+ Hits        47587    47591    +4     
+ Misses       9931     9927    -4

Impacted Files	Coverage Δ
sdks/python/apache_beam/runners/direct/executor.py	`96.29% <0.00%> (-0.53%)`	⬇️
...hon/apache_beam/runners/worker/bundle_processor.py	`93.83% <0.00%> (+0.12%)`	⬆️
sdks/python/apache_beam/dataframe/frames.py	`91.98% <0.00%> (+0.41%)`	⬆️
.../python/apache_beam/transforms/periodicsequence.py	`98.24% <0.00%> (+1.75%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f205f1...7cc00b0. Read the comment docs.

aaltay · 2020-10-08T18:40:12Z

What is Nebula plugin? Why does Beam need this?

sonam-vend · 2020-12-31T11:13:17Z

Run SQL PreCommit

sonam-vend · 2020-12-31T12:42:47Z

I didn't have time to go through all of this today. To be honest, I don't think there's any way we can merge a single PR changing the entire project at once. I recommend splitting this PR into separate PRs, one for each module, so that it's easier to review and merge safely.

A couple general comments:

If a dependency is defined in project.ext.library, always use that value instead of hard-coding it. For example, use library.java.google_oauth_client instead of "com.google.oauth-client:google-oauth-client:1.31.0".

This is purely cosmetic, but try to keep the dependency lists alphabetized. It makes them much easier to read.

@ibzib , I consider it is quite large and critical change to be merged all at once. Responding to your suggestion of splitting this PR into multiple separate PR, I am thinking to split this PR into 8 sub PRs as:

Module A | beam/example/java
Module B | beam/model
Module C | beam/runners/core-construction-java, runners/core-java, runners/direct-java, runners/extensions-java
Module D | runners/flink, runners/google-cloud-dataflow-java,runners/google-cloud-dataflow-java, runners/java-fn-execution, runners/java-job-service, runners/jet
Module E | runners/local-java, runners/portability, runners/samza, runners/spark, runners/twister2
Module F | sdks/java/bom, sdks/java/build-tools, sdks/java/container, sdks/java/core
Module G | sdks/java/expansion-service, sdks/java/extensions, sdks/java/fn-execution, sdks/java/harness
Module H | sdks/java/io, sdks/java/javadoc, sdks/java/maven-archetypes, sdks/java/testing

See if this proposal looks good to you, or else I welcome your other suggestions.

BEAM-10961.xlsx

cc @shehzaadn-vd

shehzaadn-vd · 2021-01-02T05:48:11Z

runners/flink/1.12/build.gradle

@@ -17,6 +17,7 @@
 */

 def basePath = '..'
+


@sonam-vend this is a stray change. please revert.

shehzaadn-vd · 2021-01-02T05:48:28Z

runners/flink/1.9/build.gradle

@@ -17,6 +17,7 @@
 */

 def basePath = '..'
+


stray change please revert.

shehzaadn-vd · 2021-01-02T05:48:43Z

runners/flink/1.9/build.gradle

@@ -31,3 +32,4 @@ project.ext {

 // Load the main build script which contains all build logic.
 apply from: "$basePath/flink_runner.gradle"
+


stray change - please revert

shehzaadn-vd · 2021-01-02T05:49:51Z

runners/google-cloud-dataflow-java/worker/build.gradle

  compile library.java.slf4j_jdk14
-
+  permitUnusedDeclared library.java.slf4j_jdk14


nitpick - there should be a be a blank line about the comment - like there was before.

shehzaadn-vd · 2021-01-02T05:50:23Z

runners/google-cloud-dataflow-java/worker/legacy-worker/build.gradle

    runtime library.java.slf4j_jdk14
-


nitpick - there should be blank line above the comment like before

shehzaadn-vd · 2021-01-02T05:51:09Z

runners/portability/java/build.gradle

  compile project(":runners:java-fn-execution")
  compile project(":runners:java-job-service")
  compile project(path: ":sdks:java:harness", configuration: "shadow")
  compile library.java.vendored_grpc_1_26_0
  compile library.java.slf4j_api
-
+  compile library.java.joda_time


nitpick - there should be a blank line between "compile" and "testcompile" lines like before

shehzaadn-vd · 2021-01-02T05:51:34Z

sdks/java/build-tools/build.gradle

@@ -20,3 +20,4 @@ plugins { id 'org.apache.beam.module' }
 applyJavaNature(exportJavadoc: false, publish: false)

 description = "Apache Beam :: SDKs :: Java :: Build Tools"
+


stray blank line - please remove

shehzaadn-vd · 2021-01-02T05:52:46Z

sdks/java/io/google-cloud-platform/build.gradle

  compile library.java.grpc_google_cloud_pubsub_v1
-  compile library.java.grpc_google_cloud_pubsublite_v1
+  //compile library.java.grpc_google_cloud_pubsublite_v1


this line should be removed - not commented out.

shehzaadn-vd · 2021-01-02T05:53:09Z

sdks/java/io/hadoop-format/build.gradle

@@ -68,6 +69,8 @@ dependencies {
  testCompile project(path: ":sdks:java:testing:test-utils", configuration: "testRuntime")
  testCompile project(":sdks:java:io:jdbc")
  testCompile project(path: ":examples:java", configuration: "testRuntime")
+


stray blank line - please remove

shehzaadn-vd · 2021-01-02T05:53:31Z

sdks/java/io/kinesis/expansion-service/build.gradle

  runtime library.java.slf4j_jdk14
 }
+


stray blank line - please remove

shehzaadn-vd · 2021-01-02T05:54:29Z

sdks/java/testing/kafka-service/build.gradle

@@ -13,7 +13,7 @@
 * 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.
+ * limitations under the License
 */


period removed by mistake. please bring it back.

shehzaadn-vd · 2021-01-02T05:55:11Z

sdks/java/testing/load-tests/build.gradle

  compile project(":sdks:java:io:synthetic")
  compile project(":sdks:java:testing:test-utils")
  compile project(":sdks:java:io:google-cloud-platform")
  compile project(":sdks:java:io:kafka")
  compile project(":sdks:java:io:kinesis")
-


there should be an empty line between "compile" and "gradleRun" lines - like there was before

shehzaadn-vd · 2021-01-02T05:55:56Z

sdks/java/testing/test-utils/build.gradle

@@ -30,12 +30,14 @@ dependencies {
  compile project(path: ":sdks:java:core", configuration: "shadow")
  compile library.java.vendored_guava_26_0_jre
  compile library.java.google_cloud_bigquery
-  compile project(":sdks:java:extensions:google-cloud-platform-core")
-
+  compile "com.google.code.gson:gson:2.8.6"


there should be a blank line between compile and testCompile lines - like there was before

ibzib · 2021-01-03T00:03:09Z

@ibzib , I consider it is quite large and critical change to be merged all at once. Responding to your suggestion of splitting this PR into multiple separate PR, I am thinking to split this PR into 8 sub PRs as:

Module A | beam/example/java

Module B | beam/model

Module C | beam/runners/core-construction-java, runners/core-java, runners/direct-java, runners/extensions-java

Module D | runners/flink, runners/google-cloud-dataflow-java,runners/google-cloud-dataflow-java, runners/java-fn-execution, runners/java-job-service, runners/jet

Module E | runners/local-java, runners/portability, runners/samza, runners/spark, runners/twister2

Module F | sdks/java/bom, sdks/java/build-tools, sdks/java/container, sdks/java/core

Module G | sdks/java/expansion-service, sdks/java/extensions, sdks/java/fn-execution, sdks/java/harness

Module H | sdks/java/io, sdks/java/javadoc, sdks/java/maven-archetypes, sdks/java/testing

It's up to your discretion, but in general, I would break it into the smallest PRs possible. In other words, wherever you can change one subproject independently of all the others, do so. Specifically, each runner (runners/flink, runners/spark, etc.) should probably get its own PR to be reviewed by one of the maintainers of that runner.

sonam-vend · 2021-01-04T09:47:28Z

Run Java PreCommit

ibzib · 2021-09-15T21:31:22Z

This PR was broken up and merged in parts. See BEAM-10961 for the full list.

Integrated Nebula plugin

53940fd

probot-autolabeler bot added the build label Sep 25, 2020

Unused exclude dependencies

dbab64a

probot-autolabeler bot added extensions hadoop hcatalog io java sql labels Sep 25, 2020

Excluded unused dependencies

e3d2be0

probot-autolabeler bot added core dataflow direct euphoria examples extensions-java fn-execution google-cloud-platform-core jackson jet join-library kotlin local protobuf runners spark labels Oct 1, 2020

sonam-vend added 2 commits October 12, 2020 12:40

Enabled strict dependency over all java modules

b974636

Removed nebula plugin

9fa8a1d

Fixed code standards issues

aa22d1b

fix for precommits checks

9a773af

shehzaadn-vd reviewed Jan 2, 2021

View reviewed changes

runners/flink/1.9/build.gradle Outdated

@@ -17,6 +17,7 @@

*/

def basePath = '..'

Copy link

Contributor

shehzaadn-vd Jan 2, 2021

Choose a reason for hiding this comment

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

stray change please revert.

shehzaadn-vd reviewed Jan 2, 2021

View reviewed changes

sonam-vend added 2 commits January 4, 2021 12:35

code cleaning

20703a1

code format fixing

c207c6a

probot-autolabeler bot added the samza label Jan 4, 2021

code clean

766e0eb

sonam-vend marked this pull request as draft January 4, 2021 08:46

sonam-vend added 2 commits January 4, 2021 17:13

fixed code format issues

b47662e

upstream with main repo

7cc00b0

ibzib mentioned this pull request Jan 7, 2021

[BEAM-10961] Enabled strict dependency on Spark #13668

Merged

4 tasks

ibzib changed the title ~~BEAM-10961- Enable strict dependency analysis on all Java modules~~ [BEAM-10961] Enable strict dependency analysis on all Java modules Sep 15, 2021

ibzib closed this Sep 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BEAM-10961] Enable strict dependency analysis on all Java modules #12938

[BEAM-10961] Enable strict dependency analysis on all Java modules #12938

sonam-vend commented Sep 25, 2020 •

edited

codecov bot commented Oct 1, 2020 •

edited

aaltay commented Oct 8, 2020

sonam-vend commented Dec 31, 2020

sonam-vend commented Dec 31, 2020 •

edited

shehzaadn-vd Jan 2, 2021

shehzaadn-vd Jan 2, 2021

shehzaadn-vd Jan 2, 2021

shehzaadn-vd Jan 2, 2021

shehzaadn-vd Jan 2, 2021

shehzaadn-vd Jan 2, 2021

shehzaadn-vd Jan 2, 2021

shehzaadn-vd Jan 2, 2021

shehzaadn-vd Jan 2, 2021

shehzaadn-vd Jan 2, 2021

shehzaadn-vd Jan 2, 2021

shehzaadn-vd Jan 2, 2021

shehzaadn-vd Jan 2, 2021

ibzib commented Jan 3, 2021

sonam-vend commented Jan 4, 2021

ibzib commented Sep 15, 2021

		@@ -31,3 +32,4 @@ project.ext {

		// Load the main build script which contains all build logic.
		apply from: "$basePath/flink_runner.gradle"

		compile library.java.slf4j_jdk14

		permitUnusedDeclared library.java.slf4j_jdk14

		@@ -20,3 +20,4 @@ plugins { id 'org.apache.beam.module' }
		applyJavaNature(exportJavadoc: false, publish: false)

		description = "Apache Beam :: SDKs :: Java :: Build Tools"

[BEAM-10961] Enable strict dependency analysis on all Java modules #12938

[BEAM-10961] Enable strict dependency analysis on all Java modules #12938

Conversation

sonam-vend commented Sep 25, 2020 • edited

Post-Commit Tests Status (on master branch)

Pre-Commit Tests Status (on master branch)

GitHub Actions Tests Status (on master branch)

codecov bot commented Oct 1, 2020 • edited

Codecov Report

aaltay commented Oct 8, 2020

sonam-vend commented Dec 31, 2020

sonam-vend commented Dec 31, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibzib commented Jan 3, 2021

sonam-vend commented Jan 4, 2021

ibzib commented Sep 15, 2021

sonam-vend commented Sep 25, 2020 •

edited

codecov bot commented Oct 1, 2020 •

edited

sonam-vend commented Dec 31, 2020 •

edited