)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"8e641555316ad84310c0fbdcd455609d3249bbb7","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Ensure host_routes order in subnet"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"As route may recursive, some routes must be installed after the"},{"line_number":10,"context_line":"depending route. So we need a mechanism to set the order of host_routes"},{"line_number":11,"context_line":"in subnet manually."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"AAAAVn%2F%2FAes%3D","line":9,"updated":"2014-03-10 19:28:15.000000000","message":"nit: \u0027may be\u0027 instead of \u0027may\u0027","commit_id":"63e4248e8798d49b194549eb9ef7ba877df0ba55"},{"author":{"_account_id":7170,"name":"Zang MingJie","email":"zealot0630@gmail.com","username":"zealot"},"change_message_id":"143b6a7d21db2b17eca41645d2b8cd5c6c19a12c","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Ensure host_routes order in subnet"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"As route may recursive, some routes must be installed after the"},{"line_number":10,"context_line":"depending route. So we need a mechanism to set the order of host_routes"},{"line_number":11,"context_line":"in subnet manually."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"AAAAVn%2F%2B8Uw%3D","line":9,"in_reply_to":"AAAAVn%2F%2FAes%3D","updated":"2014-03-12 07:45:02.000000000","message":"Done","commit_id":"63e4248e8798d49b194549eb9ef7ba877df0ba55"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"8e641555316ad84310c0fbdcd455609d3249bbb7","unresolved":false,"context_lines":[{"line_number":13,"context_line":"This patch add an order column to host_routes table, which describe the"},{"line_number":14,"context_line":"order of routes, routes are applied from lower order to higher."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"There is a change to subnet API, but it is backward competible."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: I3d883db64cce1f145ceb997f2e42582475ebc86f"},{"line_number":19,"context_line":"Closes-Bug: #1288606"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"AAAAVn%2F%2FAeY%3D","line":16,"updated":"2014-03-10 19:28:15.000000000","message":"s/competible/compatible","commit_id":"63e4248e8798d49b194549eb9ef7ba877df0ba55"},{"author":{"_account_id":7170,"name":"Zang MingJie","email":"zealot0630@gmail.com","username":"zealot"},"change_message_id":"143b6a7d21db2b17eca41645d2b8cd5c6c19a12c","unresolved":false,"context_lines":[{"line_number":13,"context_line":"This patch add an order column to host_routes table, which describe the"},{"line_number":14,"context_line":"order of routes, routes are applied from lower order to higher."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"There is a change to subnet API, but it is backward competible."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: I3d883db64cce1f145ceb997f2e42582475ebc86f"},{"line_number":19,"context_line":"Closes-Bug: #1288606"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"AAAAVn%2F%2B8Us%3D","line":16,"in_reply_to":"AAAAVn%2F%2FAeY%3D","updated":"2014-03-12 07:45:02.000000000","message":"Done","commit_id":"63e4248e8798d49b194549eb9ef7ba877df0ba55"}],"neutron/api/v2/attributes.py":[{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"8e641555316ad84310c0fbdcd455609d3249bbb7","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    :param expected_keys: A list of keys expected to be present."},{"line_number":42,"context_line":"    :param target_dict: The dictionary which should be verified."},{"line_number":43,"context_line":"    :param strict: Specifies whether additional keys are allowed to be present."},{"line_number":44,"context_line":"    :param optional_keys: keys allowed to be present even strict is true."},{"line_number":45,"context_line":"    :return: True, if keys in the dictionary correspond to the specification."},{"line_number":46,"context_line":"    \"\"\""},{"line_number":47,"context_line":"    if not isinstance(target_dict, dict):"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAVn%2F%2FAdw%3D","line":44,"updated":"2014-03-10 19:28:15.000000000","message":"nit: s/even/even if","commit_id":"63e4248e8798d49b194549eb9ef7ba877df0ba55"},{"author":{"_account_id":7170,"name":"Zang MingJie","email":"zealot0630@gmail.com","username":"zealot"},"change_message_id":"143b6a7d21db2b17eca41645d2b8cd5c6c19a12c","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    :param expected_keys: A list of keys expected to be present."},{"line_number":42,"context_line":"    :param target_dict: The dictionary which should be verified."},{"line_number":43,"context_line":"    :param strict: Specifies whether additional keys are allowed to be present."},{"line_number":44,"context_line":"    :param optional_keys: keys allowed to be present even strict is true."},{"line_number":45,"context_line":"    :return: True, if keys in the dictionary correspond to the specification."},{"line_number":46,"context_line":"    \"\"\""},{"line_number":47,"context_line":"    if not isinstance(target_dict, dict):"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAVn%2F%2B8Uo%3D","line":44,"in_reply_to":"AAAAVn%2F%2FAdw%3D","updated":"2014-03-12 07:45:02.000000000","message":"Done","commit_id":"63e4248e8798d49b194549eb9ef7ba877df0ba55"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"8e641555316ad84310c0fbdcd455609d3249bbb7","unresolved":false,"context_lines":[{"line_number":61,"context_line":"                 \u0027provided_keys\u0027: provided_keys})"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    if strict:"},{"line_number":64,"context_line":"        with_optional_keys \u003d expected_keys.union(optional_keys)"},{"line_number":65,"context_line":"        if not with_optional_keys.issuperset(provided_keys):"},{"line_number":66,"context_line":"            return (_(\"Validation of dictionary\u0027s keys failed.\""},{"line_number":67,"context_line":"                      \"Expected keys: %(expected_keys)s \""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAVn%2F%2FAQo%3D","line":64,"updated":"2014-03-10 19:28:15.000000000","message":"Here you modified the logic, you don\u0027t check if every key in expected_keys has a corresponding key in provided_keys.\nMy suggestion is to leave the code of this method as it was. You just need to remove from provided_keys all the keys that are in optional_keys, something like this:\nprovided_keys.remove(set(optional_keys))\nDo that before line 54 of the original file and you don\u0027t have to change anything else I think.","commit_id":"63e4248e8798d49b194549eb9ef7ba877df0ba55"},{"author":{"_account_id":7170,"name":"Zang MingJie","email":"zealot0630@gmail.com","username":"zealot"},"change_message_id":"143b6a7d21db2b17eca41645d2b8cd5c6c19a12c","unresolved":false,"context_lines":[{"line_number":61,"context_line":"                 \u0027provided_keys\u0027: provided_keys})"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    if strict:"},{"line_number":64,"context_line":"        with_optional_keys \u003d expected_keys.union(optional_keys)"},{"line_number":65,"context_line":"        if not with_optional_keys.issuperset(provided_keys):"},{"line_number":66,"context_line":"            return (_(\"Validation of dictionary\u0027s keys failed.\""},{"line_number":67,"context_line":"                      \"Expected keys: %(expected_keys)s \""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAVn%2F%2B8Sw%3D","line":64,"in_reply_to":"AAAAVn%2F%2FAQo%3D","updated":"2014-03-12 07:45:02.000000000","message":"No, I didn\u0027t.\n\nThe enforcement of expected_keys is checked in the previous if, line 56\n\nIf strict is true and optional_keys is empty, with_optional_keys should equal as expected_keys, then expected_keys is both superset and subset of provided_keys, so they are equal.","commit_id":"63e4248e8798d49b194549eb9ef7ba877df0ba55"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"8e641555316ad84310c0fbdcd455609d3249bbb7","unresolved":false,"context_lines":[{"line_number":291,"context_line":"            if msg:"},{"line_number":292,"context_line":"                LOG.debug(msg)"},{"line_number":293,"context_line":"                return msg"},{"line_number":294,"context_line":"        uniq \u003d dict((key, hostroute[key]) for key in expected_keys)"},{"line_number":295,"context_line":"        if uniq in hostroutes:"},{"line_number":296,"context_line":"            msg \u003d _(\"Duplicate hostroute \u0027%s\u0027\") % hostroute"},{"line_number":297,"context_line":"            LOG.debug(msg)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAVn%2F%2FAMs%3D","line":294,"updated":"2014-03-10 19:28:15.000000000","message":"It\u0027s ugly to do that every time, it would be more elegant to redefine __eq__ for type hostroutes","commit_id":"63e4248e8798d49b194549eb9ef7ba877df0ba55"},{"author":{"_account_id":7170,"name":"Zang MingJie","email":"zealot0630@gmail.com","username":"zealot"},"change_message_id":"143b6a7d21db2b17eca41645d2b8cd5c6c19a12c","unresolved":false,"context_lines":[{"line_number":291,"context_line":"            if msg:"},{"line_number":292,"context_line":"                LOG.debug(msg)"},{"line_number":293,"context_line":"                return msg"},{"line_number":294,"context_line":"        uniq \u003d dict((key, hostroute[key]) for key in expected_keys)"},{"line_number":295,"context_line":"        if uniq in hostroutes:"},{"line_number":296,"context_line":"            msg \u003d _(\"Duplicate hostroute \u0027%s\u0027\") % hostroute"},{"line_number":297,"context_line":"            LOG.debug(msg)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAVn%2F%2B7B8%3D","line":294,"in_reply_to":"AAAAVn%2F%2FAMs%3D","updated":"2014-03-12 07:45:02.000000000","message":"I don\u0027t want this depending db model here, and because there is no hostroute class, creating a new class is not straightforward. IMO, it is better this way.","commit_id":"63e4248e8798d49b194549eb9ef7ba877df0ba55"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"1fc5e3fa4521be2fae341e39f1a3748c615b919b","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        return msg"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    expected_keys \u003d set(expected_keys)"},{"line_number":54,"context_line":"    provided_keys \u003d set(target_dict.keys())"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    if not expected_keys.issubset(provided_keys):"},{"line_number":57,"context_line":"        return (_(\"Validation of dictionary\u0027s keys failed.\""}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAWH%2F%2FcOQ%3D","line":54,"updated":"2014-03-17 21:46:32.000000000","message":"I see your point now, you are checking at line 56 if expected_keys is a subset of expected_keys and then at line 65 if with_optional_keys is a superset of provided_keys. So if optional_keys is empty, you are checking equality.\n\nAnyway I think it would be more clear if you\u0027d modify this line like this:\n\nprovided_keys \u003d set(target_dict.keys()).remove(optional_keys)\n\nThe rest of the function can stay as it was. It\u0027s one line change and makes things more clear I think.","commit_id":"dc0efb02da0882a3dde76c9b49aabbcf8ffee4fb"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"37b0ee08ef82b5c1a4fb76c37d00325fb749e74b","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        return msg"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    expected_keys \u003d set(expected_keys)"},{"line_number":54,"context_line":"    provided_keys \u003d set(target_dict.keys())"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    if not provided_keys \u003e\u003d expected_keys:"},{"line_number":57,"context_line":"        return (_(\"Validation of dictionary\u0027s keys failed. \""}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAAW3%2F%2FSVY%3D","line":54,"updated":"2014-04-15 15:45:26.000000000","message":"I repeat the same comment I made on patchset 11, if you modify line 54 like that:\n\nprovided_keys \u003d set(target_dict.keys()).remove(optional_keys)\n\nThen you don\u0027t have to change anything else in this function. Which I think makes everything more clear.\n\nAnyway I think there\u0027s an ambiguity here: is a key that is in optional_keys allowed to be also in expected_keys?","commit_id":"e26dfdf4672751a2790ffec7aeac3da4fed5d97f"},{"author":{"_account_id":7170,"name":"Zang MingJie","email":"zealot0630@gmail.com","username":"zealot"},"change_message_id":"92e31faf049a0c8209eb9c05d3333ca8b6bd54a5","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        return msg"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    expected_keys \u003d set(expected_keys)"},{"line_number":54,"context_line":"    provided_keys \u003d set(target_dict.keys())"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    if not provided_keys \u003e\u003d expected_keys:"},{"line_number":57,"context_line":"        return (_(\"Validation of dictionary\u0027s keys failed. \""}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAAXH%2F%2F9L8%3D","line":54,"in_reply_to":"AAAAW3%2F%2FSVY%3D","updated":"2014-04-16 11:33:32.000000000","message":"Done","commit_id":"e26dfdf4672751a2790ffec7aeac3da4fed5d97f"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"7d456937883f0300c7cb0284a615b140173c581c","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"def _verify_dict_keys(expected_keys, target_dict, strict\u003dTrue,"},{"line_number":38,"context_line":"                      optional_keys\u003d[]):"},{"line_number":39,"context_line":"    \"\"\"Allows to verify keys in a dictionary."},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    :param expected_keys: A list of keys expected to be present."}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAXH%2F%2F2IA%3D","line":38,"updated":"2014-04-17 02:06:55.000000000","message":"Mutable default arguments are discouraged. See http://stackoverflow.com/questions/1132941\n\nIt\u0027s much better to use optional_keys\u003dNone as the default, then initialize it as follows:\n\n  optional_keys \u003d optional_keys or []","commit_id":"3694886dcc24c410806e9d5e3eda0ad0a149596b"},{"author":{"_account_id":7170,"name":"Zang MingJie","email":"zealot0630@gmail.com","username":"zealot"},"change_message_id":"ef91d3a2f07095cabee938f87472a5c7a83c42c4","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"def _verify_dict_keys(expected_keys, target_dict, strict\u003dTrue,"},{"line_number":38,"context_line":"                      optional_keys\u003d[]):"},{"line_number":39,"context_line":"    \"\"\"Allows to verify keys in a dictionary."},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    :param expected_keys: A list of keys expected to be present."}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAXH%2F%2Fz6Y%3D","line":38,"in_reply_to":"AAAAXH%2F%2F2IA%3D","updated":"2014-04-17 09:44:41.000000000","message":"Done","commit_id":"3694886dcc24c410806e9d5e3eda0ad0a149596b"}],"neutron/db/db_base_plugin_v2.py":[{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"8e641555316ad84310c0fbdcd455609d3249bbb7","unresolved":false,"context_lines":[{"line_number":1094,"context_line":""},{"line_number":1095,"context_line":"            if s[\u0027host_routes\u0027] is not attributes.ATTR_NOT_SPECIFIED:"},{"line_number":1096,"context_line":"                for rt in s[\u0027host_routes\u0027]:"},{"line_number":1097,"context_line":"                    if \u0027order\u0027 in rt:"},{"line_number":1098,"context_line":"                        route \u003d models_v2.SubnetRoute("},{"line_number":1099,"context_line":"                            subnet_id\u003dsubnet.id,"},{"line_number":1100,"context_line":"                            destination\u003drt[\u0027destination\u0027],"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAVn%2F%2FAAU%3D","line":1097,"updated":"2014-03-10 19:28:15.000000000","message":"I\u0027d modify this line like this:\nif not \u0027order\u0027 in rt:\n    rt[\u0027order\u0027] \u003d 10\n\n\nand remove lines from 1103 to 1107","commit_id":"63e4248e8798d49b194549eb9ef7ba877df0ba55"},{"author":{"_account_id":7170,"name":"Zang MingJie","email":"zealot0630@gmail.com","username":"zealot"},"change_message_id":"143b6a7d21db2b17eca41645d2b8cd5c6c19a12c","unresolved":false,"context_lines":[{"line_number":1094,"context_line":""},{"line_number":1095,"context_line":"            if s[\u0027host_routes\u0027] is not attributes.ATTR_NOT_SPECIFIED:"},{"line_number":1096,"context_line":"                for rt in s[\u0027host_routes\u0027]:"},{"line_number":1097,"context_line":"                    if \u0027order\u0027 in rt:"},{"line_number":1098,"context_line":"                        route \u003d models_v2.SubnetRoute("},{"line_number":1099,"context_line":"                            subnet_id\u003dsubnet.id,"},{"line_number":1100,"context_line":"                            destination\u003drt[\u0027destination\u0027],"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAVn%2F%2B7U0%3D","line":1097,"in_reply_to":"AAAAVn%2F%2FAAU%3D","updated":"2014-03-12 07:45:02.000000000","message":"Done","commit_id":"63e4248e8798d49b194549eb9ef7ba877df0ba55"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"8e641555316ad84310c0fbdcd455609d3249bbb7","unresolved":false,"context_lines":[{"line_number":1168,"context_line":"                old_route_list \u003d self._get_route_by_subnet(context, id)"},{"line_number":1169,"context_line":""},{"line_number":1170,"context_line":"                for route in old_route_list:"},{"line_number":1171,"context_line":"                    context.session.delete(route)"},{"line_number":1172,"context_line":"                for ht in s[\u0027host_routes\u0027]:"},{"line_number":1173,"context_line":"                    if \u0027order\u0027 in ht:"},{"line_number":1174,"context_line":"                        route \u003d models_v2.SubnetRoute("}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAVn%2F%2FALc%3D","line":1171,"updated":"2014-03-10 19:28:15.000000000","message":"This is not as efficient as it was before, I suggest you don\u0027t change the logic of this piece. Remove all the hostroutes that are not needed and add only the ones that are new","commit_id":"63e4248e8798d49b194549eb9ef7ba877df0ba55"},{"author":{"_account_id":7170,"name":"Zang MingJie","email":"zealot0630@gmail.com","username":"zealot"},"change_message_id":"143b6a7d21db2b17eca41645d2b8cd5c6c19a12c","unresolved":false,"context_lines":[{"line_number":1168,"context_line":"                old_route_list \u003d self._get_route_by_subnet(context, id)"},{"line_number":1169,"context_line":""},{"line_number":1170,"context_line":"                for route in old_route_list:"},{"line_number":1171,"context_line":"                    context.session.delete(route)"},{"line_number":1172,"context_line":"                for ht in s[\u0027host_routes\u0027]:"},{"line_number":1173,"context_line":"                    if \u0027order\u0027 in ht:"},{"line_number":1174,"context_line":"                        route \u003d models_v2.SubnetRoute("}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAVn%2F%2B7WM%3D","line":1171,"in_reply_to":"AAAAVn%2F%2FALc%3D","updated":"2014-03-12 07:45:02.000000000","message":"Done","commit_id":"63e4248e8798d49b194549eb9ef7ba877df0ba55"},{"author":{"_account_id":7141,"name":"Sylvain Afchain","email":"safchain@redhat.com","username":"sylvainA"},"change_message_id":"fa41a9d97e7f9dea73e94b31f2c72e2ece70fca2","unresolved":false,"context_lines":[{"line_number":1179,"context_line":"                        subnet_id\u003dsubnet.id,"},{"line_number":1180,"context_line":"                        destination\u003drt[\u0027destination\u0027],"},{"line_number":1181,"context_line":"                        nexthop\u003drt[\u0027nexthop\u0027],"},{"line_number":1182,"context_line":"                        order\u003drt[\u0027order\u0027]"},{"line_number":1183,"context_line":"                        if \u0027order\u0027 in rt else constants.ROUTE_DEFAULT_ORDER)"},{"line_number":1184,"context_line":"                    context.session.add(route)"},{"line_number":1185,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAAXH%2F%2F9yg%3D","line":1182,"updated":"2014-04-16 08:09:34.000000000","message":"You could just use : order\u003drt.get(\u0027order\u0027, constants.ROUTE_DEFAULT_ORDER)","commit_id":"e26dfdf4672751a2790ffec7aeac3da4fed5d97f"},{"author":{"_account_id":7170,"name":"Zang MingJie","email":"zealot0630@gmail.com","username":"zealot"},"change_message_id":"92e31faf049a0c8209eb9c05d3333ca8b6bd54a5","unresolved":false,"context_lines":[{"line_number":1179,"context_line":"                        subnet_id\u003dsubnet.id,"},{"line_number":1180,"context_line":"                        destination\u003drt[\u0027destination\u0027],"},{"line_number":1181,"context_line":"                        nexthop\u003drt[\u0027nexthop\u0027],"},{"line_number":1182,"context_line":"                        order\u003drt[\u0027order\u0027]"},{"line_number":1183,"context_line":"                        if \u0027order\u0027 in rt else constants.ROUTE_DEFAULT_ORDER)"},{"line_number":1184,"context_line":"                    context.session.add(route)"},{"line_number":1185,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAAXH%2F%2F9ME%3D","line":1182,"in_reply_to":"AAAAXH%2F%2F9yg%3D","updated":"2014-04-16 11:33:32.000000000","message":"Done","commit_id":"e26dfdf4672751a2790ffec7aeac3da4fed5d97f"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"37b0ee08ef82b5c1a4fb76c37d00325fb749e74b","unresolved":false,"context_lines":[{"line_number":1258,"context_line":"                        new \u003d new_route_list.pop(i)"},{"line_number":1259,"context_line":"                        if new.order !\u003d route.order:"},{"line_number":1260,"context_line":"                            route.order \u003d new.order"},{"line_number":1261,"context_line":"                            context.session.update(route)"},{"line_number":1262,"context_line":"                    except ValueError:"},{"line_number":1263,"context_line":"                        # route not in new route list"},{"line_number":1264,"context_line":"                        context.session.delete(route)"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAAW3%2F%2FSC0%3D","line":1261,"updated":"2014-04-15 15:45:26.000000000","message":"There should be an else here. It\u0027s the case when the old route is the same as the new one, you should remove it from new_route_list otherwise on line 1267 you\u0027d add the same route again","commit_id":"e26dfdf4672751a2790ffec7aeac3da4fed5d97f"},{"author":{"_account_id":7141,"name":"Sylvain Afchain","email":"safchain@redhat.com","username":"sylvainA"},"change_message_id":"fa41a9d97e7f9dea73e94b31f2c72e2ece70fca2","unresolved":false,"context_lines":[{"line_number":1258,"context_line":"                        new \u003d new_route_list.pop(i)"},{"line_number":1259,"context_line":"                        if new.order !\u003d route.order:"},{"line_number":1260,"context_line":"                            route.order \u003d new.order"},{"line_number":1261,"context_line":"                            context.session.update(route)"},{"line_number":1262,"context_line":"                    except ValueError:"},{"line_number":1263,"context_line":"                        # route not in new route list"},{"line_number":1264,"context_line":"                        context.session.delete(route)"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAAXH%2F%2F9zA%3D","line":1261,"in_reply_to":"AAAAW3%2F%2FSC0%3D","updated":"2014-04-16 08:09:34.000000000","message":"+1 not especially a else but you should remove it from the list in any cases, order changed or not. Moreover a UT for that should show the potential issue.","commit_id":"e26dfdf4672751a2790ffec7aeac3da4fed5d97f"},{"author":{"_account_id":7170,"name":"Zang MingJie","email":"zealot0630@gmail.com","username":"zealot"},"change_message_id":"92e31faf049a0c8209eb9c05d3333ca8b6bd54a5","unresolved":false,"context_lines":[{"line_number":1258,"context_line":"                        new \u003d new_route_list.pop(i)"},{"line_number":1259,"context_line":"                        if new.order !\u003d route.order:"},{"line_number":1260,"context_line":"                            route.order \u003d new.order"},{"line_number":1261,"context_line":"                            context.session.update(route)"},{"line_number":1262,"context_line":"                    except ValueError:"},{"line_number":1263,"context_line":"                        # route not in new route list"},{"line_number":1264,"context_line":"                        context.session.delete(route)"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAAXH%2F%2F9TQ%3D","line":1261,"in_reply_to":"AAAAXH%2F%2F9zA%3D","updated":"2014-04-16 11:33:32.000000000","message":"it is already removed from new list @L1258, I\u0027ll add a UT to test the case","commit_id":"e26dfdf4672751a2790ffec7aeac3da4fed5d97f"},{"author":{"_account_id":7141,"name":"Sylvain Afchain","email":"safchain@redhat.com","username":"sylvainA"},"change_message_id":"48d641f9cf00e7f1a4d275a0df84728abdec134a","unresolved":false,"context_lines":[{"line_number":1263,"context_line":"                for route in new_route_list:"},{"line_number":1264,"context_line":"                    context.session.add(route)"},{"line_number":1265,"context_line":""},{"line_number":1266,"context_line":"                new_routes \u003d s[\u0027host_routes\u0027]"},{"line_number":1267,"context_line":"                del s[\"host_routes\"]"},{"line_number":1268,"context_line":""},{"line_number":1269,"context_line":"            subnet \u003d self._get_subnet(context, id)"}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAXH%2F%2FUdA%3D","line":1266,"updated":"2014-04-22 14:39:16.000000000","message":"I think it\u0027s worth it to return the host_routes sorted with the new attribute otherwise we have no way to know the order since the default order is hidden.","commit_id":"cebfe2a2d4eab0c82d472792f4ac9ddcc937e21d"},{"author":{"_account_id":7141,"name":"Sylvain Afchain","email":"safchain@redhat.com","username":"sylvainA"},"change_message_id":"48d641f9cf00e7f1a4d275a0df84728abdec134a","unresolved":false,"context_lines":[{"line_number":1273,"context_line":"        if changed_dns:"},{"line_number":1274,"context_line":"            result[\u0027dns_nameservers\u0027] \u003d new_dns"},{"line_number":1275,"context_line":"        if changed_host_routes:"},{"line_number":1276,"context_line":"            result[\u0027host_routes\u0027] \u003d new_routes"},{"line_number":1277,"context_line":"        return result"},{"line_number":1278,"context_line":""},{"line_number":1279,"context_line":"    def delete_subnet(self, context, id):"}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAXH%2F%2FUZ8%3D","line":1276,"updated":"2014-04-22 14:39:16.000000000","message":"The field sorted by _make_subnet_dict is overridden here","commit_id":"cebfe2a2d4eab0c82d472792f4ac9ddcc937e21d"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"f8c159a7d8bec06cbb7d6694e3473ea97fb7f54c","unresolved":false,"context_lines":[{"line_number":1260,"context_line":"                    for rt in s[\u0027host_routes\u0027]"},{"line_number":1261,"context_line":"                ]"},{"line_number":1262,"context_line":""},{"line_number":1263,"context_line":"                for route in old_route_list:"},{"line_number":1264,"context_line":"                    try:"},{"line_number":1265,"context_line":"                        i \u003d new_route_list.index(route)"},{"line_number":1266,"context_line":"                        new \u003d new_route_list.pop(i)"}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAXH%2F%2FLco%3D","line":1263,"updated":"2014-04-24 09:52:52.000000000","message":"looks like set arithmetic  should be applied here, you need to derive two sets: to_delete and to_add from known data and the do deletions and insertions.\nafaik it\u0027s a typical task used in agents that work with devices/routers.\n\nIf you\u0027ll need to update the patch, please consider changing this","commit_id":"4f8d1f81e42cdc55cfcbfcae9d779a302494229e"}],"neutron/db/migration/alembic_migrations/versions/1dc6247e4fbd_order_of_subnet_host_routes.py":[{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"8e641555316ad84310c0fbdcd455609d3249bbb7","unresolved":false,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    op.add_column(\u0027subnetroutes\u0027, sa.Column(u\u0027order\u0027, sa.Integer(),"},{"line_number":43,"context_line":"                                            nullable\u003dFalse,"},{"line_number":44,"context_line":"                                            server_default\u003d\"10\"))"},{"line_number":45,"context_line":"    # routerroutes share the same model as subnetroutes, and may also need the"},{"line_number":46,"context_line":"    # order column."},{"line_number":47,"context_line":"    op.add_column(\u0027routerroutes\u0027, sa.Column(u\u0027order\u0027, sa.Integer(),"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAVn%2F%2B%2F%2Fw%3D","line":44,"updated":"2014-03-10 19:28:15.000000000","message":"why 10 is the value chosen as default?","commit_id":"63e4248e8798d49b194549eb9ef7ba877df0ba55"},{"author":{"_account_id":7170,"name":"Zang MingJie","email":"zealot0630@gmail.com","username":"zealot"},"change_message_id":"143b6a7d21db2b17eca41645d2b8cd5c6c19a12c","unresolved":false,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    op.add_column(\u0027subnetroutes\u0027, sa.Column(u\u0027order\u0027, sa.Integer(),"},{"line_number":43,"context_line":"                                            nullable\u003dFalse,"},{"line_number":44,"context_line":"                                            server_default\u003d\"10\"))"},{"line_number":45,"context_line":"    # routerroutes share the same model as subnetroutes, and may also need the"},{"line_number":46,"context_line":"    # order column."},{"line_number":47,"context_line":"    op.add_column(\u0027routerroutes\u0027, sa.Column(u\u0027order\u0027, sa.Integer(),"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAVn%2F%2B7U4%3D","line":44,"in_reply_to":"AAAAVn%2F%2B%2F%2Fw%3D","updated":"2014-03-12 07:45:02.000000000","message":"I don\u0027t know, it just jump into my head.","commit_id":"63e4248e8798d49b194549eb9ef7ba877df0ba55"}],"neutron/db/models_v2.py":[{"author":{"_account_id":7141,"name":"Sylvain Afchain","email":"safchain@redhat.com","username":"sylvainA"},"change_message_id":"fa41a9d97e7f9dea73e94b31f2c72e2ece70fca2","unresolved":false,"context_lines":[{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    def __cmp__(self, other):"},{"line_number":108,"context_line":"        eq \u003d cmp(self.destination, other.destination)"},{"line_number":109,"context_line":"        return eq if eq !\u003d 0 else cmp(self.nexthop, other.nexthop)"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def __hash__(self):"},{"line_number":112,"context_line":"        return hash(self.destination) ^ hash(self.nexthop)"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAAXH%2F%2F96k%3D","line":109,"updated":"2014-04-16 08:09:34.000000000","message":"Why not : return cmp(self.destination, other.destination) or cmp(self.nexthop, other.nexthop)","commit_id":"e26dfdf4672751a2790ffec7aeac3da4fed5d97f"},{"author":{"_account_id":7170,"name":"Zang MingJie","email":"zealot0630@gmail.com","username":"zealot"},"change_message_id":"92e31faf049a0c8209eb9c05d3333ca8b6bd54a5","unresolved":false,"context_lines":[{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    def __cmp__(self, other):"},{"line_number":108,"context_line":"        eq \u003d cmp(self.destination, other.destination)"},{"line_number":109,"context_line":"        return eq if eq !\u003d 0 else cmp(self.nexthop, other.nexthop)"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def __hash__(self):"},{"line_number":112,"context_line":"        return hash(self.destination) ^ hash(self.nexthop)"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAAXH%2F%2F9L4%3D","line":109,"in_reply_to":"AAAAXH%2F%2F96k%3D","updated":"2014-04-16 11:33:32.000000000","message":"Done","commit_id":"e26dfdf4672751a2790ffec7aeac3da4fed5d97f"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"7d456937883f0300c7cb0284a615b140173c581c","unresolved":false,"context_lines":[{"line_number":109,"context_line":"                cmp(self.nexthop, other.nexthop))"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def __hash__(self):"},{"line_number":112,"context_line":"        return hash(self.destination) ^ hash(self.nexthop)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    destination \u003d sa.Column(sa.String(64), nullable\u003dFalse, primary_key\u003dTrue)"},{"line_number":115,"context_line":"    nexthop \u003d sa.Column(sa.String(64), nullable\u003dFalse, primary_key\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAXH%2F%2F1%2FA%3D","line":112,"updated":"2014-04-17 02:06:55.000000000","message":"Is this better than hash(self.destination + self.nexthop)?","commit_id":"3694886dcc24c410806e9d5e3eda0ad0a149596b"},{"author":{"_account_id":7170,"name":"Zang MingJie","email":"zealot0630@gmail.com","username":"zealot"},"change_message_id":"ef91d3a2f07095cabee938f87472a5c7a83c42c4","unresolved":false,"context_lines":[{"line_number":109,"context_line":"                cmp(self.nexthop, other.nexthop))"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def __hash__(self):"},{"line_number":112,"context_line":"        return hash(self.destination) ^ hash(self.nexthop)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    destination \u003d sa.Column(sa.String(64), nullable\u003dFalse, primary_key\u003dTrue)"},{"line_number":115,"context_line":"    nexthop \u003d sa.Column(sa.String(64), nullable\u003dFalse, primary_key\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAXH%2F%2Fz6o%3D","line":112,"in_reply_to":"AAAAXH%2F%2F1%2FA%3D","updated":"2014-04-17 09:44:41.000000000","message":"I don\u0027t think there is any difference, but xor may faster than add","commit_id":"3694886dcc24c410806e9d5e3eda0ad0a149596b"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"64dd1448d1ccf9d0cf3d99fd2e718aa361ec8b1b","unresolved":false,"context_lines":[{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    destination \u003d sa.Column(sa.String(64), nullable\u003dFalse, primary_key\u003dTrue)"},{"line_number":115,"context_line":"    nexthop \u003d sa.Column(sa.String(64), nullable\u003dFalse, primary_key\u003dTrue)"},{"line_number":116,"context_line":"    order \u003d sa.Column(sa.Integer, nullable\u003dFalse, primary_key\u003dFalse,"},{"line_number":117,"context_line":"                      default\u003dconstants.ROUTE_DEFAULT_ORDER)"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAXH%2F%2Fm0I%3D","line":116,"updated":"2014-04-18 21:19:28.000000000","message":"nit: primary_key\u003dFalse is unnecessary","commit_id":"cebfe2a2d4eab0c82d472792f4ac9ddcc937e21d"}],"neutron/tests/unit/test_db_plugin.py":[{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"8e641555316ad84310c0fbdcd455609d3249bbb7","unresolved":false,"context_lines":[{"line_number":2197,"context_line":"                if k \u003d\u003d \u0027host_routes\u0027:"},{"line_number":2198,"context_line":"                    hrs \u003d [{\u0027destination\u0027: hr[\u0027destination\u0027],"},{"line_number":2199,"context_line":"                            \u0027nexthop\u0027: hr[\u0027nexthop\u0027],"},{"line_number":2200,"context_line":"                            \u0027order\u0027: hr[\u0027order\u0027] if \u0027order\u0027 in hr else 10}"},{"line_number":2201,"context_line":"                           for hr in keys[k]]"},{"line_number":2202,"context_line":"                    key \u003d lambda r: r[\u0027order\u0027]"},{"line_number":2203,"context_line":"                    self.assertEqual(sorted(subnet[\u0027subnet\u0027][k], key\u003dkey),"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAVn%2F%2FAJE%3D","line":2200,"updated":"2014-03-10 19:28:15.000000000","message":"why do you set 10 automatically if order is not set?","commit_id":"63e4248e8798d49b194549eb9ef7ba877df0ba55"},{"author":{"_account_id":7170,"name":"Zang MingJie","email":"zealot0630@gmail.com","username":"zealot"},"change_message_id":"143b6a7d21db2b17eca41645d2b8cd5c6c19a12c","unresolved":false,"context_lines":[{"line_number":2197,"context_line":"                if k \u003d\u003d \u0027host_routes\u0027:"},{"line_number":2198,"context_line":"                    hrs \u003d [{\u0027destination\u0027: hr[\u0027destination\u0027],"},{"line_number":2199,"context_line":"                            \u0027nexthop\u0027: hr[\u0027nexthop\u0027],"},{"line_number":2200,"context_line":"                            \u0027order\u0027: hr[\u0027order\u0027] if \u0027order\u0027 in hr else 10}"},{"line_number":2201,"context_line":"                           for hr in keys[k]]"},{"line_number":2202,"context_line":"                    key \u003d lambda r: r[\u0027order\u0027]"},{"line_number":2203,"context_line":"                    self.assertEqual(sorted(subnet[\u0027subnet\u0027][k], key\u003dkey),"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAVn%2F%2B7VY%3D","line":2200,"in_reply_to":"AAAAVn%2F%2FAJE%3D","updated":"2014-03-12 07:45:02.000000000","message":"This is the response returned from API, where order should be set to 10 if is left empty.","commit_id":"63e4248e8798d49b194549eb9ef7ba877df0ba55"},{"author":{"_account_id":7141,"name":"Sylvain Afchain","email":"safchain@redhat.com","username":"sylvainA"},"change_message_id":"48d641f9cf00e7f1a4d275a0df84728abdec134a","unresolved":false,"context_lines":[{"line_number":2253,"context_line":"                    hrs \u003d [{\u0027destination\u0027: hr[\u0027destination\u0027],"},{"line_number":2254,"context_line":"                            \u0027nexthop\u0027: hr[\u0027nexthop\u0027],"},{"line_number":2255,"context_line":"                            \u0027order\u0027: hr[\u0027order\u0027]"},{"line_number":2256,"context_line":"                            if \u0027order\u0027 in hr else"},{"line_number":2257,"context_line":"                            constants.ROUTE_DEFAULT_ORDER}"},{"line_number":2258,"context_line":"                           for hr in keys[k]]"},{"line_number":2259,"context_line":"                    key \u003d lambda r: r[\u0027order\u0027]"}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAXH%2F%2FWeI%3D","line":2256,"updated":"2014-04-22 14:39:16.000000000","message":"Sorry, I missed to report this one few patches ago, but should be\n\n    \u0027order\u0027: hr.get(\u0027order\u0027, constants.ROUTE_DEFAULT_ORDER)","commit_id":"cebfe2a2d4eab0c82d472792f4ac9ddcc937e21d"},{"author":{"_account_id":7141,"name":"Sylvain Afchain","email":"safchain@redhat.com","username":"sylvainA"},"change_message_id":"48d641f9cf00e7f1a4d275a0df84728abdec134a","unresolved":false,"context_lines":[{"line_number":3633,"context_line":"            req \u003d self.new_show_request(\u0027subnets\u0027, subnet[\u0027subnet\u0027][\u0027id\u0027])"},{"line_number":3634,"context_line":"            res \u003d self.deserialize(self.fmt, req.get_response(self.api))"},{"line_number":3635,"context_line":"            self.assertEqual(data[\u0027subnet\u0027][\u0027host_routes\u0027],"},{"line_number":3636,"context_line":"                             res[\u0027subnet\u0027][\u0027host_routes\u0027])"},{"line_number":3637,"context_line":""},{"line_number":3638,"context_line":"    def test_update_subnet_route_to_None(self):"},{"line_number":3639,"context_line":"        with self.subnet(host_routes\u003d[{\u0027destination\u0027: \u002712.0.0.0/8\u0027,"}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAXH%2F%2FUaU%3D","line":3636,"updated":"2014-04-22 14:39:16.000000000","message":"Maybe I missed something but I didn\u0027t see a test that checks that the order is respected. See my comment in neutron/db/db_base_plugin_v2.py.","commit_id":"cebfe2a2d4eab0c82d472792f4ac9ddcc937e21d"}]}