Handling update_interval values. #185
Conversation
| except (OSError, OverflowError) as err: | ||
| self.logger.error('Error in time.sleep. ' | ||
| 'Provided update_interval value may be too big. Error: {}'.format(str(err))) | ||
| raise |
There was a problem hiding this comment.
Should we still be raising in this case or just pass it to the error handler? Not sure if raising from another thread might be an issue
There was a problem hiding this comment.
I think if we hit either of these errors we should just raise so that the user sees this and fixes it. We cannot leave it silent.
There was a problem hiding this comment.
Won't this crash the application though? The job of the error handler in this case would be raise visibility (if an error handler is provided), otherwise it'll just show up in the logs (again, if a logger is provided)
There was a problem hiding this comment.
If the value (which is provided the first time) is too high to cause OverflowError or OSError the SDK won't be initialized in the first place.
There was a problem hiding this comment.
I see. Wouldn't that make this check redundant then?
There was a problem hiding this comment.
Yeah, it may seem that way, but unfortunately time.sleep has different limits. See here: https://github.com/python/cpython/blob/2.7/Modules/timemodule.c#L1030-L1155
We are not imposing any limit at init after our discussion last week. I was in favor on imposing a 1 day limit, but may be it is ok for time.sleep to fail to let people know that they should change it to a reasonable value. That way they can adjust for their platform accordingly.
There was a problem hiding this comment.
Got it. Thanks for the explanation.
Summary
Handling
update_intervalvalues if they cause overflow or are of the invalid type.