bowensong commented on a change in pull request #1249:
URL: https://github.com/apache/cassandra/pull/1249#discussion_r728124408
##########
File path: bin/cqlsh.py
##########
@@ -93,11 +93,6 @@
# is a ../lib dir, use bundled libs there preferentially.
ZIPLIB_DIRS = [os.path.join(CASSANDRA_PATH, 'lib')]
myplatform = platform.system()
Review comment:
After removing the `is_win` part, the `myplatform` variable is only
being used at `if myplatform == 'Linux':` below. It makes more sense to remove
the global variable and change the line to `if platform.system() == 'Linux':`.
##########
File path: bin/cqlsh.py
##########
@@ -1894,7 +1878,7 @@ def do_clear(self, parsed):
Clears the console.
"""
import subprocess
- subprocess.call(['clear', 'cls'][is_win], shell=True)
+ subprocess.call(['clear', 'cls'][False], shell=True)
Review comment:
That new code is equivalent to `subprocess.call('clear', shell=True)`,
but less readable.
##########
File path: pylib/cqlshlib/test/run_cqlsh.py
##########
@@ -126,21 +118,12 @@ def timing_out_alarm(seconds):
finally:
signal.alarm(0)
-if is_win():
- try:
- import eventlet
- except ImportError as e:
- sys.exit("evenlet library required to run cqlshlib tests on Windows")
-
- def timing_out(seconds):
- return eventlet.Timeout(seconds, TimeoutError)
+# setitimer is new in 2.6, but it's still worth supporting, for potentially
+# faster tests because of sub-second resolution on timeouts.
+if hasattr(signal, 'setitimer'):
Review comment:
`signal.setitimer()` is available on all Python versions (2.7 and 3)
currently supported by cqlsh. The `if hasattr(...)` is redundant.
##########
File path: pylib/cqlshlib/test/run_cqlsh.py
##########
@@ -30,15 +30,8 @@
from os.path import join, normpath
-def is_win():
- return sys.platform in ("cygwin", "win32")
-
-if is_win():
- from .winpty import WinPty
Review comment:
`WinPty` is still being referenced in the `ProcRunner.start_proc()`
method. Please double check and make sure all references to the `WinPty` module
has been taken care of.
##########
File path: pylib/cqlshlib/test/run_cqlsh.py
##########
@@ -297,11 +277,6 @@ def __init__(self, path=None, host=None, port=None,
keyspace=None, cqlver=None,
args += ('--cqlversion', str(cqlver))
if keyspace is not None:
args += ('--keyspace', keyspace.lower())
- if tty and is_win():
- args += ('--tty',)
- args += ('--encoding', 'utf-8')
- if win_force_colors:
Review comment:
After removing this line, the `win_force_colors` parameter in the
`__init__()` method is unused and should be removed too. The
`test_no_color_output` test in the test_cqlsh_output.py file needs to be
updated or removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
