[BEAM-10961] Enable strict dependency checking on sdks/java/extensions/sql #13830

shehzaadn-vd · 2021-01-28T05:01:24Z

This PR was split from #13686 to isolate sdks/java/extensions/sql changes.

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.

shehzaadn-vd · 2021-01-28T06:23:29Z

R: @ibzib

ibzib

General note: if the plugin reports that a declared compile dependency is unused, there are basically three options to fix it.

Keep it as compile and add permitUnusedDeclared.
Change it to runtimeOnly.
Remove it entirely.

Of these, 1. is the safest, since it doesn't change the build at all. 2. is slightly more risky, I am currently investigating a bug with this approach that might have slipped past our tests. 3. is the most dangerous, and should only be done if we are absolutely sure the dependency is never used at all, which is hard to verify.

cc: @apilloud

sdks/java/extensions/sql/datacatalog/build.gradle

ibzib · 2021-01-29T17:45:10Z

sdks/java/extensions/sql/datacatalog/build.gradle

+  compile library.java.slf4j_api
+  compile library.java.vendored_guava_26_0_jre
+  compile project(path: ":sdks:java:core", configuration: "shadow")
+  compile "com.google.api.grpc:proto-google-cloud-datacatalog-v1beta1:0.32.1"


We should probably used the bom for this, like we discussed on the PR for GCP IO.

@ibzib the declaration in BeamModulePlugin is
google_cloud_datacatalog_v1beta1 : "com.google.cloud:google-cloud-datacatalog", // google_cloud_platform_libraries_bom sets version
whereas what's used here is:
"com.google.api.grpc:proto-google-cloud-datacatalog-v1beta1:0.32.1"
which doesn't exist in the BOM
https://storage.googleapis.com/java-cloud-bom-dashboard/com.google.cloud/google-cloud-bom/0.147.0/index.html
or maybe there's another BOM for com.google.api.*?

Cancel the above comment. I see there is a layer of dependencies that can also be referred to using the BOM.

sdks/java/extensions/sql/expansion-service/build.gradle

sdks/java/extensions/sql/hcatalog/build.gradle

sdks/java/extensions/sql/jdbc/build.gradle

sdks/java/extensions/sql/zetasql/build.gradle

sdks/java/extensions/sql/build.gradle

codecov · 2021-01-29T19:04:07Z

Codecov Report

Merging #13830 (3835496) into master (c137e88) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #13830      +/-   ##
==========================================
+ Coverage   82.97%   82.99%   +0.01%     
==========================================
  Files         469      469              
  Lines       58300    58330      +30     
==========================================
+ Hits        48377    48413      +36     
+ Misses       9923     9917       -6

Impacted Files	Coverage Δ
...srcs/sdks/python/apache_beam/io/localfilesystem.py
...he_beam/portability/api/standard_window_fns_pb2.py
.../srcs/sdks/python/apache_beam/portability/utils.py
...ks/python/apache_beam/runners/worker/data_plane.py
.../python/apache_beam/io/gcp/datastore/v1new/util.py
...on/apache_beam/runners/portability/flink_runner.py
...hon/apache_beam/runners/worker/worker_pool_main.py
...ache_beam/examples/cookbook/datastore_wordcount.py
...ks/python/apache_beam/io/gcp/pubsub_it_pipeline.py
...testing/benchmarks/nexmark/models/nexmark_model.py
... and 928 more

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 c137e88...3835496. Read the comment docs.

…ql/build.gradle

shehzaadn-vd · 2021-02-06T13:59:12Z

@ibzib I'm converting this to draft until PR #13910 is merged (hopefully that's a quick merge :-))

…lphabeticization

shehzaadn-vd · 2021-02-10T08:11:41Z

@ibzib, requesting a review.

I've introduced permitUnusedDeclared where needed and the mongodb_java_driver is now used here as well.

…oogle-common-protos

sdks/java/extensions/sql/datacatalog/build.gradle

sdks/java/extensions/sql/build.gradle

sdks/java/extensions/sql/jdbc/build.gradle

ibzib · 2021-02-13T00:00:23Z

sdks/java/extensions/sql/zetasql/build.gradle

+  compile library.java.slf4j_api
+  compile library.java.vendored_guava_26_0_jre
+  compile library.java.proto_google_common_protos // Interfaces with ZetaSQL use this
+  permitUnusedDeclared "com.google.api.grpc:proto-google-common-protos:2.0.1" // BEAM-11761


This was added twice.

Also, if you're using the library.java version in the compile statement, you should also use it in the corresponding permitUnusedDeclared.

I couldn't get permitUnusedDeclared to work with the library.java variable when the version is coming from the bom. It seems permitUnusedDeclared sees a blank version and fails. See #13791 (comment)

@ibzib Regarding your comment about this being added twice, they are actually two different dependencies:
"com.google.api.grpc:proto-google-common-protos:2.0.1"
"com.google.api.grpc:grpc-google-common-protos:2.0.1"

It's possible both aren't really needed, but I think resolving that is probably a separate issue.

Regarding your comment about this being added twice, they are actually two different dependencies:

Oops, you're right. I didn't read carefully.

I couldn't get permitUnusedDeclared to work with the library.java variable when the version is coming from the bom. It seems permitUnusedDeclared sees a blank version and fails.

We should be able to work around this by resolving the bom versions before running analyzeClassesDependencies. But I'm not sure exactly how. Filed BEAM-11823 to track.

@shehzaadn-vd we can fix the issue with blank versions by using enforcedPlatform: ibzib@ef12251

sdks/java/extensions/sql/zetasql/build.gradle

…lared to support library.java.* variables

shehzaadn-vd · 2021-02-25T10:29:29Z

@ibzib please see if this is good to go. thanks!

ibzib

LGTM

Can you please squash the commit history before merging?

sdks/java/extensions/sql/zetasql/build.gradle

…gger * github/master: (123 commits) [BEAM-11899] Bump commons-pool to 2.8.1 and bump commons-dbcp to 2.8.0, Because there is a library dependency Update pillars.yaml (apache#14142) [BEAM-10632] Checkerframework nullness cleanups (apache#14107) [BEAM-11213] Instantiate SparkListenerApplicationStart in a Spark 3 compatible way Fix typos for excluding testMergingCustomWindowsWithoutCustomWindowTypes Specify the time resolution for TestStreamPayload. [BEAM-10961] Enable strict depdency checking for sdks/java/extensions/euphoria (second attempt) [BEAM-11848] Store Docker images in a variable for consistency. Splitting old Go Precommit and new ULR integration test precommit. Moving runner imports out of ptest. Add the TO_STRING capability to Java and Python [BEAM-11848] Fix Docker images list. jdbc python supported Dataflow runner (apache#13960) Adding a warning to use multi-workers on FnApiRunner Fix legend for Python Directrunner microbenchmarks [BEAM-11740] Estimate PCollection byte size [BEAM-10961] enable strict dependency checking for sdks/java/extensions/zetasketch (apache#14093) Map Dataflow JOB_STATE_CANCELLING to Beam RUNNING state [BEAM-11833] Fix reported watermark after restriction split in UnboundedSourceAsSDFRestrictionTracker [BEAM-10761] add reference to BEAM-11761 [BEAM-10961] enable strict dependency checking for flink/job-server Exclude MapState example integration tests from Dataflow runner v2 suite Remove InvalidWindows from Java SDK, instead track "already merged" bit Fix checkstyle in watermark latency benchmark Fix compile breakage in WindmillStateInternals Improve test, error on ALREADY_MERGED. [BEAM-10961] Strict dependency checking for sdks/java/io/gcp (apache#13791) Initial watermark latency benchmark Attempting improvements on DirectRunner Python dash [BEAM-10961] enable strict dependency checking for sdks/java/extensions/google-cloud-platform-core (apache#14084) Merge pull request apache#13802: [BEAM-1474]. Adding MapState and SetState support for the Dataflow runner Remove some false positives Remove nullness warning suppression [BEAM-11861] Add methods to explicitly provide coder for ParquetIO's Parse and ParseFiles (apache#14078) [BEAM-11531] Use pandas 1.2 for python>=3.7 (apache#14099) [BEAM-10961] add reference to BEAM-11761 [BEAM-10961] add explicit compile for auto_value_annotations in sdks/extensions/ml/build.gradle Attempting improvements on DirectRunner Python dash Recognize JOB_STATE_PENDING from Dataflow and map to RUNNING never run checkerframework on tests Puts more expensive BQ empty table check to the right of the 'and' condition (apache#14094) Use the windowing strategy of the input, not output, PCollection of GBK. Do not stage dataflow worker jar when use runner_v2. [BEAM-11870] Re-raise underlying exception for InvocationTargetException (apache#14098) [BEAM-11778] Create a wrapper for ZetaSQL catalog and refactor accordingly. (apache#13934) [BEAM-9378] Add ignored tests which fail in various ways when querying nested structures (apache#14077) Merge Fn API and runner v2 configurations for DataflowRunner Fix up! formatting Add validate runner test for testing custom merging windows fn without custom window types Revert "Revert "[BEAM-2914] Add portable merging window support to Python. (apache#12995)"" [BEAM-10961] fix stray reordering of lines [BEAM-10961] enable strict dependency checking for sdks/java/extensions/sorter [BEAM-10961] enable strict dependency checking for sdks/java/extensions/sketching [BEAM-10961] enable strict dependency checking for sdks/java/extensions/schemaio-expansion-service [BEAM-10961] enable strict dependency checking for sdks/java/extensions/protobuf [BEAM-10961] enable strict dependency checking for sdks/java/extensions/ml [BEAM-10961] enable strict dependency checking for sdks/java/extensions/kyro [BEAM-10961] enable strict dependency checking for sdks/java/extensions/join-library [BEAM-10961] undo line moves (originally intended for alphabeticization) [BEAM-10961] enable strict dependency checking for sdks/java/extensions/jackson [BEAM-10961] Enable strict dependency checking on sdks/java/extensions/sql (apache#13830) [BEAM-10961] enable strict dependency checking for sdks/java/extensions/euphoria [BEAM-10961] enable strict dependency checking for sdks/java/io/parquet (apache#14062) [BEAM-10961] enable strict dependency checking for sdks/java/io/thrift (apache#14066) Refactor ZetaSqlDialectSpecTest and add some passing tests. (apache#14080) [BEAM-11864] Use objects.equals instead of raw comparison [BEAM-11707] Change WindmillStateCache cache invalidation to be based upon reference invalidation instead of expensive set management. Reduce operations of shared cache by caching per-key object sets locally and flushing as groups to shared cache. Remove byte tracking which could be racy based upon background evictions in favor of just iterating for rendering the status page. This also lets us capture more stats. [BEAM-11730] Reduce context switching overhead for appliance reads by issuing reads directly from calling threads in the case that there is no reads being queued. Fix preview Show string from Dataflow service when job terminates in unrecognized state Log a warning when Dataflow returns an unrecognized state Merge pull request apache#14033 from [BEAM-11408] Integrate Python BigQuery sink with GroupIntoBatches Remove SYNCHRONIZED_PROCESSING_TIME from model proto Remove use of model SYNCHRONIZED_PROCESSING_TIME Merge redundant model feature columns in capability matrix Remove MapReduce runner from capability matrix, because it is on a branch and unreleased Remove JStorm runner from capability matrix, because it is on a branch and unreleased Remove retractions from capability matrix, because they do not exist yet Remove metadata-driven triggers from capability matrix, because they do not exist [BEAM-10937] Add Tour of Beam page (apache#13747) [BEAM-11344] Apply "Become a Committer" changes from Website Revamp (apache#14036) Merge pull request apache#14046 from [BEAM-11791] Adding a microbenchmark for TestStream Returning successful writes in FhirIO.Write.Result (apache#14034) Fixup [BEAM-10961] enable strict dependency checking for sdks/java/io/file-based-io-tests (apache#14052) [BEAM-10961] enable strict dependency checking for sdks/java/io/contextualtextio (apache#14049) [BEAM-10961] enable strict dependency checking for sdks/java/io/kinesis (apache#14058) [BEAM-10961] enable strict dependency checking for sdks/java/io/bigquery-io-perf-tests (apache#14048) [BEAM-10961] enable strict dependency checking for sdks/java/io/elasticsearch (apache#14050) [BEAM-10961] enable strict dependency checking for sdks/java/io/expansion-service (apache#14051) [BEAM-10961] enable strict dependency checking for sdks/java/io/jdbc (apache#14055) [BEAM-10961] enable strict dependency checking for sdks/java/io/jms (apache#14056) [BEAM-10961] enable strict dependency checking for sdks/java/io/kafka (apache#14057) [BEAM-10961] enable strict dependency checking for sdks/java/io/hcatalog (apache#14053) [BEAM-11859] Fixed bug in python S3 IO [BEAM-10114] Fix PerSubscriptionPartitionSdf to not rely on the presence of BundleFinalizer [BEAM-10114] Fix PerSubscriptionPartitionSdf to not rely on the presence of BundleFinalizer [BEAM-10961] fix spacing [BEAM-10961] enable strict dependency checking for sdks/java/io/xml [BEAM-10961] enable strict dependency checking for sdks/java/io/tika ...

shehzaadn-vd added 4 commits January 27, 2021 20:47

initial move of sdks/java/extensions/sql files to their own PR

e0d609c

enableStrictDependencies: true

2c1b817

remove unnecessary declaration of checkerqual

d37416c

use of existing variables and alphabetacization

d97ba09

ibzib reviewed Jan 29, 2021

View reviewed changes

sdks/java/extensions/sql/build.gradle Outdated Show resolved Hide resolved

shehzaadn-vd added 3 commits January 31, 2021 18:32

Incorporate review comments (various)

dba8ed6

merged from master and resolved conflict in java/extensions/sql/zetas…

a449640

…ql/build.gradle

[BEAM-10961] add compile lines due to new udf dependency

8381744

shehzaadn-vd marked this pull request as draft February 6, 2021 13:57

shehzaadn-vd added 2 commits February 9, 2021 15:46

Merge branch 'master' into sdks-extensions-sql

4df10de

[BEAM-10961] add permitUnusedDeclared to handle BEAM-11761 and some a…

fc604e4

…lphabeticization

shehzaadn-vd marked this pull request as ready for review February 10, 2021 08:10

shehzaadn-vd added 2 commits February 11, 2021 21:26

merged from upstream/master

48b6def

[BEAM-10961] account for upgraded versions of com.google.api.grpc:*-g…

994d63b

…oogle-common-protos

ibzib reviewed Feb 13, 2021

View reviewed changes

shehzaadn-vd added 2 commits February 12, 2021 16:56

[BEAM-10961] cleanup due to last review comments

621310f

[BEAM-10961] use enforcedPlatform dependency to allow permitUnusedDec…

3dc7800

…lared to support library.java.* variables

shehzaadn-vd referenced this pull request in ibzib/beam Feb 18, 2021

[BEAM-11823] Enable permitUnusedDeclared to work with BOM dependencies.

ef12251

shehzaadn-vd added 2 commits February 24, 2021 15:34

merge master

5eb8f52

[BEAM-10961] add reference to BEAM-11761

3835496

ibzib reviewed Feb 25, 2021

View reviewed changes

sdks/java/extensions/sql/zetasql/build.gradle Show resolved Hide resolved

ibzib merged commit 5566098 into apache:master Feb 25, 2021

[BEAM-10961] Enable strict dependency checking on sdks/java/extensions/sql #13830

[BEAM-10961] Enable strict dependency checking on sdks/java/extensions/sql #13830

Uh oh!

Conversation

shehzaadn-vd commented Jan 28, 2021

Post-Commit Tests Status (on master branch)

Pre-Commit Tests Status (on master branch)

GitHub Actions Tests Status (on master branch)

Uh oh!

shehzaadn-vd commented Jan 28, 2021

Uh oh!

ibzib left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ibzib Jan 29, 2021

Choose a reason for hiding this comment

Uh oh!

shehzaadn-vd Feb 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shehzaadn-vd Feb 1, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jan 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

shehzaadn-vd commented Feb 6, 2021

Uh oh!

shehzaadn-vd commented Feb 10, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ibzib Feb 13, 2021

Choose a reason for hiding this comment

Uh oh!

shehzaadn-vd Feb 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shehzaadn-vd Feb 13, 2021

Choose a reason for hiding this comment

Uh oh!

ibzib Feb 17, 2021

Choose a reason for hiding this comment

Uh oh!

ibzib Feb 17, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shehzaadn-vd commented Feb 25, 2021

Uh oh!

ibzib left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

shehzaadn-vd Feb 1, 2021 •

edited

Loading

codecov bot commented Jan 29, 2021 •

edited

Loading

shehzaadn-vd Feb 13, 2021 •

edited

Loading