)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"50bdaacc0b2a2b2a58de2824bcf7d78af949e119","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Michal Dulko \u003cmichal.dulko@intel.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-06-02 18:09:28 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Make c-vol appear down if driver is uninitialized"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This commit adds is_up method to base Manager class that can"},{"line_number":10,"context_line":"be used to indicate that service has some problems and isn\u0027t"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5afe65bd_6381674a","line":7,"updated":"2015-06-02 18:46:16.000000000","message":"I\u0027d make it explicit about what this patch is doing, how about \u0027stop sending heartbeat to DB if manager is marked not working properly\u0027","commit_id":"374dcd3081c107f6a6d8ffe0480f54eaf6fa6b60"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"2e8a23ebb009259c1944ca0f93ad1b331aea6a1b","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Michal Dulko \u003cmichal.dulko@intel.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-06-02 18:09:28 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Make c-vol appear down if driver is uninitialized"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This commit adds is_up method to base Manager class that can"},{"line_number":10,"context_line":"be used to indicate that service has some problems and isn\u0027t"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5afe65bd_de07bb7f","line":7,"in_reply_to":"5afe65bd_6381674a","updated":"2015-06-03 09:20:55.000000000","message":"Done","commit_id":"374dcd3081c107f6a6d8ffe0480f54eaf6fa6b60"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c95aeaf26d9ca31afa659598fb556c02aa76bed5","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Michal Dulko \u003cmichal.dulko@intel.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-06-02 18:09:28 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Make c-vol appear down if driver is uninitialized"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This commit adds is_up method to base Manager class that can"},{"line_number":10,"context_line":"be used to indicate that service has some problems and isn\u0027t"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5afe65bd_b4dd5716","line":7,"in_reply_to":"5afe65bd_6381674a","updated":"2015-06-02 20:04:57.000000000","message":"Unless I\u0027m missing something that sentence is well over 52 characters. I think this would fit: \"Don\u0027t send heartbeat if manager has driver problems\"","commit_id":"374dcd3081c107f6a6d8ffe0480f54eaf6fa6b60"},{"author":{"_account_id":7173,"name":"Scott DAngelo","email":"scott.dangelo@gmail.com","username":"scottda"},"change_message_id":"ae4a74ff9fe0d3c809ba7d3ada1ec4b83802d0cc","unresolved":false,"context_lines":[{"line_number":12,"context_line":"Manager will return False from is_working."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"The method is implemented in c-vol Manager to return initialization"},{"line_number":15,"context_line":"state of the underlying volume driver."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Closes-Bug: 1446750"},{"line_number":18,"context_line":"DocImpact"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"5afe65bd_dd677db5","line":15,"updated":"2015-06-04 16:11:00.000000000","message":"Perhaps change to:\n...return initialization state of the underlying volume driver, in the case of the default implementation. The driver can override the is_working function to add more checks.","commit_id":"2a84ae0fc015a43617521727ca31152066839d3c"}],"cinder/manager.py":[{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"50bdaacc0b2a2b2a58de2824bcf7d78af949e119","unresolved":false,"context_lines":[{"line_number":112,"context_line":"            config[key] \u003d CONF.get(key, None)"},{"line_number":113,"context_line":"        return config"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def is_up(self):"},{"line_number":116,"context_line":"        \"\"\"Method indicating if service is running."},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        This method is supposed to be overwritten by subclasses and return"}],"source_content_type":"text/x-python","patch_set":1,"id":"5afe65bd_43978b90","line":115,"updated":"2015-06-02 18:46:16.000000000","message":"I\u0027d prefer to use \u0027working\u0027 instead of \u0027up\u0027, because we have already \u0027service_is_up\u0027 helper function in cinder/utils.py and \u0027up\u0027 has the meaning of \u0027service is able to update its status to DB\u0027 since long time ago.  I don\u0027t want to change that.","commit_id":"374dcd3081c107f6a6d8ffe0480f54eaf6fa6b60"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"2e8a23ebb009259c1944ca0f93ad1b331aea6a1b","unresolved":false,"context_lines":[{"line_number":112,"context_line":"            config[key] \u003d CONF.get(key, None)"},{"line_number":113,"context_line":"        return config"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def is_up(self):"},{"line_number":116,"context_line":"        \"\"\"Method indicating if service is running."},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        This method is supposed to be overwritten by subclasses and return"}],"source_content_type":"text/x-python","patch_set":1,"id":"5afe65bd_1eff93ab","line":115,"in_reply_to":"5afe65bd_143f8bb5","updated":"2015-06-03 09:20:55.000000000","message":"IMO this is not a responsibility of the manager to tell that, so I\u0027ve rephrased the docstring.","commit_id":"374dcd3081c107f6a6d8ffe0480f54eaf6fa6b60"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"2e8a23ebb009259c1944ca0f93ad1b331aea6a1b","unresolved":false,"context_lines":[{"line_number":112,"context_line":"            config[key] \u003d CONF.get(key, None)"},{"line_number":113,"context_line":"        return config"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def is_up(self):"},{"line_number":116,"context_line":"        \"\"\"Method indicating if service is running."},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        This method is supposed to be overwritten by subclasses and return"}],"source_content_type":"text/x-python","patch_set":1,"id":"5afe65bd_5e6e2b97","line":115,"in_reply_to":"5afe65bd_43978b90","updated":"2015-06-03 09:20:55.000000000","message":"Done","commit_id":"374dcd3081c107f6a6d8ffe0480f54eaf6fa6b60"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c95aeaf26d9ca31afa659598fb556c02aa76bed5","unresolved":false,"context_lines":[{"line_number":112,"context_line":"            config[key] \u003d CONF.get(key, None)"},{"line_number":113,"context_line":"        return config"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def is_up(self):"},{"line_number":116,"context_line":"        \"\"\"Method indicating if service is running."},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        This method is supposed to be overwritten by subclasses and return"}],"source_content_type":"text/x-python","patch_set":1,"id":"5afe65bd_143f8bb5","line":115,"in_reply_to":"5afe65bd_43978b90","updated":"2015-06-02 20:04:57.000000000","message":"What about \"service_is_running\"? After all it\u0027s what the docstring says it indicates.","commit_id":"374dcd3081c107f6a6d8ffe0480f54eaf6fa6b60"},{"author":{"_account_id":7173,"name":"Scott DAngelo","email":"scott.dangelo@gmail.com","username":"scottda"},"change_message_id":"65d225891646d514021df67acf020a9e71655720","unresolved":false,"context_lines":[{"line_number":118,"context_line":"        This method is supposed to be overwritten by subclasses and return"},{"line_number":119,"context_line":"        if manager internal state is considered fine."},{"line_number":120,"context_line":"        \"\"\""},{"line_number":121,"context_line":"        return True"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"class SchedulerDependentManager(Manager):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5afe65bd_204feabe","line":121,"updated":"2015-06-02 16:50:19.000000000","message":"Should just \"pass\" if subclasses are supposed to override.\nReturning True in parent class will mean child does not have to override. Is this what you intended?","commit_id":"374dcd3081c107f6a6d8ffe0480f54eaf6fa6b60"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"2e8a23ebb009259c1944ca0f93ad1b331aea6a1b","unresolved":false,"context_lines":[{"line_number":118,"context_line":"        This method is supposed to be overwritten by subclasses and return"},{"line_number":119,"context_line":"        if manager internal state is considered fine."},{"line_number":120,"context_line":"        \"\"\""},{"line_number":121,"context_line":"        return True"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"class SchedulerDependentManager(Manager):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5afe65bd_7e21e74c","line":121,"in_reply_to":"5afe65bd_1438abd2","updated":"2015-06-03 09:20:55.000000000","message":"Exactly.","commit_id":"374dcd3081c107f6a6d8ffe0480f54eaf6fa6b60"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"2e8a23ebb009259c1944ca0f93ad1b331aea6a1b","unresolved":false,"context_lines":[{"line_number":118,"context_line":"        This method is supposed to be overwritten by subclasses and return"},{"line_number":119,"context_line":"        if manager internal state is considered fine."},{"line_number":120,"context_line":"        \"\"\""},{"line_number":121,"context_line":"        return True"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"class SchedulerDependentManager(Manager):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5afe65bd_3eaecf85","line":121,"in_reply_to":"5afe65bd_204feabe","updated":"2015-06-03 09:20:55.000000000","message":"Basically this way it will involve less changes - we won\u0027t need to change every manager class. Once similar mechanism can be used by other services any of them will just need to overload this method. (+I\u0027ve changed overwritten to overloaded in docstring).","commit_id":"374dcd3081c107f6a6d8ffe0480f54eaf6fa6b60"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c95aeaf26d9ca31afa659598fb556c02aa76bed5","unresolved":false,"context_lines":[{"line_number":118,"context_line":"        This method is supposed to be overwritten by subclasses and return"},{"line_number":119,"context_line":"        if manager internal state is considered fine."},{"line_number":120,"context_line":"        \"\"\""},{"line_number":121,"context_line":"        return True"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"class SchedulerDependentManager(Manager):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5afe65bd_1438abd2","line":121,"in_reply_to":"5afe65bd_204feabe","updated":"2015-06-02 20:04:57.000000000","message":"True means that if subclasses don\u0027t implement it it\u0027ll behave as it is now. Which I think is the sensible thing to do.","commit_id":"374dcd3081c107f6a6d8ffe0480f54eaf6fa6b60"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"e16d977daf6ad9181fccd318c2d9fa26fce7000d","unresolved":false,"context_lines":[{"line_number":115,"context_line":"    def is_working(self):"},{"line_number":116,"context_line":"        \"\"\"Method indicating if service is working correctly."},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        This method is supposed to be overloaded by subclasses and return if"},{"line_number":119,"context_line":"        manager\u0027s internal state is considered healthy."},{"line_number":120,"context_line":"        \"\"\""},{"line_number":121,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"5afe65bd_044e226f","line":118,"updated":"2015-06-03 09:28:54.000000000","message":"I think the word you want here is \u0027overriden\u0027, not \u0027overloaded\u0027.\n\nhttps://en.wikipedia.org/wiki/Method_overriding","commit_id":"7e3cd08ac50dcd7f89d3425967fef165def8f0ea"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"fd97237c131ee40f6192e8b8d68ea8fd68a30b9a","unresolved":false,"context_lines":[{"line_number":115,"context_line":"    def is_working(self):"},{"line_number":116,"context_line":"        \"\"\"Method indicating if service is working correctly."},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        This method is supposed to be overloaded by subclasses and return if"},{"line_number":119,"context_line":"        manager\u0027s internal state is considered healthy."},{"line_number":120,"context_line":"        \"\"\""},{"line_number":121,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"5afe65bd_0413c2b9","line":118,"in_reply_to":"5afe65bd_044e226f","updated":"2015-06-03 09:43:01.000000000","message":"I agree","commit_id":"7e3cd08ac50dcd7f89d3425967fef165def8f0ea"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"55b96bee50257cb59601f3d241275b78dbbf84f0","unresolved":false,"context_lines":[{"line_number":115,"context_line":"    def is_working(self):"},{"line_number":116,"context_line":"        \"\"\"Method indicating if service is working correctly."},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        This method is supposed to be overloaded by subclasses and return if"},{"line_number":119,"context_line":"        manager\u0027s internal state is considered healthy."},{"line_number":120,"context_line":"        \"\"\""},{"line_number":121,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"5afe65bd_e4666628","line":118,"in_reply_to":"5afe65bd_044e226f","updated":"2015-06-03 10:03:11.000000000","message":"I\u0027ve confused this with operator overloading probably.\n\nAnyway - done.","commit_id":"7e3cd08ac50dcd7f89d3425967fef165def8f0ea"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"d1205483994f481bdae959a2ff6f534af4393169","unresolved":false,"context_lines":[{"line_number":112,"context_line":"            config[key] \u003d CONF.get(key, None)"},{"line_number":113,"context_line":"        return config"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def is_working(self):"},{"line_number":116,"context_line":"        \"\"\"Method indicating if service is working correctly."},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        This method is supposed to be overriden by subclasses and return if"}],"source_content_type":"text/x-python","patch_set":3,"id":"5afe65bd_55d726bf","line":115,"updated":"2015-06-03 11:00:23.000000000","message":"Can we rename this to is_healthy which looks like a better name after reading the method description?","commit_id":"4860699b4f64bc261985a5aa748f1181a2b91db0"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"ad33643c7d52b965207d14164959fe6733ee1956","unresolved":false,"context_lines":[{"line_number":112,"context_line":"            config[key] \u003d CONF.get(key, None)"},{"line_number":113,"context_line":"        return config"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def is_working(self):"},{"line_number":116,"context_line":"        \"\"\"Method indicating if service is working correctly."},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        This method is supposed to be overriden by subclasses and return if"}],"source_content_type":"text/x-python","patch_set":3,"id":"5afe65bd_5d77483a","line":115,"in_reply_to":"5afe65bd_30e6f8b4","updated":"2015-06-03 12:51:38.000000000","message":"I\u0027ve tweaked the docstring.","commit_id":"4860699b4f64bc261985a5aa748f1181a2b91db0"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"3590146f8acc736feaf5b7b3ee17636d7c07d729","unresolved":false,"context_lines":[{"line_number":112,"context_line":"            config[key] \u003d CONF.get(key, None)"},{"line_number":113,"context_line":"        return config"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def is_working(self):"},{"line_number":116,"context_line":"        \"\"\"Method indicating if service is working correctly."},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        This method is supposed to be overriden by subclasses and return if"}],"source_content_type":"text/x-python","patch_set":3,"id":"5afe65bd_04673d26","line":115,"in_reply_to":"5afe65bd_30e6f8b4","updated":"2015-06-03 11:56:57.000000000","message":"It\u0027s not my patch, but thank you for the compliment  ;-)\n\nYou are right, it is in the docstring, and I wouldn\u0027t oppose to the name change, but I think the name is fine as it is.","commit_id":"4860699b4f64bc261985a5aa748f1181a2b91db0"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"4d7a9c49dfb86575eeeecc56721a61f32863e77a","unresolved":false,"context_lines":[{"line_number":112,"context_line":"            config[key] \u003d CONF.get(key, None)"},{"line_number":113,"context_line":"        return config"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def is_working(self):"},{"line_number":116,"context_line":"        \"\"\"Method indicating if service is working correctly."},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        This method is supposed to be overriden by subclasses and return if"}],"source_content_type":"text/x-python","patch_set":3,"id":"5afe65bd_30e6f8b4","line":115,"in_reply_to":"5afe65bd_5078542b","updated":"2015-06-03 11:30:36.000000000","message":"My comment was based on your method description.\n\n\u003e manager\u0027s internal state is considered healthy.","commit_id":"4860699b4f64bc261985a5aa748f1181a2b91db0"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6619d63e692a5d350cc29e6e1b032b6d9c8b8c6c","unresolved":false,"context_lines":[{"line_number":112,"context_line":"            config[key] \u003d CONF.get(key, None)"},{"line_number":113,"context_line":"        return config"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def is_working(self):"},{"line_number":116,"context_line":"        \"\"\"Method indicating if service is working correctly."},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        This method is supposed to be overriden by subclasses and return if"}],"source_content_type":"text/x-python","patch_set":3,"id":"5afe65bd_5078542b","line":115,"in_reply_to":"5afe65bd_55d726bf","updated":"2015-06-03 11:20:36.000000000","message":"I think working is better, as health is a relative term. For some it could mean that the service is working and the workload/response time is good.","commit_id":"4860699b4f64bc261985a5aa748f1181a2b91db0"}],"cinder/service.py":[{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"d1205483994f481bdae959a2ff6f534af4393169","unresolved":false,"context_lines":[{"line_number":290,"context_line":"        if not self.manager.is_working():"},{"line_number":291,"context_line":"            # NOTE(dulek): If manager reports a problem we\u0027re not sending"},{"line_number":292,"context_line":"            # heartbeats - to indicate that service is actually down."},{"line_number":293,"context_line":"            return"},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":296,"context_line":"        zone \u003d CONF.storage_availability_zone"}],"source_content_type":"text/x-python","patch_set":3,"id":"5afe65bd_f595f2e3","line":293,"updated":"2015-06-03 11:00:23.000000000","message":"We should log this.","commit_id":"4860699b4f64bc261985a5aa748f1181a2b91db0"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"ad33643c7d52b965207d14164959fe6733ee1956","unresolved":false,"context_lines":[{"line_number":290,"context_line":"        if not self.manager.is_working():"},{"line_number":291,"context_line":"            # NOTE(dulek): If manager reports a problem we\u0027re not sending"},{"line_number":292,"context_line":"            # heartbeats - to indicate that service is actually down."},{"line_number":293,"context_line":"            return"},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":296,"context_line":"        zone \u003d CONF.storage_availability_zone"}],"source_content_type":"text/x-python","patch_set":3,"id":"5afe65bd_7d91e451","line":293,"in_reply_to":"5afe65bd_90b7ecf4","updated":"2015-06-03 12:51:38.000000000","message":"I actually agree, we should log that there are problems. I\u0027ve tested it and log entry comes every 10 seconds. That\u0027s a lot less than for example when AMQP fails.","commit_id":"4860699b4f64bc261985a5aa748f1181a2b91db0"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"9441a4f965214642e570f268b24a0d4d51a540c6","unresolved":false,"context_lines":[{"line_number":290,"context_line":"        if not self.manager.is_working():"},{"line_number":291,"context_line":"            # NOTE(dulek): If manager reports a problem we\u0027re not sending"},{"line_number":292,"context_line":"            # heartbeats - to indicate that service is actually down."},{"line_number":293,"context_line":"            return"},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":296,"context_line":"        zone \u003d CONF.storage_availability_zone"}],"source_content_type":"text/x-python","patch_set":3,"id":"5afe65bd_b09f8802","line":293,"in_reply_to":"5afe65bd_90b7ecf4","updated":"2015-06-03 11:32:14.000000000","message":"Ok got it.","commit_id":"4860699b4f64bc261985a5aa748f1181a2b91db0"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"ad33643c7d52b965207d14164959fe6733ee1956","unresolved":false,"context_lines":[{"line_number":290,"context_line":"        if not self.manager.is_working():"},{"line_number":291,"context_line":"            # NOTE(dulek): If manager reports a problem we\u0027re not sending"},{"line_number":292,"context_line":"            # heartbeats - to indicate that service is actually down."},{"line_number":293,"context_line":"            return"},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":296,"context_line":"        zone \u003d CONF.storage_availability_zone"}],"source_content_type":"text/x-python","patch_set":3,"id":"5afe65bd_5dc98877","line":293,"in_reply_to":"5afe65bd_f595f2e3","updated":"2015-06-03 12:51:38.000000000","message":"Done","commit_id":"4860699b4f64bc261985a5aa748f1181a2b91db0"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6619d63e692a5d350cc29e6e1b032b6d9c8b8c6c","unresolved":false,"context_lines":[{"line_number":290,"context_line":"        if not self.manager.is_working():"},{"line_number":291,"context_line":"            # NOTE(dulek): If manager reports a problem we\u0027re not sending"},{"line_number":292,"context_line":"            # heartbeats - to indicate that service is actually down."},{"line_number":293,"context_line":"            return"},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":296,"context_line":"        zone \u003d CONF.storage_availability_zone"}],"source_content_type":"text/x-python","patch_set":3,"id":"5afe65bd_90b7ecf4","line":293,"in_reply_to":"5afe65bd_f595f2e3","updated":"2015-06-03 11:20:36.000000000","message":"I don\u0027t think that\u0027s such a great idea, because it would fill the logs since this is called in a loop...\n\nOn the other hand that would make it hard to miss in the logs. Although I would go with logging in the is_working method since it will have relevant information on what\u0027s going on.","commit_id":"4860699b4f64bc261985a5aa748f1181a2b91db0"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"171c4d128df03518319f0c83f24e69b758ef9da5","unresolved":false,"context_lines":[{"line_number":290,"context_line":"        if not self.manager.is_working():"},{"line_number":291,"context_line":"            # NOTE(dulek): If manager reports a problem we\u0027re not sending"},{"line_number":292,"context_line":"            # heartbeats - to indicate that service is actually down."},{"line_number":293,"context_line":"            LOG.error(_LE(\u0027Manager is reporting problems, skipping sending \u0027"},{"line_number":294,"context_line":"                          \u0027heartbeat. Service will appear \"down\".\u0027))"},{"line_number":295,"context_line":"            return"},{"line_number":296,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5afe65bd_09a2aeae","line":293,"updated":"2015-06-03 13:59:36.000000000","message":"Shouldn\u0027t we specify which manager is having problems (the class or something)?","commit_id":"0d56e32a5ee9244b3ac4e02f53ae9d29ad23041d"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"c1acc8588e97e9684fde59f733e54e3177eafacf","unresolved":false,"context_lines":[{"line_number":290,"context_line":"        if not self.manager.is_working():"},{"line_number":291,"context_line":"            # NOTE(dulek): If manager reports a problem we\u0027re not sending"},{"line_number":292,"context_line":"            # heartbeats - to indicate that service is actually down."},{"line_number":293,"context_line":"            LOG.error(_LE(\u0027Manager is reporting problems, skipping sending \u0027"},{"line_number":294,"context_line":"                          \u0027heartbeat. Service will appear \"down\".\u0027))"},{"line_number":295,"context_line":"            return"},{"line_number":296,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5afe65bd_261fd060","line":293,"in_reply_to":"5afe65bd_09a2aeae","updated":"2015-06-03 16:22:56.000000000","message":"s/skipping/skip/\n\nAccording to the commit message, this is implemented in the c-vol manager.  But I agree with Gorka, knowing the specific manager is helpful.  Probably have to look up the service in the db and use the binary column?","commit_id":"0d56e32a5ee9244b3ac4e02f53ae9d29ad23041d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c6a5e475bbcc9889e0eb6a6ce155a7abd5c8fe0b","unresolved":false,"context_lines":[{"line_number":290,"context_line":"        if not self.manager.is_working():"},{"line_number":291,"context_line":"            # NOTE(dulek): If manager reports a problem we\u0027re not sending"},{"line_number":292,"context_line":"            # heartbeats - to indicate that service is actually down."},{"line_number":293,"context_line":"            LOG.error(_LE(\u0027Manager is reporting problems, skipping sending \u0027"},{"line_number":294,"context_line":"                          \u0027heartbeat. Service will appear \"down\".\u0027))"},{"line_number":295,"context_line":"            return"},{"line_number":296,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5afe65bd_1acb0874","line":293,"in_reply_to":"5afe65bd_261fd060","updated":"2015-06-03 16:56:33.000000000","message":"I may be too lazy, but I was thinking we could use six.text_type(self.manager_class_name)","commit_id":"0d56e32a5ee9244b3ac4e02f53ae9d29ad23041d"}],"cinder/tests/unit/test_service.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"fd97237c131ee40f6192e8b8d68ea8fd68a30b9a","unresolved":false,"context_lines":[{"line_number":201,"context_line":"            serv.manager.is_working \u003d mock.Mock(return_value\u003dFalse)"},{"line_number":202,"context_line":"            serv.start()"},{"line_number":203,"context_line":"            serv.report_state()"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"            self.assertFalse(mock_db.service_update.called)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    def test_service_with_long_report_interval(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5afe65bd_44f2dad3","line":204,"updated":"2015-06-03 09:43:01.000000000","message":"You should check as well that is_working has been called once.","commit_id":"7e3cd08ac50dcd7f89d3425967fef165def8f0ea"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"55b96bee50257cb59601f3d241275b78dbbf84f0","unresolved":false,"context_lines":[{"line_number":201,"context_line":"            serv.manager.is_working \u003d mock.Mock(return_value\u003dFalse)"},{"line_number":202,"context_line":"            serv.start()"},{"line_number":203,"context_line":"            serv.report_state()"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"            self.assertFalse(mock_db.service_update.called)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    def test_service_with_long_report_interval(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5afe65bd_4fc6734a","line":204,"in_reply_to":"5afe65bd_44f2dad3","updated":"2015-06-03 10:03:11.000000000","message":"Done, I should not care in the test if method was called once or multiple times, but this seem like the easiest way to check.","commit_id":"7e3cd08ac50dcd7f89d3425967fef165def8f0ea"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"c5421c1c18eaabf67ed5379f45494caaeeb40c7d","unresolved":false,"context_lines":[{"line_number":201,"context_line":"            serv.manager.is_working \u003d mock.Mock(return_value\u003dFalse)"},{"line_number":202,"context_line":"            serv.start()"},{"line_number":203,"context_line":"            serv.report_state()"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"            self.assertFalse(mock_db.service_update.called)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    def test_service_with_long_report_interval(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5afe65bd_cad6d126","line":204,"in_reply_to":"5afe65bd_4fc6734a","updated":"2015-06-03 10:20:23.000000000","message":"If you want, you can assertTrue that serv.manager.is_working.call_count \u003e 0.","commit_id":"7e3cd08ac50dcd7f89d3425967fef165def8f0ea"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6ef538abe41ad89904992b082488875810118217","unresolved":false,"context_lines":[{"line_number":201,"context_line":"            serv.manager.is_working \u003d mock.Mock(return_value\u003dFalse)"},{"line_number":202,"context_line":"            serv.start()"},{"line_number":203,"context_line":"            serv.report_state()"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"            self.assertFalse(mock_db.service_update.called)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    def test_service_with_long_report_interval(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5afe65bd_2a21e5fb","line":204,"in_reply_to":"5afe65bd_cad6d126","updated":"2015-06-03 10:30:04.000000000","message":"That would be equivalent to\n\n self.assertTrue(serv.manager.is_working.called)\n\nBTW, I like how it\u0027s done in patch 3 because you ensure that it\u0027s only been called once and with no arguments, confirming that there will be no run exception caused by the number of arguments passed.","commit_id":"7e3cd08ac50dcd7f89d3425967fef165def8f0ea"}],"cinder/volume/manager.py":[{"author":{"_account_id":7173,"name":"Scott DAngelo","email":"scott.dangelo@gmail.com","username":"scottda"},"change_message_id":"65d225891646d514021df67acf020a9e71655720","unresolved":false,"context_lines":[{"line_number":394,"context_line":"                 resource\u003d{\u0027type\u0027: \u0027driver\u0027,"},{"line_number":395,"context_line":"                           \u0027id\u0027: self.driver.__class__.__name__})"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"    def is_up(self):"},{"line_number":398,"context_line":"        \"\"\"Indicates if there are problems with driver initialization."},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"        This is to inform Service class that the manager is actually down and"}],"source_content_type":"text/x-python","patch_set":1,"id":"5afe65bd_a09f5a02","line":397,"updated":"2015-06-02 16:50:19.000000000","message":"If this is specific, should it be called:\nis_driver_initialized()?","commit_id":"374dcd3081c107f6a6d8ffe0480f54eaf6fa6b60"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"2e8a23ebb009259c1944ca0f93ad1b331aea6a1b","unresolved":false,"context_lines":[{"line_number":394,"context_line":"                 resource\u003d{\u0027type\u0027: \u0027driver\u0027,"},{"line_number":395,"context_line":"                           \u0027id\u0027: self.driver.__class__.__name__})"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"    def is_up(self):"},{"line_number":398,"context_line":"        \"\"\"Indicates if there are problems with driver initialization."},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"        This is to inform Service class that the manager is actually down and"}],"source_content_type":"text/x-python","patch_set":1,"id":"5afe65bd_3e780ff2","line":397,"in_reply_to":"5afe65bd_54b063f4","updated":"2015-06-03 09:20:55.000000000","message":"Exactly - Winston already commented that he sees other use cases so I want to keep this generic to make sure any criteria can be used to decide if manager is running correctly.","commit_id":"374dcd3081c107f6a6d8ffe0480f54eaf6fa6b60"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c95aeaf26d9ca31afa659598fb556c02aa76bed5","unresolved":false,"context_lines":[{"line_number":394,"context_line":"                 resource\u003d{\u0027type\u0027: \u0027driver\u0027,"},{"line_number":395,"context_line":"                           \u0027id\u0027: self.driver.__class__.__name__})"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"    def is_up(self):"},{"line_number":398,"context_line":"        \"\"\"Indicates if there are problems with driver initialization."},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"        This is to inform Service class that the manager is actually down and"}],"source_content_type":"text/x-python","patch_set":1,"id":"5afe65bd_54b063f4","line":397,"in_reply_to":"5afe65bd_a09f5a02","updated":"2015-06-02 20:04:57.000000000","message":"Am I correct to assume that this method could, in the future, return False in other cases besides when the driver is not initialized?","commit_id":"374dcd3081c107f6a6d8ffe0480f54eaf6fa6b60"}]}