CaliDog / certstream-python

Python library for connecting to CertStream
MIT License
425 stars 72 forks source link

SIGPIPE not for Windows #3

Closed gvanem closed 5 years ago

gvanem commented 6 years ago

From running the certclient.exe program:

Traceback (most recent call last):
  File "f:\programfiler\python27\lib\runpy.py", line 174, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "f:\programfiler\python27\lib\runpy.py", line 72, in _run_code
    exec code in run_globals
  File "F:\ProgramFiler\Python27\Scripts\certstream.EXE\__main__.py", line 5, in <module>
  File "f:\programfiler\python27\lib\site-packages\certstream\cli.py", line 8, in <module>
    from signal import signal, SIGPIPE, SIG_DFL
ImportError: cannot import name SIGPIPE

I'm using Python 2.7.13 on Win-10. The use of termcolor tells me this package hasn't been tested on Windows at all. Why not coloramainstead?

Fitblip commented 6 years ago

Hey @gvanem!

Hmm, I don't know much about why windows would not have a SIGPIPE signal, but it's necessary for us to have unbuffered outputs so it can be chained into grep and friends.

I don't have the bandwidth to fix this right now, but if you'd like to take a stab at it a PR would be greatly appreciated (and compensated with no less than one awesome GIF)!

As for termcolor/colorama there wasn't a whole lot of thought that went into picking the package, mostly a google query. If you want to move from one to the other, I'm happy to review a separate PR for that as well!

gvanem commented 6 years ago

Well, I'm not very experienced in Python. But this patch works for me:

--- a/certstream/cli.py 2017-11-08 19:15:11
+++ b/certstream/cli.py 2017-11-30 16:25:43
@@ -1,3 +1,4 @@
+import sys
 import argparse
 import datetime
 import json
@@ -5,7 +6,10 @@
 import sys
 import termcolor

-from signal import signal, SIGPIPE, SIG_DFL
+if sys.platform == 'win32':
+  from signal import signal, SIG_DFL
+else:
+  from signal import signal, SIGPIPE, SIG_DFL

 import certstream

@@ -20,7 +24,8 @@
     args = parser.parse_args()

     # Ignore broken pipes
-    signal(SIGPIPE, SIG_DFL)
+    if sys.platform != 'win32':
+      signal(SIGPIPE, SIG_DFL)

     log_level = logging.INFO
     if args.verbose:
gvanem commented 5 years ago

Closing since no-one bothers.