Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows Support #76

Closed
heri16 opened this issue Apr 29, 2017 · 5 comments
Closed

Windows Support #76

heri16 opened this issue Apr 29, 2017 · 5 comments

Comments

@heri16
Copy link

heri16 commented Apr 29, 2017

Crashes on Windows

PS C:\Windows\system32> D:\aws-ses-proxy\venv\Scripts\python.exe -m aiosmtpd -n
Traceback (most recent call last):
  File "C:\Python36\lib\runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "C:\Python36\lib\runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "D:\aws-ses-proxy\venv\lib\site-packages\aiosmtpd\__main__.py", line 5, in <module>
    main()
  File "D:\aws-ses-proxy\venv\lib\site-packages\aiosmtpd\main.py", line 137, in main
    loop.add_signal_handler(signal.SIGINT, loop.stop)
  File "C:\Python36\lib\asyncio\events.py", line 481, in add_signal_handler
    raise NotImplementedError
NotImplementedError
@Mortal
Copy link
Contributor

Mortal commented Apr 29, 2017

On some platforms loop.add_signal_handler(signal.SIGINT, loop.stop) is an easy way of handling Ctrl-C. Unfortunately this is currently not supported on Windows. There is a patch python/asyncio#191 that seemingly was never merged into asyncio. Apparently the best workaround is to wake up the event loop often so that Ctrl-C can be handled quickly, since Ctrl-C is not handled when the event loop is sleeping, cf. python/asyncio#407. Signal handling on Windows is (apparently) complicated.

In short, we can fix this particular exception by catching the NotImplementedError on Windows.

--- a/aiosmtpd/main.py
+++ b/aiosmtpd/main.py
@@ -134,7 +134,11 @@ def main(args=None):
 
     server = loop.run_until_complete(
         loop.create_server(factory, host=args.host, port=args.port))
-    loop.add_signal_handler(signal.SIGINT, loop.stop)
+    try:
+        loop.add_signal_handler(signal.SIGINT, loop.stop)
+    except NotImplementedError:
+        if sys.platform != 'win32':
+            raise
 
     log.info('Starting asyncio loop')
     try:

@warsaw
Copy link
Contributor

warsaw commented Apr 30, 2017

I definitely want to support Windows officially. I would love it if we could include Windows in the CI, perhaps with AppVeyor. Do you have any experience integrating that with GitHub CI? If we don't do automated testing on Windows, then it's highly likely we'll break compatibility again in the future.

@Mortal
Copy link
Contributor

Mortal commented May 1, 2017

I'm affiliated with a C++ project on GitHub that uses AppVeyor but I haven't tried setting it up myself for a project, so I don't have any experience. I agree that this is important to set up.

@Mortal
Copy link
Contributor

Mortal commented May 6, 2017

I'm working on this. Hopefully I'll have a PR for you soon with an Appveyor config.

@warsaw
Copy link
Contributor

warsaw commented May 7, 2017

#81 and #82

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants