)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"a5134632d58e31eaccae4b699108176809dd193b","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Simplify the validation of required fields of pipeline source"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"- The required field \"meters\" (or \"counters\") validation is invalid"},{"line_number":10,"context_line":"- The validation of these required fields of SampleSource and EventSource"},{"line_number":11,"context_line":"  are unnecessary"},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5a2ca52d_beaf9d93","line":9,"updated":"2015-10-13 10:29:32.000000000","message":"invalid? what do you mean? the meters field is not required?","commit_id":"a04da91e03dc4636dba725691d6542a9d8c0621d"},{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"dbc2c390d47689219cd8b0c4001eeb0812b4286f","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Simplify the validation of required fields of pipeline source"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"- The required field \"meters\" (or \"counters\") validation is invalid"},{"line_number":10,"context_line":"- The validation of these required fields of SampleSource and EventSource"},{"line_number":11,"context_line":"  are unnecessary"},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5a2ca52d_e9b55dd0","line":9,"in_reply_to":"5a2ca52d_beaf9d93","updated":"2015-10-13 11:14:21.000000000","message":"I mean the following code is invalid, because the KeyError will never be caused.\n\n        try:\n            # Support \u0027counters\u0027 for backward compatibility\n            self.meters \u003d cfg.get(\u0027meters\u0027, cfg.get(\u0027counters\u0027))\n        except KeyError as err:\n            raise PipelineException(\n                \"Required field %s not specified\" % err.args[0], cfg)","commit_id":"a04da91e03dc4636dba725691d6542a9d8c0621d"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"d8867239ef437606fab6d34c8fdac3a5fdbff421","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Simplify the validation of required fields of pipeline source"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"- The required field \"meters\" (or \"counters\") validation is invalid"},{"line_number":10,"context_line":"- The validation of these required fields of SampleSource and EventSource"},{"line_number":11,"context_line":"  are unnecessary"},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5a2ca52d_8fadc933","line":9,"in_reply_to":"5a2ca52d_e9b55dd0","updated":"2015-10-13 11:33:54.000000000","message":"I think it is just unnecessary, anyway, the code looks good","commit_id":"a04da91e03dc4636dba725691d6542a9d8c0621d"}],"ceilometer/pipeline.py":[{"author":{"_account_id":13560,"name":"Rohit Jaiswal","email":"rohit.jaiswal@gmail.com","username":"rjaiswal"},"change_message_id":"8f91e1fc90fcae1ab50f75b029fae4a5c62d79be","unresolved":false,"context_lines":[{"line_number":293,"context_line":""},{"line_number":294,"context_line":"    def __init__(self, cfg):"},{"line_number":295,"context_line":"        super(EventSource, self).__init__(cfg)"},{"line_number":296,"context_line":"        self.events \u003d cfg.get(\u0027events\u0027)"},{"line_number":297,"context_line":"        self.check_source_filtering(self.events, \u0027events\u0027)"},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"    def support_event(self, event_name):"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a2fa921_f74d08c5","line":296,"updated":"2015-10-13 04:22:18.000000000","message":"I think it will still fail at https://github.com/openstack/ceilometer/blob/master/ceilometer/pipeline.py#L258, so its still required. why not fail explicitly?","commit_id":"a04da91e03dc4636dba725691d6542a9d8c0621d"},{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"e237c03c969e445f67ea509b99b1b536887cf89a","unresolved":false,"context_lines":[{"line_number":293,"context_line":""},{"line_number":294,"context_line":"    def __init__(self, cfg):"},{"line_number":295,"context_line":"        super(EventSource, self).__init__(cfg)"},{"line_number":296,"context_line":"        self.events \u003d cfg.get(\u0027events\u0027)"},{"line_number":297,"context_line":"        self.check_source_filtering(self.events, \u0027events\u0027)"},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"    def support_event(self, event_name):"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a2fa921_4f6a201c","line":296,"in_reply_to":"7a2fa921_f74d08c5","updated":"2015-10-13 06:25:04.000000000","message":"yeah, you are right, it will be fail in check_source_filtering(). hmmm, I think the exception messaging in check_source_filtering() is explicit enough, what do you think :)","commit_id":"a04da91e03dc4636dba725691d6542a9d8c0621d"},{"author":{"_account_id":13560,"name":"Rohit Jaiswal","email":"rohit.jaiswal@gmail.com","username":"rjaiswal"},"change_message_id":"8f91e1fc90fcae1ab50f75b029fae4a5c62d79be","unresolved":false,"context_lines":[{"line_number":312,"context_line":"    def __init__(self, cfg):"},{"line_number":313,"context_line":"        super(SampleSource, self).__init__(cfg)"},{"line_number":314,"context_line":"        # Support \u0027counters\u0027 for backward compatibility"},{"line_number":315,"context_line":"        self.meters \u003d cfg.get(\u0027meters\u0027, cfg.get(\u0027counters\u0027))"},{"line_number":316,"context_line":"        try:"},{"line_number":317,"context_line":"            self.interval \u003d int(cfg.get(\u0027interval\u0027, 600))"},{"line_number":318,"context_line":"        except ValueError:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a2fa921_b7ed5090","line":315,"updated":"2015-10-13 04:22:18.000000000","message":"ditto","commit_id":"a04da91e03dc4636dba725691d6542a9d8c0621d"},{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"e237c03c969e445f67ea509b99b1b536887cf89a","unresolved":false,"context_lines":[{"line_number":312,"context_line":"    def __init__(self, cfg):"},{"line_number":313,"context_line":"        super(SampleSource, self).__init__(cfg)"},{"line_number":314,"context_line":"        # Support \u0027counters\u0027 for backward compatibility"},{"line_number":315,"context_line":"        self.meters \u003d cfg.get(\u0027meters\u0027, cfg.get(\u0027counters\u0027))"},{"line_number":316,"context_line":"        try:"},{"line_number":317,"context_line":"            self.interval \u003d int(cfg.get(\u0027interval\u0027, 600))"},{"line_number":318,"context_line":"        except ValueError:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a2fa921_efc3ec36","line":315,"in_reply_to":"7a2fa921_b7ed5090","updated":"2015-10-13 06:25:04.000000000","message":"you can see the cfg.get(\u0027meters\u0027, cfg.get(\u0027counters\u0027)) will never cause KeyError, so this try is invalid, so just remove the try...except, and it can be checked in check_source_filtering() method too.","commit_id":"a04da91e03dc4636dba725691d6542a9d8c0621d"}]}