[ 
https://issues.apache.org/jira/browse/BEAM-3803?focusedWorklogId=78995&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-78995
 ]

ASF GitHub Bot logged work on BEAM-3803:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 09/Mar/18 19:32
            Start Date: 09/Mar/18 19:32
    Worklog Time Spent: 10m 
      Work Description: kennknowles commented on a change in pull request 
#4841: BEAM-3803: Dataflow runner implements metrics contract
URL: https://github.com/apache/beam/pull/4841#discussion_r173545265
 
 

 ##########
 File path: 
runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowMetrics.java
 ##########
 @@ -350,10 +352,18 @@ public static MetricQueryResults create(
     public abstract MetricName name();
     public abstract String step();
     @Nullable
-    public abstract T committed();
-    @Nullable
+    protected abstract T committedInternal();
     public abstract T attempted();
 
+    public T committed() {
+      T committed = committedInternal();
+      if (committed == null) {
+        throw new UnsupportedOperationException("This runner does not 
currently support committed"
 
 Review comment:
   I'm not so keen on the use of exception instead of null. If something is 
explicitly optional `@Nullable` seems reasonable (insert standard rant about 
how `Optional` is a better choice but Java screwed it up and Guava is shaded).
   
   I would imagine that changes to this class would just remove `@Nullable` 
from attempted, then an additional static factory method like 
`createCommitted(<just committed value>)` that populates both fields with that 
value, `createAttempted(<just attempted value>)` that leaves committed null and 
`create(<both not nullable>)`. Incidentally the existing `create` method seems 
broken as it fails to mark them as `@Nullable`. When we plug in the checker 
framework that should be caught.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 78995)
    Time Spent: 3.5h  (was: 3h 20m)

> Dataflow runner should handle metrics per the spec
> --------------------------------------------------
>
>                 Key: BEAM-3803
>                 URL: https://issues.apache.org/jira/browse/BEAM-3803
>             Project: Beam
>          Issue Type: Bug
>          Components: runner-dataflow
>            Reporter: Andrew Pilloud
>            Assignee: Andrew Pilloud
>            Priority: Major
>              Labels: nexmark
>          Time Spent: 3.5h
>  Remaining Estimate: 0h
>
> The dataflow runner only supports committed metrics for batch jobs and 
> attempted metrics for streaming jobs. It should always support attempted 
> metrics and throw an UnsupportedOperationException when the metrics are 
> missing.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to