You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/04/07 22:22:27 UTC
[GitHub] [beam] cushon opened a new pull request, #17317: Unvendor bytebuddy
cushon opened a new pull request, #17317:
URL: https://github.com/apache/beam/pull/17317
This is only for testing.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) 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](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #17317: Unvendor bytebuddy
Posted by GitBox <gi...@apache.org>.
kennknowles commented on PR #17317:
URL: https://github.com/apache/beam/pull/17317#issuecomment-1092851577
run spark validatesrunner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on a diff in pull request #17317: [BEAM-14117] Unvendor bytebuddy dependency
Posted by GitBox <gi...@apache.org>.
kennknowles commented on code in PR #17317:
URL: https://github.com/apache/beam/pull/17317#discussion_r919480737
##########
sdks/java/core/build.gradle:
##########
@@ -28,12 +28,17 @@ applyJavaNature(
dependencies {
include(dependency("org.apache.commons:.*"))
include(dependency(library.java.antlr_runtime))
+ include(dependency(library.java.byte_buddy))
Review Comment:
In other words, it should become "just like any other dependency"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] commented on pull request #17317: Unvendor bytebuddy
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #17317:
URL: https://github.com/apache/beam/pull/17317#issuecomment-1092284200
# [Codecov](https://codecov.io/gh/apache/beam/pull/17317?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17317](https://codecov.io/gh/apache/beam/pull/17317?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b48ce9c) into [master](https://codecov.io/gh/apache/beam/commit/25d14a7f12a3388cc3a437a10096c804a6e14c1e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (25d14a7) will **decrease** coverage by `0.00%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #17317 +/- ##
==========================================
- Coverage 74.10% 74.09% -0.01%
==========================================
Files 683 683
Lines 89261 89259 -2
==========================================
- Hits 66148 66140 -8
- Misses 21960 21966 +6
Partials 1153 1153
```
| Flag | Coverage Δ | |
|---|---|---|
| python | `83.62% <ø> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/17317?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...apache\_beam/runners/portability/portable\_runner.py](https://codecov.io/gh/apache/beam/pull/17317/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9wb3J0YWJsZV9ydW5uZXIucHk=) | `74.57% <0.00%> (-2.07%)` | :arrow_down: |
| [sdks/python/apache\_beam/io/localfilesystem.py](https://codecov.io/gh/apache/beam/pull/17317/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vbG9jYWxmaWxlc3lzdGVtLnB5) | `91.47% <0.00%> (-0.78%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/17317/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.51% <0.00%> (-0.13%)` | :arrow_down: |
| [...apache\_beam/runners/dataflow/internal/apiclient.py](https://codecov.io/gh/apache/beam/pull/17317/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kYXRhZmxvdy9pbnRlcm5hbC9hcGljbGllbnQucHk=) | `77.57% <0.00%> (+0.08%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/17317/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `89.06% <0.00%> (+0.15%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/17317?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/17317?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [25d14a7...b48ce9c](https://codecov.io/gh/apache/beam/pull/17317?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] cushon commented on pull request #17317: [BEAM-14117] Unvendor bytebuddy dependency
Posted by GitBox <gi...@apache.org>.
cushon commented on PR #17317:
URL: https://github.com/apache/beam/pull/17317#issuecomment-1099574831
R: @kennknowles
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #17317: Unvendor bytebuddy
Posted by GitBox <gi...@apache.org>.
kennknowles commented on PR #17317:
URL: https://github.com/apache/beam/pull/17317#issuecomment-1092851833
run dataflow validatesrunner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #17317: [BEAM-14117] Unvendor bytebuddy dependency
Posted by GitBox <gi...@apache.org>.
kennknowles commented on PR #17317:
URL: https://github.com/apache/beam/pull/17317#issuecomment-1159143343
The tests are super flaky, but I think Jenkins runs against the HEAD commit of the PR so it will need a rebase to get any recent improvements in test flakiness. Let me investigate a little bit and I'll comment when I have news.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] cushon commented on pull request #17317: [BEAM-14117] Unvendor bytebuddy dependency
Posted by GitBox <gi...@apache.org>.
cushon commented on PR #17317:
URL: https://github.com/apache/beam/pull/17317#issuecomment-1183925019
> You should declare byte_buddy within the shadow configuration
Thanks so much! Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #17317: Unvendor bytebuddy
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17317:
URL: https://github.com/apache/beam/pull/17317#issuecomment-1092265209
Can one of the admins verify this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on pull request #17317: [BEAM-14117] Unvendor bytebuddy dependency
Posted by GitBox <gi...@apache.org>.
lukecwik commented on PR #17317:
URL: https://github.com/apache/beam/pull/17317#issuecomment-1187931801
Fixes https://github.com/apache/beam/issues/21519
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #17317: [BEAM-14117] Unvendor bytebuddy dependency
Posted by GitBox <gi...@apache.org>.
kennknowles commented on PR #17317:
URL: https://github.com/apache/beam/pull/17317#issuecomment-1159140161
Thread at https://lists.apache.org/thread/sdp7tpvy5n1jlk3g8wqqp40dlyzp5llv did not encounter any major objections.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on pull request #17317: [BEAM-14117] Unvendor bytebuddy dependency
Posted by GitBox <gi...@apache.org>.
lukecwik commented on PR #17317:
URL: https://github.com/apache/beam/pull/17317#issuecomment-1183676938
> > Any idea why the regular non-shadow dependency isn't getting resolved correctly there?
>
> The tests are making more progress after adding an explicit dep to `sdks/java/harness/jmh/build.gradle`.
Adding an explicit dep doesn't seem to be the way to go since it is technically a transitive dependency.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] cushon commented on pull request #17317: [BEAM-14117] Unvendor bytebuddy dependency
Posted by GitBox <gi...@apache.org>.
cushon commented on PR #17317:
URL: https://github.com/apache/beam/pull/17317#issuecomment-1183802788
I did some more debugging, and my understanding is that this configuration is effectively removing the `implementation` dep:
https://github.com/apache/beam/blob/f2e41be4880ef47d86d975c38f353896f70365bd/sdks/java/core/build.gradle#L27-L35
because when `shadowClosure` is specified `BeamModulePlugin` removes all of the implementation deps:
https://github.com/apache/beam/blob/f2e41be4880ef47d86d975c38f353896f70365bd/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy#L808-L810
Hacking up `BeamModulePlugin` to add a hard-coded implementation dep on bytebuddy fixes the build errors I was seeing, which seems to be consistent with that theory:
```diff
diff --git a/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy b/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy
index 791277a7e1..30b94333e0 100644
--- a/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy
+++ b/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy
@@ -496,6 +496,7 @@ class BeamModulePlugin implements Plugin<Project> {
def testcontainers_version = "1.16.3"
def arrow_version = "5.0.0"
def jmh_version = "1.34"
+ def byte_buddy_version = "1.12.9"
// A map of maps containing common libraries used per language. To use:
// dependencies {
@@ -541,7 +542,7 @@ class BeamModulePlugin implements Plugin<Project> {
aws_java_sdk2_utils : "software.amazon.awssdk:utils:$aws_java_sdk2_version",
bigdataoss_gcsio : "com.google.cloud.bigdataoss:gcsio:$google_cloud_bigdataoss_version",
bigdataoss_util : "com.google.cloud.bigdataoss:util:$google_cloud_bigdataoss_version",
- byte_buddy : "net.bytebuddy:byte-buddy:1.12.9",
+ byte_buddy : "net.bytebuddy:byte-buddy:$byte_buddy_version",
cassandra_driver_core : "com.datastax.cassandra:cassandra-driver-core:$cassandra_driver_version",
cassandra_driver_mapping : "com.datastax.cassandra:cassandra-driver-mapping:$cassandra_driver_version",
cdap_api : "io.cdap.cdap:cdap-api:$cdap_version",
@@ -1063,7 +1064,15 @@ class BeamModulePlugin implements Plugin<Project> {
// This contains many improved annotations beyond javax.annotations for enhanced static checking
// of the codebase. It is runtime so users can also take advantage of them. The annotations themselves
// are MIT licensed (checkerframework is GPL and cannot be distributed)
- implementation "org.checkerframework:checker-qual:$checkerframework_version"
+
+ def implementationDeps = [
+ "org.checkerframework:checker-qual:$checkerframework_version",
+ "net.bytebuddy:byte-buddy:$byte_buddy_version",
+ ]
+
+ implementationDeps.each { dep ->
+ implementation dep
+ }
}
// Defines Targets for sonarqube analysis reporting.
```
What do you recommend? It seems like there are no existing examples of implementation deps that aren't included in the shadow closure. Does it make sense to include it in the shadow closure after all? Is there a more principled change to `BeamModulePlugin` to allow it to be configured to preserve some implementation deps?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #17317: Unvendor bytebuddy
Posted by GitBox <gi...@apache.org>.
kennknowles commented on PR #17317:
URL: https://github.com/apache/beam/pull/17317#issuecomment-1092851351
run java postcommit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik merged pull request #17317: [BEAM-14117] Unvendor bytebuddy dependency
Posted by GitBox <gi...@apache.org>.
lukecwik merged PR #17317:
URL: https://github.com/apache/beam/pull/17317
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] cushon commented on pull request #17317: [BEAM-14117] Unvendor bytebuddy dependency
Posted by GitBox <gi...@apache.org>.
cushon commented on PR #17317:
URL: https://github.com/apache/beam/pull/17317#issuecomment-1183706282
> Adding an explicit dep doesn't seem to be the way to go since it is technically a transitive dependency.
Thanks, any suggestions about what the right way to go would be?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] cushon commented on pull request #17317: [BEAM-14117] Unvendor bytebuddy dependency
Posted by GitBox <gi...@apache.org>.
cushon commented on PR #17317:
URL: https://github.com/apache/beam/pull/17317#issuecomment-1183660201
> Any idea why the regular non-shadow dependency isn't getting resolved correctly there?
The tests are making more progress after adding an explicit dep to `sdks/java/harness/jmh/build.gradle`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on a diff in pull request #17317: [BEAM-14117] Unvendor bytebuddy dependency
Posted by GitBox <gi...@apache.org>.
kennknowles commented on code in PR #17317:
URL: https://github.com/apache/beam/pull/17317#discussion_r920581729
##########
sdks/java/harness/jmh/build.gradle:
##########
@@ -34,6 +34,7 @@ dependencies {
implementation project(path: ":sdks:java:harness", configuration: "shadow")
implementation project(":runners:java-fn-execution")
runtimeOnly library.java.slf4j_jdk14
+ runtimeOnly library.java.byte_buddy
Review Comment:
I'm not sure how the failure occurred, but based on grep and IWYU this should just not be here. I understand you added it in response to an issue. It sounds like there must be an error in some other module's IWYU compliance, and presumably is suppressed? (just a guess; I don't know)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #17317: Unvendor bytebuddy
Posted by GitBox <gi...@apache.org>.
kennknowles commented on PR #17317:
URL: https://github.com/apache/beam/pull/17317#issuecomment-1092851703
run samza validatesrunner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #17317: [BEAM-14117] Unvendor bytebuddy dependency
Posted by GitBox <gi...@apache.org>.
kennknowles commented on PR #17317:
URL: https://github.com/apache/beam/pull/17317#issuecomment-1183485515
Now I just want to poke the tests until they are green, rather than ignoring red signals, even if they seem irrelevant.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] cushon commented on pull request #17317: [BEAM-14117] Unvendor bytebuddy dependency
Posted by GitBox <gi...@apache.org>.
cushon commented on PR #17317:
URL: https://github.com/apache/beam/pull/17317#issuecomment-1183520006
> Now I just want to poke the tests until they are green, rather than ignoring red signals, even if they seem irrelevant.
Definitely, thanks. The tests were all passing earlier, and I was hoping there was an unrelated breakage at head.
But looking more closely, it's related to the change to stop bundling bytebuddy. With that applied, I get:
```
$ ./gradlew -p sdks/java/harness/ test
...
> Task :sdks:java:harness:jmh:test
org.apache.beam.fn.harness.jmh.ProcessBundleBenchmarkTest > testStateWithoutCaching FAILED
java.util.concurrent.ExecutionException at ProcessBundleBenchmarkTest.java:46
Caused by: org.apache.beam.vendor.grpc.v1p43p2.io.grpc.StatusRuntimeException at Status.java:526
```
And looking at the stderr for the failing test:
```
Exception in thread "pool-3-thread-4" java.lang.NoClassDefFoundError: net/bytebuddy/NamingStrategy
at org.apache.beam.sdk.transforms.reflect.DoFnInvokers.invokerFor(DoFnInvokers.java:38)
at org.apache.beam.sdk.transforms.reflect.DoFnInvokers.tryInvokeSetupFor(DoFnInvokers.java:51)
at org.apache.beam.fn.harness.FnApiDoFnRunner.<init>(FnApiDoFnRunner.java:492)
at org.apache.beam.fn.harness.FnApiDoFnRunner$Factory.createRunnerForPTransform(FnApiDoFnRunner.java:193)
at org.apache.beam.fn.harness.FnApiDoFnRunner$Factory.createRunnerForPTransform(FnApiDoFnRunner.java:162)
at org.apache.beam.fn.harness.control.ProcessBundleHandler.createRunnerAndConsumersForPTransformRecursively(ProcessBundleHandler.java:298)
at org.apache.beam.fn.harness.control.ProcessBundleHandler.createRunnerAndConsumersForPTransformRecursively(ProcessBundleHandler.java:252)
at org.apache.beam.fn.harness.control.ProcessBundleHandler.createBundleProcessor(ProcessBundleHandler.java:825)
at org.apache.beam.fn.harness.control.ProcessBundleHandler.lambda$processBundle$0(ProcessBundleHandler.java:502)
at org.apache.beam.fn.harness.control.ProcessBundleHandler$BundleProcessorCache.get(ProcessBundleHandler.java:936)
at org.apache.beam.fn.harness.control.ProcessBundleHandler.processBundle(ProcessBundleHandler.java:498)
at org.apache.beam.fn.harness.control.BeamFnControlClient.delegateOnInstructionRequestType(BeamFnControlClient.java:151)
at org.apache.beam.fn.harness.control.BeamFnControlClient$InboundObserver.lambda$onNext$0(BeamFnControlClient.java:116)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)
at java.base/java.lang.Thread.run(Thread.java:830)
Caused by: java.lang.ClassNotFoundException: net.bytebuddy.NamingStrategy
at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:581)
at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:182)
at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:523)
... 16 more
```
Any idea why the regular non-shadow dependency isn't getting resolved correctly there?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] cushon commented on a diff in pull request #17317: [BEAM-14117] Unvendor bytebuddy dependency
Posted by GitBox <gi...@apache.org>.
cushon commented on code in PR #17317:
URL: https://github.com/apache/beam/pull/17317#discussion_r920316892
##########
sdks/java/core/build.gradle:
##########
@@ -28,12 +28,17 @@ applyJavaNature(
dependencies {
include(dependency("org.apache.commons:.*"))
include(dependency(library.java.antlr_runtime))
+ include(dependency(library.java.byte_buddy))
Review Comment:
Done, thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] cushon commented on pull request #17317: [BEAM-14117] Unvendor bytebuddy dependency
Posted by GitBox <gi...@apache.org>.
cushon commented on PR #17317:
URL: https://github.com/apache/beam/pull/17317#issuecomment-1186270155
I think all of the open issues have been resolved:
* the byte_buddy dep is now in the shadow configuration
* the CI is green (I had to retry a few times but this didn't require any additional fixes, I think some of the tests might be flaky)
Let me know if there's any additional feedback!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #17317: Unvendor bytebuddy
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17317:
URL: https://github.com/apache/beam/pull/17317#issuecomment-1092265207
Can one of the admins verify this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #17317: Unvendor bytebuddy
Posted by GitBox <gi...@apache.org>.
kennknowles commented on PR #17317:
URL: https://github.com/apache/beam/pull/17317#issuecomment-1092851455
run flink validatesrunner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on a diff in pull request #17317: [BEAM-14117] Unvendor bytebuddy dependency
Posted by GitBox <gi...@apache.org>.
kennknowles commented on code in PR #17317:
URL: https://github.com/apache/beam/pull/17317#discussion_r919480587
##########
sdks/java/core/build.gradle:
##########
@@ -28,12 +28,17 @@ applyJavaNature(
dependencies {
include(dependency("org.apache.commons:.*"))
include(dependency(library.java.antlr_runtime))
+ include(dependency(library.java.byte_buddy))
Review Comment:
I don't think we should bundle bytebuddy in here, right? It should be a residual dependency that is resolved according to everyone's poms?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on pull request #17317: [BEAM-14117] Unvendor bytebuddy dependency
Posted by GitBox <gi...@apache.org>.
lukecwik commented on PR #17317:
URL: https://github.com/apache/beam/pull/17317#issuecomment-1183917432
> I did some more debugging, and my understanding is that this configuration is effectively removing the `implementation` dep:
>
> https://github.com/apache/beam/blob/f2e41be4880ef47d86d975c38f353896f70365bd/sdks/java/core/build.gradle#L27-L35
>
> because when `shadowClosure` is specified `BeamModulePlugin` removes all of the implementation deps:
>
> https://github.com/apache/beam/blob/f2e41be4880ef47d86d975c38f353896f70365bd/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy#L808-L810
>
> Hacking up `BeamModulePlugin` to add a hard-coded implementation dep on bytebuddy fixes the build errors I was seeing, which seems to be consistent with that theory:
>
> ```diff
> diff --git a/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy b/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy
> index 791277a7e1..30b94333e0 100644
> --- a/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy
> +++ b/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy
> @@ -496,6 +496,7 @@ class BeamModulePlugin implements Plugin<Project> {
> def testcontainers_version = "1.16.3"
> def arrow_version = "5.0.0"
> def jmh_version = "1.34"
> + def byte_buddy_version = "1.12.9"
>
> // A map of maps containing common libraries used per language. To use:
> // dependencies {
> @@ -541,7 +542,7 @@ class BeamModulePlugin implements Plugin<Project> {
> aws_java_sdk2_utils : "software.amazon.awssdk:utils:$aws_java_sdk2_version",
> bigdataoss_gcsio : "com.google.cloud.bigdataoss:gcsio:$google_cloud_bigdataoss_version",
> bigdataoss_util : "com.google.cloud.bigdataoss:util:$google_cloud_bigdataoss_version",
> - byte_buddy : "net.bytebuddy:byte-buddy:1.12.9",
> + byte_buddy : "net.bytebuddy:byte-buddy:$byte_buddy_version",
> cassandra_driver_core : "com.datastax.cassandra:cassandra-driver-core:$cassandra_driver_version",
> cassandra_driver_mapping : "com.datastax.cassandra:cassandra-driver-mapping:$cassandra_driver_version",
> cdap_api : "io.cdap.cdap:cdap-api:$cdap_version",
> @@ -1063,7 +1064,15 @@ class BeamModulePlugin implements Plugin<Project> {
> // This contains many improved annotations beyond javax.annotations for enhanced static checking
> // of the codebase. It is runtime so users can also take advantage of them. The annotations themselves
> // are MIT licensed (checkerframework is GPL and cannot be distributed)
> - implementation "org.checkerframework:checker-qual:$checkerframework_version"
> +
> + def implementationDeps = [
> + "org.checkerframework:checker-qual:$checkerframework_version",
> + "net.bytebuddy:byte-buddy:$byte_buddy_version",
> + ]
> +
> + implementationDeps.each { dep ->
> + implementation dep
> + }
> }
>
> // Defines Targets for sonarqube analysis reporting.
> ```
>
> What do you recommend? It seems like there are no existing examples of implementation deps that aren't included in the shadow closure. Does it make sense to include it in the shadow closure after all? Is there a more principled change to `BeamModulePlugin` to allow it to be configured to preserve some implementation deps?
You should declare byte_buddy within the shadow configuration as per https://github.com/apache/beam/blob/f2e41be4880ef47d86d975c38f353896f70365bd/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy#L811
This will make it a compile time dependency for the source set and it will be exposed in the pom.xml file and should become a transitive dependency.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org