Deprecate rally task results command

This command produces task results in old format that missis a lot of
information. For backward compatibility, the new task results exporter
is introduced.

Change-Id: I28880642e370513c2430e8e0f67dd7a622023a92

CHANGELOG.rst

@@ -42,6 +42,9 @@ Changed
 Deprecated
 ~~~~~~~~~~
 
+* Command *rally task results* is deprecated. Use *rally task report --json*
+  instead.
+
 * Module *rally.common.sshutils* is deprecated. Use *rally.utils.sshutils*
   instead.
 

rally/cli/commands/task.py

@@ -16,7 +16,6 @@
 """Rally command: task"""
 
 from __future__ import print_function
-import collections
 import datetime as dt
 import itertools
 import json
@@ -544,70 +543,16 @@ def make_flat(r, depth=0):
     @envutils.with_default_task_id
     @cliutils.suppress_warnings
     def results(self, api, task_id=None):
-        """Display raw task results.
-
-        This will produce a lot of output data about every iteration.
-        """
-
-        task = api.task.get(task_id=task_id, detailed=True)
-        finished_statuses = (consts.TaskStatus.FINISHED,
-                             consts.TaskStatus.ABORTED)
-        if task["status"] not in finished_statuses:
-            print("Task status is %s. Results available when it is one of %s."
-                  % (task["status"], ", ".join(finished_statuses)))
+        """DEPRECATED since Rally 3.0.0."""
+        LOG.warning("CLI method `rally task results` is deprecated since "
+                    "Rally 3.0.0 and will be removed soon. "
+                    "Use `rally task report --json` instead.")
+        try:
+            self.export(api, tasks=[task_id], output_type="old-json-results")
+        except exceptions.RallyException as e:
+            print(e.format_message())
             return 1
 
-        # TODO(chenhb): Ensure `rally task results` puts out old format.
-        for workload in itertools.chain(
-                *[s["workloads"] for s in task["subtasks"]]):
-            for itr in workload["data"]:
-                itr["atomic_actions"] = collections.OrderedDict(
-                    tutils.WrapperForAtomicActions(
-                        itr["atomic_actions"]).items()
-                )
-
-        results = []
-        for w in itertools.chain(*[s["workloads"] for s in task["subtasks"]]):
-            w["runner"]["type"] = w["runner_type"]
-
-            def port_hook_cfg(h):
-                h["config"] = {
-                    "name": h["config"]["action"][0],
-                    "args": h["config"]["action"][1],
-                    "description": h["config"].get("description", ""),
-                    "trigger": {"name": h["config"]["trigger"][0],
-                                "args": h["config"]["trigger"][1]}
-                }
-                return h
-
-            hooks = [port_hook_cfg(h) for h in w["hooks"]]
-
-            created_at = dt.datetime.strptime(w["created_at"],
-                                              "%Y-%m-%dT%H:%M:%S")
-            created_at = created_at.strftime("%Y-%d-%mT%H:%M:%S")
-
-            results.append({
-                "key": {
-                    "name": w["name"],
-                    "description": w["description"],
-                    "pos": w["position"],
-                    "kw": {
-                        "args": w["args"],
-                        "runner": w["runner"],
-                        "context": w["contexts"],
-                        "sla": w["sla"],
-                        "hooks": [h["config"] for h in w["hooks"]],
-                    }
-                },
-                "result": w["data"],
-                "sla": w["sla_results"].get("sla", []),
-                "hooks": hooks,
-                "load_duration": w["load_duration"],
-                "full_duration": w["full_duration"],
-                "created_at": created_at})
-
-        print(json.dumps(results, sort_keys=False, indent=4))
-
     @cliutils.args("--deployment", dest="deployment", type=str,
                    metavar="<uuid>", required=False,
                    help="UUID or name of a deployment.")

rally/plugins/task/exporters/old_json_results.py

@@ -0,0 +1,121 @@
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import collections
+import datetime as dt
+import itertools
+import json
+
+from rally import consts
+from rally import exceptions
+from rally.task import exporter
+
+
+def _to_old_atomic_actions_format(atomic_actions):
+    """Convert atomic actions to old format. """
+    old_style = collections.OrderedDict()
+    for action in atomic_actions:
+        duration = action["finished_at"] - action["started_at"]
+        if action["name"] in old_style:
+            name_template = action["name"] + " (%i)"
+            i = 2
+            while name_template % i in old_style:
+                i += 1
+            old_style[name_template % i] = duration
+        else:
+            old_style[action["name"]] = duration
+    return old_style
+
+
+@exporter.configure("old-json-results")
+class OldJSONExporter(exporter.TaskExporter):
+    """Generates task report in JSON format as old `rally task results`."""
+    def __init__(self, *args, **kwargs):
+        super(OldJSONExporter, self).__init__(*args, **kwargs)
+        if len(self.tasks_results) != 1:
+            raise exceptions.RallyException(
+                f"'{self.get_fullname()}' task exporter can be used only for "
+                f"a one task.")
+        self.task = self.tasks_results[0]
+
+    def _get_report(self):
+        results = []
+        for w in itertools.chain(*[s["workloads"]
+                                   for s in self.task["subtasks"]]):
+            for itr in w["data"]:
+                itr["atomic_actions"] = _to_old_atomic_actions_format(
+                    itr["atomic_actions"]
+                )
+
+            w["runner"]["type"] = w["runner_type"]
+
+            def port_hook_cfg(h):
+                h["config"] = {
+                    "name": h["config"]["action"][0],
+                    "args": h["config"]["action"][1],
+                    "description": h["config"].get("description", ""),
+                    "trigger": {"name": h["config"]["trigger"][0],
+                                "args": h["config"]["trigger"][1]}
+                }
+                return h
+
+            hooks = [port_hook_cfg(h) for h in w["hooks"]]
+
+            created_at = dt.datetime.strptime(w["created_at"],
+                                              "%Y-%m-%dT%H:%M:%S")
+            created_at = created_at.strftime("%Y-%d-%mT%H:%M:%S")
+
+            results.append({
+                "key": {
+                    "name": w["name"],
+                    "description": w["description"],
+                    "pos": w["position"],
+                    "kw": {
+                        "args": w["args"],
+                        "runner": w["runner"],
+                        "context": w["contexts"],
+                        "sla": w["sla"],
+                        "hooks": [h["config"] for h in w["hooks"]],
+                    }
+                },
+                "result": w["data"],
+                "sla": w["sla_results"].get("sla", []),
+                "hooks": hooks,
+                "load_duration": w["load_duration"],
+                "full_duration": w["full_duration"],
+                "created_at": created_at})
+
+        return results
+
+    def generate(self):
+        if len(self.tasks_results) != 1:
+            raise exceptions.RallyException(
+                f"'{self.get_fullname()}' task exporter can be used only for "
+                f"a one task.")
+
+        finished_statuses = (consts.TaskStatus.FINISHED,
+                             consts.TaskStatus.ABORTED)
+        if self.task["status"] not in finished_statuses:
+            raise exceptions.RallyException(
+                f"Task status is {self.task['status']}. Results available "
+                f"when it is one of {', '.join(finished_statuses)}."
+            )
+
+        results = json.dumps(self._get_report(), sort_keys=False, indent=4)
+
+        if self.output_destination:
+            return {"files": {self.output_destination: results},
+                    "open": "file://" + self.output_destination}
+        else:
+            return {"print": results}

tests/ci/cover.sh

@@ -15,7 +15,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-ALLOWED_EXTRA_MISSING=400
+ALLOWED_EXTRA_MISSING=4
 
 show_diff () {
     head -1 $1

tests/functional/test_cli_task.py

@@ -365,7 +365,8 @@ def test_new_report_one_file_with_static_libs(self):
         rally("task start --task %s" % config.filename)
         task_result_file = rally.gen_report_path(suffix="results")
         self.addCleanup(os.remove, task_result_file)
-        rally("task results", report_path=task_result_file, raw=True)
+        rally("task results", report_path=task_result_file, raw=True,
+              getjson=True)
 
         html_report = rally.gen_report_path(extension="html")
         rally("task report --html-static %s --out %s"

tests/unit/cli/commands/test_task.py

@@ -573,18 +573,32 @@ def _make_task(self, status=None, data=None):
                 "contexts": {"users": {}},
                 "data": data or []}]}]}
 
-    @mock.patch("rally.cli.commands.task.json.dumps")
+    @mock.patch("rally.plugins.task.exporters.old_json_results.json.dumps")
     def test_results(self, mock_json_dumps):
+
+        mock_json_dumps.return_value = ""
+
+        def fake_export(tasks, output_type, output_dest=None):
+            tasks = [self.fake_api.task.get(u, detailed=True) for u in tasks]
+            return self.real_api.task.export(tasks, output_type, output_dest)
+
+        self.fake_api.task.export.side_effect = fake_export
+
         task_id = "foo_task_id"
 
-        task_obj = self._make_task(data=[{"atomic_actions": {"foo": 1.1}}])
+        task_obj = self._make_task(
+            data=[{"atomic_actions": [{"name": "foo",
+                                       "started_at": 0,
+                                       "finished_at": 1.1}]}]
+        )
         task_obj["subtasks"][0]["workloads"][0]["hooks"] = [{
             "config": {
                 "action": ("foo", "arg"),
                 "trigger": ("bar", "arg2")
             },
             "summary": {"success": 1}}
         ]
+        task_obj["uuid"] = task_id
 
         def fix_r(workload):
             cfg = workload["runner"]
@@ -633,9 +647,16 @@ def fix_r(workload):
 
     @mock.patch("rally.cli.commands.task.sys.stdout")
     def test_results_no_data(self, mock_stdout):
+        def fake_export(tasks, output_type, output_dest=None):
+            tasks = [self.fake_api.task.get(u, detailed=True) for u in tasks]
+            return self.real_api.task.export(tasks, output_type, output_dest)
+
+        self.fake_api.task.export.side_effect = fake_export
+
         task_id = "foo"
-        self.fake_api.task.get.return_value = self._make_task(
-            status=consts.TaskStatus.CRASHED)
+        task_obj = self._make_task(status=consts.TaskStatus.CRASHED)
+        task_obj["uuid"] = task_id
+        self.fake_api.task.get.return_value = task_obj
 
         self.assertEqual(1, self.task.results(self.fake_api, task_id))