Skip to content

Avoid installing signal handlers to the event loop#1014

Merged
jujubot merged 2 commits intojuju:masterfrom
cderici:remove-signal-handler-installs
Jan 29, 2024
Merged

Avoid installing signal handlers to the event loop#1014
jujubot merged 2 commits intojuju:masterfrom
cderici:remove-signal-handler-installs

Conversation

@cderici
Copy link
Copy Markdown
Contributor

@cderici cderici commented Jan 25, 2024

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 in #1011 for details about reasoning.

Fixes #1010

1. Because if someone runs pylibjuju in an off-main thread, then
add_signal_handler will (rightfully) complain (see issue juju#1010).

2. Pylibjuju as a library shouldn't enforce a certain way of handling
signals. Clients should install their handlers however they wanna
handle them.

Fixes juju#1010
@cderici cderici added bug fix hint/3.x going on main branch labels Jan 25, 2024
@cderici cderici requested review from anvial and jack-w-shaw January 25, 2024 15:57
@cderici
Copy link
Copy Markdown
Contributor Author

cderici commented Jan 29, 2024

/merge

@jujubot jujubot merged commit 6692f3d into juju:master Jan 29, 2024
@cderici cderici mentioned this pull request Feb 8, 2024
jujubot added a commit that referenced this pull request Feb 13, 2024
#1024

## What's Changed
* Remove paramiko upper-bound by @gboutry in #1005
* Remove explicit passing of event_loop into tests by @cderici in #1006
* chore: remove the upper restrictions on the websockets dependency by @tonyandrewmeyer in #1007
* Target ceiling version by @cderici in #1008
* Make it easier to run tests using `make` by @cderici in #1012
* Avoid installing signal handlers to the event loop by @cderici in #1014
* feat: remove app block until done by @yanksyoon in #1017
* feat: remove app timeout by @yanksyoon in #1018
* Forward port latest changes from 2.9 onto 3.x by @cderici in #1022

#### Notes & Discussion

JUJU-5414
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hint/3.x going on main branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can't run python-libjuju async loop on background thread in py3.8, py3.9 and py3.10? due to "set_wakeup_fd() only works in main thread"

3 participants