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

Bot fails to gracefully shut down #2

Open
ruthgrace opened this issue Aug 29, 2016 · 4 comments
Open

Bot fails to gracefully shut down #2

ruthgrace opened this issue Aug 29, 2016 · 4 comments
Labels

Comments

@ruthgrace
Copy link

The bot does not necessarily shut down gracefully. Here is an example from when reconnection after Slack team migration fails.

2016-08-28 03:43:52,136 Bot WARNING: Slack has initiated a team migration to a new server.  Attempting to reconnect...
2016-08-28 03:43:52,237 requests.packages.urllib3.connectionpool INFO: Starting new HTTPS connection (1): slack.com
2016-08-28 03:43:52,326 requests.packages.urllib3.connectionpool DEBUG: "POST /api/rtm.start HTTP/1.1" 200 64
2016-08-28 03:43:52,329 requests.packages.urllib3.connectionpool INFO: Starting new HTTP connection (1): localhost
2016-08-28 03:43:52,331 Webserver WARNING: Shutdown code received.  Stopping webserver...
2016-08-28 03:43:52,331 werkzeug INFO: 127.0.0.1 - - [28/Aug/2016 03:43:52] "POST /_/shutdown HTTP/1.1" 200 -
2016-08-28 03:43:52,331 requests.packages.urllib3.connectionpool DEBUG: "POST /_/shutdown HTTP/1.1" 200 0

After this, the docker container was not shut down, circumventing automatic restart.

@arcticfoxnv
Copy link
Collaborator

This is most likely due to a plugin starting a thread and not stopping it when the bot shuts down. Python won't force background threads to exit when the main thread exits.

@balajijegan
Copy link
Contributor

https://docs.python.org/3/library/threading.html#threading.Thread.daemon

Why not set all plugins as daemon threads? The only downside is lock or any other operation the thread might be doing.

@arcticfoxnv
Copy link
Collaborator

Mostly because the bot was written with 2.7 in mind. At the time, python 3 wasn't a viable choice for our environment, and bot frameworks at the time were written for python 3. Additionally, existing frameworks didn't handle saving their config+state to anywhere other than local storage, which was a non-starter for the project.

Given python 2.7 is going away soon, everything should be python 3 anyways, and there are much better bot frameworks for python 3 than this one, I'm not sure how much time you would want to spend on rewriting the plugin system.

balajijegan pushed a commit to balajijegan/slackminion that referenced this issue Jul 28, 2020
Add link_names option to send_message, fix tests
@balajijegan
Copy link
Contributor

This issue is now resolved and we have not seen this issue after the python 3 rewrite and fixes that @amckenna-pinterest did.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants