New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix incorrect timeout warnings in AWS Lambda and GCP integrations #854
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -891,21 +891,31 @@ def __init__(self, waiting_time, configured_timeout): | |
threading.Thread.__init__(self) | ||
self.waiting_time = waiting_time | ||
self.configured_timeout = configured_timeout | ||
self._stop_event = threading.Event() | ||
|
||
def stop(self): | ||
# type: () -> None | ||
self._stop_event.set() | ||
|
||
def stopped(self): | ||
# type: () -> Any | ||
return self._stop_event.is_set() | ||
|
||
def run(self): | ||
# type: () -> None | ||
|
||
time.sleep(self.waiting_time) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The thread doesn't need to sleep for the full That's why I suggested using https://docs.python.org/2.7/library/threading.html#threading.Event.wait You can either use the return value or There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes I figured that while I did testing and found it was still waiting for complete waiting time. |
||
|
||
integer_configured_timeout = int(self.configured_timeout) | ||
if not self.stopped(): | ||
integer_configured_timeout = int(self.configured_timeout) | ||
|
||
# Setting up the exact integer value of configured time(in seconds) | ||
if integer_configured_timeout < self.configured_timeout: | ||
integer_configured_timeout = integer_configured_timeout + 1 | ||
# Setting up the exact integer value of configured time(in seconds) | ||
if integer_configured_timeout < self.configured_timeout: | ||
integer_configured_timeout = integer_configured_timeout + 1 | ||
|
||
# Raising Exception after timeout duration is reached | ||
raise ServerlessTimeoutWarning( | ||
"WARNING : Function is expected to get timed out. Configured timeout duration = {} seconds.".format( | ||
integer_configured_timeout | ||
# Raising Exception after timeout duration is reached | ||
raise ServerlessTimeoutWarning( | ||
"WARNING : Function is expected to get timed out. Configured timeout duration = {} seconds.".format( | ||
integer_configured_timeout | ||
) | ||
) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@untitaker In this "finally" block, the code to stop the thread basically works in case of :-
But, it doesn't stop the thread in case of timeout, which is an expected behavior.
By "stop the thread" I mean explicitly stopping a thread by sending a stop signal.