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/03/30 00:13:38 UTC
[GitHub] [beam] riteshghorse opened a new pull request #17216: Simplify specifying additional dependencies in Go SDK in XLang IOs
riteshghorse opened a new pull request #17216:
URL: https://github.com/apache/beam/pull/17216
Currently, to add dependencies to the expansion service, we have to modify the build.gradle file and build it again. To simplify this, users can pass the maven-gradle style jar name and it is fetched and new jar is composed when starting an automated expansion service in Go SDK. Tested it with mysql but and the expansion service is able to find the corresponding Driver. However, due to other error connecting to mysql (which existed before this the other way as well) I added the test for just postgresql.
Overall this mechanism won't be in the test suite unless we use automated expansion service for our XLang IOs.
Attached is image showing `tmp.jar`, the composed jar including the specified dependencies.
------------------------
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] codecov[bot] edited a comment on pull request #17216: Simplify specifying additional dependencies in Go SDK in XLang IOs
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17216:
URL: https://github.com/apache/beam/pull/17216#issuecomment-1082488184
# [Codecov](https://codecov.io/gh/apache/beam/pull/17216?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 [#17216](https://codecov.io/gh/apache/beam/pull/17216?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0edf2d2) into [master](https://codecov.io/gh/apache/beam/commit/4c90af8c4e5e294cabf4474574e55ae957a557c5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4c90af8) will **decrease** coverage by `0.15%`.
> The diff coverage is `0.00%`.
> :exclamation: Current head 0edf2d2 differs from pull request most recent head 3fa8cbf. Consider uploading reports for the commit 3fa8cbf to get more accurate results
```diff
@@ Coverage Diff @@
## master #17216 +/- ##
==========================================
- Coverage 74.03% 73.88% -0.16%
==========================================
Files 672 672
Lines 88284 88470 +186
==========================================
Hits 65364 65364
- Misses 21800 21986 +186
Partials 1120 1120
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `49.39% <0.00%> (-0.37%)` | :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/17216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/go/pkg/beam/core/graph/xlang.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL3hsYW5nLmdv) | `100.00% <ø> (ø)` | |
| [sdks/go/pkg/beam/core/runtime/xlangx/expand.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUveGxhbmd4L2V4cGFuZC5nbw==) | `0.00% <0.00%> (ø)` | |
| [...kg/beam/core/runtime/xlangx/expansionx/download.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUveGxhbmd4L2V4cGFuc2lvbngvZG93bmxvYWQuZ28=) | `16.73% <0.00%> (-44.81%)` | :arrow_down: |
| [sdks/go/pkg/beam/xlang.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS94bGFuZy5nbw==) | `21.68% <0.00%> (-1.69%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/17216?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/17216?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 [4c90af8...3fa8cbf](https://codecov.io/gh/apache/beam/pull/17216?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] riteshghorse commented on pull request #17216: Simplify specifying additional dependencies in Go SDK in XLang IOs
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on pull request #17216:
URL: https://github.com/apache/beam/pull/17216#issuecomment-1082488370
R: @youngoli @damccorm @jrmccluskey @lostluck
--
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] riteshghorse commented on a change in pull request #17216: Simplify specifying additional dependencies in Go SDK in XLang IOs
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on a change in pull request #17216:
URL: https://github.com/apache/beam/pull/17216#discussion_r840126042
##########
File path: sdks/go.mod
##########
@@ -23,33 +23,38 @@ module github.com/apache/beam/sdks/v2
go 1.16
require (
- cloud.google.com/go/bigquery v1.17.0
- cloud.google.com/go/datastore v1.5.0
- cloud.google.com/go/pubsub v1.11.0-beta.schemas
- cloud.google.com/go/storage v1.15.0
+ cloud.google.com/go/bigquery v1.29.0
Review comment:
I guess I used the same environment for hackathon which added further dependencies. Doing `go mod tidy` now removes most of it.
--
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] edited a comment on pull request #17216: Simplify specifying additional dependencies in Go SDK in XLang IOs
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17216:
URL: https://github.com/apache/beam/pull/17216#issuecomment-1082488184
# [Codecov](https://codecov.io/gh/apache/beam/pull/17216?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 [#17216](https://codecov.io/gh/apache/beam/pull/17216?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3fa8cbf) into [master](https://codecov.io/gh/apache/beam/commit/4c90af8c4e5e294cabf4474574e55ae957a557c5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4c90af8) will **decrease** coverage by `0.24%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #17216 +/- ##
==========================================
- Coverage 74.03% 73.78% -0.25%
==========================================
Files 672 672
Lines 88284 88608 +324
==========================================
+ Hits 65364 65383 +19
- Misses 21800 22104 +304
- Partials 1120 1121 +1
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `49.19% <0.00%> (-0.57%)` | :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/17216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/go/pkg/beam/core/graph/xlang.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL3hsYW5nLmdv) | `100.00% <ø> (ø)` | |
| [sdks/go/pkg/beam/core/runtime/xlangx/expand.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUveGxhbmd4L2V4cGFuZC5nbw==) | `0.00% <0.00%> (ø)` | |
| [...kg/beam/core/runtime/xlangx/expansionx/download.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUveGxhbmd4L2V4cGFuc2lvbngvZG93bmxvYWQuZ28=) | `16.66% <0.00%> (-44.88%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/runtime/xlangx/registry.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUveGxhbmd4L3JlZ2lzdHJ5Lmdv) | `48.38% <0.00%> (-3.34%)` | :arrow_down: |
| [sdks/go/pkg/beam/xlang.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS94bGFuZy5nbw==) | `23.37% <0.00%> (ø)` | |
| [sdks/go/pkg/beam/core/runtime/exec/fn\_arity.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9mbl9hcml0eS5nbw==) | `8.00% <0.00%> (-0.89%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/runtime/graphx/serialize.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZ3JhcGh4L3NlcmlhbGl6ZS5nbw==) | `26.27% <0.00%> (-0.17%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/funcx/fn.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2Z1bmN4L2ZuLmdv) | `57.54% <0.00%> (-0.04%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/runtime/graphx/translate.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZ3JhcGh4L3RyYW5zbGF0ZS5nbw==) | `28.81% <0.00%> (-0.02%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/util/reflectx/calls.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3V0aWwvcmVmbGVjdHgvY2FsbHMuZ28=) | `0.00% <0.00%> (ø)` | |
| ... and [2 more](https://codecov.io/gh/apache/beam/pull/17216/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/17216?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/17216?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 [4c90af8...3fa8cbf](https://codecov.io/gh/apache/beam/pull/17216?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] codecov[bot] edited a comment on pull request #17216: Simplify specifying additional dependencies in Go SDK in XLang IOs
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17216:
URL: https://github.com/apache/beam/pull/17216#issuecomment-1082488184
# [Codecov](https://codecov.io/gh/apache/beam/pull/17216?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 [#17216](https://codecov.io/gh/apache/beam/pull/17216?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (069388c) into [master](https://codecov.io/gh/apache/beam/commit/4c90af8c4e5e294cabf4474574e55ae957a557c5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4c90af8) will **decrease** coverage by `0.24%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #17216 +/- ##
==========================================
- Coverage 74.03% 73.78% -0.25%
==========================================
Files 672 672
Lines 88284 88608 +324
==========================================
+ Hits 65364 65383 +19
- Misses 21800 22104 +304
- Partials 1120 1121 +1
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `49.19% <0.00%> (-0.57%)` | :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/17216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/go/pkg/beam/core/runtime/xlangx/expand.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUveGxhbmd4L2V4cGFuZC5nbw==) | `0.00% <0.00%> (ø)` | |
| [...kg/beam/core/runtime/xlangx/expansionx/download.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUveGxhbmd4L2V4cGFuc2lvbngvZG93bmxvYWQuZ28=) | `16.66% <0.00%> (-44.88%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/runtime/xlangx/registry.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUveGxhbmd4L3JlZ2lzdHJ5Lmdv) | `48.38% <0.00%> (-3.34%)` | :arrow_down: |
| [sdks/go/pkg/beam/xlang.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS94bGFuZy5nbw==) | `23.37% <0.00%> (ø)` | |
| [sdks/go/pkg/beam/core/runtime/exec/fn\_arity.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9mbl9hcml0eS5nbw==) | `8.00% <0.00%> (-0.89%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/runtime/graphx/serialize.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZ3JhcGh4L3NlcmlhbGl6ZS5nbw==) | `26.27% <0.00%> (-0.17%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/funcx/fn.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2Z1bmN4L2ZuLmdv) | `57.54% <0.00%> (-0.04%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/runtime/graphx/translate.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZ3JhcGh4L3RyYW5zbGF0ZS5nbw==) | `28.81% <0.00%> (-0.02%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/util/reflectx/calls.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3V0aWwvcmVmbGVjdHgvY2FsbHMuZ28=) | `0.00% <0.00%> (ø)` | |
| [sdks/go/pkg/beam/core/typex/class.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3R5cGV4L2NsYXNzLmdv) | `85.49% <0.00%> (+0.11%)` | :arrow_up: |
| ... and [1 more](https://codecov.io/gh/apache/beam/pull/17216/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/17216?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/17216?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 [4c90af8...069388c](https://codecov.io/gh/apache/beam/pull/17216?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] lostluck commented on a change in pull request #17216: Simplify specifying additional dependencies in Go SDK in XLang IOs
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #17216:
URL: https://github.com/apache/beam/pull/17216#discussion_r839821647
##########
File path: sdks/go/pkg/beam/core/runtime/xlangx/expansionx/download.go
##########
@@ -68,6 +70,225 @@ func SetDefaultRepositoryURL(repoURL string) error {
return defaultJarGetter.setRepositoryURL(repoURL)
}
+func buildJarName(artifactId, version string) string {
+ return fmt.Sprintf("%s-%s.jar", artifactId, version)
+}
+
+func getMavenJar(artifactID, groupID, version string) string {
+ return strings.Join([]string{string(apacheRepository), strings.ReplaceAll(groupID, ".", "/"), artifactID, version, buildJarName(artifactID, version)}, "/")
+}
+
+func expandJar(jar string) string {
+ if jarExists(jar) {
+ return jar
+ } else if strings.HasPrefix(jar, "http://") || strings.HasPrefix(jar, "https://") {
+ return jar
+ } else {
+ components := strings.Split(jar, ":")
+ groupID, artifactID, version := components[0], components[1], components[2]
+ path := getMavenJar(artifactID, groupID, version)
+ return path
+ }
+}
+
+func getLocalJar(url string) (string, error) {
+ jarName := path.Base(url)
+ usr, _ := user.Current()
+ cacheDir := filepath.Join(usr.HomeDir, jarCache[2:])
+ jarPath := filepath.Join(cacheDir, jarName)
+
+ if jarExists(jarPath) {
+ return jarPath, nil
+ }
+
+ resp, err := http.Get(string(url))
+ if err != nil {
+ return "", err
+ }
+ defer resp.Body.Close()
+
+ if resp.StatusCode != 200 {
+ return "", fmt.Errorf("failed to connect to %v: received non 200 response code, got %v", url, resp.StatusCode)
+ }
+
+ file, err := os.Create(jarPath)
+ if err != nil {
+ return "", errors.New(fmt.Sprintf("error in create: %v", err))
Review comment:
Prefer using fmt.Errorf instead of layering.
Please use `%w` for the error parameter as well.
##########
File path: sdks/go/pkg/beam/core/runtime/xlangx/expansionx/download.go
##########
@@ -68,6 +70,225 @@ func SetDefaultRepositoryURL(repoURL string) error {
return defaultJarGetter.setRepositoryURL(repoURL)
}
+func buildJarName(artifactId, version string) string {
+ return fmt.Sprintf("%s-%s.jar", artifactId, version)
+}
+
+func getMavenJar(artifactID, groupID, version string) string {
+ return strings.Join([]string{string(apacheRepository), strings.ReplaceAll(groupID, ".", "/"), artifactID, version, buildJarName(artifactID, version)}, "/")
+}
+
+func expandJar(jar string) string {
+ if jarExists(jar) {
+ return jar
+ } else if strings.HasPrefix(jar, "http://") || strings.HasPrefix(jar, "https://") {
+ return jar
+ } else {
+ components := strings.Split(jar, ":")
+ groupID, artifactID, version := components[0], components[1], components[2]
+ path := getMavenJar(artifactID, groupID, version)
+ return path
+ }
+}
+
+func getLocalJar(url string) (string, error) {
+ jarName := path.Base(url)
+ usr, _ := user.Current()
+ cacheDir := filepath.Join(usr.HomeDir, jarCache[2:])
+ jarPath := filepath.Join(cacheDir, jarName)
+
+ if jarExists(jarPath) {
+ return jarPath, nil
+ }
+
+ resp, err := http.Get(string(url))
+ if err != nil {
+ return "", err
+ }
+ defer resp.Body.Close()
+
+ if resp.StatusCode != 200 {
+ return "", fmt.Errorf("failed to connect to %v: received non 200 response code, got %v", url, resp.StatusCode)
+ }
+
+ file, err := os.Create(jarPath)
+ if err != nil {
+ return "", errors.New(fmt.Sprintf("error in create: %v", err))
+ }
+
+ _, err = io.Copy(file, resp.Body)
+ if err != nil {
+ return "", errors.New(fmt.Sprintf("error in copy: %v", err))
+ }
+
+ return jarPath, nil
+}
+
+func extractJar(source, dest string) error {
+ reader, err := zip.OpenReader(source)
+ if err != nil {
+ return fmt.Errorf("error extracting jar extractJar(%s,%s)= %v", source, dest, err)
+ }
+
+ if err := os.MkdirAll(dest, 0700); err != nil {
+ return fmt.Errorf("error creating directory %s in extractJar(%s,%s)= %v", dest, source, dest, err)
Review comment:
This one is very good, replace the `=` with `:` and the %v for the error parameter with %w.
##########
File path: sdks/go/pkg/beam/core/runtime/xlangx/expand.go
##########
@@ -143,18 +144,26 @@ func QueryExpansionService(ctx context.Context, p *HandlerParams) (*jobpb.Expans
return res, nil
}
-func startAutomatedJavaExpansionService(gradleTarget string) (stopFunc func() error, address string, err error) {
+func startAutomatedJavaExpansionService(gradleTarget string, classpath []string) (stopFunc func() error, address string, err error) {
jarPath, err := expansionx.GetBeamJar(gradleTarget, core.SdkVersion)
if err != nil {
return nil, "", err
}
+
+ if len(classpath) > 0 {
+ jarPath, err = expansionx.MakeJar(jarPath, classpath)
+ if err != nil {
+ return nil, "", err
+ }
+ }
+
serviceRunner, err := expansionx.NewExpansionServiceRunner(jarPath, "")
if err != nil {
- return nil, "", err
+ return nil, "", fmt.Errorf("error in new expansion service: %v", err)
}
err = serviceRunner.StartService()
if err != nil {
- return nil, "", err
+ return nil, "", fmt.Errorf("error in start: %v", err)
Review comment:
As added, the extra information is OK, but doesn't help explain where things are going wrong.
Consider also including the gradle target, or the jarpath that the call was trying to make.
This is the superpower of Go's explicit approach to error handling over just a stack trace: You can include the parameters used along the way to improve the context in the error messages.
##########
File path: sdks/go/pkg/beam/core/runtime/xlangx/expand.go
##########
@@ -143,18 +144,26 @@ func QueryExpansionService(ctx context.Context, p *HandlerParams) (*jobpb.Expans
return res, nil
}
-func startAutomatedJavaExpansionService(gradleTarget string) (stopFunc func() error, address string, err error) {
+func startAutomatedJavaExpansionService(gradleTarget string, classpath []string) (stopFunc func() error, address string, err error) {
Review comment:
Question: Is the `classpath` slice representing a single class path, or does each element in the slice represent another class path.
If it's the former, leave it as is.
If it's the latter, rename it to `classpaths` to indicate that it represents more than one.
##########
File path: sdks/go/pkg/beam/core/runtime/xlangx/expand.go
##########
@@ -143,18 +144,26 @@ func QueryExpansionService(ctx context.Context, p *HandlerParams) (*jobpb.Expans
return res, nil
}
-func startAutomatedJavaExpansionService(gradleTarget string) (stopFunc func() error, address string, err error) {
+func startAutomatedJavaExpansionService(gradleTarget string, classpath []string) (stopFunc func() error, address string, err error) {
jarPath, err := expansionx.GetBeamJar(gradleTarget, core.SdkVersion)
if err != nil {
return nil, "", err
}
+
+ if len(classpath) > 0 {
+ jarPath, err = expansionx.MakeJar(jarPath, classpath)
+ if err != nil {
+ return nil, "", err
+ }
+ }
+
serviceRunner, err := expansionx.NewExpansionServiceRunner(jarPath, "")
if err != nil {
- return nil, "", err
+ return nil, "", fmt.Errorf("error in new expansion service: %v", err)
Review comment:
When producing an error with `fmt.Errorf` prefer `%w` over `%v` for the error parameter, so the error is wrapped.
Here and everywhere.
##########
File path: sdks/go/pkg/beam/core/runtime/xlangx/expansionx/download.go
##########
@@ -68,6 +70,225 @@ func SetDefaultRepositoryURL(repoURL string) error {
return defaultJarGetter.setRepositoryURL(repoURL)
}
+func buildJarName(artifactId, version string) string {
+ return fmt.Sprintf("%s-%s.jar", artifactId, version)
+}
+
+func getMavenJar(artifactID, groupID, version string) string {
+ return strings.Join([]string{string(apacheRepository), strings.ReplaceAll(groupID, ".", "/"), artifactID, version, buildJarName(artifactID, version)}, "/")
+}
+
+func expandJar(jar string) string {
+ if jarExists(jar) {
+ return jar
+ } else if strings.HasPrefix(jar, "http://") || strings.HasPrefix(jar, "https://") {
+ return jar
+ } else {
+ components := strings.Split(jar, ":")
+ groupID, artifactID, version := components[0], components[1], components[2]
+ path := getMavenJar(artifactID, groupID, version)
+ return path
+ }
+}
+
+func getLocalJar(url string) (string, error) {
+ jarName := path.Base(url)
+ usr, _ := user.Current()
+ cacheDir := filepath.Join(usr.HomeDir, jarCache[2:])
+ jarPath := filepath.Join(cacheDir, jarName)
+
+ if jarExists(jarPath) {
+ return jarPath, nil
+ }
+
+ resp, err := http.Get(string(url))
+ if err != nil {
+ return "", err
+ }
+ defer resp.Body.Close()
+
+ if resp.StatusCode != 200 {
+ return "", fmt.Errorf("failed to connect to %v: received non 200 response code, got %v", url, resp.StatusCode)
+ }
+
+ file, err := os.Create(jarPath)
+ if err != nil {
+ return "", errors.New(fmt.Sprintf("error in create: %v", err))
+ }
+
+ _, err = io.Copy(file, resp.Body)
+ if err != nil {
+ return "", errors.New(fmt.Sprintf("error in copy: %v", err))
Review comment:
Same here.
##########
File path: sdks/go/pkg/beam/core/runtime/xlangx/expansionx/download.go
##########
@@ -68,6 +70,225 @@ func SetDefaultRepositoryURL(repoURL string) error {
return defaultJarGetter.setRepositoryURL(repoURL)
}
+func buildJarName(artifactId, version string) string {
+ return fmt.Sprintf("%s-%s.jar", artifactId, version)
+}
+
+func getMavenJar(artifactID, groupID, version string) string {
+ return strings.Join([]string{string(apacheRepository), strings.ReplaceAll(groupID, ".", "/"), artifactID, version, buildJarName(artifactID, version)}, "/")
+}
+
+func expandJar(jar string) string {
+ if jarExists(jar) {
+ return jar
+ } else if strings.HasPrefix(jar, "http://") || strings.HasPrefix(jar, "https://") {
+ return jar
+ } else {
+ components := strings.Split(jar, ":")
+ groupID, artifactID, version := components[0], components[1], components[2]
+ path := getMavenJar(artifactID, groupID, version)
+ return path
+ }
+}
+
+func getLocalJar(url string) (string, error) {
+ jarName := path.Base(url)
+ usr, _ := user.Current()
+ cacheDir := filepath.Join(usr.HomeDir, jarCache[2:])
+ jarPath := filepath.Join(cacheDir, jarName)
+
+ if jarExists(jarPath) {
+ return jarPath, nil
+ }
+
+ resp, err := http.Get(string(url))
+ if err != nil {
+ return "", err
+ }
+ defer resp.Body.Close()
+
+ if resp.StatusCode != 200 {
+ return "", fmt.Errorf("failed to connect to %v: received non 200 response code, got %v", url, resp.StatusCode)
+ }
+
+ file, err := os.Create(jarPath)
+ if err != nil {
+ return "", errors.New(fmt.Sprintf("error in create: %v", err))
+ }
+
+ _, err = io.Copy(file, resp.Body)
+ if err != nil {
+ return "", errors.New(fmt.Sprintf("error in copy: %v", err))
+ }
+
+ return jarPath, nil
+}
+
+func extractJar(source, dest string) error {
+ reader, err := zip.OpenReader(source)
+ if err != nil {
+ return fmt.Errorf("error extracting jar extractJar(%s,%s)= %v", source, dest, err)
+ }
+
+ if err := os.MkdirAll(dest, 0700); err != nil {
+ return fmt.Errorf("error creating directory %s in extractJar(%s,%s)= %v", dest, source, dest, err)
+ }
+
+ for _, file := range reader.File {
+ fileName := filepath.Join(dest, file.Name)
+ if file.FileInfo().IsDir() {
+ os.MkdirAll(fileName, 0700)
+ continue
+ }
+
+ f, err := file.Open()
+ if err != nil {
+ return fmt.Errorf("error opening file %s: %v", file.Name, err)
+ }
+ defer f.Close()
+
+ tf, err := os.OpenFile(fileName, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0777)
+ if err != nil {
+ return fmt.Errorf("error opening file %s: %v", fileName, err)
+ }
+ defer tf.Close()
+
+ if _, err := io.Copy(tf, f); err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+func packJar(source, dest string) error {
+ jar, err := os.Create(dest)
+ if err != nil {
+ return fmt.Errorf("error creating jar packJar(%s,%s)=%v", source, dest, err)
+ }
+ defer jar.Close()
+
+ jarFile := zip.NewWriter(jar)
+ defer jarFile.Close()
+
+ fileInfo, err := os.Stat(source)
+ if err != nil {
+ return fmt.Errorf("source path %s doesn't exist: %v", source, err)
+ }
+
+ var sourceDir string
+ if fileInfo.IsDir() {
+ sourceDir = filepath.Base(source)
+ }
+
+ err = filepath.Walk(source, func(path string, fileInfo os.FileInfo, err error) error {
+ if err != nil {
+ return fmt.Errorf("error accesing path %s: %v", path, err)
+ }
+ fileHeader, err := zip.FileInfoHeader(fileInfo)
+ if err != nil {
+ return fmt.Errorf("error getting FileInfoHeader: %v", err)
+ }
+
+ if sourceDir != "" {
+ fileHeader.Name = filepath.Join(sourceDir, strings.TrimPrefix(path, source))
+ }
+
+ if fileInfo.IsDir() {
+ fileHeader.Name += "/"
+ } else {
+ fileHeader.Method = zip.Deflate
+ }
+
+ writer, err := jarFile.CreateHeader(fileHeader)
+ if err != nil {
+ return fmt.Errorf("error creating jarFile header: %v", err)
+ }
+ if fileInfo.IsDir() {
+ return nil
+ }
+
+ f, err := os.Open(path)
+ if err != nil {
+ return fmt.Errorf("error opening file %s: %v", path, err)
+ }
+ defer f.Close()
+
+ if _, err = io.Copy(writer, f); err != nil {
+ return fmt.Errorf("error copying file %s: %v", path, err)
+ }
+ return nil
+ })
+ return err
+}
+
+// MakeJar fetches additional classpath JARs and adds it to the classpath of
+// main JAR file and compiles a fresh JAR.
+func MakeJar(mainJar string, classpath []string) (string, error) {
+ usr, _ := user.Current()
+ cacheDir := filepath.Join(usr.HomeDir, jarCache[2:])
+
+ // fetch jars required in classpath
+ classpathJars := []string{}
+ for _, jar := range classpath {
+ path := expandJar(jar)
+ if j, err := getLocalJar(path); err == nil {
+ classpathJars = append(classpathJars, j)
+ } else {
+ return "", errors.New(fmt.Sprintf("error in getLocal(): %v", err))
+ }
+ }
+
+ // classpath jars should have relative path
+ relClasspath := []string{}
+ for _, path := range classpathJars {
+ relPath, err := filepath.Rel(cacheDir, path)
+ if err != nil {
+ return "", fmt.Errorf("error in creating relative path: %v", err)
+ }
+ relClasspath = append(relClasspath, relPath)
+ }
+
+ tmpDir := filepath.Join(cacheDir, "tmpDir")
+
+ if err := extractJar(mainJar, tmpDir); err != nil {
+ return "", errors.New(fmt.Sprintf("error in extractJar(): %v", err))
+ }
+
+ b, err := os.ReadFile(tmpDir + "/META-INF/MANIFEST.MF")
+ if err != nil {
+ return "", fmt.Errorf("error readingf: %v", err)
+ }
+
+ // trim the empty lines present at the end of MANIFEST.MF file.
+ str := strings.Split(string(b), "\n")
+ str = str[:len(str)-2]
Review comment:
While Go is pretty terse, it generally avoids just 'b' and 'str' in contexts where they're ambiguous, or know better.
`m` would be better for `b`, but there's no harm with going with `manifest` as that's what the bytes are.
`str` doesn't explain anything, just that it's a string, but it's not a string, it's a []string. The name would be improved as `lines` or `manifestLines`, since that's what they are.
##########
File path: sdks/go/pkg/beam/core/graph/xlang.go
##########
@@ -66,6 +66,7 @@ type ExternalTransform struct {
Urn string
Payload []byte
ExpansionAddr string
+ Classpath []string
Review comment:
Same question here about whether this is a single classpath, or multiple. If multiple, please make the field plural.
##########
File path: sdks/go/pkg/beam/xlang.go
##########
@@ -173,6 +212,7 @@ func TryCrossLanguage(
expansionAddr string,
namedInputs map[string]PCollection,
namedOutputTypes map[string]FullType,
+ classpath []string,
Review comment:
This is a breaking change we shouldn't make for a one off. If anything, we should break it harder than this so we can avoid breaking it in the future.
##########
File path: sdks/go/pkg/beam/core/runtime/xlangx/expansionx/download.go
##########
@@ -68,6 +70,225 @@ func SetDefaultRepositoryURL(repoURL string) error {
return defaultJarGetter.setRepositoryURL(repoURL)
}
+func buildJarName(artifactId, version string) string {
+ return fmt.Sprintf("%s-%s.jar", artifactId, version)
+}
+
+func getMavenJar(artifactID, groupID, version string) string {
+ return strings.Join([]string{string(apacheRepository), strings.ReplaceAll(groupID, ".", "/"), artifactID, version, buildJarName(artifactID, version)}, "/")
+}
+
+func expandJar(jar string) string {
+ if jarExists(jar) {
+ return jar
+ } else if strings.HasPrefix(jar, "http://") || strings.HasPrefix(jar, "https://") {
+ return jar
+ } else {
+ components := strings.Split(jar, ":")
+ groupID, artifactID, version := components[0], components[1], components[2]
+ path := getMavenJar(artifactID, groupID, version)
+ return path
+ }
+}
+
+func getLocalJar(url string) (string, error) {
+ jarName := path.Base(url)
+ usr, _ := user.Current()
+ cacheDir := filepath.Join(usr.HomeDir, jarCache[2:])
+ jarPath := filepath.Join(cacheDir, jarName)
+
+ if jarExists(jarPath) {
+ return jarPath, nil
+ }
+
+ resp, err := http.Get(string(url))
+ if err != nil {
+ return "", err
+ }
+ defer resp.Body.Close()
+
+ if resp.StatusCode != 200 {
+ return "", fmt.Errorf("failed to connect to %v: received non 200 response code, got %v", url, resp.StatusCode)
+ }
+
+ file, err := os.Create(jarPath)
+ if err != nil {
+ return "", errors.New(fmt.Sprintf("error in create: %v", err))
+ }
+
+ _, err = io.Copy(file, resp.Body)
+ if err != nil {
+ return "", errors.New(fmt.Sprintf("error in copy: %v", err))
+ }
+
+ return jarPath, nil
+}
+
+func extractJar(source, dest string) error {
+ reader, err := zip.OpenReader(source)
+ if err != nil {
+ return fmt.Errorf("error extracting jar extractJar(%s,%s)= %v", source, dest, err)
+ }
+
+ if err := os.MkdirAll(dest, 0700); err != nil {
+ return fmt.Errorf("error creating directory %s in extractJar(%s,%s)= %v", dest, source, dest, err)
+ }
+
+ for _, file := range reader.File {
+ fileName := filepath.Join(dest, file.Name)
+ if file.FileInfo().IsDir() {
+ os.MkdirAll(fileName, 0700)
+ continue
+ }
+
+ f, err := file.Open()
+ if err != nil {
+ return fmt.Errorf("error opening file %s: %v", file.Name, err)
+ }
+ defer f.Close()
+
+ tf, err := os.OpenFile(fileName, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0777)
+ if err != nil {
+ return fmt.Errorf("error opening file %s: %v", fileName, err)
Review comment:
These are good error messages, but consider clarifying which one is the source, and which one is the dest.
##########
File path: sdks/go.mod
##########
@@ -23,33 +23,38 @@ module github.com/apache/beam/sdks/v2
go 1.16
require (
- cloud.google.com/go/bigquery v1.17.0
- cloud.google.com/go/datastore v1.5.0
- cloud.google.com/go/pubsub v1.11.0-beta.schemas
- cloud.google.com/go/storage v1.15.0
+ cloud.google.com/go/bigquery v1.29.0
Review comment:
What is your workflow? I wouldn't expect to see go.mod to be changed for every PR. One should be running `go mod tidy` when a new import is added, but that shouldn't be forcing upgrading of arbitrary deps on nearly every PR.
##########
File path: sdks/go/pkg/beam/core/runtime/xlangx/expansionx/download.go
##########
@@ -68,6 +70,225 @@ func SetDefaultRepositoryURL(repoURL string) error {
return defaultJarGetter.setRepositoryURL(repoURL)
}
+func buildJarName(artifactId, version string) string {
+ return fmt.Sprintf("%s-%s.jar", artifactId, version)
+}
+
+func getMavenJar(artifactID, groupID, version string) string {
+ return strings.Join([]string{string(apacheRepository), strings.ReplaceAll(groupID, ".", "/"), artifactID, version, buildJarName(artifactID, version)}, "/")
+}
+
+func expandJar(jar string) string {
+ if jarExists(jar) {
+ return jar
+ } else if strings.HasPrefix(jar, "http://") || strings.HasPrefix(jar, "https://") {
+ return jar
+ } else {
+ components := strings.Split(jar, ":")
+ groupID, artifactID, version := components[0], components[1], components[2]
+ path := getMavenJar(artifactID, groupID, version)
+ return path
+ }
+}
+
+func getLocalJar(url string) (string, error) {
+ jarName := path.Base(url)
+ usr, _ := user.Current()
+ cacheDir := filepath.Join(usr.HomeDir, jarCache[2:])
+ jarPath := filepath.Join(cacheDir, jarName)
+
+ if jarExists(jarPath) {
+ return jarPath, nil
+ }
+
+ resp, err := http.Get(string(url))
+ if err != nil {
+ return "", err
+ }
+ defer resp.Body.Close()
+
+ if resp.StatusCode != 200 {
+ return "", fmt.Errorf("failed to connect to %v: received non 200 response code, got %v", url, resp.StatusCode)
+ }
+
+ file, err := os.Create(jarPath)
+ if err != nil {
+ return "", errors.New(fmt.Sprintf("error in create: %v", err))
+ }
+
+ _, err = io.Copy(file, resp.Body)
+ if err != nil {
+ return "", errors.New(fmt.Sprintf("error in copy: %v", err))
+ }
+
+ return jarPath, nil
+}
+
+func extractJar(source, dest string) error {
+ reader, err := zip.OpenReader(source)
+ if err != nil {
+ return fmt.Errorf("error extracting jar extractJar(%s,%s)= %v", source, dest, err)
Review comment:
This is a fairly good error message, but it's being a bit redundant as "extracting jar" is pretty clear from having `extractJar` being mentioned. But it's not clear that what failed was opening the jar file.
The '=' doesn't help much in this context. A reader seeing this in their logs won't necessarily know that error is the only output.
Finally, %w, so that the error is properly wrapped. This allows the helpers in the [errors](https://pkg.go.dev/errors) package to work, for us and our users.
```suggestion
return fmt.Errorf("error opening jar for extractJar(%s,%s): %w", source, dest, err)
```
##########
File path: sdks/go/pkg/beam/xlang.go
##########
@@ -68,6 +68,45 @@ func CrossLanguagePayload(pl interface{}) []byte {
return bytes
}
+// CrossLanguageWithClasspath is a low-level transform for executing cross-language transforms written in other
+// SDKs. Because this is low-level, it is recommended to use one of the higher-level IO-specific
+// wrappers where available. These can be found in the pkg/beam/io/xlang subdirectory.
+// CrossLanguage is useful for executing cross-language transforms which do not have any existing
+// IO wrappers. The functions beam.CrossLanguage() and beam.CrossLanguageWithClasspath() are similar except for
+// additional classpath dependencies that the later one accepts.
+//
+// Note: This should be specifically used when using an automated expansion service where you want to add additional
+// custom dependencies in the form of classpath to the expansion service.
+//
+// Usage requires an address for an expansion service accessible during pipeline construction, a
+// URN identifying the desired transform, an optional payload with configuration information,
+// input and output names, and classpath dependencies as an array of string. It outputs a map of named output PCollections.
+//
+// Example
+//
+// This example shows using CrossLanguageWithClasspath to execute the JDBC cross-language transform.
+//
+// pl := beam.CrossLanguagePayload(JdbcConfigSchema)
+// expansionAddr := xlang.UseAutomatedJavaExpansionService(gradleTargetOfExpansionService)
+// readURN := ""
+// classpath := []string{"org.postgresql:postgresql:42.3.3"}
+// result := beam.CrossLanguageWithClasspath(s, readURN, pl, expansionAddr, nil, beam.UnnamedOutput(typex.New(outT)), classpath)
+func CrossLanguageWithClasspath(
+ s Scope,
+ urn string,
+ payload []byte,
+ expansionAddr string,
+ namedInputs map[string]PCollection,
+ namedOutputTypes map[string]FullType,
+ classpath []string,
+) map[string]PCollection {
Review comment:
Instead of a new specific purpose method (in this case same as before but with a classpath parameter), lets try building it out as a new general purpose one.
It should always have the scope, urn, payload, namedInputs, namedOutputs parameters, and a variadic xlangOption typed parameter.
This way we can have expansionAddr and class path become *optional* parameters, and enable some of the automatic start up stuff more directly, and less hacked in like it is presently.
No need to fully replace things under the hood for the others at this time, but I wouldn't say no.
--
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] edited a comment on pull request #17216: Simplify specifying additional dependencies in Go SDK in XLang IOs
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17216:
URL: https://github.com/apache/beam/pull/17216#issuecomment-1082488184
# [Codecov](https://codecov.io/gh/apache/beam/pull/17216?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 [#17216](https://codecov.io/gh/apache/beam/pull/17216?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0edf2d2) into [master](https://codecov.io/gh/apache/beam/commit/4c90af8c4e5e294cabf4474574e55ae957a557c5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4c90af8) will **decrease** coverage by `0.15%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #17216 +/- ##
==========================================
- Coverage 74.03% 73.88% -0.16%
==========================================
Files 672 672
Lines 88284 88470 +186
==========================================
Hits 65364 65364
- Misses 21800 21986 +186
Partials 1120 1120
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `49.39% <0.00%> (-0.37%)` | :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/17216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/go/pkg/beam/core/graph/xlang.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL3hsYW5nLmdv) | `100.00% <ø> (ø)` | |
| [sdks/go/pkg/beam/core/runtime/xlangx/expand.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUveGxhbmd4L2V4cGFuZC5nbw==) | `0.00% <0.00%> (ø)` | |
| [...kg/beam/core/runtime/xlangx/expansionx/download.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUveGxhbmd4L2V4cGFuc2lvbngvZG93bmxvYWQuZ28=) | `16.73% <0.00%> (-44.81%)` | :arrow_down: |
| [sdks/go/pkg/beam/xlang.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS94bGFuZy5nbw==) | `21.68% <0.00%> (-1.69%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/17216?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/17216?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 [4c90af8...0edf2d2](https://codecov.io/gh/apache/beam/pull/17216?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] pabloem commented on a change in pull request #17216: Simplify specifying additional dependencies in Go SDK in XLang IOs
Posted by GitBox <gi...@apache.org>.
pabloem commented on a change in pull request #17216:
URL: https://github.com/apache/beam/pull/17216#discussion_r839956258
##########
File path: sdks/go/pkg/beam/core/graph/xlang.go
##########
@@ -66,6 +66,7 @@ type ExternalTransform struct {
Urn string
Payload []byte
ExpansionAddr string
+ Classpath []string
Review comment:
I also wonder if there should be a separate ExternalTransform struct for Java that has a classpath, since classpaths are not a thing in other languages
--
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] riteshghorse commented on a change in pull request #17216: Simplify specifying additional dependencies in Go SDK in XLang IOs
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on a change in pull request #17216:
URL: https://github.com/apache/beam/pull/17216#discussion_r840219347
##########
File path: sdks/go/pkg/beam/core/graph/xlang.go
##########
@@ -66,6 +66,7 @@ type ExternalTransform struct {
Urn string
Payload []byte
ExpansionAddr string
+ Classpath []string
Review comment:
I guess building the expansion address handler and parsing the string later to extract classpath can get rid of this field.
--
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] riteshghorse commented on a change in pull request #17216: Simplify specifying additional dependencies in Go SDK in XLang IOs
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on a change in pull request #17216:
URL: https://github.com/apache/beam/pull/17216#discussion_r840218389
##########
File path: sdks/go/pkg/beam/xlang.go
##########
@@ -173,6 +212,7 @@ func TryCrossLanguage(
expansionAddr string,
namedInputs map[string]PCollection,
namedOutputTypes map[string]FullType,
+ classpath []string,
Review comment:
I guess building the expansion address handler can avoid doing this. Got it.
--
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] riteshghorse edited a comment on pull request #17216: Simplify specifying additional dependencies in Go SDK in XLang IOs
Posted by GitBox <gi...@apache.org>.
riteshghorse edited a comment on pull request #17216:
URL: https://github.com/apache/beam/pull/17216#issuecomment-1082488599
Run Go 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] riteshghorse commented on a change in pull request #17216: Simplify specifying additional dependencies in Go SDK in XLang IOs
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on a change in pull request #17216:
URL: https://github.com/apache/beam/pull/17216#discussion_r840225729
##########
File path: sdks/go/pkg/beam/xlang.go
##########
@@ -68,6 +68,45 @@ func CrossLanguagePayload(pl interface{}) []byte {
return bytes
}
+// CrossLanguageWithClasspath is a low-level transform for executing cross-language transforms written in other
+// SDKs. Because this is low-level, it is recommended to use one of the higher-level IO-specific
+// wrappers where available. These can be found in the pkg/beam/io/xlang subdirectory.
+// CrossLanguage is useful for executing cross-language transforms which do not have any existing
+// IO wrappers. The functions beam.CrossLanguage() and beam.CrossLanguageWithClasspath() are similar except for
+// additional classpath dependencies that the later one accepts.
+//
+// Note: This should be specifically used when using an automated expansion service where you want to add additional
+// custom dependencies in the form of classpath to the expansion service.
+//
+// Usage requires an address for an expansion service accessible during pipeline construction, a
+// URN identifying the desired transform, an optional payload with configuration information,
+// input and output names, and classpath dependencies as an array of string. It outputs a map of named output PCollections.
+//
+// Example
+//
+// This example shows using CrossLanguageWithClasspath to execute the JDBC cross-language transform.
+//
+// pl := beam.CrossLanguagePayload(JdbcConfigSchema)
+// expansionAddr := xlang.UseAutomatedJavaExpansionService(gradleTargetOfExpansionService)
+// readURN := ""
+// classpath := []string{"org.postgresql:postgresql:42.3.3"}
+// result := beam.CrossLanguageWithClasspath(s, readURN, pl, expansionAddr, nil, beam.UnnamedOutput(typex.New(outT)), classpath)
+func CrossLanguageWithClasspath(
+ s Scope,
+ urn string,
+ payload []byte,
+ expansionAddr string,
+ namedInputs map[string]PCollection,
+ namedOutputTypes map[string]FullType,
+ classpath []string,
+) map[string]PCollection {
Review comment:
I guess building the expansion address handler can get rid of this. It is good to have though for users who want to use automated expansion service.
--
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 #17216: Simplify specifying additional dependencies in Go SDK in XLang IOs
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #17216:
URL: https://github.com/apache/beam/pull/17216#issuecomment-1082488184
# [Codecov](https://codecov.io/gh/apache/beam/pull/17216?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 [#17216](https://codecov.io/gh/apache/beam/pull/17216?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0edf2d2) into [master](https://codecov.io/gh/apache/beam/commit/4c90af8c4e5e294cabf4474574e55ae957a557c5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4c90af8) will **decrease** coverage by `0.15%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #17216 +/- ##
==========================================
- Coverage 74.03% 73.88% -0.16%
==========================================
Files 672 672
Lines 88284 88470 +186
==========================================
Hits 65364 65364
- Misses 21800 21986 +186
Partials 1120 1120
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `49.39% <0.00%> (-0.37%)` | :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/17216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/go/pkg/beam/core/graph/xlang.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL3hsYW5nLmdv) | `100.00% <ø> (ø)` | |
| [sdks/go/pkg/beam/core/runtime/xlangx/expand.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUveGxhbmd4L2V4cGFuZC5nbw==) | `0.00% <0.00%> (ø)` | |
| [...kg/beam/core/runtime/xlangx/expansionx/download.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUveGxhbmd4L2V4cGFuc2lvbngvZG93bmxvYWQuZ28=) | `16.73% <0.00%> (-44.81%)` | :arrow_down: |
| [sdks/go/pkg/beam/xlang.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS94bGFuZy5nbw==) | `21.68% <0.00%> (-1.69%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/17216?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/17216?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 [4c90af8...0edf2d2](https://codecov.io/gh/apache/beam/pull/17216?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] github-actions[bot] commented on pull request #17216: Simplify specifying additional dependencies in Go SDK in XLang IOs
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17216:
URL: https://github.com/apache/beam/pull/17216#issuecomment-1082488995
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control
--
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] riteshghorse commented on pull request #17216: Simplify specifying additional dependencies in Go SDK in XLang IOs
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on pull request #17216:
URL: https://github.com/apache/beam/pull/17216#issuecomment-1082488599
Run Go 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] codecov[bot] edited a comment on pull request #17216: Simplify specifying additional dependencies in Go SDK in XLang IOs
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17216:
URL: https://github.com/apache/beam/pull/17216#issuecomment-1082488184
# [Codecov](https://codecov.io/gh/apache/beam/pull/17216?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 [#17216](https://codecov.io/gh/apache/beam/pull/17216?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0edf2d2) into [master](https://codecov.io/gh/apache/beam/commit/4c90af8c4e5e294cabf4474574e55ae957a557c5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4c90af8) will **decrease** coverage by `0.15%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #17216 +/- ##
==========================================
- Coverage 74.03% 73.88% -0.16%
==========================================
Files 672 672
Lines 88284 88470 +186
==========================================
Hits 65364 65364
- Misses 21800 21986 +186
Partials 1120 1120
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `49.39% <0.00%> (-0.37%)` | :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/17216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/go/pkg/beam/core/graph/xlang.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL3hsYW5nLmdv) | `100.00% <ø> (ø)` | |
| [sdks/go/pkg/beam/core/runtime/xlangx/expand.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUveGxhbmd4L2V4cGFuZC5nbw==) | `0.00% <0.00%> (ø)` | |
| [...kg/beam/core/runtime/xlangx/expansionx/download.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUveGxhbmd4L2V4cGFuc2lvbngvZG93bmxvYWQuZ28=) | `16.73% <0.00%> (-44.81%)` | :arrow_down: |
| [sdks/go/pkg/beam/xlang.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS94bGFuZy5nbw==) | `21.68% <0.00%> (-1.69%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/17216?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/17216?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 [4c90af8...0edf2d2](https://codecov.io/gh/apache/beam/pull/17216?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] riteshghorse commented on a change in pull request #17216: Simplify specifying additional dependencies in Go SDK in XLang IOs
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on a change in pull request #17216:
URL: https://github.com/apache/beam/pull/17216#discussion_r840126042
##########
File path: sdks/go.mod
##########
@@ -23,33 +23,38 @@ module github.com/apache/beam/sdks/v2
go 1.16
require (
- cloud.google.com/go/bigquery v1.17.0
- cloud.google.com/go/datastore v1.5.0
- cloud.google.com/go/pubsub v1.11.0-beta.schemas
- cloud.google.com/go/storage v1.15.0
+ cloud.google.com/go/bigquery v1.29.0
Review comment:
I guess I used the same environment for hackathon which added further dependencies. Doing `go mod tidy` now removes some of it.
--
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] edited a comment on pull request #17216: Simplify specifying additional dependencies in Go SDK in XLang IOs
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17216:
URL: https://github.com/apache/beam/pull/17216#issuecomment-1082488184
# [Codecov](https://codecov.io/gh/apache/beam/pull/17216?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 [#17216](https://codecov.io/gh/apache/beam/pull/17216?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3fa8cbf) into [master](https://codecov.io/gh/apache/beam/commit/4c90af8c4e5e294cabf4474574e55ae957a557c5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4c90af8) will **decrease** coverage by `0.24%`.
> The diff coverage is `0.00%`.
> :exclamation: Current head 3fa8cbf differs from pull request most recent head 069388c. Consider uploading reports for the commit 069388c to get more accurate results
```diff
@@ Coverage Diff @@
## master #17216 +/- ##
==========================================
- Coverage 74.03% 73.78% -0.25%
==========================================
Files 672 672
Lines 88284 88608 +324
==========================================
+ Hits 65364 65383 +19
- Misses 21800 22104 +304
- Partials 1120 1121 +1
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `49.19% <0.00%> (-0.57%)` | :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/17216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/go/pkg/beam/core/graph/xlang.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL3hsYW5nLmdv) | `100.00% <ø> (ø)` | |
| [sdks/go/pkg/beam/core/runtime/xlangx/expand.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUveGxhbmd4L2V4cGFuZC5nbw==) | `0.00% <0.00%> (ø)` | |
| [...kg/beam/core/runtime/xlangx/expansionx/download.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUveGxhbmd4L2V4cGFuc2lvbngvZG93bmxvYWQuZ28=) | `16.66% <0.00%> (-44.88%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/runtime/xlangx/registry.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUveGxhbmd4L3JlZ2lzdHJ5Lmdv) | `48.38% <0.00%> (-3.34%)` | :arrow_down: |
| [sdks/go/pkg/beam/xlang.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS94bGFuZy5nbw==) | `23.37% <0.00%> (ø)` | |
| [sdks/go/pkg/beam/core/runtime/exec/fn\_arity.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9mbl9hcml0eS5nbw==) | `8.00% <0.00%> (-0.89%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/runtime/graphx/serialize.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZ3JhcGh4L3NlcmlhbGl6ZS5nbw==) | `26.27% <0.00%> (-0.17%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/funcx/fn.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2Z1bmN4L2ZuLmdv) | `57.54% <0.00%> (-0.04%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/runtime/graphx/translate.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZ3JhcGh4L3RyYW5zbGF0ZS5nbw==) | `28.81% <0.00%> (-0.02%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/util/reflectx/calls.go](https://codecov.io/gh/apache/beam/pull/17216/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3V0aWwvcmVmbGVjdHgvY2FsbHMuZ28=) | `0.00% <0.00%> (ø)` | |
| ... and [2 more](https://codecov.io/gh/apache/beam/pull/17216/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/17216?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/17216?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 [4c90af8...069388c](https://codecov.io/gh/apache/beam/pull/17216?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] riteshghorse commented on pull request #17216: Simplify specifying additional dependencies in Go SDK in XLang IOs
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on pull request #17216:
URL: https://github.com/apache/beam/pull/17216#issuecomment-1085913262
Used the expansion address builder which eliminated the breaking change and the need to have a separate `CrossLanguage` function for classpath.
PTAL.
--
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