)]}'
{"monasca_agent/collector/checks_d/cpu.py":[{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"bd72d1c834f50330048ae53b65aaaba9ce4aecb7","unresolved":false,"context_lines":[{"line_number":18,"context_line":"    def factory(cls, name, init_config, agent_config, instances\u003dNone):"},{"line_number":19,"context_line":"        if sys.platform \u003d\u003d \u0027win32\u0027:"},{"line_number":20,"context_line":"            return WindowsCpu(name, init_config, agent_config)"},{"line_number":21,"context_line":"        return Cpu(name, init_config, agent_config)"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def check(self, instance):"},{"line_number":24,"context_line":"        \"\"\"Capture cpu stats"}],"source_content_type":"text/x-python","patch_set":5,"id":"7aa08908_9e1e625d","line":21,"range":{"start_line":21,"start_character":19,"end_line":21,"end_character":23},"updated":"2016-06-07 17:28:56.000000000","message":"I think it might be better to declare this factory outside of the class. The problem is that your base class \"Cpu\", now needs to know about the derived class \"WindowsCpu\", which it shouldn\u0027t have too. \n\nIf you just move this factor outside of the class, and change the name to \"create_cpu\" as this method is creating and returning a class of type \"Cpu\", then that would probably be a bit clearer.\n\nAlso, the parameter \"instances\" doesn\u0027t seem to be used. Should that be removed?","commit_id":"9b06ebf3b47e7297d8b291e07d51f061e9edeced"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"5407b086322f0fb3e34eb75ce74df373421cb209","unresolved":false,"context_lines":[{"line_number":18,"context_line":"    def factory(cls, name, init_config, agent_config, instances\u003dNone):"},{"line_number":19,"context_line":"        if sys.platform \u003d\u003d \u0027win32\u0027:"},{"line_number":20,"context_line":"            return WindowsCpu(name, init_config, agent_config)"},{"line_number":21,"context_line":"        return Cpu(name, init_config, agent_config)"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def check(self, instance):"},{"line_number":24,"context_line":"        \"\"\"Capture cpu stats"}],"source_content_type":"text/x-python","patch_set":5,"id":"7aa08908_776a4283","line":21,"range":{"start_line":21,"start_character":19,"end_line":21,"end_character":23},"in_reply_to":"7aa08908_9e1e625d","updated":"2016-06-15 08:11:55.000000000","message":"yeah, forgot to add instances.","commit_id":"9b06ebf3b47e7297d8b291e07d51f061e9edeced"},{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"bd72d1c834f50330048ae53b65aaaba9ce4aecb7","unresolved":false,"context_lines":[{"line_number":46,"context_line":"                                    cpu_stats.softirq),"},{"line_number":47,"context_line":"                \u0027cpu.wait_perc\u0027: cpu_stats.iowait,"},{"line_number":48,"context_line":"                \u0027cpu.idle_perc\u0027: cpu_stats.idle,"},{"line_number":49,"context_line":"                \u0027cpu.stolen_perc\u0027: cpu_stats.steal}"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def _format_results(self, data, dimensions, instance):"},{"line_number":52,"context_line":"        for key in data.keys():"}],"source_content_type":"text/x-python","patch_set":5,"id":"7aa08908_de8bcac1","line":49,"range":{"start_line":49,"start_character":35,"end_line":49,"end_character":44},"updated":"2016-06-07 17:28:56.000000000","message":"Not to over engineer, but just wondering if it makes sense to move this inline dict declaration to outside.\n\nFor example,\n\nLINUX_CPU_STATS \u003d {\u0027cpu.user_perc\u0027: cpu_stats.user + cpu_stats.nice,\n                \u0027cpu.system_perc\u0027: (cpu_stats.system + cpu_stats.irq +\n                                    cpu_stats.softirq),\n                \u0027cpu.wait_perc\u0027: cpu_stats.iowait,\n                \u0027cpu.idle_perc\u0027: cpu_stats.idle,\n                \u0027cpu.stolen_perc\u0027: cpu_stats.steal}\n\nWIN32_CPU_STATE \u003d ...","commit_id":"9b06ebf3b47e7297d8b291e07d51f061e9edeced"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"5407b086322f0fb3e34eb75ce74df373421cb209","unresolved":false,"context_lines":[{"line_number":46,"context_line":"                                    cpu_stats.softirq),"},{"line_number":47,"context_line":"                \u0027cpu.wait_perc\u0027: cpu_stats.iowait,"},{"line_number":48,"context_line":"                \u0027cpu.idle_perc\u0027: cpu_stats.idle,"},{"line_number":49,"context_line":"                \u0027cpu.stolen_perc\u0027: cpu_stats.steal}"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def _format_results(self, data, dimensions, instance):"},{"line_number":52,"context_line":"        for key in data.keys():"}],"source_content_type":"text/x-python","patch_set":5,"id":"7aa08908_6065c8b3","line":49,"range":{"start_line":49,"start_character":35,"end_line":49,"end_character":44},"in_reply_to":"7aa08908_de8bcac1","updated":"2016-06-15 08:11:55.000000000","message":"Not really sure I follow. You\u0027re suggesting to add LINUX_CPU_STATS and WIN32_CPU_STATE as class members?\n\n_extract_cpu_stats_info returns a dict with values based on the given cpu_stats, I can\u0027t really imagine having those dicts outside the method.","commit_id":"9b06ebf3b47e7297d8b291e07d51f061e9edeced"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"a1e3057de22c86ae4ddcced7199db8a0bc99dbf0","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    @classmethod"},{"line_number":28,"context_line":"    def factory(cls, name, init_config, agent_config, instances\u003dNone):"},{"line_number":29,"context_line":"        if sys.platform \u003d\u003d \u0027win32\u0027:"},{"line_number":30,"context_line":"            return WindowsCpu(name, init_config, agent_config)"},{"line_number":31,"context_line":"        return Cpu(name, init_config, agent_config)"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3a98d1f4_396adfb3","line":29,"range":{"start_line":29,"start_character":11,"end_line":29,"end_character":34},"updated":"2016-09-30 20:39:08.000000000","message":"I think it makes sense to move this to a utility method in  monasca_agent/common/util.py","commit_id":"44816262d5e8f0916b0ffa100307ef8f17518423"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"b2995f25a37209b2561c2854988d1f2c4f2ed73a","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    @classmethod"},{"line_number":28,"context_line":"    def factory(cls, name, init_config, agent_config, instances\u003dNone):"},{"line_number":29,"context_line":"        if sys.platform \u003d\u003d \u0027win32\u0027:"},{"line_number":30,"context_line":"            return WindowsCpu(name, init_config, agent_config)"},{"line_number":31,"context_line":"        return Cpu(name, init_config, agent_config)"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"fa6399be_e7776100","line":29,"range":{"start_line":29,"start_character":11,"end_line":29,"end_character":34},"in_reply_to":"3a98d1f4_396adfb3","updated":"2016-10-11 16:50:44.000000000","message":"Done","commit_id":"44816262d5e8f0916b0ffa100307ef8f17518423"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"a1e3057de22c86ae4ddcced7199db8a0bc99dbf0","unresolved":false,"context_lines":[{"line_number":69,"context_line":"                                     cpu_stats.softirq),"},{"line_number":70,"context_line":"            \u0027cpu.wait_\u0027 + suffix: cpu_stats.iowait,"},{"line_number":71,"context_line":"            \u0027cpu.idle_\u0027 + suffix: cpu_stats.idle,"},{"line_number":72,"context_line":"            \u0027cpu.stolen_\u0027 + suffix: cpu_stats.steal,"},{"line_number":73,"context_line":"        })"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def _add_cpu_freq(self, data):"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a98d1f4_d9e3bbd7","line":72,"range":{"start_line":72,"start_character":12,"end_line":72,"end_character":25},"updated":"2016-09-30 20:39:08.000000000","message":"There is no cpu.stolen_time in the old code. Do we want to add this as a new metric?","commit_id":"44816262d5e8f0916b0ffa100307ef8f17518423"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"b2995f25a37209b2561c2854988d1f2c4f2ed73a","unresolved":false,"context_lines":[{"line_number":69,"context_line":"                                     cpu_stats.softirq),"},{"line_number":70,"context_line":"            \u0027cpu.wait_\u0027 + suffix: cpu_stats.iowait,"},{"line_number":71,"context_line":"            \u0027cpu.idle_\u0027 + suffix: cpu_stats.idle,"},{"line_number":72,"context_line":"            \u0027cpu.stolen_\u0027 + suffix: cpu_stats.steal,"},{"line_number":73,"context_line":"        })"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def _add_cpu_freq(self, data):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa6399be_278f9924","line":72,"range":{"start_line":72,"start_character":12,"end_line":72,"end_character":25},"in_reply_to":"3a98d1f4_d9e3bbd7","updated":"2016-10-11 16:50:44.000000000","message":"hm, from what I can see in this diff, there is: L43.","commit_id":"44816262d5e8f0916b0ffa100307ef8f17518423"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"a0e4c0f8d5ad976eb6603f0e1644cd5f33db5fdf","unresolved":false,"context_lines":[{"line_number":69,"context_line":"                                     cpu_stats.softirq),"},{"line_number":70,"context_line":"            \u0027cpu.wait_\u0027 + suffix: cpu_stats.iowait,"},{"line_number":71,"context_line":"            \u0027cpu.idle_\u0027 + suffix: cpu_stats.idle,"},{"line_number":72,"context_line":"            \u0027cpu.stolen_\u0027 + suffix: cpu_stats.steal,"},{"line_number":73,"context_line":"        })"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def _add_cpu_freq(self, data):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa6399be_e26caf9e","line":72,"range":{"start_line":72,"start_character":12,"end_line":72,"end_character":25},"in_reply_to":"fa6399be_278f9924","updated":"2016-10-11 17:07:19.000000000","message":"L43 is for cpu.stolen_perc, but not cpu.stolen_time","commit_id":"44816262d5e8f0916b0ffa100307ef8f17518423"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"a1e3057de22c86ae4ddcced7199db8a0bc99dbf0","unresolved":false,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":"            data[\u0027cpu.frequency_mhz\u0027] \u003d float(frequency_ghz) * units.Ki"},{"line_number":107,"context_line":"        except Exception:"},{"line_number":108,"context_line":"            log.exception(\u0027Cannot extract CPU MHz information using wmic\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a98d1f4_195b63f7","line":108,"range":{"start_line":108,"start_character":12,"end_line":108,"end_character":25},"updated":"2016-09-30 20:39:08.000000000","message":"Do we expect this to fail other than in exceptional cases? For example, if permissions are configured differently? If this could fail often, it should only output this exception the first time. After that, it just filling the log file","commit_id":"44816262d5e8f0916b0ffa100307ef8f17518423"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"b2995f25a37209b2561c2854988d1f2c4f2ed73a","unresolved":false,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":"            data[\u0027cpu.frequency_mhz\u0027] \u003d float(frequency_ghz) * units.Ki"},{"line_number":107,"context_line":"        except Exception:"},{"line_number":108,"context_line":"            log.exception(\u0027Cannot extract CPU MHz information using wmic\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa6399be_c76a5d1a","line":108,"range":{"start_line":108,"start_character":12,"end_line":108,"end_character":25},"in_reply_to":"3a98d1f4_195b63f7","updated":"2016-10-11 16:50:44.000000000","message":"no, if the user has the proper permissions, it won\u0027t fail. if the user doesn\u0027t have them, then the admin should address this issue.","commit_id":"44816262d5e8f0916b0ffa100307ef8f17518423"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"a0e4c0f8d5ad976eb6603f0e1644cd5f33db5fdf","unresolved":false,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":"            data[\u0027cpu.frequency_mhz\u0027] \u003d float(frequency_ghz) * units.Ki"},{"line_number":107,"context_line":"        except Exception:"},{"line_number":108,"context_line":"            log.exception(\u0027Cannot extract CPU MHz information using wmic\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa6399be_6226df69","line":108,"range":{"start_line":108,"start_character":12,"end_line":108,"end_character":25},"in_reply_to":"fa6399be_c76a5d1a","updated":"2016-10-11 17:07:19.000000000","message":"So, it sounds like this could easily fail every time. You should then output the exception the first time.","commit_id":"44816262d5e8f0916b0ffa100307ef8f17518423"},{"author":{"_account_id":22157,"name":"Andrea Adams","email":"aadams@hpe.com","username":"andrea.adams"},"change_message_id":"2e486e79c04cb49af70d546af8339beef3a764b3","unresolved":false,"context_lines":[{"line_number":25,"context_line":"        psutil.cpu_times_percent(interval\u003dNone, percpu\u003dFalse)"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    @classmethod"},{"line_number":28,"context_line":"    def factory(cls, name, init_config, agent_config, instances\u003dNone):"},{"line_number":29,"context_line":"        if util.is_windows()"},{"line_number":30,"context_line":"            return WindowsCpu(name, init_config, agent_config)"},{"line_number":31,"context_line":"        return Cpu(name, init_config, agent_config)"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa6399be_6b823454","line":28,"range":{"start_line":28,"start_character":54,"end_line":28,"end_character":68},"updated":"2016-10-11 21:01:37.000000000","message":"Since you accept the instances argument here, util.py at line 566 assumes that you set instances. However, you never do anything with the argument, so instances never gets set. Please remove this argument.","commit_id":"0a59492cca0b4d3710c84d1b17e4bb009a43bd4e"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"e3137d7d086d9c0e4e1ebd54b45a7c7b8b9458a8","unresolved":false,"context_lines":[{"line_number":25,"context_line":"        psutil.cpu_times_percent(interval\u003dNone, percpu\u003dFalse)"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    @classmethod"},{"line_number":28,"context_line":"    def factory(cls, name, init_config, agent_config, instances\u003dNone):"},{"line_number":29,"context_line":"        if util.is_windows()"},{"line_number":30,"context_line":"            return WindowsCpu(name, init_config, agent_config)"},{"line_number":31,"context_line":"        return Cpu(name, init_config, agent_config)"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa6399be_988a6abe","line":28,"range":{"start_line":28,"start_character":54,"end_line":28,"end_character":68},"in_reply_to":"fa6399be_6b823454","updated":"2016-10-12 10:27:16.000000000","message":"can\u0027t remove the instances argument, as this method has to respect the parent\u0027s factory method signature.","commit_id":"0a59492cca0b4d3710c84d1b17e4bb009a43bd4e"},{"author":{"_account_id":22157,"name":"Andrea Adams","email":"aadams@hpe.com","username":"andrea.adams"},"change_message_id":"2e486e79c04cb49af70d546af8339beef3a764b3","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    @classmethod"},{"line_number":28,"context_line":"    def factory(cls, name, init_config, agent_config, instances\u003dNone):"},{"line_number":29,"context_line":"        if util.is_windows()"},{"line_number":30,"context_line":"            return WindowsCpu(name, init_config, agent_config)"},{"line_number":31,"context_line":"        return Cpu(name, init_config, agent_config)"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"fa6399be_384fb7be","line":29,"range":{"start_line":29,"start_character":8,"end_line":29,"end_character":28},"updated":"2016-10-11 21:01:37.000000000","message":"I think you are missing a colon at the end here.","commit_id":"0a59492cca0b4d3710c84d1b17e4bb009a43bd4e"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"e3137d7d086d9c0e4e1ebd54b45a7c7b8b9458a8","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    @classmethod"},{"line_number":28,"context_line":"    def factory(cls, name, init_config, agent_config, instances\u003dNone):"},{"line_number":29,"context_line":"        if util.is_windows()"},{"line_number":30,"context_line":"            return WindowsCpu(name, init_config, agent_config)"},{"line_number":31,"context_line":"        return Cpu(name, init_config, agent_config)"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"fa6399be_9878aac6","line":29,"range":{"start_line":29,"start_character":8,"end_line":29,"end_character":28},"in_reply_to":"fa6399be_384fb7be","updated":"2016-10-12 10:27:16.000000000","message":"Done","commit_id":"0a59492cca0b4d3710c84d1b17e4bb009a43bd4e"},{"author":{"_account_id":22157,"name":"Andrea Adams","email":"aadams@hpe.com","username":"andrea.adams"},"change_message_id":"848ed53bc5ab6f6028886389762ecb0620267cfd","unresolved":false,"context_lines":[{"line_number":25,"context_line":"        psutil.cpu_times_percent(interval\u003dNone, percpu\u003dFalse)"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    @classmethod"},{"line_number":28,"context_line":"    def factory(cls, name, init_config, agent_config, instances\u003dNone):"},{"line_number":29,"context_line":"        if util.is_windows():"},{"line_number":30,"context_line":"            return WindowsCpu(name, init_config, agent_config)"},{"line_number":31,"context_line":"        return Cpu(name, init_config, agent_config)"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa6399be_39bb0429","line":28,"range":{"start_line":28,"start_character":4,"end_line":28,"end_character":70},"updated":"2016-10-12 21:13:30.000000000","message":"Expecting each check that wants to implement both Windows and Linux to know about instances behavior is too fragile. It would be easier to let the util.py deal with this behavior instead of expecting each check to know about and handle both cases on the lower level. Therefor, I think the idea to use a method definition to instantiate both objects outside the classes is better because util.py can then take care of both constructor cases.\nimport sys\n\nclass CpuCheck:\n    def foo(self):\n        pass\n\n\nclass LinuxCpuCheck(CpuCheck):\n    def foo(self):\n        print \"LinuxStats\"\n\n\nclass Win32CpuCheck(CpuCheck):\n    def foo(self):\n        print \"Win32Stats\"\n\n\ndef Cpu():\n    if sys.platform \u003d\u003d \u0027win32\u0027:\n        return Win32CpuCheck()\n    else:\n        return LinuxCpuCheck()\n\n\na \u003d Cpu()\na.foo()\nUsing something like this will get a more predictable expected behavior and hopefully avoid confusion for others trying to write their own checks.","commit_id":"744c56860679fe47c0fa76aeead79e6793a0a8a7"},{"author":{"_account_id":22157,"name":"Andrea Adams","email":"aadams@hpe.com","username":"andrea.adams"},"change_message_id":"95bda1e08381a6b2d2176bae73288e984e8f4ec4","unresolved":false,"context_lines":[{"line_number":25,"context_line":"        psutil.cpu_times_percent(interval\u003dNone, percpu\u003dFalse)"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    @classmethod"},{"line_number":28,"context_line":"    def factory(cls, name, init_config, agent_config, instances\u003dNone):"},{"line_number":29,"context_line":"        if util.is_windows():"},{"line_number":30,"context_line":"            return WindowsCpu(name, init_config, agent_config)"},{"line_number":31,"context_line":"        return Cpu(name, init_config, agent_config)"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa6399be_68d12857","line":28,"range":{"start_line":28,"start_character":54,"end_line":28,"end_character":68},"updated":"2016-10-12 19:47:06.000000000","message":"In https://review.openstack.org/#/c/323034/ you have written a factory method that you intend to override here. The factory method that you override handles the case were the constructor does not take an instances variable and sets it. This factory method takes an instances variable and since the constructor does not take an instances variable, it is never set anywhere. As the code is now, cpu check will never get run because there are no instances. This is the case on both Windows and Linux.","commit_id":"744c56860679fe47c0fa76aeead79e6793a0a8a7"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"bfb9930186314c169b4e969f63a1a8ad4e1ec45e","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# (C) Copyright 2015-2016 Hewlett Packard Enterprise Development Company LP"},{"line_number":2,"context_line":""},{"line_number":3,"context_line":"import logging"},{"line_number":4,"context_line":"import psutil"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a629dbe_d129889c","line":1,"range":{"start_line":1,"start_character":65,"end_line":1,"end_character":72},"updated":"2016-11-11 14:59:57.000000000","message":"Andrea, since you made changes to this file, you need to update the copyright","commit_id":"84238648d82465ff3fa0639ad534b00166467734"},{"author":{"_account_id":22157,"name":"Andrea Adams","email":"aadams@hpe.com","username":"andrea.adams"},"change_message_id":"01d4faf9219e1fcdc5a709d1499d197a25f42a91","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# (C) Copyright 2015-2016 Hewlett Packard Enterprise Development Company LP"},{"line_number":2,"context_line":""},{"line_number":3,"context_line":"import logging"},{"line_number":4,"context_line":"import psutil"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a629dbe_08374aff","line":1,"range":{"start_line":1,"start_character":65,"end_line":1,"end_character":72},"in_reply_to":"9a629dbe_d129889c","updated":"2016-11-11 19:11:46.000000000","message":"Done","commit_id":"84238648d82465ff3fa0639ad534b00166467734"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"8fd3f9eb05dac5367ff49f866e12ba910894b9d8","unresolved":false,"context_lines":[{"line_number":13,"context_line":"log \u003d logging.getLogger(__name__)"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"def Cpu(self, name, init_config, agent_config):"},{"line_number":17,"context_line":"    if util.is_windows():"},{"line_number":18,"context_line":"        return WindowsCpu(self, name, init_config, agent_config)"},{"line_number":19,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a629dbe_79bdda01","line":16,"range":{"start_line":16,"start_character":4,"end_line":16,"end_character":7},"updated":"2016-11-14 05:49:30.000000000","message":"This is not going to work. The collector tries to instantiate the classes in the plugin file that extend checks.AgentCheck, it doesn\u0027t use the name of the file to choose which one.\n\nThis is going to have to be changed so there is one and only one class in this file that extends checks.AgentCheck and that class is going to have to use a factory or whatever to instantiate a helper class. Or something like that.","commit_id":"067e543976bc33b695cc934d8a4f832ca9e16928"},{"author":{"_account_id":14517,"name":"Ryan Brandt","email":"ryan.brandt@hpe.com","username":"ryan-brandt"},"change_message_id":"5fd5f822a818e0fb1dbb2befd02fbd8f8332f4cb","unresolved":false,"context_lines":[{"line_number":89,"context_line":""},{"line_number":90,"context_line":"class WindowsCpu(LinuxCpu):"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    is_exception_logged \u003d False"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    def _add_cpu_stats_info(self, data, cpu_stats, suffix):"},{"line_number":95,"context_line":"        data.update({"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a629dbe_b9486eb4","line":92,"range":{"start_line":92,"start_character":4,"end_line":92,"end_character":23},"updated":"2016-11-11 20:44:37.000000000","message":"this doesn\u0027t match the variable name used below","commit_id":"067e543976bc33b695cc934d8a4f832ca9e16928"}],"monasca_agent/common/util.py":[{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"38cce36c16b35a09981cf626f6e276af6704be7a","unresolved":false,"context_lines":[{"line_number":461,"context_line":"    for function in functions:"},{"line_number":462,"context_line":"        if \u0027initalize_check\u0027 in function:"},{"line_number":463,"context_line":"            __import__(\u0027checksd_\u0027 + check_name)"},{"line_number":464,"context_line":"            return check_module.initalize_check(check_name, init_config, agent_config, instances)"},{"line_number":465,"context_line":"    return None"},{"line_number":466,"context_line":""},{"line_number":467,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"5a74a57a_e84d8e9a","line":464,"range":{"start_line":464,"start_character":0,"end_line":464,"end_character":97},"updated":"2016-11-29 21:11:23.000000000","message":"We should have an error trap here.  If the initialize_check function fails I\u0027m not sure where that exception would go.","commit_id":"78d9202cd17293fa9808c034a57ba5659170f90f"},{"author":{"_account_id":22157,"name":"Andrea Adams","email":"aadams@hpe.com","username":"andrea.adams"},"change_message_id":"10d4f22909f199913af25b1bc014b3c695f42b8a","unresolved":false,"context_lines":[{"line_number":461,"context_line":"    for function in functions:"},{"line_number":462,"context_line":"        if \u0027initalize_check\u0027 in function:"},{"line_number":463,"context_line":"            __import__(\u0027checksd_\u0027 + check_name)"},{"line_number":464,"context_line":"            return check_module.initalize_check(check_name, init_config, agent_config, instances)"},{"line_number":465,"context_line":"    return None"},{"line_number":466,"context_line":""},{"line_number":467,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"5a74a57a_9707a41c","line":464,"range":{"start_line":464,"start_character":0,"end_line":464,"end_character":97},"in_reply_to":"5a74a57a_e84d8e9a","updated":"2016-11-30 21:50:58.000000000","message":"Done","commit_id":"78d9202cd17293fa9808c034a57ba5659170f90f"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"38cce36c16b35a09981cf626f6e276af6704be7a","unresolved":false,"context_lines":[{"line_number":505,"context_line":"    check_name \u003d os.path.basename(check).split(\u0027.\u0027)[0]"},{"line_number":506,"context_line":"    if check_name in initialized_checks or check_name in init_failed_checks:"},{"line_number":507,"context_line":"        log.debug("},{"line_number":508,"context_line":"            \u0027Skipping check %s because it has already been loaded from another location\u0027, check)"},{"line_number":509,"context_line":"        return None, None"},{"line_number":510,"context_line":"    try:"},{"line_number":511,"context_line":"        check_module \u003d imp.load_source(\u0027checksd_%s\u0027 % check_name, check)"}],"source_content_type":"text/x-python","patch_set":15,"id":"5a74a57a_c89372ff","line":508,"range":{"start_line":508,"start_character":13,"end_line":508,"end_character":21},"updated":"2016-11-29 21:11:23.000000000","message":"I wonder if this should be a warning.  If we\u0027re finding identically named modules and attempting to load them we should probably know so we can decide which one is the right module.","commit_id":"78d9202cd17293fa9808c034a57ba5659170f90f"},{"author":{"_account_id":22157,"name":"Andrea Adams","email":"aadams@hpe.com","username":"andrea.adams"},"change_message_id":"10d4f22909f199913af25b1bc014b3c695f42b8a","unresolved":false,"context_lines":[{"line_number":505,"context_line":"    check_name \u003d os.path.basename(check).split(\u0027.\u0027)[0]"},{"line_number":506,"context_line":"    if check_name in initialized_checks or check_name in init_failed_checks:"},{"line_number":507,"context_line":"        log.debug("},{"line_number":508,"context_line":"            \u0027Skipping check %s because it has already been loaded from another location\u0027, check)"},{"line_number":509,"context_line":"        return None, None"},{"line_number":510,"context_line":"    try:"},{"line_number":511,"context_line":"        check_module \u003d imp.load_source(\u0027checksd_%s\u0027 % check_name, check)"}],"source_content_type":"text/x-python","patch_set":15,"id":"5a74a57a_9235f2dc","line":508,"range":{"start_line":508,"start_character":13,"end_line":508,"end_character":21},"in_reply_to":"5a74a57a_c89372ff","updated":"2016-11-30 21:50:58.000000000","message":"Done","commit_id":"78d9202cd17293fa9808c034a57ba5659170f90f"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"38cce36c16b35a09981cf626f6e276af6704be7a","unresolved":false,"context_lines":[{"line_number":516,"context_line":"        conf_path \u003d os.path.join(confd_path, \u0027%s.yaml\u0027 % check_name)"},{"line_number":517,"context_line":"        if os.path.exists(conf_path):"},{"line_number":518,"context_line":"            # There is a configuration file for that check but the module can\u0027t be imported"},{"line_number":519,"context_line":"            init_failed_checks[check_name] \u003d {\u0027error\u0027: e, \u0027traceback\u0027: traceback_message}"},{"line_number":520,"context_line":"            log.exception(\u0027Unable to import check module %s.py from checks_d\u0027 % check_name)"},{"line_number":521,"context_line":"        else:  # There is no conf for that check. Let\u0027s not spam the logs for it."},{"line_number":522,"context_line":"            log.debug(\u0027Unable to import check module %s.py from checks_d\u0027 % check_name)"}],"source_content_type":"text/x-python","patch_set":15,"id":"5a74a57a_28d5c6b8","line":519,"range":{"start_line":519,"start_character":0,"end_line":519,"end_character":89},"updated":"2016-11-29 21:11:23.000000000","message":"The only piece of data we use here is the check_name.  The value portion doesn\u0027t appear to be used anywhere.  Do you think it\u0027s worth converting this to a set instead and not bothering with storing the error and the traceback_message?","commit_id":"78d9202cd17293fa9808c034a57ba5659170f90f"},{"author":{"_account_id":22157,"name":"Andrea Adams","email":"aadams@hpe.com","username":"andrea.adams"},"change_message_id":"10d4f22909f199913af25b1bc014b3c695f42b8a","unresolved":false,"context_lines":[{"line_number":516,"context_line":"        conf_path \u003d os.path.join(confd_path, \u0027%s.yaml\u0027 % check_name)"},{"line_number":517,"context_line":"        if os.path.exists(conf_path):"},{"line_number":518,"context_line":"            # There is a configuration file for that check but the module can\u0027t be imported"},{"line_number":519,"context_line":"            init_failed_checks[check_name] \u003d {\u0027error\u0027: e, \u0027traceback\u0027: traceback_message}"},{"line_number":520,"context_line":"            log.exception(\u0027Unable to import check module %s.py from checks_d\u0027 % check_name)"},{"line_number":521,"context_line":"        else:  # There is no conf for that check. Let\u0027s not spam the logs for it."},{"line_number":522,"context_line":"            log.debug(\u0027Unable to import check module %s.py from checks_d\u0027 % check_name)"}],"source_content_type":"text/x-python","patch_set":15,"id":"5a74a57a_ff5d0aed","line":519,"range":{"start_line":519,"start_character":0,"end_line":519,"end_character":89},"in_reply_to":"5a74a57a_28d5c6b8","updated":"2016-11-30 21:50:58.000000000","message":"I agree that this is weird, but it seems like a fix for a future patch set.","commit_id":"78d9202cd17293fa9808c034a57ba5659170f90f"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"38cce36c16b35a09981cf626f6e276af6704be7a","unresolved":false,"context_lines":[{"line_number":587,"context_line":"        if not check_name or not check_module:"},{"line_number":588,"context_line":"            continue"},{"line_number":589,"context_line":""},{"line_number":590,"context_line":"        check_class \u003d get_check_class(check_module)"},{"line_number":591,"context_line":"        if not check_class:"},{"line_number":592,"context_line":"            if not check_name \u003d\u003d \u0027__init__\u0027:"},{"line_number":593,"context_line":"                log.error(\u0027No check class (inheriting from AgentCheck) found in %s.py\u0027 % check_name)"}],"source_content_type":"text/x-python","patch_set":15,"id":"5a74a57a_0881eafb","line":590,"range":{"start_line":590,"start_character":22,"end_line":590,"end_character":37},"updated":"2016-11-29 21:11:23.000000000","message":"We should probably only run this when we don\u0027t find a factory function.","commit_id":"78d9202cd17293fa9808c034a57ba5659170f90f"},{"author":{"_account_id":22157,"name":"Andrea Adams","email":"aadams@hpe.com","username":"andrea.adams"},"change_message_id":"10d4f22909f199913af25b1bc014b3c695f42b8a","unresolved":false,"context_lines":[{"line_number":587,"context_line":"        if not check_name or not check_module:"},{"line_number":588,"context_line":"            continue"},{"line_number":589,"context_line":""},{"line_number":590,"context_line":"        check_class \u003d get_check_class(check_module)"},{"line_number":591,"context_line":"        if not check_class:"},{"line_number":592,"context_line":"            if not check_name \u003d\u003d \u0027__init__\u0027:"},{"line_number":593,"context_line":"                log.error(\u0027No check class (inheriting from AgentCheck) found in %s.py\u0027 % check_name)"}],"source_content_type":"text/x-python","patch_set":15,"id":"5a74a57a_520fda87","line":590,"range":{"start_line":590,"start_character":22,"end_line":590,"end_character":37},"in_reply_to":"5a74a57a_0881eafb","updated":"2016-11-30 21:50:58.000000000","message":"Done","commit_id":"78d9202cd17293fa9808c034a57ba5659170f90f"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"38cce36c16b35a09981cf626f6e276af6704be7a","unresolved":false,"context_lines":[{"line_number":590,"context_line":"        check_class \u003d get_check_class(check_module)"},{"line_number":591,"context_line":"        if not check_class:"},{"line_number":592,"context_line":"            if not check_name \u003d\u003d \u0027__init__\u0027:"},{"line_number":593,"context_line":"                log.error(\u0027No check class (inheriting from AgentCheck) found in %s.py\u0027 % check_name)"},{"line_number":594,"context_line":"                continue"},{"line_number":595,"context_line":""},{"line_number":596,"context_line":"        # Check if the config exists"}],"source_content_type":"text/x-python","patch_set":15,"id":"5a74a57a_68917e52","line":593,"range":{"start_line":593,"start_character":0,"end_line":593,"end_character":100},"updated":"2016-11-29 21:11:23.000000000","message":"If we\u0027re allowing them to define a factory method that will instantiate the check I don\u0027t think this is necessarily true.  We should only have this error if we fall back to trying to find an object to construct ourselves.","commit_id":"78d9202cd17293fa9808c034a57ba5659170f90f"},{"author":{"_account_id":22157,"name":"Andrea Adams","email":"aadams@hpe.com","username":"andrea.adams"},"change_message_id":"10d4f22909f199913af25b1bc014b3c695f42b8a","unresolved":false,"context_lines":[{"line_number":590,"context_line":"        check_class \u003d get_check_class(check_module)"},{"line_number":591,"context_line":"        if not check_class:"},{"line_number":592,"context_line":"            if not check_name \u003d\u003d \u0027__init__\u0027:"},{"line_number":593,"context_line":"                log.error(\u0027No check class (inheriting from AgentCheck) found in %s.py\u0027 % check_name)"},{"line_number":594,"context_line":"                continue"},{"line_number":595,"context_line":""},{"line_number":596,"context_line":"        # Check if the config exists"}],"source_content_type":"text/x-python","patch_set":15,"id":"5a74a57a_f278e6fa","line":593,"range":{"start_line":593,"start_character":0,"end_line":593,"end_character":100},"in_reply_to":"5a74a57a_68917e52","updated":"2016-11-30 21:50:58.000000000","message":"Done","commit_id":"78d9202cd17293fa9808c034a57ba5659170f90f"},{"author":{"_account_id":14273,"name":"Joe Keen","email":"joe.keen@hpe.com","username":"jkeen"},"change_message_id":"38cce36c16b35a09981cf626f6e276af6704be7a","unresolved":false,"context_lines":[{"line_number":611,"context_line":"                                                init_config,"},{"line_number":612,"context_line":"                                                agent_config,"},{"line_number":613,"context_line":"                                                instances) or \\"},{"line_number":614,"context_line":"            initalize_check_by_subclass(check_class,"},{"line_number":615,"context_line":"                                        check_name,"},{"line_number":616,"context_line":"                                        init_config,"},{"line_number":617,"context_line":"                                        agent_config,"}],"source_content_type":"text/x-python","patch_set":15,"id":"5a74a57a_889eda61","line":614,"range":{"start_line":614,"start_character":12,"end_line":614,"end_character":39},"updated":"2016-11-29 21:11:23.000000000","message":"I don\u0027t like this or statement here.  I\u0027d prefer to see an explicit if to make it clearer or just one \"initialize_check\" function that figured this out appropriately.","commit_id":"78d9202cd17293fa9808c034a57ba5659170f90f"},{"author":{"_account_id":22157,"name":"Andrea Adams","email":"aadams@hpe.com","username":"andrea.adams"},"change_message_id":"10d4f22909f199913af25b1bc014b3c695f42b8a","unresolved":false,"context_lines":[{"line_number":611,"context_line":"                                                init_config,"},{"line_number":612,"context_line":"                                                agent_config,"},{"line_number":613,"context_line":"                                                instances) or \\"},{"line_number":614,"context_line":"            initalize_check_by_subclass(check_class,"},{"line_number":615,"context_line":"                                        check_name,"},{"line_number":616,"context_line":"                                        init_config,"},{"line_number":617,"context_line":"                                        agent_config,"}],"source_content_type":"text/x-python","patch_set":15,"id":"5a74a57a_5286fadb","line":614,"range":{"start_line":614,"start_character":12,"end_line":614,"end_character":39},"in_reply_to":"5a74a57a_889eda61","updated":"2016-11-30 21:50:58.000000000","message":"Done","commit_id":"78d9202cd17293fa9808c034a57ba5659170f90f"}]}