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/06/24 16:03:02 UTC
[GitHub] [beam] damccorm opened a new pull request, #22046: Fix vendored dependency issue (guava)
damccorm opened a new pull request, #22046:
URL: https://github.com/apache/beam/pull/22046
This was causing precommits to fail
------------------------
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`).
- [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
- [ ] 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] asf-ci commented on pull request #22046: Fix vendored dependency issue (guava)
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #22046:
URL: https://github.com/apache/beam/pull/22046#issuecomment-1165715315
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] damccorm merged pull request #22046: Fix vendored dependency issue and other style checks
Posted by GitBox <gi...@apache.org>.
damccorm merged PR #22046:
URL: https://github.com/apache/beam/pull/22046
--
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] damondouglas commented on pull request #22046: Fix vendored dependency issue and other style checks
Posted by GitBox <gi...@apache.org>.
damondouglas commented on PR #22046:
URL: https://github.com/apache/beam/pull/22046#issuecomment-1167702823
Thank you so much @pabloem, the following commands (run at `53d03c204c`) provided insight into the problem when run locally.
```
./gradlew sdks:java:io:google-cloud-platform:checkStyleMain
./gradlew sdks:java:io:google-cloud-platform:checkStyleTest
```
`./gradlew sdks:java:io:google-cloud-platform:compileJava` did not show any errors and was successful.
Before re-submitting #21953 with the corrections, I'm going to update / create a draft language specific contribution guide and start a thread on dev@ with a recommendation to update the PR template with an additional checklist that refers to a contribution guide.
--
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 #22046: Fix vendored dependency issue (guava)
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #22046:
URL: https://github.com/apache/beam/pull/22046#issuecomment-1165715316
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] damccorm commented on pull request #22046: Fix vendored dependency issue (guava)
Posted by GitBox <gi...@apache.org>.
damccorm commented on PR #22046:
URL: https://github.com/apache/beam/pull/22046#issuecomment-1165748557
Yeah, good call - I'm not 100% sure we want to do this yet FWIW, but I lean towards doing it if its a small change (which it should 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] codecov[bot] commented on pull request #22046: Fix vendored dependency issue and other style checks
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #22046:
URL: https://github.com/apache/beam/pull/22046#issuecomment-1165801120
# [Codecov](https://codecov.io/gh/apache/beam/pull/22046?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 [#22046](https://codecov.io/gh/apache/beam/pull/22046?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c327364) into [master](https://codecov.io/gh/apache/beam/commit/10dab960d9695266fbbbeb040a378550fb440be6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (10dab96) will **decrease** coverage by `0.00%`.
> The diff coverage is `n/a`.
> :exclamation: Current head c327364 differs from pull request most recent head 1ea3a57. Consider uploading reports for the commit 1ea3a57 to get more accurate results
```diff
@@ Coverage Diff @@
## master #22046 +/- ##
==========================================
- Coverage 74.00% 73.99% -0.01%
==========================================
Files 703 703
Lines 92936 92936
==========================================
- Hits 68779 68770 -9
- Misses 22891 22900 +9
Partials 1266 1266
```
| Flag | Coverage Δ | |
|---|---|---|
| python | `83.57% <ø> (-0.02%)` | :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/22046?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/22046/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdGVzdGluZy90ZXN0X3N0cmVhbV9zZXJ2aWNlLnB5) | `88.09% <0.00%> (-4.77%)` | :arrow_down: |
| [...n/apache\_beam/ml/gcp/recommendations\_ai\_test\_it.py](https://codecov.io/gh/apache/beam/pull/22046/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvZ2NwL3JlY29tbWVuZGF0aW9uc19haV90ZXN0X2l0LnB5) | `73.46% <0.00%> (-2.05%)` | :arrow_down: |
| [...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/22046/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9ydW5uZXIucHk=) | `90.06% <0.00%> (-1.33%)` | :arrow_down: |
| [...eam/runners/portability/fn\_api\_runner/execution.py](https://codecov.io/gh/apache/beam/pull/22046/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL2V4ZWN1dGlvbi5weQ==) | `92.44% <0.00%> (-0.65%)` | :arrow_down: |
| [sdks/python/apache\_beam/runners/common.py](https://codecov.io/gh/apache/beam/pull/22046/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9jb21tb24ucHk=) | `88.59% <0.00%> (-0.13%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/22046/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.54% <0.00%> (+0.12%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/22046?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/22046?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 [10dab96...1ea3a57](https://codecov.io/gh/apache/beam/pull/22046?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] Abacn commented on a diff in pull request #22046: Fix vendored dependency issue and other style checks
Posted by GitBox <gi...@apache.org>.
Abacn commented on code in PR #22046:
URL: https://github.com/apache/beam/pull/22046#discussion_r906266413
##########
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy:
##########
@@ -1064,6 +1066,7 @@ class BeamModulePlugin implements Plugin<Project> {
// 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"
+ implementation "org.checkerframework:checker-compat-qual:$checkercompatframework_version"
Review Comment:
what does this new dep do and shall we have an issue to record this?
--
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] pabloem commented on pull request #22046: Fix vendored dependency issue and other style checks
Posted by GitBox <gi...@apache.org>.
pabloem commented on PR #22046:
URL: https://github.com/apache/beam/pull/22046#issuecomment-1167674114
hm these checks are disabled for local tests, but they run on jenkins. You should be able to run a `...:checkStyle` task to get them to reprodue locally, I believe
or `...:javaCompile` with -Werror or something like that. Can you run `...:tasks --all` and find the checkstyle task?
--
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 #22046: Fix vendored dependency issue (guava)
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #22046:
URL: https://github.com/apache/beam/pull/22046#issuecomment-1165715324
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] damccorm commented on a diff in pull request #22046: Fix vendored dependency issue and other style checks
Posted by GitBox <gi...@apache.org>.
damccorm commented on code in PR #22046:
URL: https://github.com/apache/beam/pull/22046#discussion_r906277669
##########
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy:
##########
@@ -1064,6 +1066,7 @@ class BeamModulePlugin implements Plugin<Project> {
// 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"
+ implementation "org.checkerframework:checker-compat-qual:$checkercompatframework_version"
Review Comment:
That's a good idea
##########
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy:
##########
@@ -1064,6 +1066,7 @@ class BeamModulePlugin implements Plugin<Project> {
// 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"
+ implementation "org.checkerframework:checker-compat-qual:$checkercompatframework_version"
Review Comment:
That's a good idea - 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] damondouglas commented on pull request #22046: Fix vendored dependency issue and other style checks
Posted by GitBox <gi...@apache.org>.
damondouglas commented on PR #22046:
URL: https://github.com/apache/beam/pull/22046#issuecomment-1167521649
@damccorm (cc: @pabloem , @Abacn ) What is the gradle task to run on my local machine to detect these errors prevent this from happening in the future? Prior to marking #21953 as ready for review, I ran and passed the following gradle tasks.
```
./gradlew rat
./gradlew spotlessCheck
./gradlew sdks:java:io:google-cloud-platform:test
```
When running `./gradlew :javaPreCommit`, I only see errors related to integration tests for which my credentials do not have access to dependent resources.
--
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] damccorm commented on a diff in pull request #22046: Fix vendored dependency issue and other style checks
Posted by GitBox <gi...@apache.org>.
damccorm commented on code in PR #22046:
URL: https://github.com/apache/beam/pull/22046#discussion_r906268191
##########
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy:
##########
@@ -1064,6 +1066,7 @@ class BeamModulePlugin implements Plugin<Project> {
// 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"
+ implementation "org.checkerframework:checker-compat-qual:$checkercompatframework_version"
Review Comment:
It gives us `org.checkerframework.checker.nullness.compatqual.NonNullType` which is used in the PubSubSchemaTransformMessageToRowFactory for an annotation.
I don't think we need an issue to record it (we don't generally need an issue every time we add a small new dependency), this is also coming from the same checkerframework used previously
--
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 #22046: Fix vendored dependency issue (guava)
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #22046:
URL: https://github.com/apache/beam/pull/22046#issuecomment-1165715318
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] asf-ci commented on pull request #22046: Fix vendored dependency issue (guava)
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #22046:
URL: https://github.com/apache/beam/pull/22046#issuecomment-1165715317
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] damccorm commented on pull request #22046: Fix vendored dependency issue and other style checks
Posted by GitBox <gi...@apache.org>.
damccorm commented on PR #22046:
URL: https://github.com/apache/beam/pull/22046#issuecomment-1165829336
@damondouglas FYI - your change (#22044) broke some precommits, this fixes that. I don't think any action is needed, but you may want to validate that
--
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] Abacn commented on a diff in pull request #22046: Fix vendored dependency issue and other style checks
Posted by GitBox <gi...@apache.org>.
Abacn commented on code in PR #22046:
URL: https://github.com/apache/beam/pull/22046#discussion_r906275225
##########
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy:
##########
@@ -1064,6 +1066,7 @@ class BeamModulePlugin implements Plugin<Project> {
// 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"
+ implementation "org.checkerframework:checker-compat-qual:$checkercompatframework_version"
Review Comment:
I see the annotation. May replace with @Nonnull from `org.checkerframework.checker.nullness.qual.NonNull` which is used elsewhere in the codebase. compatqual is not updated in years and looks like a legacy version?
##########
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy:
##########
@@ -1064,6 +1066,7 @@ class BeamModulePlugin implements Plugin<Project> {
// 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"
+ implementation "org.checkerframework:checker-compat-qual:$checkercompatframework_version"
Review Comment:
I see the annotation. May replace with `@Nonnull` from `org.checkerframework.checker.nullness.qual.NonNull` which is used elsewhere in the codebase. compatqual is not updated in years and looks like a legacy version?
--
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] Abacn commented on pull request #22046: Fix vendored dependency issue (guava)
Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #22046:
URL: https://github.com/apache/beam/pull/22046#issuecomment-1165746815
If we do fix forward (instead of rollback #22044) there are also several checkStyle violations in tests
--
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