From 272acb9f1f27313c658c3aa25e82e860bb2496da Mon Sep 17 00:00:00 2001 From: Ivan Daschinskiy Date: Mon, 7 Sep 2020 14:24:01 +0300 Subject: [PATCH 1/2] Fix: set DucktapeJSONEncoder as default encoder in reports. --- ducktape/tests/reporter.py | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/ducktape/tests/reporter.py b/ducktape/tests/reporter.py index eba615e1d..dd20073cb 100644 --- a/ducktape/tests/reporter.py +++ b/ducktape/tests/reporter.py @@ -43,6 +43,15 @@ def format_time(t): return r +def json_dumps(obj, **kwargs): + """ + Dump object to json string with Ducktape json encoder. + """ + assert 'cls' not in kwargs, "Passing custom json encoder is forbidden." + + return json.dumps(obj, cls=DucktapeJSONEncoder, **kwargs) + + class SingleResultReporter(object): """Helper class for creating a view of results from a single test.""" @@ -64,7 +73,7 @@ def result_string(self): result_lines.append(" " + self.result.summary) if self.result.data is not None: - result_lines.append(json.dumps(self.result.data)) + result_lines.append(json_dumps(self.result.data)) return "\n".join(result_lines) @@ -85,7 +94,7 @@ def report(self): if self.result.data is not None: data_file = os.path.join(self.result.results_dir, "data.json") with open(data_file, "w") as fp: - fp.write(json.dumps(self.result.data)) + fp.write(json_dumps(self.result.data)) class SummaryReporter(object): @@ -146,7 +155,7 @@ def __init__(self, results): def report(self): report_file = os.path.abspath(os.path.join(self.results.session_context.results_dir, "report.json")) with open(report_file, "w") as f: - f.write(json.dumps(self.results, cls=DucktapeJSONEncoder, sort_keys=True, indent=2, separators=(',', ': '))) + f.write(json_dumps(self.results, sort_keys=True, indent=2, separators=(',', ': '))) class HTMLSummaryReporter(SummaryReporter): @@ -159,7 +168,7 @@ def format_test_name(self, result): if result.injected_args is not None: lines.append("Arguments:") lines.append( - json.dumps(result.injected_args, sort_keys=True, indent=2, separators=(',', ': '))) + json_dumps(result.injected_args, sort_keys=True, indent=2, separators=(',', ': '))) return "\n".join(lines) @@ -171,7 +180,7 @@ def format_result(self, result): "test_result": test_result, "description": result.description, "run_time": format_time(result.run_time_seconds), - "data": "" if result.data is None else json.dumps(result.data, sort_keys=True, + "data": "" if result.data is None else json_dumps(result.data, sort_keys=True, indent=2, separators=(',', ': ')), "test_log": self.test_results_dir(result) } @@ -199,7 +208,7 @@ def format_report(self): ignored_result_string = "" for result in self.results: - json_string = json.dumps(self.format_result(result)) + json_string = json_dumps(self.format_result(result)) if result.test_status == PASS: num_passes += 1 passed_result_string += json_string From 67791a0c54422a4fef09247b1be17968a052d980 Mon Sep 17 00:00:00 2001 From: Ivan Daschinskiy Date: Mon, 14 Sep 2020 10:49:40 +0300 Subject: [PATCH 2/2] Fix: codereview issues --- ducktape/cluster/cluster_spec.py | 9 ++++---- ducktape/cluster/json.py | 4 ++-- ducktape/cluster/vagrant.py | 7 +++---- ducktape/command_line/main.py | 8 +++---- ducktape/json_serializable.py | 36 ++++++++++++++++++++++++++++++-- ducktape/tests/reporter.py | 12 +---------- ducktape/tests/result.py | 5 ++--- tests/cluster/check_vagrant.py | 6 +++--- tests/command_line/check_main.py | 8 +++---- 9 files changed, 57 insertions(+), 38 deletions(-) diff --git a/ducktape/cluster/cluster_spec.py b/ducktape/cluster/cluster_spec.py index 684cb3aee..b95ec0941 100644 --- a/ducktape/cluster/cluster_spec.py +++ b/ducktape/cluster/cluster_spec.py @@ -14,9 +14,8 @@ from __future__ import absolute_import -import json - from ducktape.cluster.node_container import NodeContainer +from ducktape.json_serializable import json_dumps, json_loads LINUX = "linux" @@ -40,7 +39,7 @@ def __str__(self): dict = { "os": self.operating_system, } - return json.dumps(dict, sort_keys=True) + return json_dumps(dict, sort_keys=True) class ClusterSpec(object): @@ -109,7 +108,7 @@ def __str__(self): node_spec_to_num[node_spec_str] = node_spec_to_num.get(node_spec_str, 0) + 1 rval = [] for node_spec_str in sorted(node_spec_to_num.keys()): - node_spec = json.loads(node_spec_str) + node_spec = json_loads(node_spec_str) node_spec["num_nodes"] = node_spec_to_num[node_spec_str] rval.append(node_spec) - return json.dumps(rval, sort_keys=True) + return json_dumps(rval, sort_keys=True) diff --git a/ducktape/cluster/json.py b/ducktape/cluster/json.py index ed2b3ef0a..16c80402b 100644 --- a/ducktape/cluster/json.py +++ b/ducktape/cluster/json.py @@ -17,12 +17,12 @@ from ducktape.cluster.cluster_spec import ClusterSpec, WINDOWS from ducktape.cluster.node_container import NodeContainer from ducktape.command_line.defaults import ConsoleDefaults +from ducktape.json_serializable import json_load from .cluster import Cluster, ClusterNode from ducktape.cluster.linux_remoteaccount import LinuxRemoteAccount from ducktape.cluster.windows_remoteaccount import WindowsRemoteAccount from .remoteaccount import RemoteAccountSSHConfig -import json import os import traceback @@ -82,7 +82,7 @@ def __init__(self, cluster_json=None, *args, **kwargs): cluster_file = kwargs.get("cluster_file") if cluster_file is None: cluster_file = ConsoleDefaults.CLUSTER_FILE - cluster_json = json.load(open(os.path.abspath(cluster_file))) + cluster_json = json_load(open(os.path.abspath(cluster_file))) try: for ninfo in cluster_json["nodes"]: ssh_config_dict = ninfo.get("ssh_config") diff --git a/ducktape/cluster/vagrant.py b/ducktape/cluster/vagrant.py index 462131a1a..5013504be 100644 --- a/ducktape/cluster/vagrant.py +++ b/ducktape/cluster/vagrant.py @@ -15,11 +15,10 @@ from __future__ import absolute_import from .json import JsonCluster -import json import os from .remoteaccount import RemoteAccountSSHConfig import subprocess -from ducktape.json_serializable import DucktapeJSONEncoder +from ducktape.json_serializable import json_dump, json_load class VagrantCluster(JsonCluster): @@ -40,7 +39,7 @@ def __init__(self, *args, **kwargs): cluster_file = kwargs.get("cluster_file") if cluster_file is not None: try: - cluster_json = json.load(open(os.path.abspath(cluster_file))) + cluster_json = json_load(open(os.path.abspath(cluster_file))) is_read_from_file = True except IOError: # It is OK if file is not found. Call vagrant ssh-info to read the cluster info. @@ -64,7 +63,7 @@ def __init__(self, *args, **kwargs): ] cluster_json["nodes"] = nodes with open(cluster_file, 'w+') as fd: - json.dump(cluster_json, fd, cls=DucktapeJSONEncoder, indent=2, separators=(',', ': '), sort_keys=True) + json_dump(cluster_json, fd, indent=2, separators=(',', ': '), sort_keys=True) # Release any ssh clients used in querying the nodes for metadata for node_account in self._available_accounts: diff --git a/ducktape/command_line/main.py b/ducktape/command_line/main.py index dd8385e45..b61cd0b96 100644 --- a/ducktape/command_line/main.py +++ b/ducktape/command_line/main.py @@ -15,7 +15,6 @@ from __future__ import print_function import importlib -import json import os import random from six import iteritems @@ -24,6 +23,7 @@ from ducktape.command_line.defaults import ConsoleDefaults from ducktape.command_line.parse_args import parse_args +from ducktape.json_serializable import json_loads from ducktape.tests.loader import TestLoader, LoaderException from ducktape.tests.loggermaker import close_logger from ducktape.tests.reporter import SimpleStdoutSummaryReporter, SimpleFileSummaryReporter, \ @@ -51,12 +51,12 @@ def get_user_defined_globals(globals_str): # The string appears to be a file, so try loading JSON from file # This may raise an IOError if the file can't be read or a ValueError if the contents of the file # cannot be parsed. - user_globals = json.loads(open(globals_str, "r").read()) + user_globals = json_loads(open(globals_str, "r").read()) from_file = True else: try: # try parsing directly as json if it doesn't seem to be a file - user_globals = json.loads(globals_str) + user_globals = json_loads(globals_str) except ValueError as ve: message = str(ve) message += "\nglobals parameter %s is neither valid JSON nor a valid path to a JSON file." % globals_str @@ -106,7 +106,7 @@ def main(): injected_args = None if args_dict["parameters"]: try: - injected_args = json.loads(args_dict["parameters"]) + injected_args = json_loads(args_dict["parameters"]) except ValueError as e: print("parameters are not valid json: " + str(e)) sys.exit(1) diff --git a/ducktape/json_serializable.py b/ducktape/json_serializable.py index 15d70f74d..26146da5f 100644 --- a/ducktape/json_serializable.py +++ b/ducktape/json_serializable.py @@ -12,8 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. - -from json import JSONEncoder +from json import JSONEncoder, dumps, dump, loads, load class DucktapeJSONEncoder(JSONEncoder): @@ -25,3 +24,36 @@ def default(self, obj): else: # Let the base class default method raise the TypeError return JSONEncoder.default(self, obj) + + +def json_dumps(obj, **kwargs): + """ + Dump object to json string with Ducktape json encoder. + """ + assert 'cls' not in kwargs, "Passing custom json encoder is forbidden." + + return dumps(obj, cls=DucktapeJSONEncoder, **kwargs) + + +def json_dump(obj, fp, **kwargs): + """ + Dump object to json as a JSON formatted stream to ``fp`` + (a ``.write()``-supporting file-like object) with Ducktape json encoder. + """ + assert 'cls' not in kwargs, "Passing custom json encoder is forbidden." + + return dump(obj, fp, cls=DucktapeJSONEncoder, **kwargs) + + +def json_loads(s, **kwargs): + """ + Loads object from json string. + """ + return loads(s, **kwargs) + + +def json_load(fp, **kwargs): + """ + Load object from ``fp`` (a ``.read()``-supporting file-like object containing a JSON document) + """ + return load(fp, **kwargs) diff --git a/ducktape/tests/reporter.py b/ducktape/tests/reporter.py index dd20073cb..573a504db 100644 --- a/ducktape/tests/reporter.py +++ b/ducktape/tests/reporter.py @@ -14,7 +14,6 @@ from __future__ import print_function -import json import os import shutil @@ -23,7 +22,7 @@ from ducktape.utils.terminal_size import get_terminal_size from ducktape.utils.util import ducktape_version from ducktape.tests.status import PASS, FAIL, IGNORE -from ducktape.json_serializable import DucktapeJSONEncoder +from ducktape.json_serializable import json_dumps DEFAULT_SEPARATOR_WIDTH = 100 @@ -43,15 +42,6 @@ def format_time(t): return r -def json_dumps(obj, **kwargs): - """ - Dump object to json string with Ducktape json encoder. - """ - assert 'cls' not in kwargs, "Passing custom json encoder is forbidden." - - return json.dumps(obj, cls=DucktapeJSONEncoder, **kwargs) - - class SingleResultReporter(object): """Helper class for creating a view of results from a single test.""" diff --git a/ducktape/tests/result.py b/ducktape/tests/result.py index d4035fdec..b550285f0 100644 --- a/ducktape/tests/result.py +++ b/ducktape/tests/result.py @@ -12,12 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. -import json import os import time from ducktape.tests.test import TestContext -from ducktape.json_serializable import DucktapeJSONEncoder +from ducktape.json_serializable import json_dump from ducktape.tests.reporter import SingleResultFileReporter from ducktape.utils.local_filesystem_utils import mkdir_p from ducktape.utils.util import ducktape_version @@ -100,7 +99,7 @@ def report(self): def dump_json(self): """Dump this object as json to the given location. By default, dump into self.results_dir/report.json""" with open(os.path.join(self.results_dir, "report.json"), "w") as fd: - json.dump(self, fd, cls=DucktapeJSONEncoder, sort_keys=True, indent=2) + json_dump(self, fd, sort_keys=True, indent=2) def to_json(self): return { diff --git a/tests/cluster/check_vagrant.py b/tests/cluster/check_vagrant.py index f5d68ebd0..0420f5478 100644 --- a/tests/cluster/check_vagrant.py +++ b/tests/cluster/check_vagrant.py @@ -14,7 +14,7 @@ from ducktape.cluster.vagrant import VagrantCluster from ducktape.services.service import Service -import json +from ducktape.json_serializable import json_dump, json_load import pickle import os import random @@ -115,7 +115,7 @@ def check_cluster_file_write(self, monkeypatch): cluster_json_expected["nodes"] = nodes - cluster_json_actual = json.load(open(os.path.abspath(self.cluster_file))) + cluster_json_actual = json_load(open(os.path.abspath(self.cluster_file))) assert cluster_json_actual == cluster_json_expected def check_cluster_file_read(self, monkeypatch): @@ -155,7 +155,7 @@ def check_cluster_file_read(self, monkeypatch): cluster_json_expected = {} cluster_json_expected["nodes"] = nodes_expected - json.dump(cluster_json_expected, open(self.cluster_file, 'w+'), + json_dump(cluster_json_expected, open(self.cluster_file, 'w+'), indent=2, separators=(',', ': '), sort_keys=True) # Load the cluster from the json file we just created diff --git a/tests/command_line/check_main.py b/tests/command_line/check_main.py index c340c08cc..74a2f0abb 100644 --- a/tests/command_line/check_main.py +++ b/tests/command_line/check_main.py @@ -15,8 +15,8 @@ from ducktape.command_line.main import get_user_defined_globals from ducktape.command_line.main import setup_results_directory from ducktape.command_line.main import update_latest_symlink +from ducktape.json_serializable import json_loads -import json import os import os.path import pickle @@ -112,7 +112,7 @@ def check_pickleable(self): def check_parseable_json_string(self): """Check if globals_json is parseable as JSON, we get back a dictionary view of parsed JSON.""" globals_dict = get_user_defined_globals(globals_json) - assert globals_dict == json.loads(globals_json) + assert globals_dict == json_loads(globals_json) def check_unparseable(self): """If globals string is not a path to a file, and not parseable as JSON we want to raise a ValueError @@ -128,7 +128,7 @@ def check_parse_from_file(self): fh.write(globals_json) global_dict = get_user_defined_globals(fname) - assert global_dict == json.loads(globals_json) + assert global_dict == json_loads(globals_json) assert global_dict["x"] == 200 finally: os.remove(fname) @@ -151,7 +151,7 @@ def check_non_dict(self): """Valid JSON which does not parse as a dict should raise a ValueError""" # Should be able to parse this as JSON - json.loads(valid_json_not_dict) + json_loads(valid_json_not_dict) with pytest.raises(ValueError): get_user_defined_globals(valid_json_not_dict)