You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2020/09/17 03:08:43 UTC
[GitHub] [cassandra-dtest] dcapwell opened a new pull request #94: Bug/cassandra 16127
dcapwell opened a new pull request #94:
URL: https://github.com/apache/cassandra-dtest/pull/94
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra-dtest] ekaterinadimitrova2 commented on a change in pull request #94: Bug/cassandra 16127
Posted by GitBox <gi...@apache.org>.
ekaterinadimitrova2 commented on a change in pull request #94:
URL: https://github.com/apache/cassandra-dtest/pull/94#discussion_r497120213
##########
File path: client_network_stop_start_test.py
##########
@@ -0,0 +1,126 @@
+import os
+import os.path
+import shutil
+import time
+import pytest
+import logging
+import string
+
+from dtest import Tester
+from distutils.version import LooseVersion
+from tools import sslkeygen
+from ccmlib.node import TimeoutError
+
+since = pytest.mark.since
+logger = logging.getLogger(__name__)
+
+# see https://issues.apache.org/jira/browse/CASSANDRA-16127
+class TestClientNetworkStopStart(Tester):
+
+ def _normalize(self, a):
+ return a.translate(str.maketrans(dict.fromkeys(string.whitespace)))
+
+ def _in(self, a, b):
+ return self._normalize(a) in self._normalize(b)
+
+ def _assert_client_active_msg(self, name, enabled, out):
+ assert self._in("{} active: {}".format(name, str(enabled).lower()), out), "{} is expected to be {} ({}) but was not found in output: {}".format(name, "actived" if enabled else "deactivated", str(enabled).lower(), out)
Review comment:
My bad, I missed the second bracket, looked like third argument in the format() method. I am sorry, my bad :-)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra-dtest] ekaterinadimitrova2 commented on a change in pull request #94: Bug/cassandra 16127
Posted by GitBox <gi...@apache.org>.
ekaterinadimitrova2 commented on a change in pull request #94:
URL: https://github.com/apache/cassandra-dtest/pull/94#discussion_r495593575
##########
File path: client_network_stop_start_test.py
##########
@@ -0,0 +1,126 @@
+import os
+import os.path
+import shutil
+import time
+import pytest
+import logging
+import string
+
+from dtest import Tester
+from distutils.version import LooseVersion
+from tools import sslkeygen
+from ccmlib.node import TimeoutError
+
+since = pytest.mark.since
+logger = logging.getLogger(__name__)
+
+# see https://issues.apache.org/jira/browse/CASSANDRA-16127
+class TestClientNetworkStopStart(Tester):
+
+ def _normalize(self, a):
+ return a.translate(str.maketrans(dict.fromkeys(string.whitespace)))
+
+ def _in(self, a, b):
+ return self._normalize(a) in self._normalize(b)
+
+ def _assert_client_active_msg(self, name, enabled, out):
+ assert self._in("{} active: {}".format(name, str(enabled).lower()), out), "{} is expected to be {} ({}) but was not found in output: {}".format(name, "actived" if enabled else "deactivated", str(enabled).lower(), out)
+
+ def _watch_log_for_loop(self, node, to_watch):
+ """Rely on looping and timeout in order to detect if an error is found. If the node crashes before the match is found, this will exit with an error rather than loop until the timeout."""
+ while True:
+ try:
+ return node.watch_log_for(to_watch, timeout=5)
+ except TimeoutError:
+ logger.debug("waited 5s watching for '{}' but was not found; checking for errors".format(to_watch))
+ # since the api doesn't return the mark read it isn't thread safe to use mark
+ # as the length of the file can change between calls which may mean we skip over
+ # errors; to avoid this keep reading the whole file over and over again...
+ errors = node.grep_log_for_errors_from()
+ if errors:
+ msg = "Errors were found in the logs while watching for '{}'; attempting to fail the test".format(to_watch)
+ logger.debug(msg)
+ raise AssertionError("{}:\n".format(msg) + '\n\n'.join(['\n'.join(msg) for msg in errors]))
Review comment:
Sounds like a candidate for one more ccm node.py function so others can have it in mind and use it too? WDYT?:-)
##########
File path: client_network_stop_start_test.py
##########
@@ -0,0 +1,126 @@
+import os
+import os.path
+import shutil
+import time
+import pytest
+import logging
+import string
+
+from dtest import Tester
+from distutils.version import LooseVersion
+from tools import sslkeygen
+from ccmlib.node import TimeoutError
+
+since = pytest.mark.since
+logger = logging.getLogger(__name__)
+
+# see https://issues.apache.org/jira/browse/CASSANDRA-16127
+class TestClientNetworkStopStart(Tester):
+
+ def _normalize(self, a):
+ return a.translate(str.maketrans(dict.fromkeys(string.whitespace)))
+
+ def _in(self, a, b):
+ return self._normalize(a) in self._normalize(b)
+
+ def _assert_client_active_msg(self, name, enabled, out):
+ assert self._in("{} active: {}".format(name, str(enabled).lower()), out), "{} is expected to be {} ({}) but was not found in output: {}".format(name, "actived" if enabled else "deactivated", str(enabled).lower(), out)
Review comment:
nit: "actived" should be "activated" or "active" I guess :-)
##########
File path: conftest.py
##########
@@ -374,10 +375,26 @@ def loose_version_compare(a, b):
def _skip_msg(current_running_version, since_version, max_version):
- if loose_version_compare(current_running_version, since_version) < 0:
+ if isinstance(since_version, collections.Sequence):
+ previous = None
+ since_version.sort()
+ for i in range(1, len(since_version) + 1):
Review comment:
nit: I would probably add an empty line before and after the for loop
##########
File path: client_network_stop_start_test.py
##########
@@ -0,0 +1,126 @@
+import os
+import os.path
+import shutil
+import time
+import pytest
+import logging
+import string
+
+from dtest import Tester
+from distutils.version import LooseVersion
+from tools import sslkeygen
+from ccmlib.node import TimeoutError
+
+since = pytest.mark.since
+logger = logging.getLogger(__name__)
+
+# see https://issues.apache.org/jira/browse/CASSANDRA-16127
+class TestClientNetworkStopStart(Tester):
+
+ def _normalize(self, a):
+ return a.translate(str.maketrans(dict.fromkeys(string.whitespace)))
+
+ def _in(self, a, b):
+ return self._normalize(a) in self._normalize(b)
+
+ def _assert_client_active_msg(self, name, enabled, out):
+ assert self._in("{} active: {}".format(name, str(enabled).lower()), out), "{} is expected to be {} ({}) but was not found in output: {}".format(name, "actived" if enabled else "deactivated", str(enabled).lower(), out)
Review comment:
nit: assert self._in("{} active: {}".format(name, str(enabled).lower()), **out**), "{} is expected to be {} ({}) but was not found in output: {}".format(name, "actived" if enabled else "deactivated", str(enabled).lower(), out)
I think you don't need the "out" in bold. The way Python format() works is that it ignores every argument after it is done parsing for the number of braces you have so it will ignore it without complaint but I think it's just better to remove it as not needed :-)
##########
File path: client_network_stop_start_test.py
##########
@@ -0,0 +1,126 @@
+import os
+import os.path
+import shutil
+import time
+import pytest
+import logging
+import string
Review comment:
nit: I would probably order them alphabetically
##########
File path: conftest.py
##########
@@ -374,10 +375,26 @@ def loose_version_compare(a, b):
def _skip_msg(current_running_version, since_version, max_version):
- if loose_version_compare(current_running_version, since_version) < 0:
+ if isinstance(since_version, collections.Sequence):
+ previous = None
+ since_version.sort()
+ for i in range(1, len(since_version) + 1):
+ sv = since_version[-i]
+ if loose_version_compare(current_running_version, sv) >= 0:
+ if not previous:
+ if max_version and loose_version_compare(current_running_version, max_version) > 0:
+ return "%s > %s" % (current_running_version, max_version)
+ return None
Review comment:
nit: I would probably leave an empty line here
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra-dtest] dcapwell commented on a change in pull request #94: Bug/cassandra 16127
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #94:
URL: https://github.com/apache/cassandra-dtest/pull/94#discussion_r500575497
##########
File path: client_network_stop_start_test.py
##########
@@ -0,0 +1,126 @@
+import os
+import os.path
+import shutil
+import time
+import pytest
+import logging
+import string
+
+from dtest import Tester
+from distutils.version import LooseVersion
+from tools import sslkeygen
+from ccmlib.node import TimeoutError
+
+since = pytest.mark.since
+logger = logging.getLogger(__name__)
+
+# see https://issues.apache.org/jira/browse/CASSANDRA-16127
+class TestClientNetworkStopStart(Tester):
+
+ def _normalize(self, a):
+ return a.translate(str.maketrans(dict.fromkeys(string.whitespace)))
+
+ def _in(self, a, b):
+ return self._normalize(a) in self._normalize(b)
+
+ def _assert_client_active_msg(self, name, enabled, out):
+ assert self._in("{} active: {}".format(name, str(enabled).lower()), out), "{} is expected to be {} ({}) but was not found in output: {}".format(name, "actived" if enabled else "deactivated", str(enabled).lower(), out)
Review comment:
code must be readable, so it more highlights that its not =)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra-dtest] michaelsembwever closed pull request #94: Bug/cassandra 16127
Posted by GitBox <gi...@apache.org>.
michaelsembwever closed pull request #94:
URL: https://github.com/apache/cassandra-dtest/pull/94
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra-dtest] dcapwell commented on a change in pull request #94: Bug/cassandra 16127
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #94:
URL: https://github.com/apache/cassandra-dtest/pull/94#discussion_r497106324
##########
File path: client_network_stop_start_test.py
##########
@@ -0,0 +1,126 @@
+import os
+import os.path
+import shutil
+import time
+import pytest
+import logging
+import string
+
+from dtest import Tester
+from distutils.version import LooseVersion
+from tools import sslkeygen
+from ccmlib.node import TimeoutError
+
+since = pytest.mark.since
+logger = logging.getLogger(__name__)
+
+# see https://issues.apache.org/jira/browse/CASSANDRA-16127
+class TestClientNetworkStopStart(Tester):
+
+ def _normalize(self, a):
+ return a.translate(str.maketrans(dict.fromkeys(string.whitespace)))
+
+ def _in(self, a, b):
+ return self._normalize(a) in self._normalize(b)
+
+ def _assert_client_active_msg(self, name, enabled, out):
+ assert self._in("{} active: {}".format(name, str(enabled).lower()), out), "{} is expected to be {} ({}) but was not found in output: {}".format(name, "actived" if enabled else "deactivated", str(enabled).lower(), out)
Review comment:
> I think you don't need the "out" in bold.
I can rewrite this to be readable, but I do need it!
`self._in("{} active: {}".format(name, str(enabled).lower()), out)` can be `self._in(search_msg, out)`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org