Add a try: except: around loop.add_signal_handler()

[ajkavanagh]

For py 3.8, 3.9 and 3.10, the set_wakeup_fd() call fails on a background thread, due to [1], which has been fixed in py3.11. This patch simply catches the error if the python version can't do the adding of a signal handler so that the existing/previous behaviour continues.

[1] python/cpython#87173

Closes-Bug: #1010

Description

Closes-Bug: #1010

QA Steps

<Commands / tests / steps to run to verify that the change works:>

tox -e py3 -- tests/unit/...

tox -e integration -- tests/integration/...

All CI tests need to pass.

[cderici]

Hey @ajkavanagh, yeah I can see why this can be a problem off the main thread.

In general, I like this solution (except maybe just catch the RuntimeError (instead of (ValueError, OSError, RuntimeError)) and match the entire message instead of doing if 'main thread' not in str(e):, so that we'd catch all the problems other than this particular issue.

However, this makes me wonder whether pylibjuju installing signal handlers is correct to begin with. So, pylibjuju has two notions built-in to make it useful in general; a library, and a client (I'd like to eventually split these two in the future). Adding signal handlers makes sense from a client perspective, however, it doesn't makes sense at all from a library perspective.

I'm beginning to think that it's a mistake for pylibjuju to enforce certain way of handling a signal (for projects using pylibjuju as a library). So it makes slightly more sense to remove the code that adds (and removes) the signal handlers in a connection. That'll solve your problem as well. WDYT?

[ajkavanagh]

In general, I like this solution (except maybe just catch the RuntimeError (instead of (ValueError, OSError, RuntimeError)) and match the entire message instead of doing if 'main thread' not in str(e):, so that we'd catch all the problems other than this particular issue.

Yes, that's a reasonable point; I was essentially just copying the existing model from juju/jasyncio.py to try to keep things similar.

However, this makes me wonder whether pylibjuju installing signal handlers is correct to begin with. So, pylibjuju has two notions built-in to make it useful in general; a library, and a client (I'd like to eventually split these two in the future). Adding signal handlers makes sense from a client perspective, however, it doesn't makes sense at all from a library perspective.

Well that was my first thought too. Libs generally shouldn't, I guess, be handling signals, as it ought to be something that the client-of-the-library ought to do.

I'm beginning to think that it's a mistake for pylibjuju to enforce certain way of handling a signal (for projects using pylibjuju as a library). So it makes slightly more sense to remove the code that adds (and removes) the signal handlers in a connection. That'll solve your problem as well. WDYT?

Yes, I agree with you. It would probably be good to give an example in the lib docs on how to catch signals so that the client can clean up properly as well, but I'd agree that the onus should be on the user-of-the-library to be responsible for cleaning up the code.

As such, please fee free to reject/close this change if you'd prefer a different approach. Thanks!

[ajkavanagh]

Based on #1014 merging, this is no longer needed. Thanks for the fix!