Skip to content

Commit 70a0ad9

Browse files
author
Stephen Hoover
authored
BUG Make sure that the CivisFuture always cleans up after exceptions (#40)
There were a couple of circumstances where exceptions raised in polling could have left us still subscribed to the `pubnub` notifications channel. Refactor the exception setting so that it only happens in one place; that way we can be sure that `cleanup` always gets called.
1 parent fbfb5f6 commit 70a0ad9

File tree

2 files changed

+15
-12
lines changed

2 files changed

+15
-12
lines changed

civis/futures.py

+2-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
from civis import APIClient
2-
from civis.base import FAILED, DONE
3-
from civis.response import Response
2+
from civis.base import DONE
43
from civis.polling import PollableResult
54

65
try:
@@ -148,5 +147,4 @@ def _poll_and_set_api_result(self):
148147
result = self.poller(*self.poller_args)
149148
self._set_api_result(result)
150149
except Exception as e:
151-
self._result = Response({"state": FAILED[0]})
152-
self.set_exception(e)
150+
self._set_api_exception(exc=e)

civis/polling.py

+13-8
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,7 @@ def _wait_for_completion(self):
9595
# bug in `_check_result`, however, we would get stuck
9696
# in an infinite loop without setting the `_result`.
9797
with self._condition:
98-
self._result = Response({"state": FAILED[0]})
99-
self.set_exception(e)
98+
self._set_api_exception(exc=e)
10099

101100
def _poll_wait_elapsed(self, now):
102101
# thie exists because it's easier to monkeypatch in testing
@@ -130,9 +129,7 @@ def _check_result(self):
130129
except Exception as e:
131130
# The _poller can raise API exceptions
132131
# Set those directly as this Future's exception
133-
self._result = Response({"state": FAILED[0]})
134-
self._last_result = self._result
135-
self.set_exception(e)
132+
self._set_api_exception(exc=e)
136133
else:
137134
# If the job has finished, then register completion and
138135
# store the results. Because of the `if self._result` check
@@ -148,13 +145,21 @@ def _set_api_result(self, result):
148145
err_msg = str(result['error'])
149146
except:
150147
err_msg = str(result)
151-
self.set_exception(CivisJobFailure(err_msg, result))
152-
self._result = result
153-
self.cleanup()
148+
self._set_api_exception(exc=CivisJobFailure(err_msg, result),
149+
result=result)
154150
elif result.state in DONE:
155151
self.set_result(result)
156152
self.cleanup()
157153

154+
def _set_api_exception(self, exc, result=None):
155+
with self._condition:
156+
if result is None:
157+
result = Response({"state": FAILED[0]})
158+
self._result = result
159+
self._last_result = self._result
160+
self.set_exception(exc)
161+
self.cleanup()
162+
158163
def cleanup(self):
159164
# This gets called after the result is set
160165
pass

0 commit comments

Comments
 (0)