)]}'
{"rally/task/processing/charts.py":[{"author":{"_account_id":8491,"name":"Illia Khudoshyn","email":"ikhudoshyn@mirantis.com","username":"ikhudoshyn"},"change_message_id":"467905f4a84f6f2401e7bc10239ef03e105be380","unresolved":false,"context_lines":[{"line_number":271,"context_line":""},{"line_number":272,"context_line":"@six.add_metaclass(abc.ABCMeta)"},{"line_number":273,"context_line":"class Table(Chart):"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"    @abc.abstractproperty"},{"line_number":276,"context_line":"    def COLUMNS(self):"},{"line_number":277,"context_line":"        \"\"\"List of columns names.\"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"5a710552_de4edac0","line":274,"updated":"2015-12-15 15:04:46.000000000","message":"It definitely lacks documentation.","commit_id":"15867abbb5b2635588b7e55e6edb480b1c179a98"},{"author":{"_account_id":10475,"name":"Alexander Maretskiy","email":"amaretskiy@mirantis.com","username":"maretskiy"},"change_message_id":"a16d89c399aae6d4cd25438669fe5cf93723b629","unresolved":false,"context_lines":[{"line_number":271,"context_line":""},{"line_number":272,"context_line":"@six.add_metaclass(abc.ABCMeta)"},{"line_number":273,"context_line":"class Table(Chart):"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"    @abc.abstractproperty"},{"line_number":276,"context_line":"    def COLUMNS(self):"},{"line_number":277,"context_line":"        \"\"\"List of columns names.\"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"5a710552_01e5e335","line":274,"in_reply_to":"5a710552_de4edac0","updated":"2015-12-15 15:08:55.000000000","message":"Agreed","commit_id":"15867abbb5b2635588b7e55e6edb480b1c179a98"},{"author":{"_account_id":8491,"name":"Illia Khudoshyn","email":"ikhudoshyn@mirantis.com","username":"ikhudoshyn"},"change_message_id":"467905f4a84f6f2401e7bc10239ef03e105be380","unresolved":false,"context_lines":[{"line_number":329,"context_line":"        for name, value in self._map_iteration_values(iteration).items():"},{"line_number":330,"context_line":"            self._data[name][-1][0].add()"},{"line_number":331,"context_line":"            if iteration[\"error\"]:"},{"line_number":332,"context_line":"                self._data[name][-2][0].add(0)"},{"line_number":333,"context_line":"            else:"},{"line_number":334,"context_line":"                self._data[name][-2][0].add(1)"},{"line_number":335,"context_line":"                for idx, dummy in enumerate(self._data[name][:-2]):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a710552_bec18ee6","line":332,"updated":"2015-12-15 15:04:46.000000000","message":"The line above (and actually the whole method) looks like black magic. It feels like something is wrong with our data structures. Three (sic!) magic numbers in one line is too much.","commit_id":"15867abbb5b2635588b7e55e6edb480b1c179a98"},{"author":{"_account_id":10475,"name":"Alexander Maretskiy","email":"amaretskiy@mirantis.com","username":"maretskiy"},"change_message_id":"a16d89c399aae6d4cd25438669fe5cf93723b629","unresolved":false,"context_lines":[{"line_number":329,"context_line":"        for name, value in self._map_iteration_values(iteration).items():"},{"line_number":330,"context_line":"            self._data[name][-1][0].add()"},{"line_number":331,"context_line":"            if iteration[\"error\"]:"},{"line_number":332,"context_line":"                self._data[name][-2][0].add(0)"},{"line_number":333,"context_line":"            else:"},{"line_number":334,"context_line":"                self._data[name][-2][0].add(1)"},{"line_number":335,"context_line":"                for idx, dummy in enumerate(self._data[name][:-2]):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a710552_61f3870b","line":332,"in_reply_to":"5a710552_bec18ee6","updated":"2015-12-15 15:08:55.000000000","message":"This line is just moved line L315 https://github.com/openstack/rally/blob/0d8740ce00cc8b375fc07a648866840ff397488b/rally/task/processing/charts.py#L315\n\nSo I do not think this patch must rewrite this code, it\u0027s about another changes","commit_id":"15867abbb5b2635588b7e55e6edb480b1c179a98"},{"author":{"_account_id":8491,"name":"Illia Khudoshyn","email":"ikhudoshyn@mirantis.com","username":"ikhudoshyn"},"change_message_id":"28a79a547406e3350a4ddffca987e0d071c6107e","unresolved":false,"context_lines":[{"line_number":271,"context_line":""},{"line_number":272,"context_line":"@six.add_metaclass(abc.ABCMeta)"},{"line_number":273,"context_line":"class Table(Chart):"},{"line_number":274,"context_line":"    \"\"\"Base class for tables.\"\"\""},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    @abc.abstractproperty"},{"line_number":277,"context_line":"    def COLUMNS(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a710552_68ee03b6","line":274,"updated":"2015-12-15 16:37:45.000000000","message":"It is a docstring, indeed. But what I asked for is a description of its functionality. Since it is a base class it would be great to understand what child classes could inherit.","commit_id":"f5444267d7e01ce28b94e0a0611d4c8e6e3968f4"},{"author":{"_account_id":10475,"name":"Alexander Maretskiy","email":"amaretskiy@mirantis.com","username":"maretskiy"},"change_message_id":"19dbd6f467207b4af4508bb2aea125e5cea7b89c","unresolved":false,"context_lines":[{"line_number":271,"context_line":""},{"line_number":272,"context_line":"@six.add_metaclass(abc.ABCMeta)"},{"line_number":273,"context_line":"class Table(Chart):"},{"line_number":274,"context_line":"    \"\"\"Base class for tables.\"\"\""},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    @abc.abstractproperty"},{"line_number":277,"context_line":"    def COLUMNS(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a7e1126_2be515db","line":274,"in_reply_to":"5a710552_68ee03b6","updated":"2015-12-18 14:03:42.000000000","message":"Done","commit_id":"f5444267d7e01ce28b94e0a0611d4c8e6e3968f4"},{"author":{"_account_id":10475,"name":"Alexander Maretskiy","email":"amaretskiy@mirantis.com","username":"maretskiy"},"change_message_id":"5d5e138ba199a48c74cf366b7c237aefa720ec51","unresolved":false,"context_lines":[{"line_number":271,"context_line":""},{"line_number":272,"context_line":"@six.add_metaclass(abc.ABCMeta)"},{"line_number":273,"context_line":"class Table(Chart):"},{"line_number":274,"context_line":"    \"\"\"Base class for tables.\"\"\""},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    @abc.abstractproperty"},{"line_number":277,"context_line":"    def COLUMNS(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a710552_dc4f90bb","line":274,"in_reply_to":"5a710552_68ee03b6","updated":"2015-12-16 10:58:33.000000000","message":"Will add some words :)","commit_id":"f5444267d7e01ce28b94e0a0611d4c8e6e3968f4"},{"author":{"_account_id":8491,"name":"Illia Khudoshyn","email":"ikhudoshyn@mirantis.com","username":"ikhudoshyn"},"change_message_id":"28a79a547406e3350a4ddffca987e0d071c6107e","unresolved":false,"context_lines":[{"line_number":277,"context_line":"    def COLUMNS(self):"},{"line_number":278,"context_line":"        \"\"\"List of columns names.\"\"\""},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"    def round(self, ins, no_result):"},{"line_number":281,"context_line":"        return no_result and \"n/a\" or round(ins.result(), 3)"},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"    def get_rows(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a710552_2816bba7","line":280,"updated":"2015-12-15 16:37:45.000000000","message":"I expect this is a one-time-used function. We could easily avoid it and improve readability. If I am wrong and the function is to be used more often then it needs a docstring. Anyway I must admit that the implementation is really weird for what one could expect from \u0027round\u0027","commit_id":"f5444267d7e01ce28b94e0a0611d4c8e6e3968f4"},{"author":{"_account_id":10475,"name":"Alexander Maretskiy","email":"amaretskiy@mirantis.com","username":"maretskiy"},"change_message_id":"19dbd6f467207b4af4508bb2aea125e5cea7b89c","unresolved":false,"context_lines":[{"line_number":277,"context_line":"    def COLUMNS(self):"},{"line_number":278,"context_line":"        \"\"\"List of columns names.\"\"\""},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"    def round(self, ins, no_result):"},{"line_number":281,"context_line":"        return no_result and \"n/a\" or round(ins.result(), 3)"},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"    def get_rows(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a7e1126_4beae1ad","line":280,"in_reply_to":"5a710552_2816bba7","updated":"2015-12-18 14:03:42.000000000","message":"Done","commit_id":"f5444267d7e01ce28b94e0a0611d4c8e6e3968f4"},{"author":{"_account_id":10475,"name":"Alexander Maretskiy","email":"amaretskiy@mirantis.com","username":"maretskiy"},"change_message_id":"5d5e138ba199a48c74cf366b7c237aefa720ec51","unresolved":false,"context_lines":[{"line_number":277,"context_line":"    def COLUMNS(self):"},{"line_number":278,"context_line":"        \"\"\"List of columns names.\"\"\""},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"    def round(self, ins, no_result):"},{"line_number":281,"context_line":"        return no_result and \"n/a\" or round(ins.result(), 3)"},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"    def get_rows(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a710552_dc78f0f4","line":280,"in_reply_to":"5a710552_2816bba7","updated":"2015-12-16 10:58:33.000000000","message":"I will consider maybe move this method into the place where it is explicitly used (as lambda) or add a docstring otherwise.\n\nThe reason why local-scope lambda can be criticized is a nitty gritty in performance since new lambda object is created each time but method is created once. This topic was raised some time ago in our team so I try to avoid such cases (however this really super small performance nits looks not important for me)","commit_id":"f5444267d7e01ce28b94e0a0611d4c8e6e3968f4"},{"author":{"_account_id":8491,"name":"Illia Khudoshyn","email":"ikhudoshyn@mirantis.com","username":"ikhudoshyn"},"change_message_id":"28a79a547406e3350a4ddffca987e0d071c6107e","unresolved":false,"context_lines":[{"line_number":280,"context_line":"    def round(self, ins, no_result):"},{"line_number":281,"context_line":"        return no_result and \"n/a\" or round(ins.result(), 3)"},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"    def get_rows(self):"},{"line_number":284,"context_line":"        rows \u003d []"},{"line_number":285,"context_line":"        for name, values in self._data.items():"},{"line_number":286,"context_line":"            row \u003d [name]"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a710552_e80f5386","line":283,"updated":"2015-12-15 16:37:45.000000000","message":"The function seems to prepare data in some special structure (needed by JS code?). It is hard to provide any meaningful docstring for it.\nCan we at least have a little hint about how the output should look?","commit_id":"f5444267d7e01ce28b94e0a0611d4c8e6e3968f4"},{"author":{"_account_id":10475,"name":"Alexander Maretskiy","email":"amaretskiy@mirantis.com","username":"maretskiy"},"change_message_id":"19dbd6f467207b4af4508bb2aea125e5cea7b89c","unresolved":false,"context_lines":[{"line_number":280,"context_line":"    def round(self, ins, no_result):"},{"line_number":281,"context_line":"        return no_result and \"n/a\" or round(ins.result(), 3)"},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"    def get_rows(self):"},{"line_number":284,"context_line":"        rows \u003d []"},{"line_number":285,"context_line":"        for name, values in self._data.items():"},{"line_number":286,"context_line":"            row \u003d [name]"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a7e1126_6b06bd87","line":283,"in_reply_to":"5a710552_e80f5386","updated":"2015-12-18 14:03:42.000000000","message":"Done","commit_id":"f5444267d7e01ce28b94e0a0611d4c8e6e3968f4"},{"author":{"_account_id":10475,"name":"Alexander Maretskiy","email":"amaretskiy@mirantis.com","username":"maretskiy"},"change_message_id":"5d5e138ba199a48c74cf366b7c237aefa720ec51","unresolved":false,"context_lines":[{"line_number":280,"context_line":"    def round(self, ins, no_result):"},{"line_number":281,"context_line":"        return no_result and \"n/a\" or round(ins.result(), 3)"},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"    def get_rows(self):"},{"line_number":284,"context_line":"        rows \u003d []"},{"line_number":285,"context_line":"        for name, values in self._data.items():"},{"line_number":286,"context_line":"            row \u003d [name]"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a710552_e1f3c59c","line":283,"in_reply_to":"5a710552_e80f5386","updated":"2015-12-16 10:58:33.000000000","message":"Will do","commit_id":"f5444267d7e01ce28b94e0a0611d4c8e6e3968f4"},{"author":{"_account_id":8491,"name":"Illia Khudoshyn","email":"ikhudoshyn@mirantis.com","username":"ikhudoshyn"},"change_message_id":"28a79a547406e3350a4ddffca987e0d071c6107e","unresolved":false,"context_lines":[{"line_number":294,"context_line":"                    break"},{"line_number":295,"context_line":"            for ins, fn in values:"},{"line_number":296,"context_line":"                fn \u003d fn or self.round"},{"line_number":297,"context_line":"                row.append(fn(ins, no_result))"},{"line_number":298,"context_line":"            rows.append(row)"},{"line_number":299,"context_line":"        return rows"},{"line_number":300,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5a710552_e85d137a","line":297,"updated":"2015-12-15 16:37:45.000000000","message":"Explicit is better than implicit.\nSimple is better than complex.\nReadability counts.\n\nif no_result:\n  row.append(\"n/a\")\nelse:\n  row.append(round(ins.result(), 3)","commit_id":"f5444267d7e01ce28b94e0a0611d4c8e6e3968f4"},{"author":{"_account_id":8491,"name":"Illia Khudoshyn","email":"ikhudoshyn@mirantis.com","username":"ikhudoshyn"},"change_message_id":"f70594a07ccca16a619c11b4fb4d76e2858e1978","unresolved":false,"context_lines":[{"line_number":294,"context_line":"                    break"},{"line_number":295,"context_line":"            for ins, fn in values:"},{"line_number":296,"context_line":"                fn \u003d fn or self.round"},{"line_number":297,"context_line":"                row.append(fn(ins, no_result))"},{"line_number":298,"context_line":"            rows.append(row)"},{"line_number":299,"context_line":"        return rows"},{"line_number":300,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5a710552_fa0dab3e","line":297,"in_reply_to":"5a710552_c1080994","updated":"2015-12-16 12:02:49.000000000","message":"if no_result:\n  row.append(\"n/a\")\nelif fn:\n  row.append(fn(ins))\nelse:\n  row.append(round(ins.result(), 3)\n\nI may be getting too old but nowadays I try to reduce cognitive load (my own and others) as much as possible. I realize I could miss some corner cases, but if I\u0027m right ant that \u0027round\u0027 function is a one-timer, then plain \u0027if\u0027 could save several minutes and several joules of mental energy for those who come after us.\n\nAnyway i\u0027ll +1 the patch once the docs that you promised will appear))","commit_id":"f5444267d7e01ce28b94e0a0611d4c8e6e3968f4"},{"author":{"_account_id":10475,"name":"Alexander Maretskiy","email":"amaretskiy@mirantis.com","username":"maretskiy"},"change_message_id":"5d5e138ba199a48c74cf366b7c237aefa720ec51","unresolved":false,"context_lines":[{"line_number":294,"context_line":"                    break"},{"line_number":295,"context_line":"            for ins, fn in values:"},{"line_number":296,"context_line":"                fn \u003d fn or self.round"},{"line_number":297,"context_line":"                row.append(fn(ins, no_result))"},{"line_number":298,"context_line":"            rows.append(row)"},{"line_number":299,"context_line":"        return rows"},{"line_number":300,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5a710552_c1080994","line":297,"in_reply_to":"5a710552_e85d137a","updated":"2015-12-16 10:58:33.000000000","message":"How, in this case, you will transform specific columns to percent value, for example we have \"Success\" column that must always show single value after point (even zero) plus percent sign (example: we have float `90\u0027 but must show str \"90.0%\")?","commit_id":"f5444267d7e01ce28b94e0a0611d4c8e6e3968f4"},{"author":{"_account_id":7369,"name":"Sergey Skripnick","email":"sskripnick@mirantis.com","username":"redixin"},"change_message_id":"c0f64d5475869f7fbb4dc25d713f2ec2b3eb4231","unresolved":false,"context_lines":[{"line_number":310,"context_line":"            row \u003d [name]"},{"line_number":311,"context_line":"            no_result \u003d False"},{"line_number":312,"context_line":"            for ins, fn in values:"},{"line_number":313,"context_line":"                if isinstance(ins, streaming.MinComputation):"},{"line_number":314,"context_line":"                    try:"},{"line_number":315,"context_line":"                        no_result \u003d not ins.result()"},{"line_number":316,"context_line":"                    except exceptions.RallyException:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a7e1126_3e174e49","line":313,"updated":"2015-12-21 12:15:14.000000000","message":"Is it possible to remove this somehow? In case of no_result None may be returned (or other special value)","commit_id":"2d6458438afefa3302ca9989ce37e9b05d048ec6"},{"author":{"_account_id":10475,"name":"Alexander Maretskiy","email":"amaretskiy@mirantis.com","username":"maretskiy"},"change_message_id":"e84e53a06ef22ceca677f13a28f4b8fa273bc7a0","unresolved":false,"context_lines":[{"line_number":310,"context_line":"            row \u003d [name]"},{"line_number":311,"context_line":"            no_result \u003d False"},{"line_number":312,"context_line":"            for ins, fn in values:"},{"line_number":313,"context_line":"                if isinstance(ins, streaming.MinComputation):"},{"line_number":314,"context_line":"                    try:"},{"line_number":315,"context_line":"                        no_result \u003d not ins.result()"},{"line_number":316,"context_line":"                    except exceptions.RallyException:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a7e1126_60f8cfc3","line":313,"in_reply_to":"3a7e1126_3e174e49","updated":"2015-12-21 14:07:48.000000000","message":"We need to have a way to determine if current row can actually have results or we should mark it as \"n/a\".\n\nIn old code (see L328 on left side) there was a hardcoded check of second column (which is represented by MinComputation). This is not flexible at all.\n\nCurrent proposal is actually an improvement - we detect MinComputation and then make check - so column index is not hardcoded.\n\nHowever this seems to be not flexible enough, so I will move determining of no_result value to separate method, so subclasses can control this behavior (if required)","commit_id":"2d6458438afefa3302ca9989ce37e9b05d048ec6"},{"author":{"_account_id":7369,"name":"Sergey Skripnick","email":"sskripnick@mirantis.com","username":"redixin"},"change_message_id":"b3230c14ce4a5999bf1566fa99c5ede57e6e4882","unresolved":false,"context_lines":[{"line_number":309,"context_line":"        for ins, fn in values:"},{"line_number":310,"context_line":"            if isinstance(ins, streaming.MinComputation):"},{"line_number":311,"context_line":"                return bool(ins.result())"},{"line_number":312,"context_line":"        return True"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"    def get_rows(self):"},{"line_number":315,"context_line":"        \"\"\"Collect rows values finally, after all data is processed."}],"source_content_type":"text/x-python","patch_set":5,"id":"1a7b0d38_d189fbcb","line":312,"updated":"2015-12-23 12:07:19.000000000","message":"This will return True if there is no MinComputation with false result in values. Is it ok?","commit_id":"0b2f1f88dea56e73b2e96f7f93a43a1c27b1a36b"},{"author":{"_account_id":10475,"name":"Alexander Maretskiy","email":"amaretskiy@mirantis.com","username":"maretskiy"},"change_message_id":"39eab1772773d02b8d5823d8abb33fcd63161304","unresolved":false,"context_lines":[{"line_number":309,"context_line":"        for ins, fn in values:"},{"line_number":310,"context_line":"            if isinstance(ins, streaming.MinComputation):"},{"line_number":311,"context_line":"                return bool(ins.result())"},{"line_number":312,"context_line":"        return True"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"    def get_rows(self):"},{"line_number":315,"context_line":"        \"\"\"Collect rows values finally, after all data is processed."}],"source_content_type":"text/x-python","patch_set":5,"id":"1a7b0d38_0c668aed","line":312,"in_reply_to":"1a7b0d38_d189fbcb","updated":"2015-12-23 12:11:34.000000000","message":"Yes, it is assumed to be Ok. At least this is my idea. Maybe we will have better AI in future...","commit_id":"0b2f1f88dea56e73b2e96f7f93a43a1c27b1a36b"}]}
