[beam] branch master updated: fix python metric-querying bug

[mx...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

mxm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/beam.git


The following commit(s) were added to refs/heads/master by this push:
     new 2bed9ae  fix python metric-querying bug
     new f3e156a  Merge pull request #7638: [BEAM-6518] fix python metric-querying bug
2bed9ae is described below

commit 2bed9ae6bfaf54fe825c4c091d4806242de26d94
Author: Ryan Williams <ry...@gmail.com>
AuthorDate: Sat Jan 26 17:33:50 2019 +0000

    fix python metric-querying bug
---
 sdks/python/apache_beam/metrics/metric.py      | 28 +++++++++++++++++---------
 sdks/python/apache_beam/metrics/metric_test.py | 24 +++++++++++++---------
 sdks/python/apache_beam/metrics/metricbase.py  |  1 +
 3 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/sdks/python/apache_beam/metrics/metric.py b/sdks/python/apache_beam/metrics/metric.py
index 455503a..acd4771 100644
--- a/sdks/python/apache_beam/metrics/metric.py
+++ b/sdks/python/apache_beam/metrics/metric.py
@@ -150,17 +150,25 @@ class MetricResults(object):
     return False
 
   @staticmethod
+  def _is_sub_list(needle, haystack):
+    """True iff `needle` is a sub-list of `haystack` (i.e. a contiguous slice
+    of `haystack` exactly matches `needle`"""
+    needle_len = len(needle)
+    haystack_len = len(haystack)
+    for i in range(0, haystack_len - needle_len + 1):
+      if haystack[i:i+needle_len] == needle:
+        return True
+
+    return False
+
+  @staticmethod
   def _matches_sub_path(actual_scope, filter_scope):
-    start_pos = actual_scope.find(filter_scope)
-    end_pos = start_pos + len(filter_scope)
-
-    if start_pos == -1:
-      return False  # No match at all
-    elif start_pos != 0 and actual_scope[start_pos - 1] != '/':
-      return False  # The first entry was not exactly matched
-    elif end_pos != len(actual_scope) and actual_scope[end_pos] != '/':
-      return False  # The last entry was not exactly matched
-    return True
+    """True iff the '/'-delimited pieces of filter_scope exist as a sub-list
+    of the '/'-delimited pieces of actual_scope"""
+    return MetricResults._is_sub_list(
+        filter_scope.split('/'),
+        actual_scope.split('/')
+    )
 
   @staticmethod
   def _matches_scope(filter, metric_key):
diff --git a/sdks/python/apache_beam/metrics/metric_test.py b/sdks/python/apache_beam/metrics/metric_test.py
index 0cd492f..6e8ee08 100644
--- a/sdks/python/apache_beam/metrics/metric_test.py
+++ b/sdks/python/apache_beam/metrics/metric_test.py
@@ -66,24 +66,30 @@ class MetricResultsTest(unittest.TestCase):
     self.assertTrue(MetricResults.matches(filter, key))
 
   def test_metric_filter_step_matching(self):
-    filter = MetricsFilter().with_step('Top1/Outer1/Inner1')
     name = MetricName('ns1', 'name1')
-    key = MetricKey('Top1/Outer1/Inner1', name)
+    filter = MetricsFilter().with_step('Step1')
+
+    key = MetricKey('Step1', name)
     self.assertTrue(MetricResults.matches(filter, key))
 
-    filter = MetricsFilter().with_step('step1')
-    name = MetricName('ns1', 'name1')
-    key = MetricKey('step1', name)
+    key = MetricKey('Step10', name)
+    self.assertFalse(MetricResults.matches(filter, key))
+
+    key = MetricKey('Step10/Step1', name)
     self.assertTrue(MetricResults.matches(filter, key))
 
-    filter = MetricsFilter().with_step('Top1/Outer1')
-    name = MetricName('ns1', 'name1')
     key = MetricKey('Top1/Outer1/Inner1', name)
+
+    filter = MetricsFilter().with_step('Top1/Outer1/Inner1')
+    self.assertTrue(MetricResults.matches(filter, key))
+
+    filter = MetricsFilter().with_step('Top1/Outer1')
+    self.assertTrue(MetricResults.matches(filter, key))
+
+    filter = MetricsFilter().with_step('Outer1/Inner1')
     self.assertTrue(MetricResults.matches(filter, key))
 
     filter = MetricsFilter().with_step('Top1/Inner1')
-    name = MetricName('ns1', 'name1')
-    key = MetricKey('Top1/Outer1/Inner1', name)
     self.assertFalse(MetricResults.matches(filter, key))
 
 
diff --git a/sdks/python/apache_beam/metrics/metricbase.py b/sdks/python/apache_beam/metrics/metricbase.py
index 6729534..420bfef 100644
--- a/sdks/python/apache_beam/metrics/metricbase.py
+++ b/sdks/python/apache_beam/metrics/metricbase.py
@@ -77,6 +77,7 @@ class MetricName(object):
   def __hash__(self):
     return hash((self.namespace, self.name))
 
+  # TODO: this proto structure is deprecated
   def to_runner_api(self):
     return beam_fn_api_pb2.Metrics.User.MetricName(
         namespace=self.namespace, name=self.name)