josh-mckenzie commented on a change in pull request #1249:
URL: https://github.com/apache/cassandra/pull/1249#discussion_r798771954
##########
File path: pylib/cqlshlib/test/run_cqlsh.py
##########
@@ -111,36 +103,6 @@ def timing_out_itimer(seconds):
finally:
signal.setitimer(signal.ITIMER_REAL, 0)
[email protected]
-def timing_out_alarm(seconds):
Review comment:
Why did we remove the itimer post 2.6 stuff here?
##########
File path: doc/modules/cassandra/pages/troubleshooting/index.adoc
##########
@@ -8,10 +8,7 @@ isolate the problem using logs and tools. Luckily Cassandra
had a great
set of instrospection tools to help you.
These pages include a number of command examples demonstrating various
-debugging and analysis techniques, mostly for Linux/Unix systems. If you
-don't have access to the machines running Cassandra, or are running on
Review comment:
Would keep the bit about "If you don't have access to the machines
running C*" etc etc
##########
File path: test/unit/org/apache/cassandra/io/sstable/SSTableLoaderTest.java
##########
@@ -101,6 +101,8 @@ public void cleanup()
We force a GC here to force buffer deallocation, and then try
deleting the directory again.
For more information, see:
http://bugs.java.com/bugdatabase/view_bug.do?bug_id=4715154
If this is not the problem, the exception will be rethrown
anyway.
+
Review comment:
I'd massage this message a bit to remove the "Windows does not allow"
etc. etc. completely.
##########
File path: pylib/cqlshlib/copyutil.py
##########
@@ -884,7 +884,7 @@ def __init__(self, fname, options):
self.max_rows = options.copy['maxrows']
self.skip_rows = options.copy['skiprows']
self.fname = fname
- self.sources = None # must be created later due to pickle problems on
Windows
+ self.sources = None
Review comment:
Without Windows' issues, should we instead initialize this at
declaration time now?
##########
File path: pylib/cqlshlib/test/test_cqlsh_output.py
##########
@@ -587,10 +584,7 @@ def test_prompt(self):
c.send('use NONEXISTENTKEYSPACE;\n')
outputlines = c.read_to_next_prompt().splitlines()
- start_index = 0
- if c.realtty:
- self.assertEqual(outputlines[start_index], 'use
NONEXISTENTKEYSPACE;')
Review comment:
Why did we remove the assertion check for index 0?
##########
File path: pylib/cqlshlib/copyutil.py
##########
@@ -1299,9 +1299,9 @@ def __init__(self, inpipe, outpipe, worker_pipes, fname,
options):
self.inpipe = inpipe
self.outpipe = outpipe
self.worker_pipes = worker_pipes
- self.inmsg = None # must be created after forking on Windows
- self.outmsg = None # must be created after forking on Windows
- self.worker_channels = None # must be created after forking on Windows
+ self.inmsg = None
Review comment:
Same here: without Windows' issues, should we instead initialize these
at declaration time now?
##########
File path: pylib/cqlshlib/copyutil.py
##########
@@ -1405,8 +1405,8 @@ def __init__(self, params, target):
super(ChildProcess, self).__init__(target=target)
self.inpipe = params['inpipe']
self.outpipe = params['outpipe']
- self.inmsg = None # must be initialized after fork on Windows
- self.outmsg = None # must be initialized after fork on Windows
+ self.inmsg = None
Review comment:
See above ☝️ . No reason not to init at decl?
##########
File path: test/conf/cdc.yaml
##########
@@ -1,4 +1 @@
cdc_enabled: true
-# Compression enabled since uncompressed + cdc isn't compatible w/Windows
-commitlog_compression:
Review comment:
What do we think is the most common config? I'd think compression on and
probably lz4 because it's legit. If so, we should test cdc w/the expected
common parameters. i.e. I'd leave this here on that grounds.
--
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]
