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/02/26 00:02:06 UTC
[GitHub] [beam] riteshghorse opened a new pull request #16957: [BEAM-13829] Expose status API from Go SDK Harness
riteshghorse opened a new pull request #16957:
URL: https://github.com/apache/beam/pull/16957
Expose SDK harness status from Go SDK to the runner.
------------------------
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] lostluck commented on a change in pull request #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r837982546
##########
File path: sdks/go/pkg/beam/core/runtime/harness/harness.go
##########
@@ -36,14 +36,27 @@ import (
"google.golang.org/grpc"
)
+// StatusAddress is a type of status endpoint address as an optional argument to harness.Main().
+type StatusAddress string
+
// TODO(herohde) 2/8/2017: for now, assume we stage a full binary (not a plugin).
// Main is the main entrypoint for the Go harness. It runs at "runtime" -- not
// "pipeline-construction time" -- on each worker. It is a FnAPI client and
// ultimately responsible for correctly executing user code.
-func Main(ctx context.Context, loggingEndpoint, controlEndpoint string) error {
+func Main(ctx context.Context, loggingEndpoint, controlEndpoint string, options ...interface{}) error {
hooks.DeserializeHooksFromOptions(ctx)
+ statusEndpoint := ""
+ for _, option := range options {
+ switch option := option.(type) {
+ case StatusAddress:
+ statusEndpoint = string(option)
+ default:
+ return errors.Errorf("unkown type %T, value %v in error call", option, option)
Review comment:
```suggestion
return errors.Errorf("unknown type %T, value %v in error call", option, option)
```
--
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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r819116998
##########
File path: sdks/go.mod
##########
@@ -23,33 +23,34 @@ 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.28.0
+ cloud.google.com/go/compute v1.5.0 // indirect
+ cloud.google.com/go/datastore v1.6.0
+ cloud.google.com/go/iam v0.2.0 // indirect
+ cloud.google.com/go/pubsub v1.18.0
+ cloud.google.com/go/storage v1.21.0
Review comment:
Yeah I had called `go get`.
--
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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r819017568
##########
File path: sdks/go.mod
##########
@@ -23,33 +23,39 @@ 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.28.0
+ cloud.google.com/go/compute v1.5.0 // indirect
+ cloud.google.com/go/datastore v1.6.0
+ cloud.google.com/go/iam v0.2.0 // indirect
+ cloud.google.com/go/pubsub v1.18.0
+ cloud.google.com/go/storage v1.21.0
+ github.com/bketelsen/crypt v0.0.4 // indirect
Review comment:
Agreed that it's strange. Unless one is adding a new dependency/import it largely shouldn't show up here... strange. But I'm less worried about additions than I am about unintentional version updates.
Note that with Go Modules, none of this will affect things for users unless they've got a conflicting requirement, at which point Go will pick the maximum of the two. This behavior tends to keep things up to date. (Then one gets into the issue of breaking changes or bad versions, but I won't go into that here.)
--
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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r830418153
##########
File path: sdks/go/pkg/beam/core/runtime/harness/harness.go
##########
@@ -36,14 +36,27 @@ import (
"google.golang.org/grpc"
)
+// StatusAddress is a type of status endpoint address as an optional argument to harness.Main().
+type StatusAddress string
+
// TODO(herohde) 2/8/2017: for now, assume we stage a full binary (not a plugin).
// Main is the main entrypoint for the Go harness. It runs at "runtime" -- not
// "pipeline-construction time" -- on each worker. It is a FnAPI client and
// ultimately responsible for correctly executing user code.
-func Main(ctx context.Context, loggingEndpoint, controlEndpoint string) error {
+func Main(ctx context.Context, loggingEndpoint, controlEndpoint string, options ...interface{}) error {
Review comment:
Agreed this is change we are probably OK with as this is an internal implementation detail no one should be relying on. I've also checked google3 for that, and the only users are the harness/init and the extworker packages....
Even though it's adding a variadic, it's technically still a breaking change as it's a type signature change. However, it's not likely to break any callers that aren't strictly associating the type with this function.
--
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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1054589987
# [Codecov](https://codecov.io/gh/apache/beam/pull/16957?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 [#16957](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (61b195f) into [master](https://codecov.io/gh/apache/beam/commit/bed5e19e084c3fdb02e5d2a9d97a1a51e655004c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bed5e19) will **decrease** coverage by `0.01%`.
> The diff coverage is `48.38%`.
[](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16957 +/- ##
==========================================
- Coverage 73.47% 73.46% -0.02%
==========================================
Files 663 664 +1
Lines 87056 87114 +58
==========================================
+ Hits 63963 63996 +33
- Misses 22034 22054 +20
- Partials 1059 1064 +5
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `47.69% <48.38%> (+0.02%)` | :arrow_up: |
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/16957?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/harness/harness.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9oYXJuZXNzLmdv) | `13.02% <0.00%> (+0.33%)` | :arrow_up: |
| [.../go/pkg/beam/core/runtime/harness/worker\_status.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy93b3JrZXJfc3RhdHVzLmdv) | `66.66% <66.66%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/16957?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/16957?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 [bed5e19...61b195f](https://codecov.io/gh/apache/beam/pull/16957?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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1054589987
# [Codecov](https://codecov.io/gh/apache/beam/pull/16957?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 [#16957](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (61b195f) into [master](https://codecov.io/gh/apache/beam/commit/3cd1f7f949bd476abb11bdb0b368a2f12a496cd1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3cd1f7f) will **decrease** coverage by `0.34%`.
> The diff coverage is `48.38%`.
> :exclamation: Current head 61b195f differs from pull request most recent head 63aa879. Consider uploading reports for the commit 63aa879 to get more accurate results
[](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16957 +/- ##
==========================================
- Coverage 73.80% 73.46% -0.35%
==========================================
Files 663 664 +1
Lines 87056 87114 +58
==========================================
- Hits 64251 63996 -255
- Misses 21721 22054 +333
+ Partials 1084 1064 -20
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `47.69% <48.38%> (-1.11%)` | :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/16957?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/harness/harness.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9oYXJuZXNzLmdv) | `13.02% <0.00%> (+0.33%)` | :arrow_up: |
| [.../go/pkg/beam/core/runtime/harness/worker\_status.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy93b3JrZXJfc3RhdHVzLmdv) | `66.66% <66.66%> (ø)` | |
| [sdks/go/pkg/beam/runners/direct/impulse.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL2RpcmVjdC9pbXB1bHNlLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [sdks/go/pkg/beam/runners/direct/buffer.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL2RpcmVjdC9idWZmZXIuZ28=) | `0.00% <0.00%> (-77.47%)` | :arrow_down: |
| [sdks/go/pkg/beam/runners/direct/direct.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL2RpcmVjdC9kaXJlY3QuZ28=) | `1.36% <0.00%> (-65.00%)` | :arrow_down: |
| [sdks/go/pkg/beam/runners/direct/gbk.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL2RpcmVjdC9nYmsuZ28=) | `22.76% <0.00%> (-49.60%)` | :arrow_down: |
| [.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/16957/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.37% <0.00%> (-4.66%)` | :arrow_down: |
| [...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/16957/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.97% <0.00%> (-2.26%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `87.50% <0.00%> (-1.71%)` | :arrow_down: |
| [.../python/apache\_beam/transforms/periodicsequence.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9wZXJpb2RpY3NlcXVlbmNlLnB5) | `96.72% <0.00%> (-1.64%)` | :arrow_down: |
| ... and [3 more](https://codecov.io/gh/apache/beam/pull/16957/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/16957?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/16957?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 [3cd1f7f...63aa879](https://codecov.io/gh/apache/beam/pull/16957?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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1054589987
# [Codecov](https://codecov.io/gh/apache/beam/pull/16957?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 [#16957](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e1539d0) into [master](https://codecov.io/gh/apache/beam/commit/d1b8e569fd651975f08823a3db49dbee56d491b5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d1b8e56) will **decrease** coverage by `27.65%`.
> The diff coverage is `45.45%`.
> :exclamation: Current head e1539d0 differs from pull request most recent head 6c99b51. Consider uploading reports for the commit 6c99b51 to get more accurate results
[](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16957 +/- ##
===========================================
- Coverage 74.67% 47.02% -27.66%
===========================================
Files 654 208 -446
Lines 82133 24387 -57746
===========================================
- Hits 61334 11468 -49866
+ Misses 19809 11875 -7934
- Partials 990 1044 +54
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `47.02% <45.45%> (?)` | |
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/16957?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/harness/harness.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9oYXJuZXNzLmdv) | `12.95% <0.00%> (-2.17%)` | :arrow_down: |
| [.../go/pkg/beam/core/runtime/harness/worker\_status.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy93b3JrZXJfc3RhdHVzLmdv) | `60.34% <60.34%> (ø)` | |
| [sdks/go/pkg/beam/core/runtime/exec/emit.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9lbWl0Lmdv) | `51.92% <0.00%> (-9.99%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/util/protox/any.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3V0aWwvcHJvdG94L2FueS5nbw==) | `46.93% <0.00%> (-9.16%)` | :arrow_down: |
| [sdks/go/pkg/beam/provision/provision.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9wcm92aXNpb24vcHJvdmlzaW9uLmdv) | `27.50% <0.00%> (-7.99%)` | :arrow_down: |
| [sdks/go/pkg/beam/flatten.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9mbGF0dGVuLmdv) | `44.44% <0.00%> (-7.94%)` | :arrow_down: |
| [sdks/go/pkg/beam/validate.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS92YWxpZGF0ZS5nbw==) | `42.10% <0.00%> (-7.90%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/graph/coder/int.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL2ludC5nbw==) | `70.00% <0.00%> (-7.78%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/graph/coder/time.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL3RpbWUuZ28=) | `70.00% <0.00%> (-7.78%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/graph/coder/stringutf8.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL3N0cmluZ3V0ZjguZ28=) | `65.90% <0.00%> (-7.78%)` | :arrow_down: |
| ... and [629 more](https://codecov.io/gh/apache/beam/pull/16957/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/16957?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/16957?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 [d1b8e56...6c99b51](https://codecov.io/gh/apache/beam/pull/16957?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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1063230282
R: @youngoli
yet to add lull logger.
--
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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r837987480
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status.go
##########
@@ -0,0 +1,91 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "io"
+ "runtime"
+ "sync"
+ "sync/atomic"
+ "time"
+
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/internal/errors"
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/log"
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+// workerStatusHandler stores the communication information of WorkerStatus API.
+type workerStatusHandler struct {
+ conn *grpc.ClientConn
+ shutdown int32
+}
+
+func newWorkerStatusHandler(ctx context.Context, endpoint string) (*workerStatusHandler, error) {
+ sconn, err := dial(ctx, endpoint, 60*time.Second)
+ if err != nil {
+ return &workerStatusHandler{}, errors.Wrapf(err, "failed to connect: %v\n", endpoint)
Review comment:
nit: Conventionally, when a non-nil error is being returned, the other return values are not considered valid. If they're pointer types, they should return `nil` instead of a pointer to a zeroed value.
If the returned non-error values are valid, the convention is to document this behavior, and why.
Eg. The [`io.Reader`](https://pkg.go.dev/io#Reader) interface defines this kind of thing. The `n` is always valid, and an error of `io.EOF` is considered a success.
In this case, it would be cleaner if nil is returned, and the failure case is properly handled by the caller.
--
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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1054589987
# [Codecov](https://codecov.io/gh/apache/beam/pull/16957?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 [#16957](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (96a9280) into [master](https://codecov.io/gh/apache/beam/commit/3cd1f7f949bd476abb11bdb0b368a2f12a496cd1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3cd1f7f) will **increase** coverage by `0.04%`.
> The diff coverage is `41.07%`.
> :exclamation: Current head 96a9280 differs from pull request most recent head 3a4557f. Consider uploading reports for the commit 3a4557f to get more accurate results
```diff
@@ Coverage Diff @@
## master #16957 +/- ##
==========================================
+ Coverage 73.80% 73.84% +0.04%
==========================================
Files 663 672 +9
Lines 87056 87522 +466
==========================================
+ Hits 64251 64630 +379
- Misses 21721 21777 +56
- Partials 1084 1115 +31
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `49.44% <41.07%> (+0.64%)` | :arrow_up: |
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/16957?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/harness/harness.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9oYXJuZXNzLmdv) | `10.94% <0.00%> (-1.75%)` | :arrow_down: |
| [.../go/pkg/beam/core/runtime/harness/worker\_status.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy93b3JrZXJfc3RhdHVzLmdv) | `58.97% <58.97%> (ø)` | |
| [sdks/go/pkg/beam/core/runtime/exec/plan.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9wbGFuLmdv) | `48.61% <0.00%> (-8.80%)` | :arrow_down: |
| [.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/16957/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.37% <0.00%> (-4.66%)` | :arrow_down: |
| [...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/16957/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.97% <0.00%> (-2.26%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `87.50% <0.00%> (-1.71%)` | :arrow_down: |
| [.../python/apache\_beam/transforms/periodicsequence.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9wZXJpb2RpY3NlcXVlbmNlLnB5) | `96.72% <0.00%> (-1.64%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/16957/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.51% <0.00%> (-0.13%)` | :arrow_down: |
| [sdks/go/pkg/beam/runners/direct/buffer.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL2RpcmVjdC9idWZmZXIuZ28=) | `77.46% <0.00%> (ø)` | |
| ... and [21 more](https://codecov.io/gh/apache/beam/pull/16957/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/16957?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/16957?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 [3cd1f7f...3a4557f](https://codecov.io/gh/apache/beam/pull/16957?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] youngoli commented on a change in pull request #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
youngoli commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r829487283
##########
File path: sdks/go/pkg/beam/core/runtime/harness/harness.go
##########
@@ -36,14 +36,27 @@ import (
"google.golang.org/grpc"
)
+// StatusAddress is a type of status endpoint address as an optional argument to harness.Main().
+type StatusAddress string
+
// TODO(herohde) 2/8/2017: for now, assume we stage a full binary (not a plugin).
// Main is the main entrypoint for the Go harness. It runs at "runtime" -- not
// "pipeline-construction time" -- on each worker. It is a FnAPI client and
// ultimately responsible for correctly executing user code.
-func Main(ctx context.Context, loggingEndpoint, controlEndpoint string) error {
+func Main(ctx context.Context, loggingEndpoint, controlEndpoint string, options ...interface{}) error {
Review comment:
This is changing the signature of an exported function, which is something we usually want to avoid... But on the other hand the harness package is very unlikely to be used by end-users and is meant to just be used within Beam, so I think this should be fine. But just to be sure, I think we should run this by @lostluck.
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status.go
##########
@@ -0,0 +1,91 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
Review comment:
Nit: You could probably shorten the filename to status.go (and status_test.go). Not something I'll block on though.
##########
File path: sdks/go/pkg/beam/core/runtime/harness/harness.go
##########
@@ -98,6 +111,18 @@ func Main(ctx context.Context, loggingEndpoint, controlEndpoint string) error {
log.Debugf(ctx, "control response channel closed")
}()
+ // if the runner supports worker status api then expose SDK harness status
+ if statusEndpoint != "" {
+ statusHandler, err := newWorkerStatusHandler(ctx, statusEndpoint)
+ if err != nil {
+ log.Error(ctx, err)
+ }
+ var wg sync.WaitGroup
Review comment:
Just checking, but is this meant to be a different WaitGroup than the one declared outside this scope? If so I think it would be clearer to rename this (even something like "wg2" would be okay, or "swg" for status wait group). It just makes it very explicit that this is supposed to be a different variable than the one above.
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status_test.go
##########
@@ -0,0 +1,77 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "fmt"
+ "log"
+ "net"
+ "sync"
+ "testing"
+
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+type BeamFnWorkerStatusServicer struct {
+ fnpb.UnimplementedBeamFnWorkerStatusServer
+ response chan string
+}
+
+func (w *BeamFnWorkerStatusServicer) WorkerStatus(b fnpb.BeamFnWorkerStatus_WorkerStatusServer) error {
+ b.Send(&fnpb.WorkerStatusRequest{Id: "1"})
+ resp, err := b.Recv()
+ if err != nil {
+ return fmt.Errorf("error receiving response b.recv: %v", err)
+ }
+ w.response <- resp.GetStatusInfo()
+ return nil
+}
+
+func setUp(port string, srv *BeamFnWorkerStatusServicer) {
+ l, err := net.Listen("tcp", ":9000")
Review comment:
Can you use 0 as a port to have it listen on an available random port? As-is this could be flaky due to port conflicts.
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status_test.go
##########
@@ -0,0 +1,77 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "fmt"
+ "log"
+ "net"
+ "sync"
+ "testing"
+
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+type BeamFnWorkerStatusServicer struct {
+ fnpb.UnimplementedBeamFnWorkerStatusServer
+ response chan string
+}
+
+func (w *BeamFnWorkerStatusServicer) WorkerStatus(b fnpb.BeamFnWorkerStatus_WorkerStatusServer) error {
+ b.Send(&fnpb.WorkerStatusRequest{Id: "1"})
+ resp, err := b.Recv()
+ if err != nil {
+ return fmt.Errorf("error receiving response b.recv: %v", err)
+ }
+ w.response <- resp.GetStatusInfo()
+ return nil
+}
+
+func setUp(port string, srv *BeamFnWorkerStatusServicer) {
Review comment:
This function isn't actually using the port passed into it, that looks like an oversight.
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status_test.go
##########
@@ -0,0 +1,77 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "fmt"
+ "log"
+ "net"
+ "sync"
+ "testing"
+
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+type BeamFnWorkerStatusServicer struct {
Review comment:
This is less of a comment for you and more of a question for @lostluck, but I've noticed a few unit tests now that start up actual GRPC servers to test our grpc files. Should we maybe transition to using a local package like buffcon, like in this Stack Overflow question: https://stackoverflow.com/questions/42102496/testing-a-grpc-service ?
To be clear, even if the answer is yes we can save it for a subsequent PR. This isn't something to address in this PR either way.
--
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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1055632913
R: @lostluck @damccorm @jrmccluskey
--
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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r816996939
##########
File path: sdks/go.mod
##########
@@ -23,33 +23,39 @@ 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.28.0
+ cloud.google.com/go/compute v1.5.0 // indirect
+ cloud.google.com/go/datastore v1.6.0
+ cloud.google.com/go/iam v0.2.0 // indirect
+ cloud.google.com/go/pubsub v1.18.0
+ cloud.google.com/go/storage v1.21.0
+ github.com/bketelsen/crypt v0.0.4 // indirect
Review comment:
I got them with go get when I cloned a fresh master repo. Not sure if all of them are related to this PR. But I guess I encountered errors in previous PRs tests when I didn't push go.mod and go.sum changes.
--
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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1054589987
# [Codecov](https://codecov.io/gh/apache/beam/pull/16957?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 [#16957](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (61b195f) into [master](https://codecov.io/gh/apache/beam/commit/3cd1f7f949bd476abb11bdb0b368a2f12a496cd1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3cd1f7f) will **decrease** coverage by `0.34%`.
> The diff coverage is `48.38%`.
> :exclamation: Current head 61b195f differs from pull request most recent head 2ad7048. Consider uploading reports for the commit 2ad7048 to get more accurate results
[](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16957 +/- ##
==========================================
- Coverage 73.80% 73.46% -0.35%
==========================================
Files 663 664 +1
Lines 87056 87114 +58
==========================================
- Hits 64251 63996 -255
- Misses 21721 22054 +333
+ Partials 1084 1064 -20
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `47.69% <48.38%> (-1.11%)` | :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/16957?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/harness/harness.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9oYXJuZXNzLmdv) | `13.02% <0.00%> (+0.33%)` | :arrow_up: |
| [.../go/pkg/beam/core/runtime/harness/worker\_status.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy93b3JrZXJfc3RhdHVzLmdv) | `66.66% <66.66%> (ø)` | |
| [sdks/go/pkg/beam/runners/direct/impulse.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL2RpcmVjdC9pbXB1bHNlLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [sdks/go/pkg/beam/runners/direct/buffer.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL2RpcmVjdC9idWZmZXIuZ28=) | `0.00% <0.00%> (-77.47%)` | :arrow_down: |
| [sdks/go/pkg/beam/runners/direct/direct.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL2RpcmVjdC9kaXJlY3QuZ28=) | `1.36% <0.00%> (-65.00%)` | :arrow_down: |
| [sdks/go/pkg/beam/runners/direct/gbk.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL2RpcmVjdC9nYmsuZ28=) | `22.76% <0.00%> (-49.60%)` | :arrow_down: |
| [.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/16957/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.37% <0.00%> (-4.66%)` | :arrow_down: |
| [...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/16957/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.97% <0.00%> (-2.26%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `87.50% <0.00%> (-1.71%)` | :arrow_down: |
| [.../python/apache\_beam/transforms/periodicsequence.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9wZXJpb2RpY3NlcXVlbmNlLnB5) | `96.72% <0.00%> (-1.64%)` | :arrow_down: |
| ... and [3 more](https://codecov.io/gh/apache/beam/pull/16957/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/16957?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/16957?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 [3cd1f7f...2ad7048](https://codecov.io/gh/apache/beam/pull/16957?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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r819121836
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status.go
##########
@@ -0,0 +1,99 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "io"
+ "runtime"
+ "sync"
+ "sync/atomic"
+ "time"
+
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/internal/errors"
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/log"
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+// workerStatusHandler stores the communication information of WorkerStatus API.
+type workerStatusHandler struct {
+ conn *grpc.ClientConn
+ shutdown int32
+ resp chan *fnpb.WorkerStatusResponse
+}
+
+// newWorkerStatusHandler
+func newWorkerStatusHandler(ctx context.Context, endpoint string) (*workerStatusHandler, error) {
+ sconn, err := dial(ctx, endpoint, 60*time.Second)
+ if err != nil {
+ return &workerStatusHandler{}, errors.Wrapf(err, "failed to connect: %v\n", endpoint)
+ }
+ return &workerStatusHandler{conn: sconn, shutdown: 0, resp: make(chan *fnpb.WorkerStatusResponse)}, nil
+}
+
+// handleRequest manages the WorkerStatus API.
+func (w *workerStatusHandler) handleRequest(ctx context.Context, wg *sync.WaitGroup) {
+ defer wg.Done()
+ statusClient := fnpb.NewBeamFnWorkerStatusClient(w.conn)
+ stub, err := statusClient.WorkerStatus(ctx)
+ if err != nil {
+ log.Errorf(ctx, "status client not established: %v", err)
Review comment:
yes, missed 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] codecov[bot] edited a comment on pull request #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1054589987
# [Codecov](https://codecov.io/gh/apache/beam/pull/16957?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 [#16957](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a62cb4e) into [master](https://codecov.io/gh/apache/beam/commit/3cd1f7f949bd476abb11bdb0b368a2f12a496cd1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3cd1f7f) will **decrease** coverage by `0.01%`.
> The diff coverage is `46.42%`.
[](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16957 +/- ##
==========================================
- Coverage 73.80% 73.78% -0.02%
==========================================
Files 663 668 +5
Lines 87056 87312 +256
==========================================
+ Hits 64251 64424 +173
- Misses 21721 21781 +60
- Partials 1084 1107 +23
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `49.03% <46.42%> (+0.23%)` | :arrow_up: |
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/16957?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/harness/harness.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9oYXJuZXNzLmdv) | `13.02% <0.00%> (+0.33%)` | :arrow_up: |
| [.../go/pkg/beam/core/runtime/harness/worker\_status.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy93b3JrZXJfc3RhdHVzLmdv) | `66.66% <66.66%> (ø)` | |
| [.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/16957/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.37% <0.00%> (-4.66%)` | :arrow_down: |
| [...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/16957/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.97% <0.00%> (-2.26%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `87.50% <0.00%> (-1.71%)` | :arrow_down: |
| [.../python/apache\_beam/transforms/periodicsequence.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9wZXJpb2RpY3NlcXVlbmNlLnB5) | `96.72% <0.00%> (-1.64%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/16957/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.51% <0.00%> (-0.13%)` | :arrow_down: |
| [sdks/go/pkg/beam/io/avroio/avroio.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9pby9hdnJvaW8vYXZyb2lvLmdv) | `56.45% <0.00%> (ø)` | |
| [sdks/go/pkg/beam/core/graph/window/strategy.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL3dpbmRvdy9zdHJhdGVneS5nbw==) | `0.00% <0.00%> (ø)` | |
| [sdks/go/pkg/beam/core/graph/window/windows.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL3dpbmRvdy93aW5kb3dzLmdv) | `82.60% <0.00%> (ø)` | |
| ... and [6 more](https://codecov.io/gh/apache/beam/pull/16957/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/16957?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/16957?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 [3cd1f7f...a62cb4e](https://codecov.io/gh/apache/beam/pull/16957?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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r830418153
##########
File path: sdks/go/pkg/beam/core/runtime/harness/harness.go
##########
@@ -36,14 +36,27 @@ import (
"google.golang.org/grpc"
)
+// StatusAddress is a type of status endpoint address as an optional argument to harness.Main().
+type StatusAddress string
+
// TODO(herohde) 2/8/2017: for now, assume we stage a full binary (not a plugin).
// Main is the main entrypoint for the Go harness. It runs at "runtime" -- not
// "pipeline-construction time" -- on each worker. It is a FnAPI client and
// ultimately responsible for correctly executing user code.
-func Main(ctx context.Context, loggingEndpoint, controlEndpoint string) error {
+func Main(ctx context.Context, loggingEndpoint, controlEndpoint string, options ...interface{}) error {
Review comment:
Agreed this is change we are probably OK with as this is an internal implementation detail no one should be relying on. I've also checked google3 for that, and the only users are the harness/init and the extworker packages....
Even though it's adding a variadic, it's technically still a breaking change as it's a type signature change. However, it's not likely to break any odd callers.
--
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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r831365653
##########
File path: sdks/go/pkg/beam/core/runtime/harness/harness.go
##########
@@ -98,6 +111,18 @@ func Main(ctx context.Context, loggingEndpoint, controlEndpoint string) error {
log.Debugf(ctx, "control response channel closed")
}()
+ // if the runner supports worker status api then expose SDK harness status
+ if statusEndpoint != "" {
+ statusHandler, err := newWorkerStatusHandler(ctx, statusEndpoint)
+ if err != nil {
+ log.Error(ctx, err)
+ }
+ var wg sync.WaitGroup
Review comment:
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] lostluck commented on a change in pull request #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r837997728
##########
File path: sdks/go/pkg/beam/core/runtime/harness/harness.go
##########
@@ -98,6 +111,18 @@ func Main(ctx context.Context, loggingEndpoint, controlEndpoint string) error {
log.Debugf(ctx, "control response channel closed")
}()
+ // if the runner supports worker status api then expose SDK harness status
+ if statusEndpoint != "" {
+ statusHandler, err := newWorkerStatusHandler(ctx, statusEndpoint)
+ if err != nil {
+ log.Error(ctx, err)
+ }
+ var swg sync.WaitGroup
+ swg.Add(1)
Review comment:
Since this WaitGroup is only used inside the status handler, and never called elsewhere, could we move it to be a field in workerStatusHandler instead? That way the WG doesn't need to be a pointer, and it `Add(1)` can be called on the statusHandler's field in `handleRequests`
As a rule of thumb, whenever one is passing around a side value everywhere a struct you own goes, it's worth thinking about putting it into the struct.
##########
File path: sdks/go/pkg/beam/core/runtime/harness/harness.go
##########
@@ -98,6 +111,18 @@ func Main(ctx context.Context, loggingEndpoint, controlEndpoint string) error {
log.Debugf(ctx, "control response channel closed")
}()
+ // if the runner supports worker status api then expose SDK harness status
+ if statusEndpoint != "" {
+ statusHandler, err := newWorkerStatusHandler(ctx, statusEndpoint)
+ if err != nil {
+ log.Error(ctx, err)
+ }
+ var swg sync.WaitGroup
+ swg.Add(1)
+ statusHandler.handleRequest(ctx, &swg)
+ defer statusHandler.close(ctx, &swg)
Review comment:
It feels like this block (after the error `if`) is one of those cases where this should be wrapped in an `else` after the error processing. `
log.Error` won't abort worker set up, so as written, if the set up returns an error, we still do these allocations and set ups, but those will fail anyway. So in my assessment, why go ahead with them on failure?
The defer for the `close` call is good, and will ensure things get cleaned up when the main function returns.
##########
File path: sdks/go/pkg/beam/core/runtime/harness/harness.go
##########
@@ -98,6 +111,18 @@ func Main(ctx context.Context, loggingEndpoint, controlEndpoint string) error {
log.Debugf(ctx, "control response channel closed")
}()
+ // if the runner supports worker status api then expose SDK harness status
+ if statusEndpoint != "" {
+ statusHandler, err := newWorkerStatusHandler(ctx, statusEndpoint)
+ if err != nil {
+ log.Error(ctx, err)
Review comment:
Please add context about what this is logging for, right now all that is known is "that it fails to connect", but nothing says it's trying to connect for a worker status handler.
Similar
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status_test.go
##########
@@ -0,0 +1,87 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "fmt"
+ "log"
+ "net"
+ "sync"
+ "testing"
+
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+ "google.golang.org/grpc/test/bufconn"
+)
+
+type BeamFnWorkerStatusServicer struct {
+ fnpb.UnimplementedBeamFnWorkerStatusServer
+ response chan string
+ lis *bufconn.Listener
+}
+
+func (w *BeamFnWorkerStatusServicer) WorkerStatus(b fnpb.BeamFnWorkerStatus_WorkerStatusServer) error {
+ b.Send(&fnpb.WorkerStatusRequest{Id: "1"})
+ resp, err := b.Recv()
+ if err != nil {
+ return fmt.Errorf("error receiving response b.recv: %v", err)
+ }
+ w.response <- resp.GetStatusInfo()
+ return nil
+}
+
+const buffsize = 1024 * 1024
+
+var lis *bufconn.Listener
+
+func setup(srv *BeamFnWorkerStatusServicer) {
+
Review comment:
nit: rm blank line
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status.go
##########
@@ -0,0 +1,91 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "io"
+ "runtime"
+ "sync"
+ "sync/atomic"
+ "time"
+
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/internal/errors"
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/log"
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+// workerStatusHandler stores the communication information of WorkerStatus API.
+type workerStatusHandler struct {
+ conn *grpc.ClientConn
+ shutdown int32
+}
+
+func newWorkerStatusHandler(ctx context.Context, endpoint string) (*workerStatusHandler, error) {
+ sconn, err := dial(ctx, endpoint, 60*time.Second)
+ if err != nil {
+ return &workerStatusHandler{}, errors.Wrapf(err, "failed to connect: %v\n", endpoint)
+ }
+ return &workerStatusHandler{conn: sconn, shutdown: 0}, nil
+}
+
+func (w *workerStatusHandler) isAlive() bool {
+ return atomic.LoadInt32(&w.shutdown) == 0
+}
+
+func (w *workerStatusHandler) stop() {
+ atomic.StoreInt32(&w.shutdown, 1)
+}
+
+// handleRequest manages the WorkerStatus API.
+func (w *workerStatusHandler) handleRequest(ctx context.Context, wg *sync.WaitGroup) {
+ statusClient := fnpb.NewBeamFnWorkerStatusClient(w.conn)
+ stub, err := statusClient.WorkerStatus(ctx)
+ if err != nil {
+ log.Errorf(ctx, "status client not established: %v", err)
+ return
+ }
+ go w.reader(ctx, stub, wg)
+}
+
+// reader reads the WorkerStatusRequest from the stream and sends a processed WorkerStatusResponse to
+// a response channel.
+func (w *workerStatusHandler) reader(ctx context.Context, stub fnpb.BeamFnWorkerStatus_WorkerStatusClient, wg *sync.WaitGroup) {
+ defer wg.Done()
+ buf := make([]byte, 1<<16)
+ for w.isAlive() {
+ req, err := stub.Recv()
+ if err != nil {
+ log.Debugf(ctx, "exiting workerStatusHandler.Reader(): %v", err)
+ return
+ }
+ log.Debugf(ctx, "RECV-status: %v", req.GetId())
+ runtime.Stack(buf, true)
+ response := &fnpb.WorkerStatusResponse{Id: req.GetId(), StatusInfo: string(buf)}
+ if err := stub.Send(response); err != nil && err != io.EOF {
+ log.Errorf(ctx, "workerStatus.Writer: Failed to respond: %v", err)
+ }
+ }
+}
+
+// close stops the reader first, closes the response channel thereby stopping writer and finally closes the gRPC connection.
Review comment:
Please update the documentation, writer no longer exists.
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status.go
##########
@@ -0,0 +1,91 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "io"
+ "runtime"
+ "sync"
+ "sync/atomic"
+ "time"
+
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/internal/errors"
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/log"
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+// workerStatusHandler stores the communication information of WorkerStatus API.
+type workerStatusHandler struct {
+ conn *grpc.ClientConn
+ shutdown int32
+}
+
+func newWorkerStatusHandler(ctx context.Context, endpoint string) (*workerStatusHandler, error) {
+ sconn, err := dial(ctx, endpoint, 60*time.Second)
+ if err != nil {
+ return &workerStatusHandler{}, errors.Wrapf(err, "failed to connect: %v\n", endpoint)
+ }
+ return &workerStatusHandler{conn: sconn, shutdown: 0}, nil
+}
+
+func (w *workerStatusHandler) isAlive() bool {
+ return atomic.LoadInt32(&w.shutdown) == 0
+}
+
+func (w *workerStatusHandler) stop() {
+ atomic.StoreInt32(&w.shutdown, 1)
+}
+
+// handleRequest manages the WorkerStatus API.
+func (w *workerStatusHandler) handleRequest(ctx context.Context, wg *sync.WaitGroup) {
Review comment:
naming nit: This doesn't handle a single request. `start` or `startHandling` are better names for this, which describes what it does.
The only nit is then it doesn't pair with `close` seamlessly.
I do like `stop()` for the atomic wrapper method, and you're already using `shutdown` for the atomic field.
What do you think about:
Name this method `start`
Rename the `shutdown` field to `shouldShutdown`
Keep `stop` for the atomic wrapper.
Use `shutdown` instead of `close`.
?
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status_test.go
##########
@@ -0,0 +1,87 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "fmt"
+ "log"
+ "net"
+ "sync"
+ "testing"
+
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+ "google.golang.org/grpc/test/bufconn"
+)
+
+type BeamFnWorkerStatusServicer struct {
+ fnpb.UnimplementedBeamFnWorkerStatusServer
+ response chan string
+ lis *bufconn.Listener
+}
+
+func (w *BeamFnWorkerStatusServicer) WorkerStatus(b fnpb.BeamFnWorkerStatus_WorkerStatusServer) error {
+ b.Send(&fnpb.WorkerStatusRequest{Id: "1"})
+ resp, err := b.Recv()
+ if err != nil {
+ return fmt.Errorf("error receiving response b.recv: %v", err)
+ }
+ w.response <- resp.GetStatusInfo()
+ return nil
+}
+
+const buffsize = 1024 * 1024
+
+var lis *bufconn.Listener
+
+func setup(srv *BeamFnWorkerStatusServicer) {
Review comment:
Please pass in `t` here and also have `t.Cleanup` be called so the grpc server can be Stopped at the end of the test, and avoid a leak.
--
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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1054589987
# [Codecov](https://codecov.io/gh/apache/beam/pull/16957?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 [#16957](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (48a335f) into [master](https://codecov.io/gh/apache/beam/commit/d1b8e569fd651975f08823a3db49dbee56d491b5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d1b8e56) will **decrease** coverage by `27.64%`.
> The diff coverage is `47.29%`.
[](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16957 +/- ##
===========================================
- Coverage 74.67% 47.03% -27.65%
===========================================
Files 654 208 -446
Lines 82133 24384 -57749
===========================================
- Hits 61334 11468 -49866
+ Misses 19809 11873 -7936
- Partials 990 1043 +53
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `47.03% <47.29%> (?)` | |
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/16957?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/harness/harness.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9oYXJuZXNzLmdv) | `12.95% <0.00%> (-2.17%)` | :arrow_down: |
| [.../go/pkg/beam/core/runtime/harness/worker\_status.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy93b3JrZXJfc3RhdHVzLmdv) | `63.63% <63.63%> (ø)` | |
| [sdks/go/pkg/beam/core/runtime/exec/emit.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9lbWl0Lmdv) | `51.92% <0.00%> (-9.99%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/util/protox/any.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3V0aWwvcHJvdG94L2FueS5nbw==) | `46.93% <0.00%> (-9.16%)` | :arrow_down: |
| [sdks/go/pkg/beam/provision/provision.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9wcm92aXNpb24vcHJvdmlzaW9uLmdv) | `27.50% <0.00%> (-7.99%)` | :arrow_down: |
| [sdks/go/pkg/beam/flatten.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9mbGF0dGVuLmdv) | `44.44% <0.00%> (-7.94%)` | :arrow_down: |
| [sdks/go/pkg/beam/validate.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS92YWxpZGF0ZS5nbw==) | `42.10% <0.00%> (-7.90%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/graph/coder/int.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL2ludC5nbw==) | `70.00% <0.00%> (-7.78%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/graph/coder/time.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL3RpbWUuZ28=) | `70.00% <0.00%> (-7.78%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/graph/coder/stringutf8.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL3N0cmluZ3V0ZjguZ28=) | `65.90% <0.00%> (-7.78%)` | :arrow_down: |
| ... and [629 more](https://codecov.io/gh/apache/beam/pull/16957/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/16957?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/16957?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 [d1b8e56...48a335f](https://codecov.io/gh/apache/beam/pull/16957?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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r819124347
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status.go
##########
@@ -0,0 +1,99 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "io"
+ "runtime"
+ "sync"
+ "sync/atomic"
+ "time"
+
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/internal/errors"
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/log"
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+// workerStatusHandler stores the communication information of WorkerStatus API.
+type workerStatusHandler struct {
+ conn *grpc.ClientConn
+ shutdown int32
+ resp chan *fnpb.WorkerStatusResponse
+}
+
+// newWorkerStatusHandler
+func newWorkerStatusHandler(ctx context.Context, endpoint string) (*workerStatusHandler, error) {
+ sconn, err := dial(ctx, endpoint, 60*time.Second)
+ if err != nil {
+ return &workerStatusHandler{}, errors.Wrapf(err, "failed to connect: %v\n", endpoint)
+ }
+ return &workerStatusHandler{conn: sconn, shutdown: 0, resp: make(chan *fnpb.WorkerStatusResponse)}, nil
+}
+
+// handleRequest manages the WorkerStatus API.
+func (w *workerStatusHandler) handleRequest(ctx context.Context, wg *sync.WaitGroup) {
+ defer wg.Done()
+ statusClient := fnpb.NewBeamFnWorkerStatusClient(w.conn)
+ stub, err := statusClient.WorkerStatus(ctx)
+ if err != nil {
+ log.Errorf(ctx, "status client not established: %v", err)
+ }
+ go w.writer(ctx, stub)
+ w.reader(ctx, stub)
+}
+
+// writer writes the WorkerStatusResponse recevied from the response channel.
+func (w *workerStatusHandler) writer(ctx context.Context, stub fnpb.BeamFnWorkerStatus_WorkerStatusClient) {
+ for resp := range w.resp {
+ log.Debugf(ctx, "RESP-status: %v", resp.GetId())
+ if err := stub.Send(resp); err != nil && err != io.EOF {
+ log.Errorf(ctx, "workerStatus.Writer: Failed to respond: %v", err)
+ }
+ }
+ log.Debugf(ctx, "exiting workerStatusHandler.Writer()")
+}
+
+// reader reads the WorkerStatusRequest from the stream and sends a processed WorkerStatusResponse to
+// a response channel.
+func (w *workerStatusHandler) reader(ctx context.Context, stub fnpb.BeamFnWorkerStatus_WorkerStatusClient) {
+ for atomic.LoadInt32(&w.shutdown) == 0 {
+ req, err := stub.Recv()
+ if err != nil {
+ log.Debugf(ctx, "exiting workerStatusHandler.Reader(): %v", err)
+ return
+ }
+ log.Debugf(ctx, "RECV-status: %v", req.GetId())
+ buf := make([]byte, 1<<16)
+ runtime.Stack(buf, true)
+ response := &fnpb.WorkerStatusResponse{Id: req.GetId(), StatusInfo: string(buf)}
+ if w.resp != nil {
+ w.resp <- response
Review comment:
Yeah request is sent every minute by dataflow runner. So it is possible to get them stacked as `stub.Recv()` followed by `stub.Send()`.
--
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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1054589987
# [Codecov](https://codecov.io/gh/apache/beam/pull/16957?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 [#16957](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e1539d0) into [master](https://codecov.io/gh/apache/beam/commit/d1b8e569fd651975f08823a3db49dbee56d491b5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d1b8e56) will **decrease** coverage by `27.65%`.
> The diff coverage is `45.45%`.
[](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16957 +/- ##
===========================================
- Coverage 74.67% 47.02% -27.66%
===========================================
Files 654 208 -446
Lines 82133 24387 -57746
===========================================
- Hits 61334 11468 -49866
+ Misses 19809 11875 -7934
- Partials 990 1044 +54
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `47.02% <45.45%> (?)` | |
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/16957?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/harness/harness.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9oYXJuZXNzLmdv) | `12.95% <0.00%> (-2.17%)` | :arrow_down: |
| [.../go/pkg/beam/core/runtime/harness/worker\_status.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy93b3JrZXJfc3RhdHVzLmdv) | `60.34% <60.34%> (ø)` | |
| [sdks/go/pkg/beam/core/runtime/exec/emit.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9lbWl0Lmdv) | `51.92% <0.00%> (-9.99%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/util/protox/any.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3V0aWwvcHJvdG94L2FueS5nbw==) | `46.93% <0.00%> (-9.16%)` | :arrow_down: |
| [sdks/go/pkg/beam/provision/provision.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9wcm92aXNpb24vcHJvdmlzaW9uLmdv) | `27.50% <0.00%> (-7.99%)` | :arrow_down: |
| [sdks/go/pkg/beam/flatten.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9mbGF0dGVuLmdv) | `44.44% <0.00%> (-7.94%)` | :arrow_down: |
| [sdks/go/pkg/beam/validate.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS92YWxpZGF0ZS5nbw==) | `42.10% <0.00%> (-7.90%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/graph/coder/int.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL2ludC5nbw==) | `70.00% <0.00%> (-7.78%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/graph/coder/time.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL3RpbWUuZ28=) | `70.00% <0.00%> (-7.78%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/graph/coder/stringutf8.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL3N0cmluZ3V0ZjguZ28=) | `65.90% <0.00%> (-7.78%)` | :arrow_down: |
| ... and [629 more](https://codecov.io/gh/apache/beam/pull/16957/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/16957?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/16957?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 [d1b8e56...e1539d0](https://codecov.io/gh/apache/beam/pull/16957?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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1054589987
# [Codecov](https://codecov.io/gh/apache/beam/pull/16957?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 [#16957](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (63aa879) into [master](https://codecov.io/gh/apache/beam/commit/3cd1f7f949bd476abb11bdb0b368a2f12a496cd1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3cd1f7f) will **decrease** coverage by `0.02%`.
> The diff coverage is `48.38%`.
> :exclamation: Current head 63aa879 differs from pull request most recent head b35af85. Consider uploading reports for the commit b35af85 to get more accurate results
[](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16957 +/- ##
==========================================
- Coverage 73.80% 73.78% -0.03%
==========================================
Files 663 664 +1
Lines 87056 87114 +58
==========================================
+ Hits 64251 64274 +23
- Misses 21721 21751 +30
- Partials 1084 1089 +5
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `48.82% <48.38%> (+0.01%)` | :arrow_up: |
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/16957?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/harness/harness.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9oYXJuZXNzLmdv) | `13.02% <0.00%> (+0.33%)` | :arrow_up: |
| [.../go/pkg/beam/core/runtime/harness/worker\_status.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy93b3JrZXJfc3RhdHVzLmdv) | `66.66% <66.66%> (ø)` | |
| [.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/16957/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.37% <0.00%> (-4.66%)` | :arrow_down: |
| [...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/16957/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.97% <0.00%> (-2.26%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `87.50% <0.00%> (-1.71%)` | :arrow_down: |
| [.../python/apache\_beam/transforms/periodicsequence.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9wZXJpb2RpY3NlcXVlbmNlLnB5) | `96.72% <0.00%> (-1.64%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/16957/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.51% <0.00%> (-0.13%)` | :arrow_down: |
| [sdks/python/apache\_beam/io/source\_test\_utils.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vc291cmNlX3Rlc3RfdXRpbHMucHk=) | `89.40% <0.00%> (+1.38%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/16957?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/16957?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 [3cd1f7f...b35af85](https://codecov.io/gh/apache/beam/pull/16957?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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r819020717
##########
File path: sdks/go.mod
##########
@@ -23,33 +23,34 @@ 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.28.0
+ cloud.google.com/go/compute v1.5.0 // indirect
+ cloud.google.com/go/datastore v1.6.0
+ cloud.google.com/go/iam v0.2.0 // indirect
+ cloud.google.com/go/pubsub v1.18.0
+ cloud.google.com/go/storage v1.21.0
Review comment:
OK so these are being updated. Did you call `go get` explicitly?
`go mod tidy` shouldn't be updating things AFACIT from the docs, unless we are actually changing/adding deps. https://go.dev/doc/modules/managing-dependencies
But I might be mis-reading it.
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status.go
##########
@@ -0,0 +1,99 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "io"
+ "runtime"
+ "sync"
+ "sync/atomic"
+ "time"
+
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/internal/errors"
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/log"
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+// workerStatusHandler stores the communication information of WorkerStatus API.
+type workerStatusHandler struct {
+ conn *grpc.ClientConn
+ shutdown int32
+ resp chan *fnpb.WorkerStatusResponse
+}
+
+// newWorkerStatusHandler
+func newWorkerStatusHandler(ctx context.Context, endpoint string) (*workerStatusHandler, error) {
+ sconn, err := dial(ctx, endpoint, 60*time.Second)
+ if err != nil {
+ return &workerStatusHandler{}, errors.Wrapf(err, "failed to connect: %v\n", endpoint)
+ }
+ return &workerStatusHandler{conn: sconn, shutdown: 0, resp: make(chan *fnpb.WorkerStatusResponse)}, nil
+}
+
+// handleRequest manages the WorkerStatus API.
+func (w *workerStatusHandler) handleRequest(ctx context.Context, wg *sync.WaitGroup) {
+ defer wg.Done()
+ statusClient := fnpb.NewBeamFnWorkerStatusClient(w.conn)
+ stub, err := statusClient.WorkerStatus(ctx)
+ if err != nil {
+ log.Errorf(ctx, "status client not established: %v", err)
+ }
+ go w.writer(ctx, stub)
+ w.reader(ctx, stub)
+}
+
+// writer writes the WorkerStatusResponse recevied from the response channel.
+func (w *workerStatusHandler) writer(ctx context.Context, stub fnpb.BeamFnWorkerStatus_WorkerStatusClient) {
+ for resp := range w.resp {
+ log.Debugf(ctx, "RESP-status: %v", resp.GetId())
+ if err := stub.Send(resp); err != nil && err != io.EOF {
+ log.Errorf(ctx, "workerStatus.Writer: Failed to respond: %v", err)
+ }
+ }
+ log.Debugf(ctx, "exiting workerStatusHandler.Writer()")
+}
+
+// reader reads the WorkerStatusRequest from the stream and sends a processed WorkerStatusResponse to
+// a response channel.
+func (w *workerStatusHandler) reader(ctx context.Context, stub fnpb.BeamFnWorkerStatus_WorkerStatusClient) {
+ for atomic.LoadInt32(&w.shutdown) == 0 {
+ req, err := stub.Recv()
+ if err != nil {
+ log.Debugf(ctx, "exiting workerStatusHandler.Reader(): %v", err)
+ return
+ }
+ log.Debugf(ctx, "RECV-status: %v", req.GetId())
+ buf := make([]byte, 1<<16)
+ runtime.Stack(buf, true)
+ response := &fnpb.WorkerStatusResponse{Id: req.GetId(), StatusInfo: string(buf)}
+ if w.resp != nil {
+ w.resp <- response
+ } else {
+ return
+ }
+ }
+}
+
+// close stops the reader first, closes the response channel thereby stopping writer and finally closes the gRPC connection.
+func (w *workerStatusHandler) close(ctx context.Context, wg *sync.WaitGroup) {
+ atomic.StoreInt32(&w.shutdown, 1)
+ close(w.resp)
Review comment:
It's a bad idea to close a channel from goroutines that aren't doing the writing. A race condition exists where this is closed, isn't yet niled out, and the reader tries to send on the goroutine. Have the reader close the channel, after it exists it's loop with a defer instead.
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status.go
##########
@@ -0,0 +1,99 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "io"
+ "runtime"
+ "sync"
+ "sync/atomic"
+ "time"
+
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/internal/errors"
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/log"
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+// workerStatusHandler stores the communication information of WorkerStatus API.
+type workerStatusHandler struct {
+ conn *grpc.ClientConn
+ shutdown int32
+ resp chan *fnpb.WorkerStatusResponse
+}
+
+// newWorkerStatusHandler
Review comment:
Consider either filling out the comment, or remove it since it's unexported and empty.
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status.go
##########
@@ -0,0 +1,99 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "io"
+ "runtime"
+ "sync"
+ "sync/atomic"
+ "time"
+
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/internal/errors"
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/log"
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+// workerStatusHandler stores the communication information of WorkerStatus API.
+type workerStatusHandler struct {
+ conn *grpc.ClientConn
+ shutdown int32
+ resp chan *fnpb.WorkerStatusResponse
+}
+
+// newWorkerStatusHandler
+func newWorkerStatusHandler(ctx context.Context, endpoint string) (*workerStatusHandler, error) {
+ sconn, err := dial(ctx, endpoint, 60*time.Second)
+ if err != nil {
+ return &workerStatusHandler{}, errors.Wrapf(err, "failed to connect: %v\n", endpoint)
+ }
+ return &workerStatusHandler{conn: sconn, shutdown: 0, resp: make(chan *fnpb.WorkerStatusResponse)}, nil
+}
+
+// handleRequest manages the WorkerStatus API.
+func (w *workerStatusHandler) handleRequest(ctx context.Context, wg *sync.WaitGroup) {
+ defer wg.Done()
+ statusClient := fnpb.NewBeamFnWorkerStatusClient(w.conn)
+ stub, err := statusClient.WorkerStatus(ctx)
+ if err != nil {
+ log.Errorf(ctx, "status client not established: %v", err)
+ }
+ go w.writer(ctx, stub)
+ w.reader(ctx, stub)
+}
+
+// writer writes the WorkerStatusResponse recevied from the response channel.
Review comment:
```suggestion
// writer sends the WorkerStatusResponse received from the response channel.
```
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status.go
##########
@@ -0,0 +1,99 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "io"
+ "runtime"
+ "sync"
+ "sync/atomic"
+ "time"
+
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/internal/errors"
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/log"
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+// workerStatusHandler stores the communication information of WorkerStatus API.
+type workerStatusHandler struct {
+ conn *grpc.ClientConn
+ shutdown int32
+ resp chan *fnpb.WorkerStatusResponse
+}
+
+// newWorkerStatusHandler
+func newWorkerStatusHandler(ctx context.Context, endpoint string) (*workerStatusHandler, error) {
+ sconn, err := dial(ctx, endpoint, 60*time.Second)
+ if err != nil {
+ return &workerStatusHandler{}, errors.Wrapf(err, "failed to connect: %v\n", endpoint)
+ }
+ return &workerStatusHandler{conn: sconn, shutdown: 0, resp: make(chan *fnpb.WorkerStatusResponse)}, nil
+}
+
+// handleRequest manages the WorkerStatus API.
+func (w *workerStatusHandler) handleRequest(ctx context.Context, wg *sync.WaitGroup) {
+ defer wg.Done()
+ statusClient := fnpb.NewBeamFnWorkerStatusClient(w.conn)
+ stub, err := statusClient.WorkerStatus(ctx)
+ if err != nil {
+ log.Errorf(ctx, "status client not established: %v", err)
+ }
+ go w.writer(ctx, stub)
+ w.reader(ctx, stub)
+}
+
+// writer writes the WorkerStatusResponse recevied from the response channel.
+func (w *workerStatusHandler) writer(ctx context.Context, stub fnpb.BeamFnWorkerStatus_WorkerStatusClient) {
+ for resp := range w.resp {
+ log.Debugf(ctx, "RESP-status: %v", resp.GetId())
+ if err := stub.Send(resp); err != nil && err != io.EOF {
+ log.Errorf(ctx, "workerStatus.Writer: Failed to respond: %v", err)
+ }
+ }
+ log.Debugf(ctx, "exiting workerStatusHandler.Writer()")
+}
+
+// reader reads the WorkerStatusRequest from the stream and sends a processed WorkerStatusResponse to
+// a response channel.
+func (w *workerStatusHandler) reader(ctx context.Context, stub fnpb.BeamFnWorkerStatus_WorkerStatusClient) {
+ for atomic.LoadInt32(&w.shutdown) == 0 {
+ req, err := stub.Recv()
+ if err != nil {
+ log.Debugf(ctx, "exiting workerStatusHandler.Reader(): %v", err)
+ return
+ }
+ log.Debugf(ctx, "RECV-status: %v", req.GetId())
+ buf := make([]byte, 1<<16)
Review comment:
Consider allocating this outside the loop so it's not constantly GC'd and re-allocated, and documenting the intended human readable size cap. (though, if it's once every 5 minutes, it's probably a non-issue, but in that case, comment about that here instead, since moving a buffer out of a loop is a common target for optimization passes). The conversion to string will avoid aliasing problems with simultaneous calls issues.
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status_test.go
##########
@@ -0,0 +1,83 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "fmt"
+ "log"
+ "net"
+ "sync"
+ "testing"
+
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+type BeamFnWorkerStatusServicer struct {
+ fnpb.UnimplementedBeamFnWorkerStatusServer
+ response chan string
+}
+
+func (w BeamFnWorkerStatusServicer) mustEmbedUnimplementedBeamFnWorkerStatusServer() {
Review comment:
Nit: You shouldn't need to implement this, since you've embedded it the servicer status.
You're running into that you're using a value, rather than a pointer method receiver for WorkerStatus.
Change `(w BeamFnWorkerStatusServicer)` to `(w *BeamFnWorkerStatusServicer)` and `srv := BeamFnWorkerStatusServicer` to `srv := &BeamFnWorkerStatusServicer` and it should clear up. (and save you from needing to do `&srv` later. Either will be stack or heap allocated as needed.)
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status.go
##########
@@ -0,0 +1,99 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "io"
+ "runtime"
+ "sync"
+ "sync/atomic"
+ "time"
+
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/internal/errors"
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/log"
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+// workerStatusHandler stores the communication information of WorkerStatus API.
+type workerStatusHandler struct {
+ conn *grpc.ClientConn
+ shutdown int32
+ resp chan *fnpb.WorkerStatusResponse
+}
+
+// newWorkerStatusHandler
+func newWorkerStatusHandler(ctx context.Context, endpoint string) (*workerStatusHandler, error) {
+ sconn, err := dial(ctx, endpoint, 60*time.Second)
+ if err != nil {
+ return &workerStatusHandler{}, errors.Wrapf(err, "failed to connect: %v\n", endpoint)
+ }
+ return &workerStatusHandler{conn: sconn, shutdown: 0, resp: make(chan *fnpb.WorkerStatusResponse)}, nil
+}
+
+// handleRequest manages the WorkerStatus API.
+func (w *workerStatusHandler) handleRequest(ctx context.Context, wg *sync.WaitGroup) {
+ defer wg.Done()
+ statusClient := fnpb.NewBeamFnWorkerStatusClient(w.conn)
+ stub, err := statusClient.WorkerStatus(ctx)
+ if err != nil {
+ log.Errorf(ctx, "status client not established: %v", err)
+ }
+ go w.writer(ctx, stub)
+ w.reader(ctx, stub)
Review comment:
Consider also launching the reader as a goroutine here instead of launching the handleRequest as a goroutine. It doesn't make sense to split the launching to multiple levels and will make it harder to track what's going on.
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status.go
##########
@@ -0,0 +1,99 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "io"
+ "runtime"
+ "sync"
+ "sync/atomic"
+ "time"
+
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/internal/errors"
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/log"
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+// workerStatusHandler stores the communication information of WorkerStatus API.
+type workerStatusHandler struct {
+ conn *grpc.ClientConn
+ shutdown int32
+ resp chan *fnpb.WorkerStatusResponse
+}
+
+// newWorkerStatusHandler
+func newWorkerStatusHandler(ctx context.Context, endpoint string) (*workerStatusHandler, error) {
+ sconn, err := dial(ctx, endpoint, 60*time.Second)
+ if err != nil {
+ return &workerStatusHandler{}, errors.Wrapf(err, "failed to connect: %v\n", endpoint)
+ }
+ return &workerStatusHandler{conn: sconn, shutdown: 0, resp: make(chan *fnpb.WorkerStatusResponse)}, nil
+}
+
+// handleRequest manages the WorkerStatus API.
+func (w *workerStatusHandler) handleRequest(ctx context.Context, wg *sync.WaitGroup) {
+ defer wg.Done()
+ statusClient := fnpb.NewBeamFnWorkerStatusClient(w.conn)
+ stub, err := statusClient.WorkerStatus(ctx)
+ if err != nil {
+ log.Errorf(ctx, "status client not established: %v", err)
+ }
+ go w.writer(ctx, stub)
+ w.reader(ctx, stub)
+}
+
+// writer writes the WorkerStatusResponse recevied from the response channel.
+func (w *workerStatusHandler) writer(ctx context.Context, stub fnpb.BeamFnWorkerStatus_WorkerStatusClient) {
+ for resp := range w.resp {
+ log.Debugf(ctx, "RESP-status: %v", resp.GetId())
+ if err := stub.Send(resp); err != nil && err != io.EOF {
+ log.Errorf(ctx, "workerStatus.Writer: Failed to respond: %v", err)
+ }
+ }
+ log.Debugf(ctx, "exiting workerStatusHandler.Writer()")
+}
+
+// reader reads the WorkerStatusRequest from the stream and sends a processed WorkerStatusResponse to
+// a response channel.
+func (w *workerStatusHandler) reader(ctx context.Context, stub fnpb.BeamFnWorkerStatus_WorkerStatusClient) {
+ for atomic.LoadInt32(&w.shutdown) == 0 {
+ req, err := stub.Recv()
+ if err != nil {
+ log.Debugf(ctx, "exiting workerStatusHandler.Reader(): %v", err)
+ return
+ }
+ log.Debugf(ctx, "RECV-status: %v", req.GetId())
+ buf := make([]byte, 1<<16)
+ runtime.Stack(buf, true)
+ response := &fnpb.WorkerStatusResponse{Id: req.GetId(), StatusInfo: string(buf)}
+ if w.resp != nil {
+ w.resp <- response
Review comment:
I suppose my question here is: Do we need two separate goroutines for handling these responses? Could we not have a single receive/send loop that receives a status request, processes it, replying back immeadiately? IIRC it took a several minute job to even trigger this.
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status.go
##########
@@ -0,0 +1,99 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "io"
+ "runtime"
+ "sync"
+ "sync/atomic"
+ "time"
+
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/internal/errors"
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/log"
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+// workerStatusHandler stores the communication information of WorkerStatus API.
+type workerStatusHandler struct {
+ conn *grpc.ClientConn
+ shutdown int32
+ resp chan *fnpb.WorkerStatusResponse
+}
+
+// newWorkerStatusHandler
+func newWorkerStatusHandler(ctx context.Context, endpoint string) (*workerStatusHandler, error) {
+ sconn, err := dial(ctx, endpoint, 60*time.Second)
+ if err != nil {
+ return &workerStatusHandler{}, errors.Wrapf(err, "failed to connect: %v\n", endpoint)
+ }
+ return &workerStatusHandler{conn: sconn, shutdown: 0, resp: make(chan *fnpb.WorkerStatusResponse)}, nil
+}
+
+// handleRequest manages the WorkerStatus API.
+func (w *workerStatusHandler) handleRequest(ctx context.Context, wg *sync.WaitGroup) {
+ defer wg.Done()
+ statusClient := fnpb.NewBeamFnWorkerStatusClient(w.conn)
+ stub, err := statusClient.WorkerStatus(ctx)
+ if err != nil {
+ log.Errorf(ctx, "status client not established: %v", err)
Review comment:
If an error is returned, the stub is invalid, so this should also return out instead of allowing the writer and reader to be launched.
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status.go
##########
@@ -0,0 +1,99 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "io"
+ "runtime"
+ "sync"
+ "sync/atomic"
+ "time"
+
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/internal/errors"
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/log"
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+// workerStatusHandler stores the communication information of WorkerStatus API.
+type workerStatusHandler struct {
+ conn *grpc.ClientConn
+ shutdown int32
+ resp chan *fnpb.WorkerStatusResponse
+}
+
+// newWorkerStatusHandler
+func newWorkerStatusHandler(ctx context.Context, endpoint string) (*workerStatusHandler, error) {
+ sconn, err := dial(ctx, endpoint, 60*time.Second)
+ if err != nil {
+ return &workerStatusHandler{}, errors.Wrapf(err, "failed to connect: %v\n", endpoint)
+ }
+ return &workerStatusHandler{conn: sconn, shutdown: 0, resp: make(chan *fnpb.WorkerStatusResponse)}, nil
+}
+
+// handleRequest manages the WorkerStatus API.
+func (w *workerStatusHandler) handleRequest(ctx context.Context, wg *sync.WaitGroup) {
+ defer wg.Done()
+ statusClient := fnpb.NewBeamFnWorkerStatusClient(w.conn)
+ stub, err := statusClient.WorkerStatus(ctx)
+ if err != nil {
+ log.Errorf(ctx, "status client not established: %v", err)
+ }
+ go w.writer(ctx, stub)
+ w.reader(ctx, stub)
+}
+
+// writer writes the WorkerStatusResponse recevied from the response channel.
+func (w *workerStatusHandler) writer(ctx context.Context, stub fnpb.BeamFnWorkerStatus_WorkerStatusClient) {
+ for resp := range w.resp {
+ log.Debugf(ctx, "RESP-status: %v", resp.GetId())
+ if err := stub.Send(resp); err != nil && err != io.EOF {
+ log.Errorf(ctx, "workerStatus.Writer: Failed to respond: %v", err)
+ }
+ }
+ log.Debugf(ctx, "exiting workerStatusHandler.Writer()")
+}
+
+// reader reads the WorkerStatusRequest from the stream and sends a processed WorkerStatusResponse to
+// a response channel.
+func (w *workerStatusHandler) reader(ctx context.Context, stub fnpb.BeamFnWorkerStatus_WorkerStatusClient) {
+ for atomic.LoadInt32(&w.shutdown) == 0 {
+ req, err := stub.Recv()
+ if err != nil {
+ log.Debugf(ctx, "exiting workerStatusHandler.Reader(): %v", err)
+ return
+ }
+ log.Debugf(ctx, "RECV-status: %v", req.GetId())
+ buf := make([]byte, 1<<16)
+ runtime.Stack(buf, true)
+ response := &fnpb.WorkerStatusResponse{Id: req.GetId(), StatusInfo: string(buf)}
+ if w.resp != nil {
+ w.resp <- response
+ } else {
+ return
+ }
+ }
+}
+
+// close stops the reader first, closes the response channel thereby stopping writer and finally closes the gRPC connection.
+func (w *workerStatusHandler) close(ctx context.Context, wg *sync.WaitGroup) {
+ atomic.StoreInt32(&w.shutdown, 1)
Review comment:
Neat pattern! Consider isolating the atomic stuff into methods for documentation purposes. It's opaque to a reader what these are supposed to do, but in their own methods, they become documented and implementation details.
The other place we're doing this should likely be doing the same, but that's a separate problem.
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status_test.go
##########
@@ -0,0 +1,83 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "fmt"
+ "log"
+ "net"
+ "sync"
+ "testing"
+
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+type BeamFnWorkerStatusServicer struct {
+ fnpb.UnimplementedBeamFnWorkerStatusServer
+ response chan string
+}
+
+func (w BeamFnWorkerStatusServicer) mustEmbedUnimplementedBeamFnWorkerStatusServer() {
+
+}
+
+func (w BeamFnWorkerStatusServicer) WorkerStatus(b fnpb.BeamFnWorkerStatus_WorkerStatusServer) error {
+ b.Send(&fnpb.WorkerStatusRequest{Id: "1"})
+ resp, err := b.Recv()
+ if err != nil {
+ return fmt.Errorf("error receiving response b.recv: %v", err)
+ }
+ w.response <- resp.GetStatusInfo()
+ return nil
+}
+
+type serverData struct {
Review comment:
It doesn't seem like you need this type. Consider just having setUp be a method instead, or even a method on your BeamFnWorkerStatusServicer type instead.
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status_test.go
##########
@@ -0,0 +1,83 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "fmt"
+ "log"
+ "net"
+ "sync"
+ "testing"
+
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+type BeamFnWorkerStatusServicer struct {
+ fnpb.UnimplementedBeamFnWorkerStatusServer
+ response chan string
+}
+
+func (w BeamFnWorkerStatusServicer) mustEmbedUnimplementedBeamFnWorkerStatusServer() {
+
+}
+
+func (w BeamFnWorkerStatusServicer) WorkerStatus(b fnpb.BeamFnWorkerStatus_WorkerStatusServer) error {
+ b.Send(&fnpb.WorkerStatusRequest{Id: "1"})
+ resp, err := b.Recv()
+ if err != nil {
+ return fmt.Errorf("error receiving response b.recv: %v", err)
+ }
+ w.response <- resp.GetStatusInfo()
+ return nil
+}
+
+type serverData struct {
+ server *grpc.Server
+}
+
+func (s *serverData) setUp(port string, srv *BeamFnWorkerStatusServicer) {
+ l, err := net.Listen("tcp", ":9000")
+ if err != nil {
+ log.Fatalf("failed to listen on port 9000 %v", err)
+ }
+
+ fnpb.RegisterBeamFnWorkerStatusServer(s.server, srv)
+ if err := s.server.Serve(l); err != nil {
+ log.Fatalf("failed to serve: %v", err)
+ }
+}
+
+func TestSendStatusResponse(t *testing.T) {
+ server := &serverData{server: grpc.NewServer()}
+ srv := BeamFnWorkerStatusServicer{response: make(chan string)}
+ go server.setUp("9000", &srv)
+ defer server.server.Stop()
Review comment:
In tests, prefer t.Cleanup(...) to defer.
--
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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1054589987
# [Codecov](https://codecov.io/gh/apache/beam/pull/16957?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 [#16957](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a62cb4e) into [master](https://codecov.io/gh/apache/beam/commit/3cd1f7f949bd476abb11bdb0b368a2f12a496cd1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3cd1f7f) will **decrease** coverage by `0.01%`.
> The diff coverage is `46.42%`.
> :exclamation: Current head a62cb4e differs from pull request most recent head 96a9280. Consider uploading reports for the commit 96a9280 to get more accurate results
```diff
@@ Coverage Diff @@
## master #16957 +/- ##
==========================================
- Coverage 73.80% 73.78% -0.02%
==========================================
Files 663 668 +5
Lines 87056 87312 +256
==========================================
+ Hits 64251 64424 +173
- Misses 21721 21781 +60
- Partials 1084 1107 +23
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `49.03% <46.42%> (+0.23%)` | :arrow_up: |
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/16957?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/harness/harness.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9oYXJuZXNzLmdv) | `13.02% <0.00%> (+0.33%)` | :arrow_up: |
| [.../go/pkg/beam/core/runtime/harness/worker\_status.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy93b3JrZXJfc3RhdHVzLmdv) | `66.66% <66.66%> (ø)` | |
| [.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/16957/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.37% <0.00%> (-4.66%)` | :arrow_down: |
| [...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/16957/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.97% <0.00%> (-2.26%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `87.50% <0.00%> (-1.71%)` | :arrow_down: |
| [.../python/apache\_beam/transforms/periodicsequence.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9wZXJpb2RpY3NlcXVlbmNlLnB5) | `96.72% <0.00%> (-1.64%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/16957/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.51% <0.00%> (-0.13%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/graph/window/windows.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL3dpbmRvdy93aW5kb3dzLmdv) | `82.60% <0.00%> (ø)` | |
| [sdks/go/pkg/beam/core/graph/window/strategy.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL3dpbmRvdy9zdHJhdGVneS5nbw==) | `0.00% <0.00%> (ø)` | |
| [sdks/go/pkg/beam/core/graph/window/fn.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL3dpbmRvdy9mbi5nbw==) | `53.84% <0.00%> (ø)` | |
| ... and [6 more](https://codecov.io/gh/apache/beam/pull/16957/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/16957?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/16957?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 [3cd1f7f...96a9280](https://codecov.io/gh/apache/beam/pull/16957?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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r838630664
##########
File path: sdks/go/pkg/beam/core/runtime/harness/harness.go
##########
@@ -98,6 +111,18 @@ func Main(ctx context.Context, loggingEndpoint, controlEndpoint string) error {
log.Debugf(ctx, "control response channel closed")
}()
+ // if the runner supports worker status api then expose SDK harness status
+ if statusEndpoint != "" {
+ statusHandler, err := newWorkerStatusHandler(ctx, statusEndpoint)
+ if err != nil {
+ log.Error(ctx, err)
+ }
+ var swg sync.WaitGroup
+ swg.Add(1)
+ statusHandler.handleRequest(ctx, &swg)
+ defer statusHandler.close(ctx, &swg)
Review comment:
Yes, missed 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] codecov[bot] edited a comment on pull request #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1054589987
# [Codecov](https://codecov.io/gh/apache/beam/pull/16957?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 [#16957](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b35af85) into [master](https://codecov.io/gh/apache/beam/commit/3cd1f7f949bd476abb11bdb0b368a2f12a496cd1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3cd1f7f) will **decrease** coverage by `0.00%`.
> The diff coverage is `48.38%`.
[](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16957 +/- ##
==========================================
- Coverage 73.80% 73.79% -0.01%
==========================================
Files 663 665 +2
Lines 87056 87250 +194
==========================================
+ Hits 64251 64388 +137
- Misses 21721 21754 +33
- Partials 1084 1108 +24
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `49.01% <48.38%> (+0.21%)` | :arrow_up: |
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/16957?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/harness/harness.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9oYXJuZXNzLmdv) | `13.02% <0.00%> (+0.33%)` | :arrow_up: |
| [.../go/pkg/beam/core/runtime/harness/worker\_status.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy93b3JrZXJfc3RhdHVzLmdv) | `66.66% <66.66%> (ø)` | |
| [.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/16957/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.37% <0.00%> (-4.66%)` | :arrow_down: |
| [...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/16957/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.97% <0.00%> (-2.26%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `87.50% <0.00%> (-1.71%)` | :arrow_down: |
| [.../python/apache\_beam/transforms/periodicsequence.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9wZXJpb2RpY3NlcXVlbmNlLnB5) | `96.72% <0.00%> (-1.64%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/16957/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.51% <0.00%> (-0.13%)` | :arrow_down: |
| [sdks/go/pkg/beam/io/avroio/avroio.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9pby9hdnJvaW8vYXZyb2lvLmdv) | `56.45% <0.00%> (ø)` | |
| [...kg/beam/core/runtime/xlangx/expansionx/download.go](https://codecov.io/gh/apache/beam/pull/16957/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=) | `61.53% <0.00%> (+1.16%)` | :arrow_up: |
| ... and [3 more](https://codecov.io/gh/apache/beam/pull/16957/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/16957?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/16957?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 [3cd1f7f...b35af85](https://codecov.io/gh/apache/beam/pull/16957?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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r838625510
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status.go
##########
@@ -0,0 +1,91 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "io"
+ "runtime"
+ "sync"
+ "sync/atomic"
+ "time"
+
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/internal/errors"
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/log"
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+// workerStatusHandler stores the communication information of WorkerStatus API.
+type workerStatusHandler struct {
+ conn *grpc.ClientConn
+ shutdown int32
+}
+
+func newWorkerStatusHandler(ctx context.Context, endpoint string) (*workerStatusHandler, error) {
+ sconn, err := dial(ctx, endpoint, 60*time.Second)
+ if err != nil {
+ return &workerStatusHandler{}, errors.Wrapf(err, "failed to connect: %v\n", endpoint)
Review comment:
Makes sense
--
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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r816997462
##########
File path: sdks/go/pkg/beam/core/runtime/harness/harness.go
##########
@@ -36,14 +36,27 @@ import (
"google.golang.org/grpc"
)
+// StatusAddress is a type of status endpoint address as an optional argument to harness.Main().
+type StatusAddress string
+
// TODO(herohde) 2/8/2017: for now, assume we stage a full binary (not a plugin).
// Main is the main entrypoint for the Go harness. It runs at "runtime" -- not
// "pipeline-construction time" -- on each worker. It is a FnAPI client and
// ultimately responsible for correctly executing user code.
-func Main(ctx context.Context, loggingEndpoint, controlEndpoint string) error {
+func Main(ctx context.Context, loggingEndpoint, controlEndpoint string, options ...interface{}) error {
hooks.DeserializeHooksFromOptions(ctx)
+ statusEndpoint := ""
+ for _, option := range options {
+ switch option := option.(type) {
+ case StatusAddress:
+ statusEndpoint = string(option)
+ default:
+ return errors.Errorf("unkown type %T, value %v in error call", option, option)
Review comment:
Yes, the first one is for type %T
--
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 change in pull request #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
damccorm commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r817010912
##########
File path: sdks/go/pkg/beam/core/runtime/harness/harness.go
##########
@@ -36,14 +36,27 @@ import (
"google.golang.org/grpc"
)
+// StatusAddress is a type of status endpoint address as an optional argument to harness.Main().
+type StatusAddress string
+
// TODO(herohde) 2/8/2017: for now, assume we stage a full binary (not a plugin).
// Main is the main entrypoint for the Go harness. It runs at "runtime" -- not
// "pipeline-construction time" -- on each worker. It is a FnAPI client and
// ultimately responsible for correctly executing user code.
-func Main(ctx context.Context, loggingEndpoint, controlEndpoint string) error {
+func Main(ctx context.Context, loggingEndpoint, controlEndpoint string, options ...interface{}) error {
hooks.DeserializeHooksFromOptions(ctx)
+ statusEndpoint := ""
+ for _, option := range options {
+ switch option := option.(type) {
+ case StatusAddress:
+ statusEndpoint = string(option)
+ default:
+ return errors.Errorf("unkown type %T, value %v in error call", option, option)
Review comment:
Oh - you're so right! My mistake
--
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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r830417694
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status_test.go
##########
@@ -0,0 +1,77 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "fmt"
+ "log"
+ "net"
+ "sync"
+ "testing"
+
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+type BeamFnWorkerStatusServicer struct {
Review comment:
+100, I wasn't aware of the [bufcon](https://pkg.go.dev/google.golang.org/grpc/test/bufconn) package before. Good find.
If we can remove the network portion in our tests, we absolutely should, as that can cause issues in some environments.
--
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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1054589987
# [Codecov](https://codecov.io/gh/apache/beam/pull/16957?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 [#16957](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (96a9280) into [master](https://codecov.io/gh/apache/beam/commit/3cd1f7f949bd476abb11bdb0b368a2f12a496cd1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3cd1f7f) will **increase** coverage by `0.04%`.
> The diff coverage is `41.07%`.
```diff
@@ Coverage Diff @@
## master #16957 +/- ##
==========================================
+ Coverage 73.80% 73.84% +0.04%
==========================================
Files 663 672 +9
Lines 87056 87522 +466
==========================================
+ Hits 64251 64630 +379
- Misses 21721 21777 +56
- Partials 1084 1115 +31
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `49.44% <41.07%> (+0.64%)` | :arrow_up: |
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/16957?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/harness/harness.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9oYXJuZXNzLmdv) | `10.94% <0.00%> (-1.75%)` | :arrow_down: |
| [.../go/pkg/beam/core/runtime/harness/worker\_status.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy93b3JrZXJfc3RhdHVzLmdv) | `58.97% <58.97%> (ø)` | |
| [sdks/go/pkg/beam/core/runtime/exec/plan.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9wbGFuLmdv) | `48.61% <0.00%> (-8.80%)` | :arrow_down: |
| [.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/16957/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.37% <0.00%> (-4.66%)` | :arrow_down: |
| [...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/16957/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.97% <0.00%> (-2.26%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `87.50% <0.00%> (-1.71%)` | :arrow_down: |
| [.../python/apache\_beam/transforms/periodicsequence.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9wZXJpb2RpY3NlcXVlbmNlLnB5) | `96.72% <0.00%> (-1.64%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/16957/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.51% <0.00%> (-0.13%)` | :arrow_down: |
| [sdks/go/pkg/beam/runners/direct/buffer.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL2RpcmVjdC9idWZmZXIuZ28=) | `77.46% <0.00%> (ø)` | |
| ... and [21 more](https://codecov.io/gh/apache/beam/pull/16957/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/16957?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/16957?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 [3cd1f7f...96a9280](https://codecov.io/gh/apache/beam/pull/16957?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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r838629742
##########
File path: sdks/go/pkg/beam/core/runtime/harness/harness.go
##########
@@ -98,6 +111,18 @@ func Main(ctx context.Context, loggingEndpoint, controlEndpoint string) error {
log.Debugf(ctx, "control response channel closed")
}()
+ // if the runner supports worker status api then expose SDK harness status
+ if statusEndpoint != "" {
+ statusHandler, err := newWorkerStatusHandler(ctx, statusEndpoint)
+ if err != nil {
+ log.Error(ctx, err)
Review comment:
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] codecov[bot] edited a comment on pull request #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1054589987
# [Codecov](https://codecov.io/gh/apache/beam/pull/16957?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 [#16957](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3a4557f) into [master](https://codecov.io/gh/apache/beam/commit/3cd1f7f949bd476abb11bdb0b368a2f12a496cd1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3cd1f7f) will **increase** coverage by `0.11%`.
> The diff coverage is `41.07%`.
```diff
@@ Coverage Diff @@
## master #16957 +/- ##
==========================================
+ Coverage 73.80% 73.92% +0.11%
==========================================
Files 663 673 +10
Lines 87056 87551 +495
==========================================
+ Hits 64251 64719 +468
+ Misses 21721 21709 -12
- Partials 1084 1123 +39
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `49.74% <41.07%> (+0.94%)` | :arrow_up: |
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/16957?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/harness/harness.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9oYXJuZXNzLmdv) | `10.94% <0.00%> (-1.75%)` | :arrow_down: |
| [.../go/pkg/beam/core/runtime/harness/worker\_status.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy93b3JrZXJfc3RhdHVzLmdv) | `58.97% <58.97%> (ø)` | |
| [sdks/go/pkg/beam/core/runtime/exec/plan.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9wbGFuLmdv) | `48.61% <0.00%> (-8.80%)` | :arrow_down: |
| [.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/16957/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.37% <0.00%> (-4.66%)` | :arrow_down: |
| [...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/16957/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.97% <0.00%> (-2.26%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `87.50% <0.00%> (-1.71%)` | :arrow_down: |
| [.../python/apache\_beam/transforms/periodicsequence.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9wZXJpb2RpY3NlcXVlbmNlLnB5) | `96.72% <0.00%> (-1.64%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/16957/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.51% <0.00%> (-0.13%)` | :arrow_down: |
| [sdks/go/pkg/beam/runners/direct/buffer.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL2RpcmVjdC9idWZmZXIuZ28=) | `77.46% <0.00%> (ø)` | |
| ... and [23 more](https://codecov.io/gh/apache/beam/pull/16957/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/16957?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/16957?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 [3cd1f7f...3a4557f](https://codecov.io/gh/apache/beam/pull/16957?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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1063231197
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] youngoli edited a comment on pull request #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
youngoli edited a comment on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1071900580
R: @lostluck
Edit: Disregard that, I forgot he already reviewed 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] youngoli commented on pull request #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
youngoli commented on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1071900580
R: @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] codecov[bot] edited a comment on pull request #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1054589987
# [Codecov](https://codecov.io/gh/apache/beam/pull/16957?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 [#16957](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b35af85) into [master](https://codecov.io/gh/apache/beam/commit/3cd1f7f949bd476abb11bdb0b368a2f12a496cd1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3cd1f7f) will **decrease** coverage by `0.00%`.
> The diff coverage is `48.38%`.
> :exclamation: Current head b35af85 differs from pull request most recent head a62cb4e. Consider uploading reports for the commit a62cb4e to get more accurate results
[](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16957 +/- ##
==========================================
- Coverage 73.80% 73.79% -0.01%
==========================================
Files 663 665 +2
Lines 87056 87250 +194
==========================================
+ Hits 64251 64388 +137
- Misses 21721 21754 +33
- Partials 1084 1108 +24
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `49.01% <48.38%> (+0.21%)` | :arrow_up: |
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/16957?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/harness/harness.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9oYXJuZXNzLmdv) | `13.02% <0.00%> (+0.33%)` | :arrow_up: |
| [.../go/pkg/beam/core/runtime/harness/worker\_status.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy93b3JrZXJfc3RhdHVzLmdv) | `66.66% <66.66%> (ø)` | |
| [.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/16957/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.37% <0.00%> (-4.66%)` | :arrow_down: |
| [...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/16957/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.97% <0.00%> (-2.26%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `87.50% <0.00%> (-1.71%)` | :arrow_down: |
| [.../python/apache\_beam/transforms/periodicsequence.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9wZXJpb2RpY3NlcXVlbmNlLnB5) | `96.72% <0.00%> (-1.64%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/16957/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.51% <0.00%> (-0.13%)` | :arrow_down: |
| [sdks/go/pkg/beam/io/avroio/avroio.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9pby9hdnJvaW8vYXZyb2lvLmdv) | `56.45% <0.00%> (ø)` | |
| [...kg/beam/core/runtime/xlangx/expansionx/download.go](https://codecov.io/gh/apache/beam/pull/16957/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=) | `61.53% <0.00%> (+1.16%)` | :arrow_up: |
| ... and [3 more](https://codecov.io/gh/apache/beam/pull/16957/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/16957?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/16957?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 [3cd1f7f...a62cb4e](https://codecov.io/gh/apache/beam/pull/16957?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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1054589987
# [Codecov](https://codecov.io/gh/apache/beam/pull/16957?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 [#16957](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e1539d0) into [master](https://codecov.io/gh/apache/beam/commit/bed5e19e084c3fdb02e5d2a9d97a1a51e655004c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bed5e19) will **decrease** coverage by `26.44%`.
> The diff coverage is `45.45%`.
> :exclamation: Current head e1539d0 differs from pull request most recent head 61b195f. Consider uploading reports for the commit 61b195f to get more accurate results
[](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16957 +/- ##
===========================================
- Coverage 73.47% 47.02% -26.45%
===========================================
Files 663 208 -455
Lines 87056 24387 -62669
===========================================
- Hits 63963 11468 -52495
+ Misses 22034 11875 -10159
+ Partials 1059 1044 -15
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `47.02% <45.45%> (-0.65%)` | :arrow_down: |
| python | `?` | |
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/16957?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/harness/harness.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9oYXJuZXNzLmdv) | `12.95% <0.00%> (+0.26%)` | :arrow_up: |
| [.../go/pkg/beam/core/runtime/harness/worker\_status.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy93b3JrZXJfc3RhdHVzLmdv) | `60.34% <60.34%> (ø)` | |
| [sdks/go/pkg/beam/io/textio/textio.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9pby90ZXh0aW8vdGV4dGlvLmdv) | `23.77% <0.00%> (-41.81%)` | :arrow_down: |
| [sdks/go/pkg/beam/io/databaseio/database.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9pby9kYXRhYmFzZWlvL2RhdGFiYXNlLmdv) | `2.27% <0.00%> (-21.97%)` | :arrow_down: |
| [sdks/go/pkg/beam/io/textio/sdf.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9pby90ZXh0aW8vc2RmLmdv) | `42.99% <0.00%> (-3.74%)` | :arrow_down: |
| [...python/apache\_beam/runners/direct/direct\_runner.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kaXJlY3QvZGlyZWN0X3J1bm5lci5weQ==) | | |
| [sdks/python/apache\_beam/io/iobase.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vaW9iYXNlLnB5) | | |
| [...am/examples/snippets/transforms/aggregation/sum.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZXhhbXBsZXMvc25pcHBldHMvdHJhbnNmb3Jtcy9hZ2dyZWdhdGlvbi9zdW0ucHk=) | | |
| [...ache\_beam/runners/dataflow/ptransform\_overrides.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kYXRhZmxvdy9wdHJhbnNmb3JtX292ZXJyaWRlcy5weQ==) | | |
| [sdks/python/apache\_beam/internal/gcp/json\_value.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvZ2NwL2pzb25fdmFsdWUucHk=) | | |
| ... and [452 more](https://codecov.io/gh/apache/beam/pull/16957/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/16957?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/16957?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 [bed5e19...61b195f](https://codecov.io/gh/apache/beam/pull/16957?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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r838632290
##########
File path: sdks/go/pkg/beam/core/runtime/harness/harness.go
##########
@@ -98,6 +111,18 @@ func Main(ctx context.Context, loggingEndpoint, controlEndpoint string) error {
log.Debugf(ctx, "control response channel closed")
}()
+ // if the runner supports worker status api then expose SDK harness status
+ if statusEndpoint != "" {
+ statusHandler, err := newWorkerStatusHandler(ctx, statusEndpoint)
+ if err != nil {
+ log.Error(ctx, err)
+ }
+ var swg sync.WaitGroup
+ swg.Add(1)
Review comment:
Yeah, that would clear things. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on a change in pull request #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r837987480
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status.go
##########
@@ -0,0 +1,91 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "io"
+ "runtime"
+ "sync"
+ "sync/atomic"
+ "time"
+
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/internal/errors"
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/log"
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+// workerStatusHandler stores the communication information of WorkerStatus API.
+type workerStatusHandler struct {
+ conn *grpc.ClientConn
+ shutdown int32
+}
+
+func newWorkerStatusHandler(ctx context.Context, endpoint string) (*workerStatusHandler, error) {
+ sconn, err := dial(ctx, endpoint, 60*time.Second)
+ if err != nil {
+ return &workerStatusHandler{}, errors.Wrapf(err, "failed to connect: %v\n", endpoint)
Review comment:
nit: Conventionally, when a non-nil error is being returned, the other return values are not considered valid. If they're pointer types, they should return `nil` instead of a pointer to a zeroed value.
If the returned non-error values are valid, the convention is to document this behavior, and why.
Eg. The [`io.Reader`](https://pkg.go.dev/io#Reader) interface defines this kind of thing. The `n` is always valid, and an error of `io.EOF` is considered a success.
--
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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1054589987
# [Codecov](https://codecov.io/gh/apache/beam/pull/16957?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 [#16957](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b35af85) into [master](https://codecov.io/gh/apache/beam/commit/3cd1f7f949bd476abb11bdb0b368a2f12a496cd1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3cd1f7f) will **decrease** coverage by `0.00%`.
> The diff coverage is `48.38%`.
[](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16957 +/- ##
==========================================
- Coverage 73.80% 73.79% -0.01%
==========================================
Files 663 665 +2
Lines 87056 87250 +194
==========================================
+ Hits 64251 64388 +137
- Misses 21721 21754 +33
- Partials 1084 1108 +24
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `49.01% <48.38%> (+0.21%)` | :arrow_up: |
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/16957?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/harness/harness.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9oYXJuZXNzLmdv) | `13.02% <0.00%> (+0.33%)` | :arrow_up: |
| [.../go/pkg/beam/core/runtime/harness/worker\_status.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy93b3JrZXJfc3RhdHVzLmdv) | `66.66% <66.66%> (ø)` | |
| [.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/16957/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.37% <0.00%> (-4.66%)` | :arrow_down: |
| [...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/16957/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.97% <0.00%> (-2.26%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `87.50% <0.00%> (-1.71%)` | :arrow_down: |
| [.../python/apache\_beam/transforms/periodicsequence.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9wZXJpb2RpY3NlcXVlbmNlLnB5) | `96.72% <0.00%> (-1.64%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/16957/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.51% <0.00%> (-0.13%)` | :arrow_down: |
| [sdks/go/pkg/beam/io/avroio/avroio.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9pby9hdnJvaW8vYXZyb2lvLmdv) | `56.45% <0.00%> (ø)` | |
| [...kg/beam/core/runtime/xlangx/expansionx/download.go](https://codecov.io/gh/apache/beam/pull/16957/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=) | `61.53% <0.00%> (+1.16%)` | :arrow_up: |
| ... and [3 more](https://codecov.io/gh/apache/beam/pull/16957/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/16957?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/16957?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 [3cd1f7f...b35af85](https://codecov.io/gh/apache/beam/pull/16957?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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r819124347
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status.go
##########
@@ -0,0 +1,99 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "io"
+ "runtime"
+ "sync"
+ "sync/atomic"
+ "time"
+
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/internal/errors"
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/log"
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+// workerStatusHandler stores the communication information of WorkerStatus API.
+type workerStatusHandler struct {
+ conn *grpc.ClientConn
+ shutdown int32
+ resp chan *fnpb.WorkerStatusResponse
+}
+
+// newWorkerStatusHandler
+func newWorkerStatusHandler(ctx context.Context, endpoint string) (*workerStatusHandler, error) {
+ sconn, err := dial(ctx, endpoint, 60*time.Second)
+ if err != nil {
+ return &workerStatusHandler{}, errors.Wrapf(err, "failed to connect: %v\n", endpoint)
+ }
+ return &workerStatusHandler{conn: sconn, shutdown: 0, resp: make(chan *fnpb.WorkerStatusResponse)}, nil
+}
+
+// handleRequest manages the WorkerStatus API.
+func (w *workerStatusHandler) handleRequest(ctx context.Context, wg *sync.WaitGroup) {
+ defer wg.Done()
+ statusClient := fnpb.NewBeamFnWorkerStatusClient(w.conn)
+ stub, err := statusClient.WorkerStatus(ctx)
+ if err != nil {
+ log.Errorf(ctx, "status client not established: %v", err)
+ }
+ go w.writer(ctx, stub)
+ w.reader(ctx, stub)
+}
+
+// writer writes the WorkerStatusResponse recevied from the response channel.
+func (w *workerStatusHandler) writer(ctx context.Context, stub fnpb.BeamFnWorkerStatus_WorkerStatusClient) {
+ for resp := range w.resp {
+ log.Debugf(ctx, "RESP-status: %v", resp.GetId())
+ if err := stub.Send(resp); err != nil && err != io.EOF {
+ log.Errorf(ctx, "workerStatus.Writer: Failed to respond: %v", err)
+ }
+ }
+ log.Debugf(ctx, "exiting workerStatusHandler.Writer()")
+}
+
+// reader reads the WorkerStatusRequest from the stream and sends a processed WorkerStatusResponse to
+// a response channel.
+func (w *workerStatusHandler) reader(ctx context.Context, stub fnpb.BeamFnWorkerStatus_WorkerStatusClient) {
+ for atomic.LoadInt32(&w.shutdown) == 0 {
+ req, err := stub.Recv()
+ if err != nil {
+ log.Debugf(ctx, "exiting workerStatusHandler.Reader(): %v", err)
+ return
+ }
+ log.Debugf(ctx, "RECV-status: %v", req.GetId())
+ buf := make([]byte, 1<<16)
+ runtime.Stack(buf, true)
+ response := &fnpb.WorkerStatusResponse{Id: req.GetId(), StatusInfo: string(buf)}
+ if w.resp != nil {
+ w.resp <- response
Review comment:
Yeah, the request is sent every minute by dataflow runner. So it is possible to get them stacked as `stub.Recv()` followed by `stub.Send()`.
--
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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1054589987
# [Codecov](https://codecov.io/gh/apache/beam/pull/16957?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 [#16957](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (48a335f) into [master](https://codecov.io/gh/apache/beam/commit/d1b8e569fd651975f08823a3db49dbee56d491b5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d1b8e56) will **decrease** coverage by `27.64%`.
> The diff coverage is `47.29%`.
[](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16957 +/- ##
===========================================
- Coverage 74.67% 47.03% -27.65%
===========================================
Files 654 208 -446
Lines 82133 24384 -57749
===========================================
- Hits 61334 11468 -49866
+ Misses 19809 11873 -7936
- Partials 990 1043 +53
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `47.03% <47.29%> (?)` | |
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/16957?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/harness/harness.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9oYXJuZXNzLmdv) | `12.95% <0.00%> (-2.17%)` | :arrow_down: |
| [.../go/pkg/beam/core/runtime/harness/worker\_status.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy93b3JrZXJfc3RhdHVzLmdv) | `63.63% <63.63%> (ø)` | |
| [sdks/go/pkg/beam/core/runtime/exec/emit.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9lbWl0Lmdv) | `51.92% <0.00%> (-9.99%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/util/protox/any.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3V0aWwvcHJvdG94L2FueS5nbw==) | `46.93% <0.00%> (-9.16%)` | :arrow_down: |
| [sdks/go/pkg/beam/provision/provision.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9wcm92aXNpb24vcHJvdmlzaW9uLmdv) | `27.50% <0.00%> (-7.99%)` | :arrow_down: |
| [sdks/go/pkg/beam/flatten.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9mbGF0dGVuLmdv) | `44.44% <0.00%> (-7.94%)` | :arrow_down: |
| [sdks/go/pkg/beam/validate.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS92YWxpZGF0ZS5nbw==) | `42.10% <0.00%> (-7.90%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/graph/coder/int.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL2ludC5nbw==) | `70.00% <0.00%> (-7.78%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/graph/coder/time.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL3RpbWUuZ28=) | `70.00% <0.00%> (-7.78%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/graph/coder/stringutf8.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL3N0cmluZ3V0ZjguZ28=) | `65.90% <0.00%> (-7.78%)` | :arrow_down: |
| ... and [629 more](https://codecov.io/gh/apache/beam/pull/16957/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/16957?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/16957?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 [d1b8e56...48a335f](https://codecov.io/gh/apache/beam/pull/16957?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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1054589987
# [Codecov](https://codecov.io/gh/apache/beam/pull/16957?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 [#16957](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (48a335f) into [master](https://codecov.io/gh/apache/beam/commit/d1b8e569fd651975f08823a3db49dbee56d491b5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d1b8e56) will **decrease** coverage by `27.64%`.
> The diff coverage is `47.29%`.
> :exclamation: Current head 48a335f differs from pull request most recent head e1539d0. Consider uploading reports for the commit e1539d0 to get more accurate results
[](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16957 +/- ##
===========================================
- Coverage 74.67% 47.03% -27.65%
===========================================
Files 654 208 -446
Lines 82133 24384 -57749
===========================================
- Hits 61334 11468 -49866
+ Misses 19809 11873 -7936
- Partials 990 1043 +53
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `47.03% <47.29%> (?)` | |
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/16957?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/harness/harness.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9oYXJuZXNzLmdv) | `12.95% <0.00%> (-2.17%)` | :arrow_down: |
| [.../go/pkg/beam/core/runtime/harness/worker\_status.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy93b3JrZXJfc3RhdHVzLmdv) | `63.63% <63.63%> (ø)` | |
| [sdks/go/pkg/beam/core/runtime/exec/emit.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9lbWl0Lmdv) | `51.92% <0.00%> (-9.99%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/util/protox/any.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3V0aWwvcHJvdG94L2FueS5nbw==) | `46.93% <0.00%> (-9.16%)` | :arrow_down: |
| [sdks/go/pkg/beam/provision/provision.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9wcm92aXNpb24vcHJvdmlzaW9uLmdv) | `27.50% <0.00%> (-7.99%)` | :arrow_down: |
| [sdks/go/pkg/beam/flatten.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9mbGF0dGVuLmdv) | `44.44% <0.00%> (-7.94%)` | :arrow_down: |
| [sdks/go/pkg/beam/validate.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS92YWxpZGF0ZS5nbw==) | `42.10% <0.00%> (-7.90%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/graph/coder/int.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL2ludC5nbw==) | `70.00% <0.00%> (-7.78%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/graph/coder/time.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL3RpbWUuZ28=) | `70.00% <0.00%> (-7.78%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/graph/coder/stringutf8.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL3N0cmluZ3V0ZjguZ28=) | `65.90% <0.00%> (-7.78%)` | :arrow_down: |
| ... and [629 more](https://codecov.io/gh/apache/beam/pull/16957/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/16957?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/16957?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 [d1b8e56...e1539d0](https://codecov.io/gh/apache/beam/pull/16957?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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r816999494
##########
File path: sdks/go/pkg/beam/core/runtime/harness/harness.go
##########
@@ -98,6 +111,18 @@ func Main(ctx context.Context, loggingEndpoint, controlEndpoint string) error {
log.Debugf(ctx, "control response channel closed")
}()
+ // if the runner supports worker status api then expose SDK harness status
+ if statusEndpoint != "" {
+ statusHandler, err := newWorkerStatusHandler(ctx, statusEndpoint)
+ if err != nil {
+ log.Error(ctx, err)
Review comment:
yes, it just logs that it is not able to make a connection and fails quietly as this is optional feature for pipeline execution.
--
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] youngoli commented on a change in pull request #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
youngoli commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r829487283
##########
File path: sdks/go/pkg/beam/core/runtime/harness/harness.go
##########
@@ -36,14 +36,27 @@ import (
"google.golang.org/grpc"
)
+// StatusAddress is a type of status endpoint address as an optional argument to harness.Main().
+type StatusAddress string
+
// TODO(herohde) 2/8/2017: for now, assume we stage a full binary (not a plugin).
// Main is the main entrypoint for the Go harness. It runs at "runtime" -- not
// "pipeline-construction time" -- on each worker. It is a FnAPI client and
// ultimately responsible for correctly executing user code.
-func Main(ctx context.Context, loggingEndpoint, controlEndpoint string) error {
+func Main(ctx context.Context, loggingEndpoint, controlEndpoint string, options ...interface{}) error {
Review comment:
This is changing the signature of an exported function, which is something we usually want to avoid... But on the other hand the harness package is very unlikely to be used by end-users and is meant to just be used within Beam, so I think this should be fine. But just to be sure, I think we should run this by @lostluck.
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status.go
##########
@@ -0,0 +1,91 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
Review comment:
Nit: You could probably shorten the filename to status.go (and status_test.go). Not something I'll block on though.
##########
File path: sdks/go/pkg/beam/core/runtime/harness/harness.go
##########
@@ -98,6 +111,18 @@ func Main(ctx context.Context, loggingEndpoint, controlEndpoint string) error {
log.Debugf(ctx, "control response channel closed")
}()
+ // if the runner supports worker status api then expose SDK harness status
+ if statusEndpoint != "" {
+ statusHandler, err := newWorkerStatusHandler(ctx, statusEndpoint)
+ if err != nil {
+ log.Error(ctx, err)
+ }
+ var wg sync.WaitGroup
Review comment:
Just checking, but is this meant to be a different WaitGroup than the one declared outside this scope? If so I think it would be clearer to rename this (even something like "wg2" would be okay, or "swg" for status wait group). It just makes it very explicit that this is supposed to be a different variable than the one above.
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status_test.go
##########
@@ -0,0 +1,77 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "fmt"
+ "log"
+ "net"
+ "sync"
+ "testing"
+
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+type BeamFnWorkerStatusServicer struct {
+ fnpb.UnimplementedBeamFnWorkerStatusServer
+ response chan string
+}
+
+func (w *BeamFnWorkerStatusServicer) WorkerStatus(b fnpb.BeamFnWorkerStatus_WorkerStatusServer) error {
+ b.Send(&fnpb.WorkerStatusRequest{Id: "1"})
+ resp, err := b.Recv()
+ if err != nil {
+ return fmt.Errorf("error receiving response b.recv: %v", err)
+ }
+ w.response <- resp.GetStatusInfo()
+ return nil
+}
+
+func setUp(port string, srv *BeamFnWorkerStatusServicer) {
+ l, err := net.Listen("tcp", ":9000")
Review comment:
Can you use 0 as a port to have it listen on an available random port? As-is this could be flaky due to port conflicts.
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status_test.go
##########
@@ -0,0 +1,77 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "fmt"
+ "log"
+ "net"
+ "sync"
+ "testing"
+
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+type BeamFnWorkerStatusServicer struct {
+ fnpb.UnimplementedBeamFnWorkerStatusServer
+ response chan string
+}
+
+func (w *BeamFnWorkerStatusServicer) WorkerStatus(b fnpb.BeamFnWorkerStatus_WorkerStatusServer) error {
+ b.Send(&fnpb.WorkerStatusRequest{Id: "1"})
+ resp, err := b.Recv()
+ if err != nil {
+ return fmt.Errorf("error receiving response b.recv: %v", err)
+ }
+ w.response <- resp.GetStatusInfo()
+ return nil
+}
+
+func setUp(port string, srv *BeamFnWorkerStatusServicer) {
Review comment:
This function isn't actually using the port passed into it, that looks like an oversight.
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status_test.go
##########
@@ -0,0 +1,77 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "fmt"
+ "log"
+ "net"
+ "sync"
+ "testing"
+
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+type BeamFnWorkerStatusServicer struct {
Review comment:
This is less of a comment for you and more of a question for @lostluck, but I've noticed a few unit tests now that start up actual GRPC servers to test our grpc files. Should we maybe transition to using a local package like buffcon, like in this Stack Overflow question: https://stackoverflow.com/questions/42102496/testing-a-grpc-service ?
To be clear, even if the answer is yes we can save it for a subsequent PR. This isn't something to address in this PR either way.
--
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 change in pull request #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
damccorm commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r816958547
##########
File path: sdks/go/pkg/beam/core/runtime/harness/harness.go
##########
@@ -36,14 +36,27 @@ import (
"google.golang.org/grpc"
)
+// StatusAddress is a type of status endpoint address as an optional argument to harness.Main().
+type StatusAddress string
+
// TODO(herohde) 2/8/2017: for now, assume we stage a full binary (not a plugin).
// Main is the main entrypoint for the Go harness. It runs at "runtime" -- not
// "pipeline-construction time" -- on each worker. It is a FnAPI client and
// ultimately responsible for correctly executing user code.
-func Main(ctx context.Context, loggingEndpoint, controlEndpoint string) error {
+func Main(ctx context.Context, loggingEndpoint, controlEndpoint string, options ...interface{}) error {
hooks.DeserializeHooksFromOptions(ctx)
+ statusEndpoint := ""
+ for _, option := range options {
+ switch option := option.(type) {
+ case StatusAddress:
+ statusEndpoint = string(option)
+ default:
+ return errors.Errorf("unkown type %T, value %v in error call", option, option)
Review comment:
You're printing option twice here - did you mean for the first to be option.(type)?
##########
File path: sdks/go.mod
##########
@@ -23,33 +23,39 @@ 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.28.0
+ cloud.google.com/go/compute v1.5.0 // indirect
+ cloud.google.com/go/datastore v1.6.0
+ cloud.google.com/go/iam v0.2.0 // indirect
+ cloud.google.com/go/pubsub v1.18.0
+ cloud.google.com/go/storage v1.21.0
+ github.com/bketelsen/crypt v0.0.4 // indirect
Review comment:
Were these version changes intentional/do you have a need for them? I'm not opposed to doing it if you don't need it, just trying to understand how we normally handle this - in general, do we have a strategy on when to update 3rd party dependencies?
##########
File path: sdks/go/pkg/beam/core/runtime/harness/harness.go
##########
@@ -98,6 +111,18 @@ func Main(ctx context.Context, loggingEndpoint, controlEndpoint string) error {
log.Debugf(ctx, "control response channel closed")
}()
+ // if the runner supports worker status api then expose SDK harness status
+ if statusEndpoint != "" {
+ statusHandler, err := newWorkerStatusHandler(ctx, statusEndpoint)
+ if err != nil {
+ log.Error(ctx, err)
Review comment:
If we return an error response, is it worth trying to handle requests or should we stick the remaining logic in an else? I'd imagine that its just going to fail since we failed to set up our connection.
--
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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1054589987
# [Codecov](https://codecov.io/gh/apache/beam/pull/16957?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 [#16957](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (63aa879) into [master](https://codecov.io/gh/apache/beam/commit/3cd1f7f949bd476abb11bdb0b368a2f12a496cd1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3cd1f7f) will **decrease** coverage by `0.02%`.
> The diff coverage is `48.38%`.
[](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16957 +/- ##
==========================================
- Coverage 73.80% 73.78% -0.03%
==========================================
Files 663 664 +1
Lines 87056 87114 +58
==========================================
+ Hits 64251 64274 +23
- Misses 21721 21751 +30
- Partials 1084 1089 +5
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `48.82% <48.38%> (+0.01%)` | :arrow_up: |
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/16957?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/harness/harness.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9oYXJuZXNzLmdv) | `13.02% <0.00%> (+0.33%)` | :arrow_up: |
| [.../go/pkg/beam/core/runtime/harness/worker\_status.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy93b3JrZXJfc3RhdHVzLmdv) | `66.66% <66.66%> (ø)` | |
| [.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/16957/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.37% <0.00%> (-4.66%)` | :arrow_down: |
| [...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/16957/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.97% <0.00%> (-2.26%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `87.50% <0.00%> (-1.71%)` | :arrow_down: |
| [.../python/apache\_beam/transforms/periodicsequence.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9wZXJpb2RpY3NlcXVlbmNlLnB5) | `96.72% <0.00%> (-1.64%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/16957/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.51% <0.00%> (-0.13%)` | :arrow_down: |
| [sdks/python/apache\_beam/io/source\_test\_utils.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vc291cmNlX3Rlc3RfdXRpbHMucHk=) | `89.40% <0.00%> (+1.38%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/16957?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/16957?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 [3cd1f7f...63aa879](https://codecov.io/gh/apache/beam/pull/16957?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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1054589987
# [Codecov](https://codecov.io/gh/apache/beam/pull/16957?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 [#16957](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b35af85) into [master](https://codecov.io/gh/apache/beam/commit/3cd1f7f949bd476abb11bdb0b368a2f12a496cd1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3cd1f7f) will **decrease** coverage by `0.00%`.
> The diff coverage is `48.38%`.
[](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16957 +/- ##
==========================================
- Coverage 73.80% 73.79% -0.01%
==========================================
Files 663 665 +2
Lines 87056 87250 +194
==========================================
+ Hits 64251 64388 +137
- Misses 21721 21754 +33
- Partials 1084 1108 +24
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `49.01% <48.38%> (+0.21%)` | :arrow_up: |
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/16957?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/harness/harness.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9oYXJuZXNzLmdv) | `13.02% <0.00%> (+0.33%)` | :arrow_up: |
| [.../go/pkg/beam/core/runtime/harness/worker\_status.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy93b3JrZXJfc3RhdHVzLmdv) | `66.66% <66.66%> (ø)` | |
| [.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/16957/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.37% <0.00%> (-4.66%)` | :arrow_down: |
| [...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/16957/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.97% <0.00%> (-2.26%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `87.50% <0.00%> (-1.71%)` | :arrow_down: |
| [.../python/apache\_beam/transforms/periodicsequence.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9wZXJpb2RpY3NlcXVlbmNlLnB5) | `96.72% <0.00%> (-1.64%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/16957/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.51% <0.00%> (-0.13%)` | :arrow_down: |
| [sdks/go/pkg/beam/io/avroio/avroio.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9pby9hdnJvaW8vYXZyb2lvLmdv) | `56.45% <0.00%> (ø)` | |
| [...kg/beam/core/runtime/xlangx/expansionx/download.go](https://codecov.io/gh/apache/beam/pull/16957/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=) | `61.53% <0.00%> (+1.16%)` | :arrow_up: |
| ... and [3 more](https://codecov.io/gh/apache/beam/pull/16957/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/16957?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/16957?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 [3cd1f7f...b35af85](https://codecov.io/gh/apache/beam/pull/16957?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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1054589987
# [Codecov](https://codecov.io/gh/apache/beam/pull/16957?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 [#16957](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a62cb4e) into [master](https://codecov.io/gh/apache/beam/commit/3cd1f7f949bd476abb11bdb0b368a2f12a496cd1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3cd1f7f) will **decrease** coverage by `0.01%`.
> The diff coverage is `46.42%`.
[](https://codecov.io/gh/apache/beam/pull/16957?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16957 +/- ##
==========================================
- Coverage 73.80% 73.78% -0.02%
==========================================
Files 663 668 +5
Lines 87056 87312 +256
==========================================
+ Hits 64251 64424 +173
- Misses 21721 21781 +60
- Partials 1084 1107 +23
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `49.03% <46.42%> (+0.23%)` | :arrow_up: |
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/16957?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/harness/harness.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9oYXJuZXNzLmdv) | `13.02% <0.00%> (+0.33%)` | :arrow_up: |
| [.../go/pkg/beam/core/runtime/harness/worker\_status.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy93b3JrZXJfc3RhdHVzLmdv) | `66.66% <66.66%> (ø)` | |
| [.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/16957/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.37% <0.00%> (-4.66%)` | :arrow_down: |
| [...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/16957/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.97% <0.00%> (-2.26%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `87.50% <0.00%> (-1.71%)` | :arrow_down: |
| [.../python/apache\_beam/transforms/periodicsequence.py](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9wZXJpb2RpY3NlcXVlbmNlLnB5) | `96.72% <0.00%> (-1.64%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/16957/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.51% <0.00%> (-0.13%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/graph/window/windows.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL3dpbmRvdy93aW5kb3dzLmdv) | `82.60% <0.00%> (ø)` | |
| [sdks/go/pkg/beam/io/avroio/avroio.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9pby9hdnJvaW8vYXZyb2lvLmdv) | `56.45% <0.00%> (ø)` | |
| [sdks/go/pkg/beam/core/graph/window/fn.go](https://codecov.io/gh/apache/beam/pull/16957/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL3dpbmRvdy9mbi5nbw==) | `53.84% <0.00%> (ø)` | |
| ... and [6 more](https://codecov.io/gh/apache/beam/pull/16957/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/16957?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/16957?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 [3cd1f7f...a62cb4e](https://codecov.io/gh/apache/beam/pull/16957?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] youngoli edited a comment on pull request #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
youngoli edited a comment on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1071900580
R: @lostluck
Edit: Disregard that, I forgot he already reviewed 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] youngoli commented on pull request #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
youngoli commented on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1071900580
R: @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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on a change in pull request #16957:
URL: https://github.com/apache/beam/pull/16957#discussion_r838658774
##########
File path: sdks/go/pkg/beam/core/runtime/harness/worker_status.go
##########
@@ -0,0 +1,91 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package harness
+
+import (
+ "context"
+ "io"
+ "runtime"
+ "sync"
+ "sync/atomic"
+ "time"
+
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/internal/errors"
+ "github.com/apache/beam/sdks/v2/go/pkg/beam/log"
+ fnpb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/fnexecution_v1"
+ "google.golang.org/grpc"
+)
+
+// workerStatusHandler stores the communication information of WorkerStatus API.
+type workerStatusHandler struct {
+ conn *grpc.ClientConn
+ shutdown int32
+}
+
+func newWorkerStatusHandler(ctx context.Context, endpoint string) (*workerStatusHandler, error) {
+ sconn, err := dial(ctx, endpoint, 60*time.Second)
+ if err != nil {
+ return &workerStatusHandler{}, errors.Wrapf(err, "failed to connect: %v\n", endpoint)
+ }
+ return &workerStatusHandler{conn: sconn, shutdown: 0}, nil
+}
+
+func (w *workerStatusHandler) isAlive() bool {
+ return atomic.LoadInt32(&w.shutdown) == 0
+}
+
+func (w *workerStatusHandler) stop() {
+ atomic.StoreInt32(&w.shutdown, 1)
+}
+
+// handleRequest manages the WorkerStatus API.
+func (w *workerStatusHandler) handleRequest(ctx context.Context, wg *sync.WaitGroup) {
Review comment:
Good suggestion. I guess I'm gonna make:
`handleRequest` as `start`
`shutdown` field as `shouldShutdown`
`stop` as `shutdown`
`close` as `stop`.
So we can have `statusHandler.start()` and `statusHandler.stop()`
--
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 #16957: [BEAM-13829] Expose status API from Go SDK Harness
Posted by GitBox <gi...@apache.org>.
riteshghorse commented on pull request #16957:
URL: https://github.com/apache/beam/pull/16957#issuecomment-1083326684
PTAL. I guess things left to do are more debug information instead of just stacktrace, lull logger, bundle progress.
--
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