[GitHub] [beam] lukecwik commented on a change in pull request #13078: Rename ProcessBundleProgressMetadataRequest to MonitoringInfosRequest. And rename ProcessBundleProgressMetadataResponse to MonitoringInfosResponse

[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