)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"5b78e4d6e46a5572a108b39d47ac8858011e0ef3","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Allows sorting of locations by a weight key defined in metadata."},{"line_number":10,"context_line":"Locations with a higher weight will be returned first. Locations with no"},{"line_number":11,"context_line":"weight key will be returned in original order."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I4cbb9d0ee4779619b478c1bc5186646c44f24191"},{"line_number":14,"context_line":"Closes-Bug: 1528453"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9a68dd71_91b4a0d1","line":12,"updated":"2016-01-21 21:40:21.000000000","message":"It would be nice if we can mark this as DocImpact, since this could be very useful driver for end user. And user need to know how to use it. Thanks.","commit_id":"ae07fc2a1cde00ffa5a6798371b7ff9a70dcd1d7"}],"glance/common/location_strategy/metadata_rank.py":[{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"520bcdf92a73bea52f4e3f453c1515b01976ac89","unresolved":false,"context_lines":[{"line_number":50,"context_line":"        metadata \u003d location.get(\u0027metadata\u0027)"},{"line_number":51,"context_line":"        rank \u003d metadata.get(rank_key)"},{"line_number":52,"context_line":"        if rank is not None:"},{"line_number":53,"context_line":"            ret.insert(rank, location)"},{"line_number":54,"context_line":"        else:"},{"line_number":55,"context_line":"            others.append(location)"},{"line_number":56,"context_line":"    ret.extend(others)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba63e191_64070a1d","line":53,"updated":"2016-01-19 17:39:18.000000000","message":"I don\u0027t think the rank key works as expected. And it\u0027s easy to reproduce the issue. Let\u0027s say you have 5 locations. And the first one you get from the \u0027locations\u0027 list has a rank key 5, then the result will be wrong.\n\nPersonally, I prefer to use \u0027weight\u0027 instead of \u0027rank\u0027. Sample code as below:\n\nhas_weight_locs \u003d [l for l in locations if \u0027metadata\u0027 in l]\nno_weight_locs \u003d list(set(locations)-set(has_weight_locs)\n# no_weight_locs \u003d [l for l in locations if \u0027metadata\u0027 in l]\nret \u003d has_weight_locs.sort(key\u003dlambda l: l[\u0027metadata\u0027][\u0027weight\u0027])\nret.extend(no_weight_locs)","commit_id":"bdb14e4b467456cc7d8b614b4cef0e1cf639d91c"}],"glance/common/location_strategy/metadata_weight.py":[{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"16db0db1185a07d770e2457a4394d9be90504d4d","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from oslo_config import cfg"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from glance import i18n"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"_ \u003d i18n._"},{"line_number":23,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9a68dd71_418e0dd1","line":20,"updated":"2016-01-23 12:07:15.000000000","message":"Nit we prefer\n\n from glance.i18n import _","commit_id":"ce447da76aeb522bdce70feb175bc6283283b9f9"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"16db0db1185a07d770e2457a4394d9be90504d4d","unresolved":false,"context_lines":[{"line_number":63,"context_line":"    for location in locations:"},{"line_number":64,"context_line":"        metadata \u003d location.get(\u0027metadata\u0027)"},{"line_number":65,"context_line":"        weight \u003d metadata.get(weight_key)"},{"line_number":66,"context_line":"        if weight is not None:"},{"line_number":67,"context_line":"            ret.append(location)"},{"line_number":68,"context_line":"        else:"},{"line_number":69,"context_line":"            others.append(location)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a68dd71_45a378b2","line":66,"updated":"2016-01-23 12:07:15.000000000","message":"May be worthwhile to check weight.isdigit(). Otherwise, this may cause issues in python3 if the weights have mixed types.","commit_id":"ce447da76aeb522bdce70feb175bc6283283b9f9"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"765d76bd28b45a701bfd810aa05eb13a0e745358","unresolved":false,"context_lines":[{"line_number":63,"context_line":"    for location in locations:"},{"line_number":64,"context_line":"        metadata \u003d location.get(\u0027metadata\u0027)"},{"line_number":65,"context_line":"        weight \u003d metadata.get(weight_key)"},{"line_number":66,"context_line":"        if weight is not None:"},{"line_number":67,"context_line":"            ret.append(location)"},{"line_number":68,"context_line":"        else:"},{"line_number":69,"context_line":"            others.append(location)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a68dd71_e594ec43","line":66,"in_reply_to":"9a68dd71_45a378b2","updated":"2016-01-23 12:08:09.000000000","message":"I meant in addition to the None check.","commit_id":"ce447da76aeb522bdce70feb175bc6283283b9f9"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"69a4b60bde6b6d410fc7988ded9d269c6d4d737a","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# All Rights Reserved."},{"line_number":2,"context_line":"#"},{"line_number":3,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":4,"context_line":"#    not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_33d3fb05","line":1,"updated":"2016-02-23 18:57:49.000000000","message":"Could you add the copyright line for the foundation?","commit_id":"c7836630835c7e1feb49f120051f7d5e05fc9d49"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"69a4b60bde6b6d410fc7988ded9d269c6d4d737a","unresolved":false,"context_lines":[{"line_number":19,"context_line":"from glance.i18n import _"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"store_type_opts \u003d ["},{"line_number":22,"context_line":"    cfg.ListOpt(\"metadata_weight_key\","},{"line_number":23,"context_line":"                default\u003dNone,"},{"line_number":24,"context_line":"                help\u003d_(\"Name of the metadata key to weight locations by. \""},{"line_number":25,"context_line":"                       \"Locations with higher weight will be returned first. \""}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_bec22032","line":22,"updated":"2016-02-23 18:57:49.000000000","message":"Why do we need a weight key? Can we hardcode it?","commit_id":"c7836630835c7e1feb49f120051f7d5e05fc9d49"},{"author":{"_account_id":8064,"name":"Jake Yip","email":"jake.yip@ardc.edu.au","username":"jake"},"change_message_id":"2cecf79bb2d68d2b861e81479e20ccebaff5be27","unresolved":false,"context_lines":[{"line_number":19,"context_line":"from glance.i18n import _"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"store_type_opts \u003d ["},{"line_number":22,"context_line":"    cfg.ListOpt(\"metadata_weight_key\","},{"line_number":23,"context_line":"                default\u003dNone,"},{"line_number":24,"context_line":"                help\u003d_(\"Name of the metadata key to weight locations by. \""},{"line_number":25,"context_line":"                       \"Locations with higher weight will be returned first. \""}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_be2bdc93","line":22,"in_reply_to":"bae84128_bec22032","updated":"2016-03-14 00:04:12.000000000","message":"Hi Flavio, the idea is that different glance-api servers can have their own key, and order the location unique to itself. So if I am a sysadmin of a glance-api server in cell-a, I can make the location of image cell-a be returned first when order by cell-a\u0027s key, and a sysadmin in cell-b can make the location of an image in cell-b be returned first when order by cell-b\u0027s key.\n\nPlease let me know if that makes sense.","commit_id":"c7836630835c7e1feb49f120051f7d5e05fc9d49"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"4bc0a506aacb09145823dafbd2379342461f79d1","unresolved":false,"context_lines":[{"line_number":19,"context_line":"from glance.i18n import _"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"store_type_opts \u003d ["},{"line_number":22,"context_line":"    cfg.ListOpt(\"metadata_weight_key\","},{"line_number":23,"context_line":"                default\u003dNone,"},{"line_number":24,"context_line":"                help\u003d_(\"Name of the metadata key to weight locations by. \""},{"line_number":25,"context_line":"                       \"Locations with higher weight will be returned first. \""}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_42c4ce3e","line":22,"in_reply_to":"bae84128_bec22032","updated":"2016-02-23 23:03:13.000000000","message":"It\u0027s good point. I think hardcode the key is an option, but we need to make sure it\u0027s documented somewhere. Given it\u0027s in metadata.","commit_id":"c7836630835c7e1feb49f120051f7d5e05fc9d49"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"69a4b60bde6b6d410fc7988ded9d269c6d4d737a","unresolved":false,"context_lines":[{"line_number":31,"context_line":"                       \"defined by the \u0027location_strategy\u0027 config option.\"))]"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"CONF \u003d cfg.CONF"},{"line_number":34,"context_line":"CONF.register_opts(store_type_opts, group\u003d\u0027metadata_weight_location_strategy\u0027)"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"def get_strategy_name():"}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_de956433","line":34,"updated":"2016-02-23 18:57:49.000000000","message":"Can we call this `weighted_location_strategy` ?","commit_id":"c7836630835c7e1feb49f120051f7d5e05fc9d49"},{"author":{"_account_id":8064,"name":"Jake Yip","email":"jake.yip@ardc.edu.au","username":"jake"},"change_message_id":"2cecf79bb2d68d2b861e81479e20ccebaff5be27","unresolved":false,"context_lines":[{"line_number":31,"context_line":"                       \"defined by the \u0027location_strategy\u0027 config option.\"))]"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"CONF \u003d cfg.CONF"},{"line_number":34,"context_line":"CONF.register_opts(store_type_opts, group\u003d\u0027metadata_weight_location_strategy\u0027)"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"def get_strategy_name():"}],"source_content_type":"text/x-python","patch_set":4,"id":"3afc51ec_9eaa3cd3","line":34,"in_reply_to":"bae84128_de956433","updated":"2016-03-14 00:04:12.000000000","message":"I can do this, do we still want the \u0027metadata\u0027 name in there?","commit_id":"c7836630835c7e1feb49f120051f7d5e05fc9d49"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"69a4b60bde6b6d410fc7988ded9d269c6d4d737a","unresolved":false,"context_lines":[{"line_number":45,"context_line":""},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"def get_ordered_locations(locations, **kwargs):"},{"line_number":48,"context_line":"    \"\"\""},{"line_number":49,"context_line":"    :param locations: The original image location list."},{"line_number":50,"context_line":"    :return: The image location list weighted by metadata_weight_key."},{"line_number":51,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_931807d5","line":48,"updated":"2016-02-23 18:57:49.000000000","message":"Could you add a description for this function in the docstring?","commit_id":"c7836630835c7e1feb49f120051f7d5e05fc9d49"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"bd8566815619bfb4859e74a75e4614b343c53a8d","unresolved":false,"context_lines":[{"line_number":45,"context_line":""},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"def get_ordered_locations(locations, **kwargs):"},{"line_number":48,"context_line":"    \"\"\""},{"line_number":49,"context_line":"    :param locations: The original image location list."},{"line_number":50,"context_line":"    :return: The image location list weighted by metadata_weight_key."},{"line_number":51,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"3afc51ec_d09e57b7","line":48,"in_reply_to":"bae84128_931807d5","updated":"2016-03-14 13:44:29.000000000","message":"+1","commit_id":"c7836630835c7e1feb49f120051f7d5e05fc9d49"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"69a4b60bde6b6d410fc7988ded9d269c6d4d737a","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    \"\"\""},{"line_number":52,"context_line":"    weight_key \u003d CONF.metadata_weight_location_strategy.metadata_weight_key"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    # return locations in original order if weight_key is not set"},{"line_number":55,"context_line":"    if weight_key is None:"},{"line_number":56,"context_line":"        return locations"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_be9d0068","line":54,"updated":"2016-02-23 18:57:49.000000000","message":"pls, use `NOTE($nick):`","commit_id":"c7836630835c7e1feb49f120051f7d5e05fc9d49"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"69a4b60bde6b6d410fc7988ded9d269c6d4d737a","unresolved":false,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    # return locations in original order if weight_key is not set"},{"line_number":55,"context_line":"    if weight_key is None:"},{"line_number":56,"context_line":"        return locations"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    ret \u003d []"},{"line_number":59,"context_line":"    others \u003d []"}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_5ed034f6","line":56,"updated":"2016-02-23 18:57:49.000000000","message":"Should this raise an error? If this strategy is used and this config is required, I think it\u0027d be better to fail here (or to have a default value, at least :D)","commit_id":"c7836630835c7e1feb49f120051f7d5e05fc9d49"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"4bc0a506aacb09145823dafbd2379342461f79d1","unresolved":false,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    # return locations in original order if weight_key is not set"},{"line_number":55,"context_line":"    if weight_key is None:"},{"line_number":56,"context_line":"        return locations"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    ret \u003d []"},{"line_number":59,"context_line":"    others \u003d []"}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_e2cfe253","line":56,"in_reply_to":"bae84128_5ed034f6","updated":"2016-02-23 23:03:13.000000000","message":"I think current locations is the default value, no?","commit_id":"c7836630835c7e1feb49f120051f7d5e05fc9d49"},{"author":{"_account_id":8064,"name":"Jake Yip","email":"jake.yip@ardc.edu.au","username":"jake"},"change_message_id":"2cecf79bb2d68d2b861e81479e20ccebaff5be27","unresolved":false,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    # return locations in original order if weight_key is not set"},{"line_number":55,"context_line":"    if weight_key is None:"},{"line_number":56,"context_line":"        return locations"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    ret \u003d []"},{"line_number":59,"context_line":"    others \u003d []"}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_7c5b1d2f","line":56,"in_reply_to":"bae84128_e2cfe253","updated":"2016-03-14 00:04:12.000000000","message":"By default it will return the locations in the default order if no key is specified. I rather it not fail hard if a key wasn\u0027t specified?","commit_id":"c7836630835c7e1feb49f120051f7d5e05fc9d49"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"bd8566815619bfb4859e74a75e4614b343c53a8d","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        weight \u003d metadata.get(weight_key)"},{"line_number":63,"context_line":"        if weight is not None:"},{"line_number":64,"context_line":"            try:"},{"line_number":65,"context_line":"                int(weight)"},{"line_number":66,"context_line":"                ret.append(location)"},{"line_number":67,"context_line":"            except ValueError:"},{"line_number":68,"context_line":"                others.append(location)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3afc51ec_b04f4bbc","line":65,"range":{"start_line":65,"start_character":26,"end_line":65,"end_character":27},"updated":"2016-03-14 13:44:29.000000000","message":"In what case  could \"weight\" not be a valid integer? I\u0027d be in favor of *not* working around type issues, and would rather have the code fail here.","commit_id":"c7836630835c7e1feb49f120051f7d5e05fc9d49"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"69a4b60bde6b6d410fc7988ded9d269c6d4d737a","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    if weight_key is None:"},{"line_number":56,"context_line":"        return locations"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    ret \u003d []"},{"line_number":59,"context_line":"    others \u003d []"},{"line_number":60,"context_line":"    for location in locations:"},{"line_number":61,"context_line":"        metadata \u003d location.get(\u0027metadata\u0027)"},{"line_number":62,"context_line":"        weight \u003d metadata.get(weight_key)"},{"line_number":63,"context_line":"        if weight is not None:"},{"line_number":64,"context_line":"            try:"},{"line_number":65,"context_line":"                int(weight)"},{"line_number":66,"context_line":"                ret.append(location)"},{"line_number":67,"context_line":"            except ValueError:"},{"line_number":68,"context_line":"                others.append(location)"},{"line_number":69,"context_line":"        else:"},{"line_number":70,"context_line":"            others.append(location)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    # sort locations based on weight key"},{"line_number":73,"context_line":"    ret.sort(key\u003dlambda loc: int(loc[\u0027metadata\u0027][weight_key]), reverse\u003dTrue)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    # locations without weight key are returned behind those with weight key"},{"line_number":76,"context_line":"    ret.extend(others)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    return ret"}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_39558af9","line":76,"range":{"start_line":58,"start_character":0,"end_line":76,"end_character":22},"updated":"2016-02-23 18:57:49.000000000","message":"Can we create a utility function that does the above and this in one iteration?\n\n\tdef _sort_weight(loc):\n\t    metadata \u003d location.get(\u0027metadata\u0027)\n\t    weight \u003d metadata.get(weight_key)\n\t    if weight is not None:\n\t        try:\n\t            return int(weight)\n\t        except ValueError:\n\t            pass\n\t    return 0\n\n\n         return sorted(locations, _sort_weight, reverse\u003dTrue)\n\n\nThe above is obviously not tested but you get the idea :D","commit_id":"c7836630835c7e1feb49f120051f7d5e05fc9d49"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"4bc0a506aacb09145823dafbd2379342461f79d1","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    if weight_key is None:"},{"line_number":56,"context_line":"        return locations"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    ret \u003d []"},{"line_number":59,"context_line":"    others \u003d []"},{"line_number":60,"context_line":"    for location in locations:"},{"line_number":61,"context_line":"        metadata \u003d location.get(\u0027metadata\u0027)"},{"line_number":62,"context_line":"        weight \u003d metadata.get(weight_key)"},{"line_number":63,"context_line":"        if weight is not None:"},{"line_number":64,"context_line":"            try:"},{"line_number":65,"context_line":"                int(weight)"},{"line_number":66,"context_line":"                ret.append(location)"},{"line_number":67,"context_line":"            except ValueError:"},{"line_number":68,"context_line":"                others.append(location)"},{"line_number":69,"context_line":"        else:"},{"line_number":70,"context_line":"            others.append(location)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    # sort locations based on weight key"},{"line_number":73,"context_line":"    ret.sort(key\u003dlambda loc: int(loc[\u0027metadata\u0027][weight_key]), reverse\u003dTrue)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    # locations without weight key are returned behind those with weight key"},{"line_number":76,"context_line":"    ret.extend(others)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    return ret"}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_9dad979d","line":76,"range":{"start_line":58,"start_character":0,"end_line":76,"end_character":22},"in_reply_to":"bae84128_39558af9","updated":"2016-02-23 23:03:13.000000000","message":"Hi Flavio, are you suggesting defining a customized key? If so, I don\u0027t think it works. Since for each location, its structure is like this: {\u0027url\u0027: \u0027http://a.b\u0027, \u0027metadata\": {\u0027weight\u0027: 10}}.\n\nIMHO, I would suggest define a custom cmp function. Like this:\n\n        def _cmp_location_weight(x, y):\n\t    x_metadata \u003d x.get(\u0027metadata\u0027)\n\t    x_weight \u003d x_metadata.get(weight_key)    \n            y_metadata \u003d y.get(\u0027metadata\u0027)\n\t    y_weight \u003d y_metadata.get(weight_key)\n            return cmp(x,y)\n        \n        return sorted(locations, cmp\u003d_cmp_location_weight, reverse\u003dTrue)\n\nDoes that make more sense?","commit_id":"c7836630835c7e1feb49f120051f7d5e05fc9d49"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"bd8566815619bfb4859e74a75e4614b343c53a8d","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    if weight_key is None:"},{"line_number":56,"context_line":"        return locations"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    ret \u003d []"},{"line_number":59,"context_line":"    others \u003d []"},{"line_number":60,"context_line":"    for location in locations:"},{"line_number":61,"context_line":"        metadata \u003d location.get(\u0027metadata\u0027)"},{"line_number":62,"context_line":"        weight \u003d metadata.get(weight_key)"},{"line_number":63,"context_line":"        if weight is not None:"},{"line_number":64,"context_line":"            try:"},{"line_number":65,"context_line":"                int(weight)"},{"line_number":66,"context_line":"                ret.append(location)"},{"line_number":67,"context_line":"            except ValueError:"},{"line_number":68,"context_line":"                others.append(location)"},{"line_number":69,"context_line":"        else:"},{"line_number":70,"context_line":"            others.append(location)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    # sort locations based on weight key"},{"line_number":73,"context_line":"    ret.sort(key\u003dlambda loc: int(loc[\u0027metadata\u0027][weight_key]), reverse\u003dTrue)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    # locations without weight key are returned behind those with weight key"},{"line_number":76,"context_line":"    ret.extend(others)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    return ret"}],"source_content_type":"text/x-python","patch_set":4,"id":"3afc51ec_f02b93e9","line":76,"range":{"start_line":58,"start_character":0,"end_line":76,"end_character":22},"in_reply_to":"bae84128_39558af9","updated":"2016-03-14 13:44:29.000000000","message":"I kind of like this.","commit_id":"c7836630835c7e1feb49f120051f7d5e05fc9d49"},{"author":{"_account_id":8064,"name":"Jake Yip","email":"jake.yip@ardc.edu.au","username":"jake"},"change_message_id":"2cecf79bb2d68d2b861e81479e20ccebaff5be27","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    if weight_key is None:"},{"line_number":56,"context_line":"        return locations"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    ret \u003d []"},{"line_number":59,"context_line":"    others \u003d []"},{"line_number":60,"context_line":"    for location in locations:"},{"line_number":61,"context_line":"        metadata \u003d location.get(\u0027metadata\u0027)"},{"line_number":62,"context_line":"        weight \u003d metadata.get(weight_key)"},{"line_number":63,"context_line":"        if weight is not None:"},{"line_number":64,"context_line":"            try:"},{"line_number":65,"context_line":"                int(weight)"},{"line_number":66,"context_line":"                ret.append(location)"},{"line_number":67,"context_line":"            except ValueError:"},{"line_number":68,"context_line":"                others.append(location)"},{"line_number":69,"context_line":"        else:"},{"line_number":70,"context_line":"            others.append(location)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    # sort locations based on weight key"},{"line_number":73,"context_line":"    ret.sort(key\u003dlambda loc: int(loc[\u0027metadata\u0027][weight_key]), reverse\u003dTrue)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    # locations without weight key are returned behind those with weight key"},{"line_number":76,"context_line":"    ret.extend(others)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    return ret"}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_bcd92586","line":76,"range":{"start_line":58,"start_character":0,"end_line":76,"end_character":22},"in_reply_to":"bae84128_39558af9","updated":"2016-03-14 00:04:12.000000000","message":"let me test this out and put it in the next patch set","commit_id":"c7836630835c7e1feb49f120051f7d5e05fc9d49"},{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"390a2fa98f0fc4fc62ebb5f8f59e899e1fe84fad","unresolved":false,"context_lines":[{"line_number":52,"context_line":""},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"def get_ordered_locations(locations, **kwargs):"},{"line_number":55,"context_line":"    \"\"\"Based on the weight key defined in location metadata. This method"},{"line_number":56,"context_line":"    will return an ordered location list by sorting the locations based on"},{"line_number":57,"context_line":"    the weight key."},{"line_number":58,"context_line":"    :param locations: The original image location list."},{"line_number":59,"context_line":"    :return: The image location list weighted by metadata_weight_key."},{"line_number":60,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a122d0e_a27cd220","line":57,"range":{"start_line":55,"start_character":7,"end_line":57,"end_character":19},"updated":"2016-05-05 18:44:25.000000000","message":"It would be great if we can document what the intended behavior is for this method. The sorting is one aspect but the how is another.\n\nIf developer is fixing bugs it will be hard to write and review what they change.","commit_id":"8ba790134ee2413b1138c1c0372f19accd7f7809"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"3fd374dff90e2f481f65a55ec3ea426225a567ac","unresolved":false,"context_lines":[{"line_number":52,"context_line":""},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"def get_ordered_locations(locations, **kwargs):"},{"line_number":55,"context_line":"    \"\"\"Based on the weight key defined in location metadata. This method"},{"line_number":56,"context_line":"    will return an ordered location list by sorting the locations based on"},{"line_number":57,"context_line":"    the weight key."},{"line_number":58,"context_line":"    :param locations: The original image location list."},{"line_number":59,"context_line":"    :return: The image location list weighted by metadata_weight_key."},{"line_number":60,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_61b322ea","line":57,"range":{"start_line":55,"start_character":7,"end_line":57,"end_character":19},"in_reply_to":"1a122d0e_a27cd220","updated":"2016-07-23 20:11:05.000000000","message":"No problem, I will fix it in next patch set.","commit_id":"8ba790134ee2413b1138c1c0372f19accd7f7809"},{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"390a2fa98f0fc4fc62ebb5f8f59e899e1fe84fad","unresolved":false,"context_lines":[{"line_number":65,"context_line":"    if weight_key is None:"},{"line_number":66,"context_line":"        return locations"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def _cmp_location_weight(x, y):"},{"line_number":69,"context_line":"        x_metadata \u003d x.get(\u0027metadata\u0027)"},{"line_number":70,"context_line":"        x_weight \u003d x_metadata.get(weight_key, -six.MAXSIZE)"},{"line_number":71,"context_line":"        x_weight \u003d x_weight if isinstance(x_weight, int) else -six.MAXSIZE"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a122d0e_82e4360d","line":68,"range":{"start_line":68,"start_character":28,"end_line":68,"end_character":35},"updated":"2016-05-05 18:44:25.000000000","message":"Hi Fei Long,\n\nThanks for working ahead on this code. \n\nPlease do not take this the wrong way but this is a hard to read code especially if you see the behavioral complexity that shows up as described by Brian on his review.\n\nPlease use some variable names that will highlight what x and y represent.","commit_id":"8ba790134ee2413b1138c1c0372f19accd7f7809"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"3fd374dff90e2f481f65a55ec3ea426225a567ac","unresolved":false,"context_lines":[{"line_number":65,"context_line":"    if weight_key is None:"},{"line_number":66,"context_line":"        return locations"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def _cmp_location_weight(x, y):"},{"line_number":69,"context_line":"        x_metadata \u003d x.get(\u0027metadata\u0027)"},{"line_number":70,"context_line":"        x_weight \u003d x_metadata.get(weight_key, -six.MAXSIZE)"},{"line_number":71,"context_line":"        x_weight \u003d x_weight if isinstance(x_weight, int) else -six.MAXSIZE"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_c1fad674","line":68,"range":{"start_line":68,"start_character":28,"end_line":68,"end_character":35},"in_reply_to":"1a122d0e_82e4360d","updated":"2016-07-23 20:11:05.000000000","message":"I see. But it\u0027s just a normal comparing method, so x and y means location x and location y. I can add a docstring if it\u0027s helpful but I don\u0027t think a long name could help :) But I will see how can i do to improve it.","commit_id":"8ba790134ee2413b1138c1c0372f19accd7f7809"},{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"390a2fa98f0fc4fc62ebb5f8f59e899e1fe84fad","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def _cmp_location_weight(x, y):"},{"line_number":69,"context_line":"        x_metadata \u003d x.get(\u0027metadata\u0027)"},{"line_number":70,"context_line":"        x_weight \u003d x_metadata.get(weight_key, -six.MAXSIZE)"},{"line_number":71,"context_line":"        x_weight \u003d x_weight if isinstance(x_weight, int) else -six.MAXSIZE"},{"line_number":72,"context_line":"        y_metadata \u003d y.get(\u0027metadata\u0027)"},{"line_number":73,"context_line":"        y_weight \u003d y_metadata.get(weight_key, -six.MAXSIZE)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a122d0e_ed2a3b7d","line":70,"range":{"start_line":70,"start_character":46,"end_line":70,"end_character":58},"updated":"2016-05-05 18:44:25.000000000","message":"Why do we need -MAXSIZE here?\n\nWhy isn\u0027t -1 enough? Looks like this is merely being used in greater than or less than comparison.","commit_id":"8ba790134ee2413b1138c1c0372f19accd7f7809"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"3fd374dff90e2f481f65a55ec3ea426225a567ac","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def _cmp_location_weight(x, y):"},{"line_number":69,"context_line":"        x_metadata \u003d x.get(\u0027metadata\u0027)"},{"line_number":70,"context_line":"        x_weight \u003d x_metadata.get(weight_key, -six.MAXSIZE)"},{"line_number":71,"context_line":"        x_weight \u003d x_weight if isinstance(x_weight, int) else -six.MAXSIZE"},{"line_number":72,"context_line":"        y_metadata \u003d y.get(\u0027metadata\u0027)"},{"line_number":73,"context_line":"        y_weight \u003d y_metadata.get(weight_key, -six.MAXSIZE)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_618682dc","line":70,"range":{"start_line":70,"start_character":46,"end_line":70,"end_character":58},"in_reply_to":"1a122d0e_ed2a3b7d","updated":"2016-07-23 20:11:05.000000000","message":"-1 maybe enough if we require user must give the weight a positive value normally. But I think -six.MAXSIZE is more safer so that the location can always at the bottom if there is no valid weight setting.","commit_id":"8ba790134ee2413b1138c1c0372f19accd7f7809"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"e74e245352bc8ac586294b05a72544f8b2a278d4","unresolved":false,"context_lines":[{"line_number":68,"context_line":"    def _cmp_location_weight(x, y):"},{"line_number":69,"context_line":"        x_metadata \u003d x.get(\u0027metadata\u0027)"},{"line_number":70,"context_line":"        x_weight \u003d x_metadata.get(weight_key, -six.MAXSIZE)"},{"line_number":71,"context_line":"        x_weight \u003d x_weight if isinstance(x_weight, int) else -six.MAXSIZE"},{"line_number":72,"context_line":"        y_metadata \u003d y.get(\u0027metadata\u0027)"},{"line_number":73,"context_line":"        y_weight \u003d y_metadata.get(weight_key, -six.MAXSIZE)"},{"line_number":74,"context_line":"        y_weight \u003d y_weight if isinstance(y_weight, int) else -six.MAXSIZE"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a122d0e_fc62efab","line":71,"updated":"2016-05-05 17:35:04.000000000","message":"I suggest casting the value to an int and setting it to -six.MAXSIZE if a ValueError is thrown.  This is going to be confusing enough for an operator to configure, it\u0027s going to be easy to put \"w\":\"10\" instead of \"w\": 10 by mistake.\n\nSame thing when you set y_weight below.","commit_id":"8ba790134ee2413b1138c1c0372f19accd7f7809"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"3fd374dff90e2f481f65a55ec3ea426225a567ac","unresolved":false,"context_lines":[{"line_number":68,"context_line":"    def _cmp_location_weight(x, y):"},{"line_number":69,"context_line":"        x_metadata \u003d x.get(\u0027metadata\u0027)"},{"line_number":70,"context_line":"        x_weight \u003d x_metadata.get(weight_key, -six.MAXSIZE)"},{"line_number":71,"context_line":"        x_weight \u003d x_weight if isinstance(x_weight, int) else -six.MAXSIZE"},{"line_number":72,"context_line":"        y_metadata \u003d y.get(\u0027metadata\u0027)"},{"line_number":73,"context_line":"        y_weight \u003d y_metadata.get(weight_key, -six.MAXSIZE)"},{"line_number":74,"context_line":"        y_weight \u003d y_weight if isinstance(y_weight, int) else -six.MAXSIZE"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_e149d2b8","line":71,"in_reply_to":"1a122d0e_fc62efab","updated":"2016-07-23 20:11:05.000000000","message":"No problem. I can fix it in next patch set.","commit_id":"8ba790134ee2413b1138c1c0372f19accd7f7809"}],"glance/tests/unit/common/test_location_strategy.py":[{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"5b78e4d6e46a5572a108b39d47ac8858011e0ef3","unresolved":false,"context_lines":[{"line_number":201,"context_line":"        self.config(metadata_weight_key\u003d\u0027weight\u0027,"},{"line_number":202,"context_line":"                    group\u003d\u0027metadata_weight_location_strategy\u0027)"},{"line_number":203,"context_line":"#        locs \u003d [{\u0027url\u0027: \u0027file://image0\u0027, \u0027metadata\u0027: {\u0027weight\u0027: 3, \u0027idx\u0027: 0}},"},{"line_number":204,"context_line":"#                ]"},{"line_number":205,"context_line":"        locs \u003d [{\u0027url\u0027: \u0027file://image0\u0027, \u0027metadata\u0027: {\u0027weight\u0027: 3, \u0027idx\u0027: 0}},"},{"line_number":206,"context_line":"                {\u0027url\u0027: \u0027rbd://image1\u0027, \u0027metadata\u0027: {\u0027weight\u0027: 0, \u0027idx\u0027: 3}},"},{"line_number":207,"context_line":"                {\u0027url\u0027: \u0027s3://image2\u0027, \u0027metadata\u0027: {\u0027idx\u0027: 4}},"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a68dd71_f135dcd9","line":204,"updated":"2016-01-21 21:40:21.000000000","message":"Remove it if we don\u0027t need it.","commit_id":"ae07fc2a1cde00ffa5a6798371b7ff9a70dcd1d7"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"5b78e4d6e46a5572a108b39d47ac8858011e0ef3","unresolved":false,"context_lines":[{"line_number":202,"context_line":"                    group\u003d\u0027metadata_weight_location_strategy\u0027)"},{"line_number":203,"context_line":"#        locs \u003d [{\u0027url\u0027: \u0027file://image0\u0027, \u0027metadata\u0027: {\u0027weight\u0027: 3, \u0027idx\u0027: 0}},"},{"line_number":204,"context_line":"#                ]"},{"line_number":205,"context_line":"        locs \u003d [{\u0027url\u0027: \u0027file://image0\u0027, \u0027metadata\u0027: {\u0027weight\u0027: 3, \u0027idx\u0027: 0}},"},{"line_number":206,"context_line":"                {\u0027url\u0027: \u0027rbd://image1\u0027, \u0027metadata\u0027: {\u0027weight\u0027: 0, \u0027idx\u0027: 3}},"},{"line_number":207,"context_line":"                {\u0027url\u0027: \u0027s3://image2\u0027, \u0027metadata\u0027: {\u0027idx\u0027: 4}},"},{"line_number":208,"context_line":"                {\u0027url\u0027: \u0027file://image3\u0027, \u0027metadata\u0027: {\u0027idx\u0027: 5}},"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a68dd71_115f500f","line":205,"updated":"2016-01-21 21:40:21.000000000","message":"Instead of using 0,1,2,3 for the weight, could you use 0, 1, 100, 10000","commit_id":"ae07fc2a1cde00ffa5a6798371b7ff9a70dcd1d7"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"5b78e4d6e46a5572a108b39d47ac8858011e0ef3","unresolved":false,"context_lines":[{"line_number":208,"context_line":"                {\u0027url\u0027: \u0027file://image3\u0027, \u0027metadata\u0027: {\u0027idx\u0027: 5}},"},{"line_number":209,"context_line":"                {\u0027url\u0027: \u0027rbd://image7\u0027, \u0027metadata\u0027: {\u0027weight\u0027: 1, \u0027idx\u0027: 2}},"},{"line_number":210,"context_line":"                {\u0027url\u0027: \u0027file://image8\u0027, \u0027metadata\u0027: {\u0027weight\u0027: 2, \u0027idx\u0027: 1}}]"},{"line_number":211,"context_line":"        ordered_locs \u003d metadata_weight.get_ordered_locations(copy.deepcopy(locs)) "},{"line_number":212,"context_line":"        locs.sort(key\u003dlambda loc: loc[\u0027metadata\u0027][\u0027idx\u0027])"},{"line_number":213,"context_line":"        # The result will ordered by metadata key."},{"line_number":214,"context_line":"        self.assertEqual(ordered_locs, locs)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a68dd71_91a900e2","line":211,"updated":"2016-01-21 21:40:21.000000000","message":"Remove the space","commit_id":"ae07fc2a1cde00ffa5a6798371b7ff9a70dcd1d7"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"16db0db1185a07d770e2457a4394d9be90504d4d","unresolved":false,"context_lines":[{"line_number":209,"context_line":"        o_locs \u003d metadata_weight.get_ordered_locations(copy.deepcopy(locs))"},{"line_number":210,"context_line":"        locs.sort(key\u003dlambda loc: loc[\u0027metadata\u0027][\u0027idx\u0027])"},{"line_number":211,"context_line":"        # The result will ordered by metadata key."},{"line_number":212,"context_line":"        self.assertEqual(o_locs, locs)"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    def test_get_ordered_locations_with_no_key(self):"},{"line_number":215,"context_line":"        self.config(metadata_weight_key\u003dNone,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a68dd71_250c443a","line":212,"range":{"start_line":212,"start_character":25,"end_line":212,"end_character":31},"updated":"2016-01-23 12:07:15.000000000","message":"Nit: This should be self.assertEqual(locs, o_locs) i.e expected, actual.","commit_id":"ce447da76aeb522bdce70feb175bc6283283b9f9"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"16db0db1185a07d770e2457a4394d9be90504d4d","unresolved":false,"context_lines":[{"line_number":216,"context_line":"                    group\u003d\u0027metadata_weight_location_strategy\u0027)"},{"line_number":217,"context_line":"        locs \u003d [{\u0027url\u0027: \u0027loc0\u0027}, {\u0027url\u0027: \u0027loc1\u0027}, {\u0027url\u0027: \u0027loc2\u0027}]"},{"line_number":218,"context_line":"        o_locs \u003d metadata_weight.get_ordered_locations(copy.deepcopy(locs))"},{"line_number":219,"context_line":"        self.assertEqual(o_locs, locs)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a68dd71_c5152840","line":219,"updated":"2016-01-23 12:07:15.000000000","message":"ditto","commit_id":"ce447da76aeb522bdce70feb175bc6283283b9f9"}]}