[2/2] incubator-beam git commit: Fix shared state across retry decorated functions

[ro...@apache.org]

Fix shared state across retry decorated functions


Project: http://git-wip-us.apache.org/repos/asf/incubator-beam/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-beam/commit/e7f689a7
Tree: http://git-wip-us.apache.org/repos/asf/incubator-beam/tree/e7f689a7
Diff: http://git-wip-us.apache.org/repos/asf/incubator-beam/diff/e7f689a7

Branch: refs/heads/python-sdk
Commit: e7f689a72143cc5c821449c862c732f877ca4645
Parents: 115cf33
Author: Sourabh Bajaj <so...@google.com>
Authored: Tue Nov 15 15:04:37 2016 -0800
Committer: Robert Bradshaw <ro...@google.com>
Committed: Thu Nov 17 12:21:43 2016 -0800

----------------------------------------------------------------------
 sdks/python/apache_beam/utils/retry.py      |  8 ++---
 sdks/python/apache_beam/utils/retry_test.py | 42 ++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-beam/blob/e7f689a7/sdks/python/apache_beam/utils/retry.py
----------------------------------------------------------------------
diff --git a/sdks/python/apache_beam/utils/retry.py b/sdks/python/apache_beam/utils/retry.py
index 1f5af88..b3016fd 100644
--- a/sdks/python/apache_beam/utils/retry.py
+++ b/sdks/python/apache_beam/utils/retry.py
@@ -152,12 +152,10 @@ def with_exponential_backoff(
 
   def real_decorator(fun):
     """The real decorator whose purpose is to return the wrapped function."""
-
-    retry_intervals = iter(
-        FuzzedExponentialIntervals(
-            initial_delay_secs, num_retries, fuzz=0.5 if fuzz else 0))
-
     def wrapper(*args, **kwargs):
+      retry_intervals = iter(
+          FuzzedExponentialIntervals(
+              initial_delay_secs, num_retries, fuzz=0.5 if fuzz else 0))
       while True:
         try:
           return fun(*args, **kwargs)

http://git-wip-us.apache.org/repos/asf/incubator-beam/blob/e7f689a7/sdks/python/apache_beam/utils/retry_test.py
----------------------------------------------------------------------
diff --git a/sdks/python/apache_beam/utils/retry_test.py b/sdks/python/apache_beam/utils/retry_test.py
index 705c555..7570ca0 100644
--- a/sdks/python/apache_beam/utils/retry_test.py
+++ b/sdks/python/apache_beam/utils/retry_test.py
@@ -164,5 +164,47 @@ class RetryTest(unittest.TestCase):
       self.assertEqual(func_name, 'transient_failure')
 
 
+class DummyClass(object):
+  def __init__(self, results):
+    self.index = 0
+    self.results = results
+
+  @retry.with_exponential_backoff(num_retries=2, initial_delay_secs=0.1)
+  def func(self):
+    self.index += 1
+    if self.index > len(self.results) or \
+        self.results[self.index - 1] == "Error":
+      raise ValueError("Error")
+    return self.results[self.index - 1]
+
+
+class RetryStateTest(unittest.TestCase):
+  """The test_two_failures and test_single_failure would fail if we have
+  any shared state for the retry decorator. This test tries to prevent a bug we
+  found where the state in the decorator was shared across objects and retries
+  were not available correctly.
+
+  The test_call_two_objects would test this inside the same test.
+  """
+  def test_two_failures(self):
+    dummy = DummyClass(["Error", "Error", "Success"])
+    dummy.func()
+    self.assertEqual(3, dummy.index)
+
+  def test_single_failure(self):
+    dummy = DummyClass(["Error", "Success"])
+    dummy.func()
+    self.assertEqual(2, dummy.index)
+
+  def test_call_two_objects(self):
+    dummy = DummyClass(["Error", "Error", "Success"])
+    dummy.func()
+    self.assertEqual(3, dummy.index)
+
+    dummy2 = DummyClass(["Error", "Success"])
+    dummy2.func()
+    self.assertEqual(2, dummy2.index)
+
+
 if __name__ == '__main__':
   unittest.main()