echauchot commented on a change in pull request #12001:
URL: https://github.com/apache/beam/pull/12001#discussion_r440982503
##########
File path:
runners/extensions-java/metrics/src/main/java/org/apache/beam/runners/extensions/metrics/MetricsHttpSink.java
##########
@@ -74,7 +74,9 @@ public void writeMetrics(MetricQueryResults
metricQueryResults) throws Exception
}
int responseCode = connection.getResponseCode();
if (responseCode != 200) {
- throw new IOException("Expected OK 200 response but received: " +
responseCode);
+ throw new IOException(
+ "Expected HTTP 200 OK response while writing metrics to
MetricsSinkHttp but received "
Review comment:
sed s/MetricsSinkHttp/MetricsHttpSink/
##########
File path:
runners/extensions-java/metrics/src/main/java/org/apache/beam/runners/extensions/metrics/MetricsHttpSink.java
##########
@@ -86,8 +93,8 @@ public MetricNameSerializer(Class<MetricName> t) {
public void serialize(MetricName value, JsonGenerator gen,
SerializerProvider provider)
throws IOException {
gen.writeStartObject();
- gen.writeObjectField("name", value.name());
- gen.writeObjectField("namespace", value.namespace());
Review comment:
yes I replaced them with get* versions to simplify serialization code
but they were re-introduced to be compatible with dataflow hosted engine which
uses .name and .namespace which I could not know of because it is not
opensource. Anyway good catch ! Dataflow is supposed to have migrated to .get*
versions now, can you check ? If so we can remove the deprecated versions from
the api.
----------------------------------------------------------------
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:
[email protected]
