Update start/stop handling of credential_manager_shim#130
Update start/stop handling of credential_manager_shim#130arenekosreal wants to merge 1 commit intolinux-credentials:mainfrom
Conversation
|
Stick to one framework, if we need uvloop, change the README.md file to pip install it. |
|
Actually |
|
Could you also motivate in the PR description why we need to quit gracefully instead of just terminating abruptly. Thanks. |
|
Hello! Thanks for your contribution! I copied in the text from your commit message into the PR description for visibility.
We only target Linux, so we don't need to worry about Windows. 👍 |
| try: | ||
| uvloop = True | ||
| from uvloop import run as asyncio_run | ||
| except ImportError: | ||
| uvloop = False | ||
| from asyncio import run as asyncio_run | ||
| from asyncio import Event |
There was a problem hiding this comment.
This is not a hot code path, so let's drop the uvloop implementation.
| signal(SIGTERM, lambda _, __ : quit.set()) | ||
| if uvloop: | ||
| logging.debug("using uvloop as async event loop") | ||
| asyncio_run(main()) |
There was a problem hiding this comment.
please import asyncio, and use asyncio.run to refer to this method.
webext/README.md
Outdated
| Then, optionally, you can use [uvloop](https://github.com/MagicStack/uvloop)>=0.18 as the web extension's async event loop implementation. | ||
| This is not required, we prefer using uvloop, while can still fallback to use the built-in asyncio if failed to import it. | ||
|
|
||
| If you are sure to use it, simply install `uvloop` like installing `dbus-next` mentioned above. | ||
| We will print a log to let you know if we are using uvloop. | ||
|
|
||
| # Setup Instructions | ||
|
|
|
@kalvdans At first I was creating a package on AUR, and found that this helper cannot start because |
Using asyncio.run is preferred by Python. `asyncio.get_event_loop` raises a `RuntimeError` on my machine, which means that there is no event loop set. This is a change in 3.14, seems that we need to use `asyncio.set_event_loop` before running. So I use `asyncio.run` directly to avoid this. Handle `SIGTERM` signal so we can quit gracefully. Firefox will send `SIGTERM` on *nix systems, and use Windows's way to kill the native app. Not using Windows so not sure if `SIGTERM` will also be raised on Windows, but I do not find a clean way to notify killing. See also: https://docs.python.org/3.14/library/asyncio-eventloop.html#asyncio.get_event_loop https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Native_messaging#closing_the_native_app
5fbb82a to
f77047f
Compare
|
@iinuwa I have force-pushed changes, uvloop support is removed completely. |
asyncio.runoruvloop.runto start main loopSIGTERMto quit gracefullyUsing asyncio.run is preferred by Python.
asyncio.get_event_loopraises aRuntimeErroron my machine, which means that there is no event loop set.This is a change in 3.14, seems that we need to use
asyncio.set_event_loopbefore running. So I useasyncio.rundirectly to avoid this.Handle
SIGTERMsignal so we can quit gracefully.Firefox will send
SIGTERMon *nix systems, and use Windows's way to kill the native app. Not using Windows so not sure ifSIGTERMwill also be raised on Windows, but I do not find a clean way to notify killing.