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 2020/10/12 22:50:01 UTC
[GitHub] [beam] ajamato opened a new pull request #13078: Rename ProcessBundleProgressMetadataRequest to MonitoringInfosRequest. And rename ProcessBundleProgressMetadataResponse to MonitoringInfosResponse
ajamato opened a new pull request #13078:
URL: https://github.com/apache/beam/pull/13078
…. And rename ProcessBundleProgressMetadataResponse to MonitoringInfosResponse
**Please** add a meaningful description for your change here
------------------------
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).
Post-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
--- | --- | --- | --- | --- | --- | ---
Go | [](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/) | ---
Java | [](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
Python | [](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/) | ---
XLang | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/) | ---
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website | Whitespace | Typescript
--- | --- | --- | --- | --- | --- | ---
Non-portable | [](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/lastCompletedBuild/) <br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/)
Portable | --- | [](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | --- | --- | ---
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on pull request #13078: Rename ProcessBundleProgressMetadataRequest to MonitoringInfosMetadataRequest. And rename ProcessBundleProgressMetadataResponse to MonitoringInfosMetadataResponse
Posted by GitBox <gi...@apache.org>.
lukecwik commented on pull request #13078:
URL: https://github.com/apache/beam/pull/13078#issuecomment-708522376
Run Python PreCommit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on pull request #13078: Rename ProcessBundleProgressMetadataRequest to MonitoringInfosMetadataRequest. And rename ProcessBundleProgressMetadataResponse to MonitoringInfosMetadataResponse
Posted by GitBox <gi...@apache.org>.
lostluck commented on pull request #13078:
URL: https://github.com/apache/beam/pull/13078#issuecomment-708618178
I've now got a compiling and passing version of the go protos. It's in PR https://github.com/apache/beam/pull/13115 which will be updated once this PR is in.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] ajamato commented on pull request #13078: Rename ProcessBundleProgressMetadataRequest to MonitoringInfosMetadataRequest. And rename ProcessBundleProgressMetadataResponse to MonitoringInfosMetadataResponse
Posted by GitBox <gi...@apache.org>.
ajamato commented on pull request #13078:
URL: https://github.com/apache/beam/pull/13078#issuecomment-708084132
R: @lostluck Please review. I removed the go changes. Please follow up and include those with the generated go protos in a separate PR as discussed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on pull request #13078: Rename ProcessBundleProgressMetadataRequest to MonitoringInfosMetadataRequest. And rename ProcessBundleProgressMetadataResponse to MonitoringInfosMetadataResponse
Posted by GitBox <gi...@apache.org>.
lukecwik commented on pull request #13078:
URL: https://github.com/apache/beam/pull/13078#issuecomment-712307019
Run Python PreCommit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik merged pull request #13078: Rename ProcessBundleProgressMetadataRequest to MonitoringInfosMetadataRequest. And rename ProcessBundleProgressMetadataResponse to MonitoringInfosMetadataResponse
Posted by GitBox <gi...@apache.org>.
lukecwik merged pull request #13078:
URL: https://github.com/apache/beam/pull/13078
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on pull request #13078: Rename ProcessBundleProgressMetadataRequest to MonitoringInfosMetadataRequest. And rename ProcessBundleProgressMetadataResponse to MonitoringInfosMetadataResponse
Posted by GitBox <gi...@apache.org>.
lukecwik commented on pull request #13078:
URL: https://github.com/apache/beam/pull/13078#issuecomment-708671760
Run Python PreCommit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on a change in pull request #13078: Rename ProcessBundleProgressMetadataRequest to MonitoringInfosRequest. And rename ProcessBundleProgressMetadataResponse to MonitoringInfosResponse
Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #13078:
URL: https://github.com/apache/beam/pull/13078#discussion_r504064444
##########
File path: model/fn-execution/src/main/proto/beam_fn_api.proto
##########
@@ -348,12 +348,12 @@ message ProcessBundleProgressResponse {
// An SDK can report metrics using an identifier that only contains the
// associated payload. A runner who wants to receive the full metrics
// information can request all the monitoring metadata via a
-// ProcessBundleProgressMetadataRequest providing a list of ids as necessary.
+// MonitoringInfosMetadataRequest providing a list of ids as necessary.
//
// The SDK is allowed to reuse the identifiers across multiple bundles as long
// as the MonitoringInfo could be reconstructed fully by overwriting its
// payload field with the bytes specified here.
-message ProcessBundleProgressMetadataResponse {
+message MonitoringInfosMetadataResponse {
// A mapping from an identifier to the full metrics information.
Review comment:
```suggestion
// A mapping from a requested identifier to a MonitoringInfo. All fields except for
// the payload of the MonitoringInfo will be specified.
```
##########
File path: model/fn-execution/src/main/proto/beam_fn_api.proto
##########
@@ -312,12 +312,12 @@ message ProcessBundleProgressRequest {
// An SDK can report metrics using an identifier that only contains the
Review comment:
I can't provide a suggestion for the first line, please adjust to `A request to provide full MonitoringInfo for a given id.`
##########
File path: model/fn-execution/src/main/proto/beam_fn_api.proto
##########
@@ -312,12 +312,12 @@ message ProcessBundleProgressRequest {
// An SDK can report metrics using an identifier that only contains the
// associated payload. A runner who wants to receive the full metrics
// information can request all the monitoring metadata via a
-// ProcessBundleProgressMetadataRequest providing a list of ids as necessary.
+// MonitoringInfosMetadataRequest providing a list of ids as necessary.
//
// The SDK is allowed to reuse the identifiers across multiple bundles as long
// as the MonitoringInfo could be reconstructed fully by overwriting its
// payload field with the bytes specified here.
Review comment:
```suggestion
// The MonitoringInfo ids are scoped to the associated control connection. For
// example, an SDK may reuse the ids across multiple bundles.
```
##########
File path: model/fn-execution/src/main/proto/beam_fn_api.proto
##########
@@ -312,12 +312,12 @@ message ProcessBundleProgressRequest {
// An SDK can report metrics using an identifier that only contains the
// associated payload. A runner who wants to receive the full metrics
// information can request all the monitoring metadata via a
-// ProcessBundleProgressMetadataRequest providing a list of ids as necessary.
+// MonitoringInfosMetadataRequest providing a list of ids as necessary.
//
// The SDK is allowed to reuse the identifiers across multiple bundles as long
// as the MonitoringInfo could be reconstructed fully by overwriting its
// payload field with the bytes specified here.
-message ProcessBundleProgressMetadataRequest {
+message MonitoringInfosMetadataRequest {
// A list of ids for which the full MonitoringInfo is requested for.
Review comment:
```suggestion
// A list of ids for which MonitoringInfo are requested. All but the payload field will be populated.
```
##########
File path: model/fn-execution/src/main/proto/beam_fn_api.proto
##########
@@ -348,12 +348,12 @@ message ProcessBundleProgressResponse {
// An SDK can report metrics using an identifier that only contains the
// associated payload. A runner who wants to receive the full metrics
// information can request all the monitoring metadata via a
-// ProcessBundleProgressMetadataRequest providing a list of ids as necessary.
+// MonitoringInfosMetadataRequest providing a list of ids as necessary.
//
// The SDK is allowed to reuse the identifiers across multiple bundles as long
// as the MonitoringInfo could be reconstructed fully by overwriting its
// payload field with the bytes specified here.
Review comment:
```suggestion
// The MonitoringInfo ids are scoped to the associated control connection. For
// examplem an SDK may reuse the ids across multiple bundles.
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] ajamato commented on pull request #13078: Rename ProcessBundleProgressMetadataRequest to MonitoringInfosRequest. And rename ProcessBundleProgressMetadataResponse to MonitoringInfosResponse
Posted by GitBox <gi...@apache.org>.
ajamato commented on pull request #13078:
URL: https://github.com/apache/beam/pull/13078#issuecomment-707409797
R: @lukecwik
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on pull request #13078: Rename ProcessBundleProgressMetadataRequest to MonitoringInfosMetadataRequest. And rename ProcessBundleProgressMetadataResponse to MonitoringInfosMetadataResponse
Posted by GitBox <gi...@apache.org>.
lostluck commented on pull request #13078:
URL: https://github.com/apache/beam/pull/13078#issuecomment-708570925
Ack. It looks like the head process for Go Protos has change enough that the instructions need to be adjusted and updated as well. I don't anticipate any additional changes to the SDK as a result other than incorporating these renames, but the Go proto files themselves will be updated drastically to handle the change to the v2 version of Go protocol buffers.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck edited a comment on pull request #13078: Rename ProcessBundleProgressMetadataRequest to MonitoringInfosMetadataRequest. And rename ProcessBundleProgressMetadataResponse to MonitoringInfosMetadataResponse
Posted by GitBox <gi...@apache.org>.
lostluck edited a comment on pull request #13078:
URL: https://github.com/apache/beam/pull/13078#issuecomment-708618178
I've now got a compiling and passing version of the go protos. It's in PR https://github.com/apache/beam/pull/13115 which will be updated once this PR is in.
Edit: now to figure out why it's passing locally but not on jenkins....
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on pull request #13078: Rename ProcessBundleProgressMetadataRequest to MonitoringInfosMetadataRequest. And rename ProcessBundleProgressMetadataResponse to MonitoringInfosMetadataResponse
Posted by GitBox <gi...@apache.org>.
lukecwik commented on pull request #13078:
URL: https://github.com/apache/beam/pull/13078#issuecomment-712364530
@lostluck Was waiting on tests to pass and they finally did so you should be able to update #13115 now.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on pull request #13078: Rename ProcessBundleProgressMetadataRequest to MonitoringInfosMetadataRequest. And rename ProcessBundleProgressMetadataResponse to MonitoringInfosMetadataResponse
Posted by GitBox <gi...@apache.org>.
lostluck commented on pull request #13078:
URL: https://github.com/apache/beam/pull/13078#issuecomment-712378037
Thanks! date #13115 should be updated now.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on pull request #13078: Rename ProcessBundleProgressMetadataRequest to MonitoringInfosMetadataRequest. And rename ProcessBundleProgressMetadataResponse to MonitoringInfosMetadataResponse
Posted by GitBox <gi...@apache.org>.
lostluck commented on pull request #13078:
URL: https://github.com/apache/beam/pull/13078#issuecomment-712359250
I'm ready to update #13115 with this once it's merged in.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on pull request #13078: Rename ProcessBundleProgressMetadataRequest to MonitoringInfosMetadataRequest. And rename ProcessBundleProgressMetadataResponse to MonitoringInfosMetadataResponse
Posted by GitBox <gi...@apache.org>.
lukecwik commented on pull request #13078:
URL: https://github.com/apache/beam/pull/13078#issuecomment-708629004
Run Python PreCommit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] ajamato commented on pull request #13078: Rename ProcessBundleProgressMetadataRequest to MonitoringInfosMetadataRequest. And rename ProcessBundleProgressMetadataResponse to MonitoringInfosMetadataResponse
Posted by GitBox <gi...@apache.org>.
ajamato commented on pull request #13078:
URL: https://github.com/apache/beam/pull/13078#issuecomment-712357682
Thanks for the review, looks like its ready now
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org