[GitHub] [beam] echauchot commented on a change in pull request #12001: [BEAM-10082] Remove dependency on java.xml.ws.

[GitBox <gi...@apache.org>]

echauchot commented on a change in pull request #12001:
URL: https://github.com/apache/beam/pull/12001#discussion_r440838547



##########
File path: runners/extensions-java/metrics/src/main/java/org/apache/beam/runners/extensions/metrics/MetricsHttpSink.java
##########
@@ -69,7 +74,7 @@ public void writeMetrics(MetricQueryResults metricQueryResults) throws Exception
     }
     int responseCode = connection.getResponseCode();
     if (responseCode != 200) {
-      throw new HTTPException(responseCode);
+      throw new IOException("Expected OK 200 response but received: " + responseCode);

Review comment:
       Agreed as there is no suitable http exception in java.net regular package. But please change the message to add more context "Exception while writing metrics to MetricsHttpSink : expected HTTP 200 OK response but received "

##########
File path: runners/extensions-java/metrics/src/main/java/org/apache/beam/runners/extensions/metrics/MetricsHttpSink.java
##########
@@ -50,6 +49,12 @@ public MetricsHttpSink(MetricsOptions pipelineOptions) {
     this.urlString = pipelineOptions.getMetricsHttpSinkUrl();
   }
 
+  /**
+   * Writes the metricQueryResults via HTTP POST to metrics sink endpoint.
+   *
+   * @param metricQueryResults query results to write.
+   * @throws Exception throws IOException for non-200 response from endpoint.
+   */

Review comment:
       thanks for adding this missing javadoc




----------------------------------------------------------------
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