Skip to content

Commit

Permalink
Merge pull request #1014 from cderici/remove-signal-handler-installs
Browse files Browse the repository at this point in the history
#1014

#### Description

In #1010 it's reported that running pylibjuju in an off-main thread blows up the `add_signal_handler` (for a good reason). Also per the discussion in the #1011 about signal handlers, this PR removes the code that installs signal handlers to the event loop at the connection creation. See [my comment](#1011 (comment)) in #1011 for details about reasoning.

Fixes #1010
  • Loading branch information
jujubot authored Jan 29, 2024
2 parents 32ce250 + c6af97a commit 6692f3d
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 11 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ run-unit-tests: lint .tox
tox -e py3

.PHONY: run-integration-tests
run-unit-tests: lint .tox
run-integration-tests: lint .tox
tox -e integration

.PHONY: run-all-tests
Expand Down
10 changes: 0 additions & 10 deletions juju/client/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import json
import logging
import ssl
import signal
import urllib.request
import weakref
from http.client import HTTPSConnection
Expand Down Expand Up @@ -425,10 +424,6 @@ def _exit_tasks():
for task in jasyncio.all_tasks():
task.cancel()

loop = jasyncio.get_running_loop()
for sig in (signal.SIGINT, signal.SIGTERM):
loop.add_signal_handler(sig, _exit_tasks)

return (await websockets.connect(
url,
ssl=self._get_ssl(cacert),
Expand Down Expand Up @@ -473,11 +468,6 @@ async def close(self, to_reconnect=False):
if self.proxy is not None:
self.proxy.close()

# Remove signal handlers
loop = jasyncio.get_running_loop()
for sig in (signal.SIGINT, signal.SIGTERM):
loop.remove_signal_handler(sig)

async def _recv(self, request_id):
if not self.is_open:
raise websockets.exceptions.ConnectionClosed(0, 'websocket closed')
Expand Down

0 comments on commit 6692f3d

Please sign in to comment.