Skip to content
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

Log an INFO-level message when SIGINT received #946

Closed
Makman2 opened this issue Dec 23, 2017 · 7 comments · Fixed by #951
Closed

Log an INFO-level message when SIGINT received #946

Makman2 opened this issue Dec 23, 2017 · 7 comments · Fixed by #951

Comments

@Makman2
Copy link
Contributor

Makman2 commented Dec 23, 2017

Hi ;)

I'm recently experimenting with your nice bot. However, I stumbled upon this during my "learning-process":

I've created a bot, it worked normally. I just did updater.start_polling like in the examples, but without using updater.idle() (I thought first start_polling is blocking). Exiting with CTRL+C worked immediately, but has thrown a KeyboardInterrupt.
After a while I found the idle() function, but I was actually a bit confused of CTRL+C's behaviour, because I've configured logging with a minimum level of INFO. So I've seen nothing when I pressed the keys. The bot actually needs a while to shutdown, and because nothing was logged I assumed the SIGINT signal wasn't received (/ assuming due to internal threading stuff not properly processed), so I pressed always CTRL+C two times causing a SIGTERM which worked immediately.
SIGINT notifications are only visible when enabling DEBUG-level logging. I would suggest to add an INFO message, that tells something like Received SIGINT, shutting down..., so nobody is worried ;)

@JosXa
Copy link
Contributor

JosXa commented Dec 23, 2017

@Makman2 To make it clear, this is all intended behavior. I kind of see your point, but it's probably not something that the library should spam into log files for users that collect them for later (if I'm making any sense). The issue is debatable, but since it's an event that clearly belongs to the "debugging" stage of application development, it makes sense to have it displayed only when debug logging is enabled, which every user should do at least once in his contact with ptb, since this offers many other clues to the internals of ptb that would fall into the same category of what the library does under the hood.

@Makman2
Copy link
Contributor Author

Makman2 commented Dec 23, 2017

it's an event that clearly belongs to the "debugging" stage of application development

This is the thing where I'm not so sure ;) It's kind of right that you usually keep your bot alive forever once it's in production state, however even then you sometimes have to shut it down (and I'm not sure if a production state bot should run in DEBUG mode). An indication might just help seeing that everything went fine.
Also a SIGTERM signal gets displayed as a WARNING, so not sure if it's better to display SIGINT for sake of consistency (but not as a warning).

The issue is debatable

Indeed :) Personally I would like to receive an info, that's why I try to find arguments for it :)

@Eldinnie
Copy link
Member

Hi,

I tend to lean towards making this a INFO-level message. It's not really a spammy message (only occurs on the SIGINT signal) and alerts the user that the signal is received and the bot is closing down.
@jh0ker @tsnoam @bomjacob thoughts?

@jh0ker
Copy link
Member

jh0ker commented Dec 23, 2017

@Eldinnie I Agree

@tsnoam
Copy link
Member

tsnoam commented Dec 23, 2017

@jh0ker I agree too

@JosXa
Copy link
Contributor

JosXa commented Dec 23, 2017

You got my blessing. @Makman2 would you be willing to make a PR? :)

@Makman2
Copy link
Contributor Author

Makman2 commented Dec 23, 2017

I'll try, currently Christmas and stuff, and Google Code-In still running on my main open source project :) But shouldn't be too hard I guess ;)

Makman2 added a commit to Makman2/python-telegram-bot that referenced this issue Dec 25, 2017
@github-actions github-actions bot locked and limited conversation to collaborators Aug 24, 2020
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 a pull request may close this issue.

5 participants