On Fri, Feb 21, 2014 at 3:58 PM, Petr Pudlák <[email protected]> wrote:
> > > > On Tue, Feb 18, 2014 at 3:39 PM, Hrvoje Ribicic <[email protected]> wrote: > >> This patch changes gnt-job info to use standard functions defined in >> cli.py, and output valid YAML. >> >> Signed-off-by: Hrvoje Ribicic <[email protected]> >> --- >> lib/client/gnt_job.py | 119 >> ++++++++++++++++++++++---------------------------- >> 1 file changed, 52 insertions(+), 67 deletions(-) >> >> diff --git a/lib/client/gnt_job.py b/lib/client/gnt_job.py >> index 762ad51..d4cf2a7 100644 >> --- a/lib/client/gnt_job.py >> +++ b/lib/client/gnt_job.py >> @@ -286,17 +286,6 @@ def ShowJobs(opts, args): >> @return: the desired exit code >> >> """ >> - def format_msg(level, text): >> - """Display the text indented.""" >> - ToStdout("%s%s", " " * level, text) >> - >> - def result_helper(value): >> - """Format a result field in a nice way.""" >> - if isinstance(value, (tuple, list)): >> - return "[%s]" % utils.CommaJoin(value) >> - else: >> - return str(value) >> - >> selected_fields = [ >> "id", "status", "ops", "opresult", "opstatus", "oplog", >> "opstart", "opexec", "opend", "received_ts", "start_ts", "end_ts", >> @@ -306,35 +295,32 @@ def ShowJobs(opts, args): >> cl = GetClient() >> result = cl.Query(constants.QR_JOB, selected_fields, qfilter).data >> >> - first = True >> + job_info_container = [] >> >> for entry in result: >> - if not first: >> - format_msg(0, "") >> - else: >> - first = False >> - >> ((_, job_id), (rs_status, status), (_, ops), (_, opresult), (_, >> opstatus), >> (_, oplog), (_, opstart), (_, opexec), (_, opend), (_, recv_ts), >> (_, start_ts), (_, end_ts)) = entry >> >> # Detect non-normal results >> if rs_status != constants.RS_NORMAL: >> - format_msg(0, "Job ID %s not found" % job_id) >> + job_info_container.append("Job ID %s not found" % job_id) >> continue >> >> - format_msg(0, "Job ID: %s" % job_id) >> + # Container for produced data >> + job_info = [("Job ID", job_id)] >> + >> if status in _USER_JOB_STATUS: >> status = _USER_JOB_STATUS[status] >> else: >> raise errors.ProgrammerError("Unknown job status code '%s'" % >> status) >> >> - format_msg(1, "Status: %s" % status) >> + job_info.append(("Status", status)) >> >> if recv_ts is not None: >> - format_msg(1, "Received: %s" % FormatTimestamp(recv_ts)) >> + job_info.append(("Received", FormatTimestamp(recv_ts))) >> else: >> - format_msg(1, "Missing received timestamp (%s)" % str(recv_ts)) >> + job_info.append(("Received", "unknown (%s)" % str(recv_ts))) >> >> if start_ts is not None: >> if recv_ts is not None: >> @@ -342,10 +328,10 @@ def ShowJobs(opts, args): >> delta = " (delta %.6fs)" % d1 >> else: >> delta = "" >> - format_msg(1, "Processing start: %s%s" % >> - (FormatTimestamp(start_ts), delta)) >> + job_info.append(("Processing start", "%s%s" % >> + (FormatTimestamp(start_ts), delta))) >> else: >> - format_msg(1, "Processing start: unknown (%s)" % str(start_ts)) >> + job_info.append(("Processing start", "unknown (%s)" % >> str(start_ts))) >> >> if end_ts is not None: >> if start_ts is not None: >> @@ -353,64 +339,63 @@ def ShowJobs(opts, args): >> delta = " (delta %.6fs)" % d2 >> else: >> delta = "" >> - format_msg(1, "Processing end: %s%s" % >> - (FormatTimestamp(end_ts), delta)) >> + job_info.append(("Processing end", "%s%s" % >> + (FormatTimestamp(end_ts), delta))) >> else: >> - format_msg(1, "Processing end: unknown (%s)" % str(end_ts)) >> + job_info.append(("Processing end", "unknown (%s)" % str(end_ts))) >> >> if end_ts is not None and recv_ts is not None: >> d3 = end_ts[0] - recv_ts[0] + (end_ts[1] - recv_ts[1]) / 1000000.0 >> - format_msg(1, "Total processing time: %.6f seconds" % d3) >> + job_info.append(("Total processing time", "%.6f seconds" % d3)) >> else: >> - format_msg(1, "Total processing time: N/A") >> - format_msg(1, "Opcodes:") >> + job_info.append(("Total processing time", "N/A")) >> + >> + opcode_container = [] >> for (opcode, result, status, log, s_ts, x_ts, e_ts) in \ >> zip(ops, opresult, opstatus, oplog, opstart, opexec, opend): >> - format_msg(2, "%s" % opcode["OP_ID"]) >> - format_msg(3, "Status: %s" % status) >> + opcode_info = [] >> + opcode_info.append(("Opcode", opcode["OP_ID"])) >> + opcode_info.append(("Status", status)) >> + >> if isinstance(s_ts, (tuple, list)): >> - format_msg(3, "Processing start: %s" % FormatTimestamp(s_ts)) >> + opcode_info.append(("Processing start", FormatTimestamp(s_ts))) >> else: >> - format_msg(3, "No processing start time") >> + opcode_info.append(("Processing start", "N/A")) >> > > This looks like a recurring piece of code in the following few lines, so > perhaps we could further improve it by adding an inner function that'd just > take the name of the key and a something_ts value. > > This is addressed in the following patches, as I wanted to both limit the number of changes in one patch, and add alignment support. > + >> if isinstance(x_ts, (tuple, list)): >> - format_msg(3, "Execution start: %s" % FormatTimestamp(x_ts)) >> + opcode_info.append(("Execution start", FormatTimestamp(x_ts))) >> else: >> - format_msg(3, "No execution start time") >> + opcode_info.append(("Execution start", "N/A")) >> + >> if isinstance(e_ts, (tuple, list)): >> - format_msg(3, "Processing end: %s" % FormatTimestamp(e_ts)) >> + opcode_info.append(("Processing end", FormatTimestamp(e_ts))) >> else: >> - format_msg(3, "No processing end time") >> - format_msg(3, "Input fields:") >> - for key in utils.NiceSort(opcode.keys()): >> - if key == "OP_ID": >> - continue >> - val = opcode[key] >> - if isinstance(val, (tuple, list)): >> - val = ",".join([str(item) for item in val]) >> - format_msg(4, "%s: %s" % (key, val)) >> - if result is None: >> - format_msg(3, "No output data") >> - elif isinstance(result, (tuple, list)): >> - if not result: >> - format_msg(3, "Result: empty sequence") >> - else: >> - format_msg(3, "Result:") >> - for elem in result: >> - format_msg(4, result_helper(elem)) >> - elif isinstance(result, dict): >> - if not result: >> - format_msg(3, "Result: empty dictionary") >> - else: >> - format_msg(3, "Result:") >> - for key, val in result.iteritems(): >> - format_msg(4, "%s: %s" % (key, result_helper(val))) >> - else: >> - format_msg(3, "Result: %s" % result) >> - format_msg(3, "Execution log:") >> + opcode_info.append(("Processing end", "N/A")) >> + >> + opcode_info.append(("Input fields", opcode)) >> + opcode_info.append(("Result", result)) >> + >> + exec_log_container = [] >> for serial, log_ts, log_type, log_msg in log: >> time_txt = FormatTimestamp(log_ts) >> encoded = FormatLogMessage(log_type, log_msg) >> - format_msg(4, "%s:%s:%s %s" % (serial, time_txt, log_type, >> encoded)) >> + >> + # Arranged in this curious way to preserve the brevity for >> multiple >> + # logs. This content cannot be exposed as a 4-tuple, as time >> contains >> + # the colon, causing some YAML parsers to fail. >> + exec_log_info = [("Time", time_txt), >> + ("Content", (serial, log_type, encoded,)), >> + ] >> + exec_log_container.append(exec_log_info) >> + opcode_info.append(("Execution log", exec_log_container)) >> + >> + opcode_container.append(opcode_info) >> + >> + job_info.append(("Opcodes", opcode_container)) >> + job_info_container.append(job_info) >> + >> + PrintGenericInfo(job_info_container) >> + >> return 0 >> >> >> -- >> 1.9.0.rc1.175.g0b1dcb5 >> >> > Rest LGTM. > > Note: This is a great improvement. I just found one small "regression", > and that is that result keys were NiceSorted before, but now they're sorted > just lexicographically. I don't know if it's worth doing something about > that, perhaps to sort all dictionary keys in _SerializeGenericInfo using > NiceSort, or just keep it as it is (both cases LGTM). > >
