surahman commented on a change in pull request #3786:
URL: https://github.com/apache/incubator-heron/pull/3786#discussion_r836593031



##########
File path: heron/tools/tracker/src/python/query_operators.py
##########
@@ -150,11 +150,11 @@ async def execute(
       raise Exception(metrics["message"])
 
     # Put a blank timeline.
-    if not metrics.get("timeline"):
-      metrics["timeline"] = {
+    if not metrics.timeline:
+      metrics.timeline = {

Review comment:
       `timeline` is a `dict`, test suite caught this error. Reverting.
   
   ```bash
         # Put a blank timeline.
   >     if not metrics.timeline:
   E     AttributeError: 'dict' object has no attribute 'timeline'
   ```
   
   

##########
File path: heron/tools/tracker/src/python/query_operators.py
##########
@@ -476,7 +477,7 @@ async def execute(self, tracker, tmanager: 
TManagerLocation, start: int, end: in
       # Initialize with first metrics timeline and its instance
       met = Metrics(None, None, metric.instance, start, end, 
metric.timeline.copy())
       for timestamp in list(met.timeline.keys()):
-        v = self._f(met.timeline[timestamp], 
metrics2[""].timeline.get(timestamp))
+        v = self._f(met.timeline[timestamp], metrics2[""].timeline[timestamp])

Review comment:
       Reverting. Please see the comment above.

##########
File path: heron/tools/tracker/src/python/query_operators.py
##########
@@ -460,14 +460,15 @@ async def execute(self, tracker, tmanager: 
TManagerLocation, start: int, end: in
       for key, metric in metrics2.items():
         # Initialize with first metrics timeline, but second metric's instance
         # because that is multivariate
-        met = Metrics(None, None, metric.instance, start, end, 
metrics[""].timeline.copy())
-        for timestamp in list(met.timeline.keys()):
-          v = self._f(met.timeline[timestamp], metric.timeline.get(timestamp))
-          if v is None:
-            met.timeline.pop(timestamp, None)
-          else:
-            met.timeline[timestamp] = v
-        all_metrics.append(met)
+        if metrics:
+          met = Metrics(None, None, metric.instance, start, end, 
metrics[""].timeline.copy())
+          for timestamp in list(met.timeline.keys()):
+            v = self._f(met.timeline[timestamp], metric.timeline[timestamp])

Review comment:
       The method `.get` in `dict` is safer than the brackets because it will 
return `None` if the key is not found. This is currently raising a `KeyError` 
in the test suite. Reverting.




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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to