crossbario / crossbar

Crossbar.io - WAMP application router
https://crossbar.io/
Other
2.05k stars 275 forks source link

Crossbar prints empty lines to the terminal #2050

Open klimkin opened 1 year ago

klimkin commented 1 year ago

During initialization crossbar prints empty lines to the terminal. Output redirection cannot mute it since crossbar writes directly to /dev/tty. This is especially annoying when running crossbar as a subprocess when no extra output is expected.

https://github.com/crossbario/crossbar/blob/8a261c5502cac4e0b77fd8f857dc9f720a620a31/crossbar/_util.py#L58-L60

oberstet commented 1 year ago

yes, crossbar tests if it can use direct raw terminal output with no redirection by writing to the terminal device https://github.com/crossbario/crossbar/blob/8a261c5502cac4e0b77fd8f857dc9f720a620a31/crossbar/_util.py#L55

klimkin commented 1 year ago

@oberstet Would you consider using an invisible character to test the terminal device? For example \0 or \r? This would minimize output pollution.

klimkin commented 1 year ago

Or a simple check if it's writable?

import os
os.access('/dev/tty', os.W_OK)
oberstet commented 1 year ago

Would you consider using an invisible character to test the terminal device?

\n is invisible? at least I can't see whitespace and whitespace doesn't have a color one can set. using a control character rather than whitespace is also invisible, but might lead to unrelated effects (writing a NUL terminator ... not sure .. lots of C around ..)

Or a simple check if it's writable?

because that's just a flag, not the same as actually writing to the device, which is the real deal we want to test for here.

klimkin commented 1 year ago

The issue with the current implementation is that it prints new lines even if not using --debug-lifecycle. To keep the code to the same meaning, we can do a lazy check on the /dev/tty.

+++ b/crossbar/_util.py
@@ -14,6 +14,7 @@ import re
 import inspect
 import uuid
 import copy
+import functools
 from collections.abc import Mapping

 import click
@@ -39,28 +40,32 @@ def set_flags_from_args(_args):
             DEBUG_PROGRAMFLOW = True

-# FS path to controlling terminal
-_TERMINAL = None
+@functools.lru_cache(1)
+def _terminal():
+    # FS path to controlling terminal
+    _TERMINAL = None

-# Linux, *BSD and MacOSX
-if sys.platform.startswith('linux') or 'bsd' in sys.platform or sys.platform.startswith('darwin'):
-    _TERMINAL = '/dev/tty' if os.path.exists('/dev/tty') else None
-# Windows
-elif sys.platform in ['win32']:
-    pass
-# Other OS
-else:
-    pass
+    # Linux, *BSD and MacOSX
+    if sys.platform.startswith('linux') or 'bsd' in sys.platform or sys.platform.startswith('darwin'):
+        _TERMINAL = '/dev/tty' if os.path.exists('/dev/tty') else None
+    # Windows
+    elif sys.platform in ['win32']:
+        pass
+    # Other OS
+    else:
+        pass
+
+    # still, we might not be able to use TTY, so duck test it:
+    if _TERMINAL:
+        try:
+            with open('/dev/tty', 'w') as f:
+                f.write('\n')
+                f.flush()
+        except:
+            # under systemd: OSError: [Errno 6] No such device or address: '/dev/tty'
+            _TERMINAL = None

-# still, we might not be able to use TTY, so duck test it:
-if _TERMINAL:
-    try:
-        with open('/dev/tty', 'w') as f:
-            f.write('\n')
-            f.flush()
-    except:
-        # under systemd: OSError: [Errno 6] No such device or address: '/dev/tty'
-        _TERMINAL = None
+    return _TERMINAL

 def class_name(obj):
@@ -165,7 +170,7 @@ def term_print(text):
     if DEBUG_LIFECYCLE:
         text = '{:<44}'.format(text)
         text = click.style(text, fg='blue', bold=True)
-        if _TERMINAL:
+        if _terminal():
             with open('/dev/tty', 'w') as f:
                 f.write(text + '\n')
                 f.flush()
oberstet commented 1 year ago

ok, right, if --debug-lifecycle is not set, it should not print or even test (and hence output). however, since the test setting _TERMINAL is always done at import time, this isn't the case currently. ok, got you. luckily, it only is used in _util.py it seems. in this case, eg making _TERMINAL global, and lazy check & set it in term_print, seems simpler though than "functools.lru_cache(1)" and such stuff. too fancy for my eyes;) anyways, I don't care much about some random whitespace somewhere, just ignoring is even simpler! who cares? nevertheless, should you care, I'd merge a simple PR with the simple (non func) approach that runs green ...