-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Error intervals to stop flooding logs & set non_stop to true by default #2332
Conversation
@Badiboy will need your review on this one too |
@@ -2598,6 +2598,10 @@ def send_document( | |||
logger.warning('The parameter "thumb" is deprecated. Use "thumbnail" instead.') | |||
thumbnail = thumb | |||
|
|||
if isinstance(document, types.InputFile) and visible_file_name: | |||
# inputfile name ignored, warn | |||
logger.warning('Cannot use both InputFile and visible_file_name. InputFile name will be ignored.') |
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.
files[key] = (value.file_name, value.file)
visible_file_name will be ignoread, isn't it?
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.
No, it returns a tuple and in apihelper, tuple value[0] is unchanged, thus ignoring the inputfile naming
telebot/async_telebot.py
Outdated
|
||
if non_stop or handled: | ||
await asyncio.sleep(2) | ||
#await asyncio.sleep(2) # used error_interval instead |
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.
Changed behaviour!
In old version sleep(2) is for non_stop or handled. In new version it is for not hadled and independently from non_stop.
- Adding for not handled is fine.
- Removing from handled is not fine, because handled exceptions will cause now immediate re-run instead of 2 second delay.
- Make sleep() for non_stop looks also not needed.
I propose:
if not handled:
logger.error('Unhandled exception (full traceback for debug level): %s', self.__hide_token(str(e)))
logger.debug(self.__hide_token(traceback.format_exc()))
if non_stop:
error_interval = await self._handle_error_interval(error_interval)
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.
so we still don't need sleep for handled? or should I add sleep to handled (2 seconds) back?
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.
We need to sleep both for handled (as before) and not handled (it's reasonable).
But I propose only if non_stop: otherwise sleeping is useless, because polling will stop anyway.
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.
wait, before, script slept 2 seconds for handled or non_stop. So, should I:
- Change your last condition to "if non_stop or handled"?
- Add another condition for handled, making the script sleep for 2 seconds?
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.
this will not affect not handled: there will be no sleeping as you said, the sleep will be useless. currently we are talking about handled. Or, do you think the user should manually do that and not rely on internal behaviour?
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.
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.
I am talking about the need to have "if handled" condition.
In previous version, as you know, sleep is triggered on handled or non_stop. After I copy your suggestion, sleep is not triggered for handled. is that fine?
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.
After I copy your suggestion, sleep is not triggered for handled. is that fine?
It is not. But if you copy my version - it will work both for handled and not handled.
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.
After I copy your suggestion, sleep is not triggered for handled. is that fine?
It is not.
I don't know what you did, but if you'll take my version - it will work both for handled and not handled. Isn't it?
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.
ah, sorry
was sleepy a little, should work, yep
@@ -457,6 +464,7 @@ async def _process_polling(self, non_stop: bool=False, interval: int=0, timeout: | |||
if not handled: | |||
logger.error('Unhandled exception (full traceback for debug level): %s', self.__hide_token(str(e))) | |||
logger.debug(self.__hide_token(traceback.format_exc())) | |||
error_interval = await self._handle_error_interval(error_interval) |
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.
The same here.
if not handled:
logger.error('Unhandled exception (full traceback for debug level): %s', self.__hide_token(str(e)))
logger.debug(self.__hide_token(traceback.format_exc()))
if non_stop:
error_interval = await self._handle_error_interval(error_interval)
if non_stop or handled:
continue
@@ -467,6 +475,7 @@ async def _process_polling(self, non_stop: bool=False, interval: int=0, timeout: | |||
if not handled: | |||
logger.error('Unhandled exception (full traceback for debug level): %s', str(e)) | |||
logger.debug(traceback.format_exc()) | |||
error_interval = await self._handle_error_interval(error_interval) |
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.
And here.
@@ -130,6 +130,8 @@ def _prepare_data(params=None, files=None): | |||
if isinstance(f, tuple): | |||
if len(f) == 2: | |||
file_name, file = f | |||
if isinstance(file, types.InputFile): |
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.
Cannot verify this, never worked with InputFile.
self._file, self.file_name = self._resolve_file(file) | ||
def __init__(self, file: Union[str, IOBase, Path], file_name: Optional[str] = None): | ||
self._file, self._file_name = self._resolve_file(file) | ||
if file_name: |
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.
Why not )
#2327 #2331 #2320