From 422c0e151bb4907b815a254aacfc4a4e4a88a0b1 Mon Sep 17 00:00:00 2001 From: Stephen Kiely Date: Fri, 20 Dec 2024 10:20:10 -0600 Subject: [PATCH 1/4] Replace sync_to_async with database_sync_to_async This change allows the Slack Socket Mode to properly close and reopen the database connection when finished processing a command. --- changes/355.fixed | 1 + nautobot_chatops/sockets/slack.py | 8 ++++---- nautobot_chatops/utils.py | 4 ++-- poetry.lock | 23 +++++++++++++++++++++-- pyproject.toml | 1 + 5 files changed, 29 insertions(+), 8 deletions(-) create mode 100644 changes/355.fixed diff --git a/changes/355.fixed b/changes/355.fixed new file mode 100644 index 00000000..9a5641f9 --- /dev/null +++ b/changes/355.fixed @@ -0,0 +1 @@ +Fixed "Server has gone away" error in Slack Socket Mode. diff --git a/nautobot_chatops/sockets/slack.py b/nautobot_chatops/sockets/slack.py index 5f95975a..88d4a968 100644 --- a/nautobot_chatops/sockets/slack.py +++ b/nautobot_chatops/sockets/slack.py @@ -4,7 +4,7 @@ import json import shlex -from asgiref.sync import sync_to_async +from channels.db import database_sync_to_async from django.conf import settings from slack_sdk.socket_mode.aiohttp import SocketModeClient from slack_sdk.socket_mode.request import SocketModeRequest @@ -72,7 +72,7 @@ async def process_slash_command(client, req): client.logger.error("%s", err) return - registry = await sync_to_async(get_commands_registry)() + registry = await database_sync_to_async(get_commands_registry)() if command not in registry: SlackDispatcher(context).send_markdown(commands_help(prefix=SLASH_PREFIX)) @@ -211,7 +211,7 @@ async def process_interactive(client, req): client.logger.info(f"command: {command}, subcommand: {subcommand}, params: {params}") - registry = await sync_to_async(get_commands_registry)() + registry = await database_sync_to_async(get_commands_registry)() if command not in registry: SlackDispatcher(context).send_markdown(commands_help(prefix=SLASH_PREFIX)) @@ -242,7 +242,7 @@ async def process_mention(client, req): client.logger.error("%s", err) return - registry = await sync_to_async(get_commands_registry)() + registry = await database_sync_to_async(get_commands_registry)() if command not in registry: SlackDispatcher(context).send_markdown(commands_help(prefix=SLASH_PREFIX)) diff --git a/nautobot_chatops/utils.py b/nautobot_chatops/utils.py index 59be36d3..ff03e074 100644 --- a/nautobot_chatops/utils.py +++ b/nautobot_chatops/utils.py @@ -4,7 +4,7 @@ import sys from datetime import datetime, timezone -from asgiref.sync import sync_to_async +from channels.db import database_sync_to_async from django.conf import settings from django.db.models import Q from django.http import HttpResponse, JsonResponse @@ -113,7 +113,7 @@ def create_command_log(dispatcher, registry, command, subcommand, params=()): ) -@sync_to_async +@database_sync_to_async def socket_check_and_enqueue_command(*args, **kwargs): """Calls check_and_enqueue_command() when in Socket mode.""" return check_and_enqueue_command(*args, **kwargs) diff --git a/poetry.lock b/poetry.lock index 02282a63..a7157f6c 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 1.8.3 and should not be changed by hand. +# This file is automatically @generated by Poetry 1.8.2 and should not be changed by hand. [[package]] name = "aiodns" @@ -635,6 +635,25 @@ files = [ [package.dependencies] pycparser = "*" +[[package]] +name = "channels" +version = "4.2.0" +description = "Brings async, event-driven capabilities to Django." +optional = false +python-versions = ">=3.8" +files = [ + {file = "channels-4.2.0-py3-none-any.whl", hash = "sha256:6b75bc8d6888fb7236e7e7bf1948520b72d296ad08216a242fc56b1db0ffde1a"}, + {file = "channels-4.2.0.tar.gz", hash = "sha256:d9e707487431ba5dbce9af982970dab3b0efd786580fadb99e45dca5e39fdd59"}, +] + +[package.dependencies] +asgiref = ">=3.6.0,<4" +Django = ">=4.2" + +[package.extras] +daphne = ["daphne (>=4.0.0)"] +tests = ["async-timeout", "coverage (>=4.5,<5.0)", "pytest", "pytest-asyncio", "pytest-django"] + [[package]] name = "charset-normalizer" version = "3.4.0" @@ -5692,4 +5711,4 @@ panorama = ["defusedxml", "ipaddr", "netmiko", "netutils", "pan-os-python"] [metadata] lock-version = "2.0" python-versions = ">=3.8,<3.13" -content-hash = "ad68c716ef369fde92f50ce853a1ae1da4a0eadc4dd65a25e97b2e65f7df4e62" +content-hash = "c71adb8f4527879e3b5b78f33b41dbe4f1fb8b06c721ba6bf40a7dce2e3ce1e9" diff --git a/pyproject.toml b/pyproject.toml index 644e3c57..851c95d7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -50,6 +50,7 @@ aiodns = "^1.0" aiohttp = "^3.7.3" asgiref = "^3.4.1" certifi = { version = ">=2021.5.30", optional = true } +channels = "^4.2.0" cloudvision = { version = "^1.1", optional = true } cvprac = { version = "^1.0.6", optional = true } defusedxml = { version = "^0.7.1", optional = true } From 6e65ebd4953e19f6b13dec62206bf3e4ba69dbef Mon Sep 17 00:00:00 2001 From: Stephen Kiely Date: Fri, 20 Dec 2024 11:56:35 -0600 Subject: [PATCH 2/4] Bring database_sync_to_async into repo Removes the bulk of having Channels installed when no functionality from the app aside from this command is being used. --- NOTICE | 1 + nautobot_chatops/sockets/slack.py | 3 +-- nautobot_chatops/utils.py | 20 +++++++++++++++++++- poetry.lock | 21 +-------------------- pyproject.toml | 1 - 5 files changed, 22 insertions(+), 24 deletions(-) create mode 100644 NOTICE diff --git a/NOTICE b/NOTICE new file mode 100644 index 00000000..3c471e11 --- /dev/null +++ b/NOTICE @@ -0,0 +1 @@ +Some parts of Nautobot-App-Chatops are based upon `channels`, licensed under BSD-3-Clause by Django Software Foundation. \ No newline at end of file diff --git a/nautobot_chatops/sockets/slack.py b/nautobot_chatops/sockets/slack.py index 88d4a968..bb71193b 100644 --- a/nautobot_chatops/sockets/slack.py +++ b/nautobot_chatops/sockets/slack.py @@ -4,7 +4,6 @@ import json import shlex -from channels.db import database_sync_to_async from django.conf import settings from slack_sdk.socket_mode.aiohttp import SocketModeClient from slack_sdk.socket_mode.request import SocketModeRequest @@ -12,7 +11,7 @@ from slack_sdk.web.async_client import AsyncWebClient from nautobot_chatops.dispatchers.slack import SlackDispatcher -from nautobot_chatops.utils import socket_check_and_enqueue_command +from nautobot_chatops.utils import database_sync_to_async, socket_check_and_enqueue_command from nautobot_chatops.workers import commands_help, get_commands_registry, parse_command_string diff --git a/nautobot_chatops/utils.py b/nautobot_chatops/utils.py index ff03e074..c4030761 100644 --- a/nautobot_chatops/utils.py +++ b/nautobot_chatops/utils.py @@ -4,8 +4,9 @@ import sys from datetime import datetime, timezone -from channels.db import database_sync_to_async +from asgiref.sync import SyncToAsync from django.conf import settings +from django.db import close_old_connections from django.db.models import Q from django.http import HttpResponse, JsonResponse from nautobot.core.celery import nautobot_task @@ -18,6 +19,23 @@ logger = logging.getLogger(__name__) +class DatabaseSyncToAsync(SyncToAsync): + """ + SyncToAsync version that cleans up old database connections when it exits. + Sourced from Channels, see NOTICE file for license information. + """ + + def thread_handler(self, loop, *args, **kwargs): + close_old_connections() + try: + return super().thread_handler(loop, *args, **kwargs) + finally: + close_old_connections() + + +# The class is TitleCased, but we want to encourage use as a callable/decorator +database_sync_to_async = DatabaseSyncToAsync + def get_app_config_part(prefix: str) -> dict: """Get part of the app config. diff --git a/poetry.lock b/poetry.lock index a7157f6c..f859ee15 100644 --- a/poetry.lock +++ b/poetry.lock @@ -635,25 +635,6 @@ files = [ [package.dependencies] pycparser = "*" -[[package]] -name = "channels" -version = "4.2.0" -description = "Brings async, event-driven capabilities to Django." -optional = false -python-versions = ">=3.8" -files = [ - {file = "channels-4.2.0-py3-none-any.whl", hash = "sha256:6b75bc8d6888fb7236e7e7bf1948520b72d296ad08216a242fc56b1db0ffde1a"}, - {file = "channels-4.2.0.tar.gz", hash = "sha256:d9e707487431ba5dbce9af982970dab3b0efd786580fadb99e45dca5e39fdd59"}, -] - -[package.dependencies] -asgiref = ">=3.6.0,<4" -Django = ">=4.2" - -[package.extras] -daphne = ["daphne (>=4.0.0)"] -tests = ["async-timeout", "coverage (>=4.5,<5.0)", "pytest", "pytest-asyncio", "pytest-django"] - [[package]] name = "charset-normalizer" version = "3.4.0" @@ -5711,4 +5692,4 @@ panorama = ["defusedxml", "ipaddr", "netmiko", "netutils", "pan-os-python"] [metadata] lock-version = "2.0" python-versions = ">=3.8,<3.13" -content-hash = "c71adb8f4527879e3b5b78f33b41dbe4f1fb8b06c721ba6bf40a7dce2e3ce1e9" +content-hash = "ad68c716ef369fde92f50ce853a1ae1da4a0eadc4dd65a25e97b2e65f7df4e62" diff --git a/pyproject.toml b/pyproject.toml index 851c95d7..644e3c57 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -50,7 +50,6 @@ aiodns = "^1.0" aiohttp = "^3.7.3" asgiref = "^3.4.1" certifi = { version = ">=2021.5.30", optional = true } -channels = "^4.2.0" cloudvision = { version = "^1.1", optional = true } cvprac = { version = "^1.0.6", optional = true } defusedxml = { version = "^0.7.1", optional = true } From 2254a4d6ca26346e7bb326181fe792a3ed82107b Mon Sep 17 00:00:00 2001 From: Stephen Kiely Date: Fri, 20 Dec 2024 12:22:23 -0600 Subject: [PATCH 3/4] Update docstrings. --- nautobot_chatops/utils.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/nautobot_chatops/utils.py b/nautobot_chatops/utils.py index c4030761..b2fe8982 100644 --- a/nautobot_chatops/utils.py +++ b/nautobot_chatops/utils.py @@ -22,10 +22,12 @@ class DatabaseSyncToAsync(SyncToAsync): """ SyncToAsync version that cleans up old database connections when it exits. + Sourced from Channels, see NOTICE file for license information. """ def thread_handler(self, loop, *args, **kwargs): + """Run the handler in a thread, closing old database connections before and after.""" close_old_connections() try: return super().thread_handler(loop, *args, **kwargs) @@ -36,6 +38,7 @@ def thread_handler(self, loop, *args, **kwargs): # The class is TitleCased, but we want to encourage use as a callable/decorator database_sync_to_async = DatabaseSyncToAsync + def get_app_config_part(prefix: str) -> dict: """Get part of the app config. From cec8d225c089017aafb53abc92e0b252f6ad8d5a Mon Sep 17 00:00:00 2001 From: Stephen Kiely Date: Fri, 20 Dec 2024 12:38:17 -0600 Subject: [PATCH 4/4] pylint --- nautobot_chatops/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nautobot_chatops/utils.py b/nautobot_chatops/utils.py index b2fe8982..f85b33f9 100644 --- a/nautobot_chatops/utils.py +++ b/nautobot_chatops/utils.py @@ -36,7 +36,7 @@ def thread_handler(self, loop, *args, **kwargs): # The class is TitleCased, but we want to encourage use as a callable/decorator -database_sync_to_async = DatabaseSyncToAsync +database_sync_to_async = DatabaseSyncToAsync # pylint: disable=invalid-name def get_app_config_part(prefix: str) -> dict: