)]}'
{"neutron/db/agents_db.py":[{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"11af7e6bfbb0abcd15e0eb677067c57865d6cb30","unresolved":false,"context_lines":[{"line_number":220,"context_line":"    def get_configuration_dict(self, agent_db):"},{"line_number":221,"context_line":"        return self._get_dict(agent_db, \u0027configurations\u0027)"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"    def _get_dict(self, agent_db, dict_name, ignore_missing\u003dFalse):"},{"line_number":224,"context_line":"        try:"},{"line_number":225,"context_line":"            dictionary \u003d getattr(agent_db, dict_name, None)"},{"line_number":226,"context_line":"            if dictionary is None and ignore_missing:"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_04f25f6f","line":223,"range":{"start_line":223,"start_character":43,"end_line":223,"end_character":65},"updated":"2016-04-18 11:07:05.000000000","message":"\u0027ignore_missing\u0027 seems to be too specific. Could you instead mimic the \u0027getattr\u0027 \u0027default\u0027 behaviour (or e.g. \u0027default\u003dNone\u0027)? Then use \u0027default\u003d{}\u0027 in place of \u0027ignore_missing\u003dTrue\u0027.","commit_id":"5f82abf00e13cdd7a303211abb2e3d1c3d85ef60"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"cabe5aa0987832306d4c69682e0829d9cd774219","unresolved":false,"context_lines":[{"line_number":224,"context_line":"        try:"},{"line_number":225,"context_line":"            dictionary \u003d getattr(agent_db, dict_name, None)"},{"line_number":226,"context_line":"            if dictionary is None and ignore_missing:"},{"line_number":227,"context_line":"                return"},{"line_number":228,"context_line":"            conf \u003d jsonutils.loads(dictionary)"},{"line_number":229,"context_line":"        except Exception:"},{"line_number":230,"context_line":"            msg \u003d _LW(\u0027Dictionary %(dict_name)s for agent %(agent_type)s on \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_2f45b916","line":227,"range":{"start_line":227,"start_character":16,"end_line":227,"end_character":22},"updated":"2016-04-18 09:55:40.000000000","message":"Shouldn\u0027t we return {} here as in line 235?","commit_id":"5f82abf00e13cdd7a303211abb2e3d1c3d85ef60"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"11af7e6bfbb0abcd15e0eb677067c57865d6cb30","unresolved":false,"context_lines":[{"line_number":224,"context_line":"        try:"},{"line_number":225,"context_line":"            dictionary \u003d getattr(agent_db, dict_name, None)"},{"line_number":226,"context_line":"            if dictionary is None and ignore_missing:"},{"line_number":227,"context_line":"                return"},{"line_number":228,"context_line":"            conf \u003d jsonutils.loads(dictionary)"},{"line_number":229,"context_line":"        except Exception:"},{"line_number":230,"context_line":"            msg \u003d _LW(\u0027Dictionary %(dict_name)s for agent %(agent_type)s on \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_700f8e2e","line":227,"range":{"start_line":227,"start_character":16,"end_line":227,"end_character":22},"in_reply_to":"1a122d0e_2f45b916","updated":"2016-04-18 11:07:05.000000000","message":"Could you please remove this early return statement so we keep 1-return-at-the-end behaviour?","commit_id":"5f82abf00e13cdd7a303211abb2e3d1c3d85ef60"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"cabe5aa0987832306d4c69682e0829d9cd774219","unresolved":false,"context_lines":[{"line_number":225,"context_line":"            dictionary \u003d getattr(agent_db, dict_name, None)"},{"line_number":226,"context_line":"            if dictionary is None and ignore_missing:"},{"line_number":227,"context_line":"                return"},{"line_number":228,"context_line":"            conf \u003d jsonutils.loads(dictionary)"},{"line_number":229,"context_line":"        except Exception:"},{"line_number":230,"context_line":"            msg \u003d _LW(\u0027Dictionary %(dict_name)s for agent %(agent_type)s on \u0027"},{"line_number":231,"context_line":"                      \u0027host %(host)s is invalid.\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_cf6365db","line":228,"range":{"start_line":228,"start_character":19,"end_line":228,"end_character":46},"updated":"2016-04-18 09:55:40.000000000","message":"if ignore_missing \u003d\u003d False, and \u0027dictionary\u0027 is None, should we really try to .loads() it? I guess we are then should proceed to line 230 right away.","commit_id":"5f82abf00e13cdd7a303211abb2e3d1c3d85ef60"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"243a9187b08fc4ed82c2ef45d54e3a5ee379a8e2","unresolved":false,"context_lines":[{"line_number":227,"context_line":"            if dictionary is not None:"},{"line_number":228,"context_line":"                conf \u003d jsonutils.loads(dictionary)"},{"line_number":229,"context_line":"            elif not ignore_missing:"},{"line_number":230,"context_line":"                raise AttributeError()"},{"line_number":231,"context_line":"        except Exception:"},{"line_number":232,"context_line":"            msg \u003d _LW(\u0027Dictionary %(dict_name)s for agent %(agent_type)s on \u0027"},{"line_number":233,"context_line":"                      \u0027host %(host)s is invalid.\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_4e246eae","line":230,"range":{"start_line":230,"start_character":16,"end_line":230,"end_character":38},"updated":"2016-04-18 12:44:48.000000000","message":"That\u0027s ugly. You don\u0027t really have an attribute error anywhere here. ValueError would probably work a bit better.\n\nHow about:\n\nconf \u003d None\n\ntry:\n    ...\n    conf \u003d jsonutils.loads(...)\nexcept Exception:\n    pass\n\nif conf is None and not ignore_missing:\n    log.warning()\nreturn conf or {}","commit_id":"a123d15897b106f1de2db2710702b8840b0dcac0"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"635ccae0172858d49230b17ad8035a0e34fcc369","unresolved":false,"context_lines":[{"line_number":227,"context_line":"            if dictionary is not None:"},{"line_number":228,"context_line":"                conf \u003d jsonutils.loads(dictionary)"},{"line_number":229,"context_line":"            elif not ignore_missing:"},{"line_number":230,"context_line":"                raise AttributeError()"},{"line_number":231,"context_line":"        except Exception:"},{"line_number":232,"context_line":"            msg \u003d _LW(\u0027Dictionary %(dict_name)s for agent %(agent_type)s on \u0027"},{"line_number":233,"context_line":"                      \u0027host %(host)s is invalid.\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_708ca47f","line":230,"range":{"start_line":230,"start_character":16,"end_line":230,"end_character":38},"in_reply_to":"1a122d0e_2e193289","updated":"2016-04-18 13:36:33.000000000","message":"I think it would be much cleaner this way:\n    def _get_dict(self, agent_db, dict_name, default\u003dNone):\n        conf \u003d default\n        try:\n            conf \u003d jsonutils.loads(getattr(agent_db, dict_name))\n        except Exception:\n            if conf is None:\n                msg \u003d _LW(\u0027Dictionary %(dict_name)s for agent %(agent_type)s on \u0027\n                          \u0027host %(host)s is invalid.\u0027)\n                LOG.warning(msg, {\u0027dict_name\u0027: dict_name,\n                                  \u0027agent_type\u0027: agent_db.agent_type,\n                                  \u0027host\u0027: agent_db.host})\n                conf \u003d {}\n        return conf","commit_id":"a123d15897b106f1de2db2710702b8840b0dcac0"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"d86a08787c5a7c66e833c013f5c02530cd285c62","unresolved":false,"context_lines":[{"line_number":227,"context_line":"            if dictionary is not None:"},{"line_number":228,"context_line":"                conf \u003d jsonutils.loads(dictionary)"},{"line_number":229,"context_line":"            elif not ignore_missing:"},{"line_number":230,"context_line":"                raise AttributeError()"},{"line_number":231,"context_line":"        except Exception:"},{"line_number":232,"context_line":"            msg \u003d _LW(\u0027Dictionary %(dict_name)s for agent %(agent_type)s on \u0027"},{"line_number":233,"context_line":"                      \u0027host %(host)s is invalid.\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_aeb0a244","line":230,"range":{"start_line":230,"start_character":16,"end_line":230,"end_character":38},"in_reply_to":"1a122d0e_4e246eae","updated":"2016-04-18 12:46:12.000000000","message":"Well, it\u0027d actually be a KeyError, in fact.\n\nBut I like your version much better.","commit_id":"a123d15897b106f1de2db2710702b8840b0dcac0"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"ed9818bef72ad67917f92626e481c72fe95171c4","unresolved":false,"context_lines":[{"line_number":227,"context_line":"            if dictionary is not None:"},{"line_number":228,"context_line":"                conf \u003d jsonutils.loads(dictionary)"},{"line_number":229,"context_line":"            elif not ignore_missing:"},{"line_number":230,"context_line":"                raise AttributeError()"},{"line_number":231,"context_line":"        except Exception:"},{"line_number":232,"context_line":"            msg \u003d _LW(\u0027Dictionary %(dict_name)s for agent %(agent_type)s on \u0027"},{"line_number":233,"context_line":"                      \u0027host %(host)s is invalid.\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_2e193289","line":230,"range":{"start_line":230,"start_character":16,"end_line":230,"end_character":38},"in_reply_to":"1a122d0e_6ee96a80","updated":"2016-04-18 12:58:49.000000000","message":"Even cleaner:\n\nconf \u003d {}\ndictionary \u003d None\nlog_warning \u003d False\ntry:\n    dictionary \u003d getattr(agent_db, dict_name)\nexcept AttributeError:\n    if not ignore_missing:\n        log_warning \u003d True\nelse:\n    # All good, the dict is found\n    try:\n        ...\n        conf \u003d jsonutils.loads(...)\n    except Exception:\n        log_warning \u003d True\nif log_warning:\n    log.warning()\nreturn conf","commit_id":"a123d15897b106f1de2db2710702b8840b0dcac0"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"680a674c670578bdea6c0c02e89dab43ae50fbbe","unresolved":false,"context_lines":[{"line_number":227,"context_line":"            if dictionary is not None:"},{"line_number":228,"context_line":"                conf \u003d jsonutils.loads(dictionary)"},{"line_number":229,"context_line":"            elif not ignore_missing:"},{"line_number":230,"context_line":"                raise AttributeError()"},{"line_number":231,"context_line":"        except Exception:"},{"line_number":232,"context_line":"            msg \u003d _LW(\u0027Dictionary %(dict_name)s for agent %(agent_type)s on \u0027"},{"line_number":233,"context_line":"                      \u0027host %(host)s is invalid.\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_f0a2744a","line":230,"range":{"start_line":230,"start_character":16,"end_line":230,"end_character":38},"in_reply_to":"1a122d0e_708ca47f","updated":"2016-04-18 13:45:13.000000000","message":"Or if you insist on \u0027ignore_missing\u0027 semantics:\n    def _get_dict(self, agent_db, dict_name, ignore_missing\u003dFalse):\n        try:\n            conf \u003d jsonutils.loads(getattr(agent_db, dict_name))\n        except Exception:\n            if not ignore_missing:\n                msg \u003d _LW(\u0027Dictionary %(dict_name)s for agent %(agent_type)s on \u0027\n                          \u0027host %(host)s is invalid.\u0027)\n                LOG.warning(msg, {\u0027dict_name\u0027: dict_name,\n                                  \u0027agent_type\u0027: agent_db.agent_type,\n                                  \u0027host\u0027: agent_db.host})\n            conf \u003d {}\n        return conf","commit_id":"a123d15897b106f1de2db2710702b8840b0dcac0"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"821d13819b22c87121db097f5cee962ec4d694ff","unresolved":false,"context_lines":[{"line_number":227,"context_line":"            if dictionary is not None:"},{"line_number":228,"context_line":"                conf \u003d jsonutils.loads(dictionary)"},{"line_number":229,"context_line":"            elif not ignore_missing:"},{"line_number":230,"context_line":"                raise AttributeError()"},{"line_number":231,"context_line":"        except Exception:"},{"line_number":232,"context_line":"            msg \u003d _LW(\u0027Dictionary %(dict_name)s for agent %(agent_type)s on \u0027"},{"line_number":233,"context_line":"                      \u0027host %(host)s is invalid.\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_ee6d7a99","line":230,"range":{"start_line":230,"start_character":16,"end_line":230,"end_character":38},"in_reply_to":"1a122d0e_aeb0a244","updated":"2016-04-18 12:49:11.000000000","message":"Oh wait, your version misses one case (that the warning was supposed to catch).\n\nWhen ignore_missing \u003d True, but it\u0027s not missing but a broken dictionary that json is not able to decode.","commit_id":"a123d15897b106f1de2db2710702b8840b0dcac0"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5dd4ee147b85eb804a3f6f9faee29d9377f0106f","unresolved":false,"context_lines":[{"line_number":227,"context_line":"            if dictionary is not None:"},{"line_number":228,"context_line":"                conf \u003d jsonutils.loads(dictionary)"},{"line_number":229,"context_line":"            elif not ignore_missing:"},{"line_number":230,"context_line":"                raise AttributeError()"},{"line_number":231,"context_line":"        except Exception:"},{"line_number":232,"context_line":"            msg \u003d _LW(\u0027Dictionary %(dict_name)s for agent %(agent_type)s on \u0027"},{"line_number":233,"context_line":"                      \u0027host %(host)s is invalid.\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_6ee96a80","line":230,"range":{"start_line":230,"start_character":16,"end_line":230,"end_character":38},"in_reply_to":"1a122d0e_ee6d7a99","updated":"2016-04-18 12:56:05.000000000","message":"Then:\n\nconf \u003d {}\ndictionary \u003d None\nlog_warning \u003d False\n\ntry:\n    dictionary \u003d getattr(agent_db, dict_name)\nexcept AttributeError:\n    if not ignore_missing:\n        log_warning \u003d True\n\nif dictionary is not None:\n    try:\n        ...\n        conf \u003d jsonutils.loads(...)\n    except Exception:\n        log_warning \u003d True\n\nif log_warning:\n    log.warning()\n\nreturn conf","commit_id":"a123d15897b106f1de2db2710702b8840b0dcac0"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"156144fc070cfcdfb318424c9f053b5bbcdf9079","unresolved":false,"context_lines":[{"line_number":227,"context_line":"            if dictionary is not None:"},{"line_number":228,"context_line":"                conf \u003d jsonutils.loads(dictionary)"},{"line_number":229,"context_line":"            elif not ignore_missing:"},{"line_number":230,"context_line":"                raise AttributeError()"},{"line_number":231,"context_line":"        except Exception:"},{"line_number":232,"context_line":"            msg \u003d _LW(\u0027Dictionary %(dict_name)s for agent %(agent_type)s on \u0027"},{"line_number":233,"context_line":"                      \u0027host %(host)s is invalid.\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_73ee6eb5","line":230,"range":{"start_line":230,"start_character":16,"end_line":230,"end_character":38},"in_reply_to":"1a122d0e_f0a2744a","updated":"2016-04-18 13:56:27.000000000","message":"Illya, thanks for the suggestion, that misses the case when he dict data is not well formed (for some reason) and that the previous function was able to detect and still log.","commit_id":"a123d15897b106f1de2db2710702b8840b0dcac0"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"6a983e432356b49db30c4739ccc74ed00a68bf89","unresolved":false,"context_lines":[{"line_number":229,"context_line":"                              \u0027host\u0027: agent_db.host})"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"        conf \u003d {}"},{"line_number":232,"context_line":"        dictionary \u003d None"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        try:"},{"line_number":235,"context_line":"            dictionary \u003d getattr(agent_db, dict_name)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_61ffb8aa","line":232,"updated":"2016-04-18 14:21:16.000000000","message":"hmm, this var is useless in this implementation, fixing.","commit_id":"6359c342e7abe1c5b8ac0373365037d282e704d2"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"6a983e432356b49db30c4739ccc74ed00a68bf89","unresolved":false,"context_lines":[{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        try:"},{"line_number":235,"context_line":"            dictionary \u003d getattr(agent_db, dict_name)"},{"line_number":236,"context_line":"            conf \u003d jsonutils.loads(dictionary)"},{"line_number":237,"context_line":"        except AttributeError:"},{"line_number":238,"context_line":"            if not ignore_missing:"},{"line_number":239,"context_line":"                _log_warning()"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_0127e464","line":236,"updated":"2016-04-18 14:21:16.000000000","message":"those two can be a line now.","commit_id":"6359c342e7abe1c5b8ac0373365037d282e704d2"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"03e3241c48c1fa781dba86521d3317109d2b8bdf","unresolved":false,"context_lines":[{"line_number":231,"context_line":"        conf \u003d {}"},{"line_number":232,"context_line":"        try:"},{"line_number":233,"context_line":"            conf \u003d jsonutils.loads(getattr(agent_db, dict_name))"},{"line_number":234,"context_line":"        except AttributeError:"},{"line_number":235,"context_line":"            if not ignore_missing:"},{"line_number":236,"context_line":"                _log_warning()"},{"line_number":237,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a122d0e_61ee98b5","line":234,"range":{"start_line":234,"start_character":15,"end_line":234,"end_character":29},"updated":"2016-04-18 14:25:10.000000000","message":"And what if .loads() raises AttributeError? I guess it\u0027s better to split the matters.","commit_id":"3be02aae061dc5343cc586badc1c623efd578e35"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"d4e61140674ecd926829eb7a2292402407c996b3","unresolved":false,"context_lines":[{"line_number":231,"context_line":"        conf \u003d {}"},{"line_number":232,"context_line":"        try:"},{"line_number":233,"context_line":"            conf \u003d jsonutils.loads(getattr(agent_db, dict_name))"},{"line_number":234,"context_line":"        except AttributeError:"},{"line_number":235,"context_line":"            if not ignore_missing:"},{"line_number":236,"context_line":"                _log_warning()"},{"line_number":237,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a122d0e_67a9887b","line":234,"range":{"start_line":234,"start_character":15,"end_line":234,"end_character":29},"in_reply_to":"1a122d0e_61ee98b5","updated":"2016-04-18 14:59:44.000000000","message":"That gets overly complicated. This should cover the loads exception case:\n\n    def _get_dict(self, agent_db, dict_name, ignore_missing\u003dFalse):\n        json_value \u003d None\n        try:\n            json_value \u003d getattr(agent_db, dict_name)\n            conf \u003d jsonutils.loads(json_value)\n        except Exception:\n            if json_value or not ignore_missing:\n                ... warn ...\n            conf \u003d {}\n        return conf\n\nNote that it will also consider None and \"\" returned by getattr as missing (if such cases are even possible). If it is not intended, than you\u0027ll need double try-except as Ihar Hrachyshka suggested.\n\nAlso, I recommend renaming \u0027dictionary\u0027 variable, as it is a string containing json, but not a dict.","commit_id":"3be02aae061dc5343cc586badc1c623efd578e35"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"9ee498d5907f79ace8f1fae4ee701040688ab1f1","unresolved":false,"context_lines":[{"line_number":231,"context_line":"        conf \u003d {}"},{"line_number":232,"context_line":"        try:"},{"line_number":233,"context_line":"            conf \u003d jsonutils.loads(getattr(agent_db, dict_name))"},{"line_number":234,"context_line":"        except AttributeError:"},{"line_number":235,"context_line":"            if not ignore_missing:"},{"line_number":236,"context_line":"                _log_warning()"},{"line_number":237,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a122d0e_1980f445","line":234,"range":{"start_line":234,"start_character":15,"end_line":234,"end_character":29},"in_reply_to":"1a122d0e_61ee98b5","updated":"2016-04-18 15:45:48.000000000","message":"oh nitpickings!!, you\u0027re right, I was aware of that, but loads didn\u0027t seem to raise anything else but ValueError.","commit_id":"3be02aae061dc5343cc586badc1c623efd578e35"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"9ee498d5907f79ace8f1fae4ee701040688ab1f1","unresolved":false,"context_lines":[{"line_number":231,"context_line":"        conf \u003d {}"},{"line_number":232,"context_line":"        try:"},{"line_number":233,"context_line":"            conf \u003d jsonutils.loads(getattr(agent_db, dict_name))"},{"line_number":234,"context_line":"        except AttributeError:"},{"line_number":235,"context_line":"            if not ignore_missing:"},{"line_number":236,"context_line":"                _log_warning()"},{"line_number":237,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a122d0e_b9baa021","line":234,"range":{"start_line":234,"start_character":15,"end_line":234,"end_character":29},"in_reply_to":"1a122d0e_67a9887b","updated":"2016-04-18 15:45:48.000000000","message":"I like it short, your version is short and covers both cases.\n\n@ihar, what do you think?","commit_id":"3be02aae061dc5343cc586badc1c623efd578e35"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"648c77ca1b1468aa296f785b58ae962603ebc5c4","unresolved":false,"context_lines":[{"line_number":231,"context_line":"        conf \u003d {}"},{"line_number":232,"context_line":"        try:"},{"line_number":233,"context_line":"            conf \u003d jsonutils.loads(getattr(agent_db, dict_name))"},{"line_number":234,"context_line":"        except AttributeError:"},{"line_number":235,"context_line":"            if not ignore_missing:"},{"line_number":236,"context_line":"                _log_warning()"},{"line_number":237,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a122d0e_59c6ec49","line":234,"range":{"start_line":234,"start_character":15,"end_line":234,"end_character":29},"in_reply_to":"1a122d0e_b9baa021","updated":"2016-04-18 15:49:58.000000000","message":"I like Ilya\u0027s suggestion.","commit_id":"3be02aae061dc5343cc586badc1c623efd578e35"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"a041d06b40f483f2e40fc902d2e86a63e70b5f93","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        return self._get_dict(agent_db, \u0027configurations\u0027)"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"    def _get_dict(self, agent_db, dict_name, ignore_missing\u003dFalse):"},{"line_number":224,"context_line":"        conf \u003d {}"},{"line_number":225,"context_line":"        json_value \u003d None"},{"line_number":226,"context_line":"        try:"},{"line_number":227,"context_line":"            json_value \u003d getattr(agent_db, dict_name)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a122d0e_ac21c0fd","line":224,"range":{"start_line":224,"start_character":8,"end_line":224,"end_character":17},"updated":"2016-04-18 16:16:07.000000000","message":"nit: this is redundant. either try or except would set conf anyway","commit_id":"f85c2e5ba9719c7d8c35b53eaf39bd771c3a9567"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"fd3dd65521f45230e01ec831ea0b6182e8a92904","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        return self._get_dict(agent_db, \u0027configurations\u0027)"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"    def _get_dict(self, agent_db, dict_name, ignore_missing\u003dFalse):"},{"line_number":224,"context_line":"        conf \u003d {}"},{"line_number":225,"context_line":"        json_value \u003d None"},{"line_number":226,"context_line":"        try:"},{"line_number":227,"context_line":"            json_value \u003d getattr(agent_db, dict_name)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a122d0e_d7e8f3b6","line":224,"range":{"start_line":224,"start_character":8,"end_line":224,"end_character":17},"in_reply_to":"1a122d0e_ac21c0fd","updated":"2016-04-19 01:26:32.000000000","message":"I think Ilya is right\nYou are setting conf in Line 228 and Line 236.\n224 is not useful","commit_id":"f85c2e5ba9719c7d8c35b53eaf39bd771c3a9567"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"c11aaa63e7e2fd597f03a281398c0943605e2168","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        return self._get_dict(agent_db, \u0027configurations\u0027)"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"    def _get_dict(self, agent_db, dict_name, ignore_missing\u003dFalse):"},{"line_number":224,"context_line":"        conf \u003d {}"},{"line_number":225,"context_line":"        json_value \u003d None"},{"line_number":226,"context_line":"        try:"},{"line_number":227,"context_line":"            json_value \u003d getattr(agent_db, dict_name)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a122d0e_cb92079f","line":224,"range":{"start_line":224,"start_character":8,"end_line":224,"end_character":17},"in_reply_to":"1a122d0e_d7e8f3b6","updated":"2016-04-19 08:04:56.000000000","message":"True, Done","commit_id":"f85c2e5ba9719c7d8c35b53eaf39bd771c3a9567"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"407e797a7cb9cb44a5bc4cf2a2030a6dd8781cfc","unresolved":false,"context_lines":[{"line_number":227,"context_line":"            json_value \u003d getattr(agent_db, dict_name)"},{"line_number":228,"context_line":"            conf \u003d jsonutils.loads(json_value)"},{"line_number":229,"context_line":"        except Exception:"},{"line_number":230,"context_line":"            if json_value or not ignore_missing:"},{"line_number":231,"context_line":"                msg \u003d _LW(\u0027Dictionary %(dict_name)s for agent %(agent_type)s \u0027"},{"line_number":232,"context_line":"                          \u0027on host %(host)s is invalid.\u0027)"},{"line_number":233,"context_line":"                LOG.warning(msg, {\u0027dict_name\u0027: dict_name,"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a122d0e_2e0e3b3b","line":230,"range":{"start_line":230,"start_character":15,"end_line":230,"end_character":33},"updated":"2016-04-18 20:58:30.000000000","message":"I still don\u0027t get why we need to check the json_value here. If we get an exception isn\u0027t it one of the 2 cases:\n(a) agent_db doesn\u0027t have dict_name\n(b) agent_db does have dict_name but its not proper json (i.e. loads() fails)\n\nIn either case it\u0027s invalid is it not?","commit_id":"f85c2e5ba9719c7d8c35b53eaf39bd771c3a9567"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"fd5630f161ea55889992f5e7fbdd4e9e0a2e7832","unresolved":false,"context_lines":[{"line_number":227,"context_line":"            json_value \u003d getattr(agent_db, dict_name)"},{"line_number":228,"context_line":"            conf \u003d jsonutils.loads(json_value)"},{"line_number":229,"context_line":"        except Exception:"},{"line_number":230,"context_line":"            if json_value or not ignore_missing:"},{"line_number":231,"context_line":"                msg \u003d _LW(\u0027Dictionary %(dict_name)s for agent %(agent_type)s \u0027"},{"line_number":232,"context_line":"                          \u0027on host %(host)s is invalid.\u0027)"},{"line_number":233,"context_line":"                LOG.warning(msg, {\u0027dict_name\u0027: dict_name,"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a122d0e_99d7d722","line":230,"range":{"start_line":230,"start_character":15,"end_line":230,"end_character":33},"in_reply_to":"1a122d0e_2e0e3b3b","updated":"2016-04-18 21:15:05.000000000","message":"boden, you are correct, but the purpose of this patch is to make it possible to suppress logging for (a) only.","commit_id":"f85c2e5ba9719c7d8c35b53eaf39bd771c3a9567"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"c11aaa63e7e2fd597f03a281398c0943605e2168","unresolved":false,"context_lines":[{"line_number":227,"context_line":"            json_value \u003d getattr(agent_db, dict_name)"},{"line_number":228,"context_line":"            conf \u003d jsonutils.loads(json_value)"},{"line_number":229,"context_line":"        except Exception:"},{"line_number":230,"context_line":"            if json_value or not ignore_missing:"},{"line_number":231,"context_line":"                msg \u003d _LW(\u0027Dictionary %(dict_name)s for agent %(agent_type)s \u0027"},{"line_number":232,"context_line":"                          \u0027on host %(host)s is invalid.\u0027)"},{"line_number":233,"context_line":"                LOG.warning(msg, {\u0027dict_name\u0027: dict_name,"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a122d0e_6b9c93c0","line":230,"range":{"start_line":230,"start_character":15,"end_line":230,"end_character":33},"in_reply_to":"1a122d0e_99d7d722","updated":"2016-04-19 08:04:56.000000000","message":"exactly, thanks @illya.","commit_id":"f85c2e5ba9719c7d8c35b53eaf39bd771c3a9567"}],"neutron/tests/unit/db/test_agents_db.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5ac0eea03f93d27aaaceeee17ad5a000c3b4ec04","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        self.assertIn(\u0027test\u0027, conf1)"},{"line_number":203,"context_line":"        self.assertEqual(\"1234\", conf1[\u0027test\u0027])"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    def test_get_dict_missing(self):"},{"line_number":206,"context_line":"        with mock.patch.object(agents_db.LOG, \u0027warning\u0027) as warn:"},{"line_number":207,"context_line":"            db_obj \u003d mock.Mock(spec\u003d[\u0027agent_type\u0027, \u0027host\u0027])"},{"line_number":208,"context_line":"            self.plugin._get_dict(db_obj, \u0027missing_conf\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_6f6e9197","line":205,"range":{"start_line":205,"start_character":8,"end_line":205,"end_character":29},"updated":"2016-04-18 09:58:29.000000000","message":"I think test__get_dict_missing is better (note two underscores), since it reflects the name of the method targeted better.","commit_id":"5f82abf00e13cdd7a303211abb2e3d1c3d85ef60"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"6907b74753b0e3b20a74eadcce52f7db1ac153a0","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        self.assertIn(\u0027test\u0027, conf1)"},{"line_number":203,"context_line":"        self.assertEqual(\"1234\", conf1[\u0027test\u0027])"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    def test_get_dict_missing(self):"},{"line_number":206,"context_line":"        with mock.patch.object(agents_db.LOG, \u0027warning\u0027) as warn:"},{"line_number":207,"context_line":"            db_obj \u003d mock.Mock(spec\u003d[\u0027agent_type\u0027, \u0027host\u0027])"},{"line_number":208,"context_line":"            self.plugin._get_dict(db_obj, \u0027missing_conf\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_acbf00fa","line":205,"range":{"start_line":205,"start_character":8,"end_line":205,"end_character":29},"in_reply_to":"1a122d0e_6f6e9197","updated":"2016-04-18 16:09:41.000000000","message":"True, Done","commit_id":"5f82abf00e13cdd7a303211abb2e3d1c3d85ef60"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5ac0eea03f93d27aaaceeee17ad5a000c3b4ec04","unresolved":false,"context_lines":[{"line_number":206,"context_line":"        with mock.patch.object(agents_db.LOG, \u0027warning\u0027) as warn:"},{"line_number":207,"context_line":"            db_obj \u003d mock.Mock(spec\u003d[\u0027agent_type\u0027, \u0027host\u0027])"},{"line_number":208,"context_line":"            self.plugin._get_dict(db_obj, \u0027missing_conf\u0027)"},{"line_number":209,"context_line":"            warn.assert_called_once_with(mock.ANY, mock.ANY)"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    def test_get_dict_ignore_missing(self):"},{"line_number":212,"context_line":"        with mock.patch.object(agents_db.LOG, \u0027warning\u0027) as warn:"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_4fde3540","line":209,"range":{"start_line":209,"start_character":12,"end_line":209,"end_character":60},"updated":"2016-04-18 09:58:29.000000000","message":"If you don\u0027t care about arguments, you can just self.assertTrue(warn.called)","commit_id":"5f82abf00e13cdd7a303211abb2e3d1c3d85ef60"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"6907b74753b0e3b20a74eadcce52f7db1ac153a0","unresolved":false,"context_lines":[{"line_number":206,"context_line":"        with mock.patch.object(agents_db.LOG, \u0027warning\u0027) as warn:"},{"line_number":207,"context_line":"            db_obj \u003d mock.Mock(spec\u003d[\u0027agent_type\u0027, \u0027host\u0027])"},{"line_number":208,"context_line":"            self.plugin._get_dict(db_obj, \u0027missing_conf\u0027)"},{"line_number":209,"context_line":"            warn.assert_called_once_with(mock.ANY, mock.ANY)"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    def test_get_dict_ignore_missing(self):"},{"line_number":212,"context_line":"        with mock.patch.object(agents_db.LOG, \u0027warning\u0027) as warn:"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_8cba0409","line":209,"range":{"start_line":209,"start_character":12,"end_line":209,"end_character":60},"in_reply_to":"1a122d0e_4fde3540","updated":"2016-04-18 16:09:41.000000000","message":":-) Done","commit_id":"5f82abf00e13cdd7a303211abb2e3d1c3d85ef60"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5ac0eea03f93d27aaaceeee17ad5a000c3b4ec04","unresolved":false,"context_lines":[{"line_number":211,"context_line":"    def test_get_dict_ignore_missing(self):"},{"line_number":212,"context_line":"        with mock.patch.object(agents_db.LOG, \u0027warning\u0027) as warn:"},{"line_number":213,"context_line":"            db_obj \u003d mock.Mock(spec\u003d[\u0027agent_type\u0027, \u0027host\u0027])"},{"line_number":214,"context_line":"            self.plugin._get_dict(db_obj, \u0027missing_conf\u0027, ignore_missing\u003dTrue)"},{"line_number":215,"context_line":"            warn.assert_not_called()"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    def get_configurations_dict(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_4f275564","line":214,"updated":"2016-04-18 09:58:29.000000000","message":"Should we validate the result?","commit_id":"5f82abf00e13cdd7a303211abb2e3d1c3d85ef60"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"6907b74753b0e3b20a74eadcce52f7db1ac153a0","unresolved":false,"context_lines":[{"line_number":211,"context_line":"    def test_get_dict_ignore_missing(self):"},{"line_number":212,"context_line":"        with mock.patch.object(agents_db.LOG, \u0027warning\u0027) as warn:"},{"line_number":213,"context_line":"            db_obj \u003d mock.Mock(spec\u003d[\u0027agent_type\u0027, \u0027host\u0027])"},{"line_number":214,"context_line":"            self.plugin._get_dict(db_obj, \u0027missing_conf\u0027, ignore_missing\u003dTrue)"},{"line_number":215,"context_line":"            warn.assert_not_called()"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    def get_configurations_dict(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_8c720448","line":214,"in_reply_to":"1a122d0e_4f275564","updated":"2016-04-18 16:09:41.000000000","message":"Yes, we should.","commit_id":"5f82abf00e13cdd7a303211abb2e3d1c3d85ef60"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"a041d06b40f483f2e40fc902d2e86a63e70b5f93","unresolved":false,"context_lines":[{"line_number":221,"context_line":"            db_obj \u003d mock.Mock(conf1\u003d\u0027{\"test\": BROKEN\u0027)"},{"line_number":222,"context_line":"            conf1 \u003d self.plugin._get_dict(db_obj, \u0027conf1\u0027, ignore_missing\u003dTrue)"},{"line_number":223,"context_line":"            self.assertEqual({}, conf1)"},{"line_number":224,"context_line":"            warn.assert_called_once_with(mock.ANY, mock.ANY)"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"    def get_configurations_dict(self):"},{"line_number":227,"context_line":"        db_obj \u003d mock.Mock(configurations\u003d\u0027{\"cfg1\": \"val1\"}\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a122d0e_0c02746f","line":224,"range":{"start_line":224,"start_character":12,"end_line":224,"end_character":60},"updated":"2016-04-18 16:16:07.000000000","message":"nit: maybe use self.assertTrue(warn.called), same as in test__get_dict_missing?","commit_id":"f85c2e5ba9719c7d8c35b53eaf39bd771c3a9567"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"c11aaa63e7e2fd597f03a281398c0943605e2168","unresolved":false,"context_lines":[{"line_number":221,"context_line":"            db_obj \u003d mock.Mock(conf1\u003d\u0027{\"test\": BROKEN\u0027)"},{"line_number":222,"context_line":"            conf1 \u003d self.plugin._get_dict(db_obj, \u0027conf1\u0027, ignore_missing\u003dTrue)"},{"line_number":223,"context_line":"            self.assertEqual({}, conf1)"},{"line_number":224,"context_line":"            warn.assert_called_once_with(mock.ANY, mock.ANY)"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"    def get_configurations_dict(self):"},{"line_number":227,"context_line":"        db_obj \u003d mock.Mock(configurations\u003d\u0027{\"cfg1\": \"val1\"}\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a122d0e_eb21c34e","line":224,"range":{"start_line":224,"start_character":12,"end_line":224,"end_character":60},"in_reply_to":"1a122d0e_0c02746f","updated":"2016-04-19 08:04:56.000000000","message":"Correct, thanks!.\n\nDone!","commit_id":"f85c2e5ba9719c7d8c35b53eaf39bd771c3a9567"}]}