)]}'
{"nova/compute/monitors/base.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9c05289a06a50e1d92598fd2dd0dd5fcf3880020","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"@six.add_metaclass(abc.ABCMeta)"},{"line_number":21,"context_line":"class MonitorBase(object):"},{"line_number":22,"context_line":"    \"\"\"Base class for all resource monitor plugins."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    A monitor is responsible for adding a set of related metrics to"}],"source_content_type":"text/x-python","patch_set":9,"id":"7aa08908_f6bb3473","line":21,"updated":"2016-06-17 21:54:12.000000000","message":"Are out of tree monitor plugins able to hook into this and extend this class? If so, changing the signature of get_metrics is going to break any out of tree implementations of this ABC.","commit_id":"ea7e0d003bd8970828d4dd03b36404dd824b274c"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"94ebd5f67210372ec36b2e181b2ff7f9f2556e60","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"@six.add_metaclass(abc.ABCMeta)"},{"line_number":21,"context_line":"class MonitorBase(object):"},{"line_number":22,"context_line":"    \"\"\"Base class for all resource monitor plugins."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    A monitor is responsible for adding a set of related metrics to"}],"source_content_type":"text/x-python","patch_set":9,"id":"7aa08908_3d3754e4","line":21,"in_reply_to":"7aa08908_f6bb3473","updated":"2016-06-18 05:27:52.000000000","message":"True. But I believe the change is tuned for future flexibility   to allow monitors the ability to select what fields they want to set in the monitor metric object.","commit_id":"ea7e0d003bd8970828d4dd03b36404dd824b274c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9c05289a06a50e1d92598fd2dd0dd5fcf3880020","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        raise NotImplementedError(\u0027get_metric_names\u0027)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @abc.abstractmethod"},{"line_number":46,"context_line":"    def get_metrics(self, metric_list):"},{"line_number":47,"context_line":"        \"\"\"Returns a list of tuples containing information for all metrics"},{"line_number":48,"context_line":"        tracked by the monitor."},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"7aa08908_51acd2ec","line":46,"range":{"start_line":46,"start_character":8,"end_line":46,"end_character":19},"updated":"2016-06-17 21:54:12.000000000","message":"This seems really weird, isn\u0027t it really more like \u0027populate_metrics\u0027 or \u0027load_metrics\u0027? Especially since it doesn\u0027t \u0027get\u0027 anything (doesn\u0027t return anything).","commit_id":"ea7e0d003bd8970828d4dd03b36404dd824b274c"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"94ebd5f67210372ec36b2e181b2ff7f9f2556e60","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        raise NotImplementedError(\u0027get_metric_names\u0027)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @abc.abstractmethod"},{"line_number":46,"context_line":"    def get_metrics(self, metric_list):"},{"line_number":47,"context_line":"        \"\"\"Returns a list of tuples containing information for all metrics"},{"line_number":48,"context_line":"        tracked by the monitor."},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"7aa08908_dd4c5078","line":46,"range":{"start_line":46,"start_character":8,"end_line":46,"end_character":19},"in_reply_to":"7aa08908_51acd2ec","updated":"2016-06-18 05:27:52.000000000","message":"Agreed. Will fix this in the next patchset.","commit_id":"ea7e0d003bd8970828d4dd03b36404dd824b274c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9c05289a06a50e1d92598fd2dd0dd5fcf3880020","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        implementation to do a single sampling call to the underlying monitor"},{"line_number":54,"context_line":"        to ensure that related metric values make logical sense."},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        :param name: Name of the metric"},{"line_number":57,"context_line":"        :param metric_list: A mutable reference of the metric list object"},{"line_number":58,"context_line":"        \"\"\""},{"line_number":59,"context_line":"        raise NotImplementedError(\u0027get_metrics\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"7aa08908_d12a827d","line":56,"range":{"start_line":56,"start_character":8,"end_line":56,"end_character":39},"updated":"2016-06-17 21:54:12.000000000","message":"This is wrong.","commit_id":"ea7e0d003bd8970828d4dd03b36404dd824b274c"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"94ebd5f67210372ec36b2e181b2ff7f9f2556e60","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        implementation to do a single sampling call to the underlying monitor"},{"line_number":54,"context_line":"        to ensure that related metric values make logical sense."},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        :param name: Name of the metric"},{"line_number":57,"context_line":"        :param metric_list: A mutable reference of the metric list object"},{"line_number":58,"context_line":"        \"\"\""},{"line_number":59,"context_line":"        raise NotImplementedError(\u0027get_metrics\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"7aa08908_bd518410","line":56,"range":{"start_line":56,"start_character":8,"end_line":56,"end_character":39},"in_reply_to":"7aa08908_d12a827d","updated":"2016-06-18 05:27:52.000000000","message":"Agreed. Will fix it in the next patchset.","commit_id":"ea7e0d003bd8970828d4dd03b36404dd824b274c"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a210c4c4b2c4deb29e39983c47b5fd96b21759d8","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        raise NotImplementedError(\u0027get_metric_names\u0027)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @abc.abstractmethod"},{"line_number":46,"context_line":"    def populate_metrics(self, metric_list):"},{"line_number":47,"context_line":"        \"\"\"Monitors are responsible for populating this metric_list object"},{"line_number":48,"context_line":"        with nova.objects.MonitorMetric objects with values collected via"},{"line_number":49,"context_line":"        the respective compute drivers."}],"source_content_type":"text/x-python","patch_set":10,"id":"3aaa91ec_6048ba5c","line":46,"updated":"2016-06-21 17:13:46.000000000","message":"Not sure you needed to change the name of the API method from add_metrics_to_list(), but OK... :)","commit_id":"2cd0e886130f5adc23c9e4f6ee32e5412deaf5f1"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"ab50fbc0b48f1f7367f9be1360a7a256534ea74d","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        raise NotImplementedError(\u0027get_metric_names\u0027)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @abc.abstractmethod"},{"line_number":46,"context_line":"    def populate_metrics(self, metric_list):"},{"line_number":47,"context_line":"        \"\"\"Monitors are responsible for populating this metric_list object"},{"line_number":48,"context_line":"        with nova.objects.MonitorMetric objects with values collected via"},{"line_number":49,"context_line":"        the respective compute drivers."}],"source_content_type":"text/x-python","patch_set":10,"id":"3aaa91ec_0dd303d6","line":46,"in_reply_to":"3aaa91ec_46bfcc60","updated":"2016-06-27 07:03:24.000000000","message":"Done","commit_id":"2cd0e886130f5adc23c9e4f6ee32e5412deaf5f1"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"5a6b5da537f4ee9c9405430de5bbc793f2df829e","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        raise NotImplementedError(\u0027get_metric_names\u0027)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @abc.abstractmethod"},{"line_number":46,"context_line":"    def populate_metrics(self, metric_list):"},{"line_number":47,"context_line":"        \"\"\"Monitors are responsible for populating this metric_list object"},{"line_number":48,"context_line":"        with nova.objects.MonitorMetric objects with values collected via"},{"line_number":49,"context_line":"        the respective compute drivers."}],"source_content_type":"text/x-python","patch_set":10,"id":"3aaa91ec_57a85b8d","line":46,"in_reply_to":"3aaa91ec_4778f82b","updated":"2016-06-23 17:26:10.000000000","message":"I guess Jay\u0027s answer to this on IRC has been to not support out of tree metric providers.\nIf we are worried about supporting out of tree metric providers - how would we ever change an interface without breaking them?\n\nI am not sure if that answers the question here, but I hope that does not land us up in a limbo.","commit_id":"2cd0e886130f5adc23c9e4f6ee32e5412deaf5f1"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"982dba7d7f551b318e3c04e21a56630c1c0fa4cc","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        raise NotImplementedError(\u0027get_metric_names\u0027)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @abc.abstractmethod"},{"line_number":46,"context_line":"    def populate_metrics(self, metric_list):"},{"line_number":47,"context_line":"        \"\"\"Monitors are responsible for populating this metric_list object"},{"line_number":48,"context_line":"        with nova.objects.MonitorMetric objects with values collected via"},{"line_number":49,"context_line":"        the respective compute drivers."}],"source_content_type":"text/x-python","patch_set":10,"id":"3aaa91ec_46bfcc60","line":46,"in_reply_to":"3aaa91ec_57a85b8d","updated":"2016-06-24 21:14:22.000000000","message":"To be clear, I\u0027m fine with changing the method name because the returned signature would be different. Also, out-of-tree monitors are not something we need to support given [1] even if we have an entrypoint for helping the operators.\n\n[1] http://docs.openstack.org/developer/nova/policies.html#public-contractual-apis\n\nThat said, given we have an entrypoint for that, we should maybe need to add an \u0027upgrade\u0027 release note for that, explaining that out-of-tree monitors should be modified for that new signature.","commit_id":"2cd0e886130f5adc23c9e4f6ee32e5412deaf5f1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"22e18cabcda02ff20a9ffc24f2ebd9c4f3e3210e","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        raise NotImplementedError(\u0027get_metric_names\u0027)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @abc.abstractmethod"},{"line_number":46,"context_line":"    def populate_metrics(self, metric_list):"},{"line_number":47,"context_line":"        \"\"\"Monitors are responsible for populating this metric_list object"},{"line_number":48,"context_line":"        with nova.objects.MonitorMetric objects with values collected via"},{"line_number":49,"context_line":"        the respective compute drivers."}],"source_content_type":"text/x-python","patch_set":10,"id":"3aaa91ec_4778f82b","line":46,"in_reply_to":"3aaa91ec_6048ba5c","updated":"2016-06-22 14:49:44.000000000","message":"Yeah, again as in a previous patch set, this will break anyone with out of tree metric providers, right? Or do we have something in place to not support those so we don\u0027t care?","commit_id":"2cd0e886130f5adc23c9e4f6ee32e5412deaf5f1"}],"nova/compute/monitors/cpu/virt_driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9c05289a06a50e1d92598fd2dd0dd5fcf3880020","unresolved":false,"context_lines":[{"line_number":62,"context_line":"            self._data[\"cpu.idle.time\"] \u003d stats[\"idle\"]"},{"line_number":63,"context_line":"            self._data[\"cpu.iowait.time\"] \u003d stats[\"iowait\"]"},{"line_number":64,"context_line":"            self._data[\"cpu.frequency\"] \u003d stats[\"frequency\"]"},{"line_number":65,"context_line":"        except (NotImplementedError, TypeError, KeyError):"},{"line_number":66,"context_line":"            LOG.exception(_LE(\"Not all properties needed are implemented \""},{"line_number":67,"context_line":"                              \"in the compute driver\"))"},{"line_number":68,"context_line":"            raise exception.ResourceMonitorError("},{"line_number":69,"context_line":"                monitor\u003dself.__class__.__name__)"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"        # The compute driver API returns the absolute values for CPU times."},{"line_number":72,"context_line":"        # We compute the utilization percentages for each specific CPU time"}],"source_content_type":"text/x-python","patch_set":9,"id":"7aa08908_11f40aef","line":69,"range":{"start_line":65,"start_character":8,"end_line":69,"end_character":48},"updated":"2016-06-17 21:54:12.000000000","message":"Unrelated, but yikes, this would be better as enumerating over the list of keys and setting them so if one at the beginning isn\u0027t implemented you don\u0027t lose everything.","commit_id":"ea7e0d003bd8970828d4dd03b36404dd824b274c"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"94ebd5f67210372ec36b2e181b2ff7f9f2556e60","unresolved":false,"context_lines":[{"line_number":62,"context_line":"            self._data[\"cpu.idle.time\"] \u003d stats[\"idle\"]"},{"line_number":63,"context_line":"            self._data[\"cpu.iowait.time\"] \u003d stats[\"iowait\"]"},{"line_number":64,"context_line":"            self._data[\"cpu.frequency\"] \u003d stats[\"frequency\"]"},{"line_number":65,"context_line":"        except (NotImplementedError, TypeError, KeyError):"},{"line_number":66,"context_line":"            LOG.exception(_LE(\"Not all properties needed are implemented \""},{"line_number":67,"context_line":"                              \"in the compute driver\"))"},{"line_number":68,"context_line":"            raise exception.ResourceMonitorError("},{"line_number":69,"context_line":"                monitor\u003dself.__class__.__name__)"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"        # The compute driver API returns the absolute values for CPU times."},{"line_number":72,"context_line":"        # We compute the utilization percentages for each specific CPU time"}],"source_content_type":"text/x-python","patch_set":9,"id":"7aa08908_420cfb41","line":69,"range":{"start_line":65,"start_character":8,"end_line":69,"end_character":48},"in_reply_to":"7aa08908_11f40aef","updated":"2016-06-18 05:27:52.000000000","message":"I think there\u0027s another problem with partial data here. The code logic below too would have to be cognizant about the key\u0027s existence in the _data dict. But that\u0027s not the biggest of problems, I think your point is valid - if we consider a different compute driver implementation of get_host_cpu_stats where it reports only a few of the fields as here. This is the reason, I feel that the entire way of doing this dict based approach is wrong. This has infact made the monitor class very specific to a linux way of implementing it. This is another motivation why the memory bandwidth monitor tries to abstract the \u0027actual name\u0027 of the metric from the compute driver and the exchanges are made only in the form of metric objects which are common.\n\nSo yeah in short, I feel, fixing the above comment is only a partial solution.","commit_id":"ea7e0d003bd8970828d4dd03b36404dd824b274c"}]}