Skip to content

Conversation

@tsnoam
Copy link
Member

@tsnoam tsnoam commented Feb 15, 2018

TimedOut exception is an expected an normal event. To reduce noise and
make things more "fluent" we now:

  • Make sure that we don't sleep after the timeout but rather retry
    immediately.
  • Log debug instead of error level.

Fixes #802

TimedOut exception is an expected an normal event. To reduce noise and
make things more "fluent" we now:
 - Make sure that we don't sleep after the timeout but rather retry
immediately.
 - Log debug instead of error level.

Fixes #802
@tsnoam tsnoam requested a review from Eldinnie February 15, 2018 16:13
Copy link
Member

@Eldinnie Eldinnie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally it looks good. But I'm wondering. This will probably work fine for a shortly interrupted connection, but won't this hog memory when timedout is fired often?

cur_interval = 0.5 + e.retry_after
except TimedOut as toe:
self.logger.debug('Timed out getting Updates: %s', toe)
# If getUpdates() failed due to timeout, we should retry asap.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get_updates() maybe?

@tsnoam
Copy link
Member Author

tsnoam commented Feb 17, 2018

@Eldinnie
There will be no memory hogging. If you've meant CPU hogging, I don't see how it's possible as if the next get_updates() will be interrupted by:

  1. A detectable network error, then the except TelegramError as te: block will come into affect.
  2. A timeout - the wait itself for the timeout to occur will take enough time to avoid any CPU hogging.

@Eldinnie
Copy link
Member

Good, merge after #1006

@tsnoam
Copy link
Member Author

tsnoam commented Feb 17, 2018

@Eldinnie Why after?

@Eldinnie
Copy link
Member

@tsnoam not really needed maybe, but that will make CI pass. And I prefer t if master passes CI

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TimedOut on get_updates

3 participants