Updater improvements #1018
Updater improvements #1018
Conversation
telegram/ext/updater.py
Outdated
| while 1: | ||
| def _bootstrap(self, max_retries, clean, webhook_url, allowed_updates, cert=None, | ||
| bootstrap_interval=5): | ||
| retries = [0] |
There was a problem hiding this comment.
Why did you make this a list? It's still only used for one int value
There was a problem hiding this comment.
Because otherwise it will overridden as a local variable of the inner function.
A different workaround would be to do:
global retries
retries = 0But I don't like global inside such an inner code.
Of course we can do it a class method or something more "generic" but then we'll lose code locality.
If you can think of something else, I'll be happy for a more elegant solution.
telegram/ext/updater.py
Outdated
| return False | ||
|
|
||
| def bootstrap_clean_updates(): | ||
| self._clean_updates() |
There was a problem hiding this comment.
The _clean_updates() method is only ever called once here. I suggest the code is added in this place.
There was a problem hiding this comment.
mmm... I tried not to touch _clean_updates() (move less code if possible)
but I guess you're right, code locality is preferred here.
telegram/ext/updater.py
Outdated
| sleep(1) | ||
|
|
||
| self._network_loop_retry(bootstrap_set_webhook, bootstrap_onerr_cb, |
There was a problem hiding this comment.
If I read it right, this means a webhook is set when Updater.start_polling is called, but with an empty url. This is an unnecessary API call to a method that is prone to flood actions. I would like to prevent it.
Also see the comment on delete webhook
There was a problem hiding this comment.
This call was "written in blood".
If user had messed around with webhooks and now he wants a polling server, we make sure that Telegram servers are properly configured for it.
@jh0ker Maybe you remember the origin of this better?
telegram/ext/updater.py
Outdated
| else: | ||
| self.logger.exception(msg) | ||
| raise | ||
| def bootstrap_del_webhook(): |
There was a problem hiding this comment.
This is also called when Updater.start_webhook() is called, but this is not necessary. However the set/delete_webhook methods are prone to flood limits. I would like to prevent that.
| raise | ||
| self.logger.debug('{0} - ended'.format(thr_name)) | ||
|
|
||
| def start_polling(self, | ||
| poll_interval=0.0, | ||
| timeout=10, | ||
| clean=False, | ||
| bootstrap_retries=0, | ||
| bootstrap_retries=-1, |
There was a problem hiding this comment.
The docstring doesn;t reflect this change to the default value
|
I just left some review comments. |
…trap_retries value [ci skip]
|
Yes good! |
fixes #605