)]}'
{"nova/api/openstack/compute/extended_volumes.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b043617ce46f0a7879980879ea3b555c1da39bf0","unresolved":false,"context_lines":[{"line_number":30,"context_line":"        self.api_version_2_3 \u003d api_version_request.APIVersionRequest(\u00272.3\u0027)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    def _extend_server("},{"line_number":33,"context_line":"            self, context, server, instance, requested_version, bdms):"},{"line_number":34,"context_line":"        volumes_attached \u003d []"},{"line_number":35,"context_line":"        for bdm in bdms:"},{"line_number":36,"context_line":"            if bdm.get(\u0027volume_id\u0027):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_0c7219ae","line":33,"updated":"2015-10-01 17:41:50.000000000","message":"To be more consistent with our style, this should be:\n\n def _extend_server(self, context, server, instance, requested_version,\n                    bdms):","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"ba2491f06a8d57d683d1796eed57e63e943caf55","unresolved":false,"context_lines":[{"line_number":30,"context_line":"        self.api_version_2_3 \u003d api_version_request.APIVersionRequest(\u00272.3\u0027)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    def _extend_server("},{"line_number":33,"context_line":"            self, context, server, instance, requested_version, bdms):"},{"line_number":34,"context_line":"        volumes_attached \u003d []"},{"line_number":35,"context_line":"        for bdm in bdms:"},{"line_number":36,"context_line":"            if bdm.get(\u0027volume_id\u0027):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_ef0d57e9","line":33,"in_reply_to":"9a1a9d01_0c7219ae","updated":"2015-10-01 18:33:39.000000000","message":"Done","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b043617ce46f0a7879980879ea3b555c1da39bf0","unresolved":false,"context_lines":[{"line_number":56,"context_line":"                server,"},{"line_number":57,"context_line":"                db_instance,"},{"line_number":58,"context_line":"                req.api_version_request,"},{"line_number":59,"context_line":"                bdms[server[\u0027id\u0027]],"},{"line_number":60,"context_line":"            )"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    @wsgi.extends"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_4c3b319d","line":59,"updated":"2015-10-01 17:41:50.000000000","message":"Here server should be an instance object, so you should use attribute syntax:\n\n bdms[server.id]\n\nI know this file is not consistent because it was never updated after the object conversion. If you want to follow up with another patch to convert dict to attribute syntax, that\u0027d be cool.","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"ba2491f06a8d57d683d1796eed57e63e943caf55","unresolved":false,"context_lines":[{"line_number":56,"context_line":"                server,"},{"line_number":57,"context_line":"                db_instance,"},{"line_number":58,"context_line":"                req.api_version_request,"},{"line_number":59,"context_line":"                bdms[server[\u0027id\u0027]],"},{"line_number":60,"context_line":"            )"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    @wsgi.extends"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_6f92a7f8","line":59,"in_reply_to":"9a1a9d01_4c3b319d","updated":"2015-10-01 18:33:39.000000000","message":"\u0027server\u0027 is a plain old python dict at this point, not an object","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"eeae6b7763f483f96b804a36a1f8ae79e364b99f","unresolved":false,"context_lines":[{"line_number":56,"context_line":"                server,"},{"line_number":57,"context_line":"                db_instance,"},{"line_number":58,"context_line":"                req.api_version_request,"},{"line_number":59,"context_line":"                bdms[server[\u0027id\u0027]],"},{"line_number":60,"context_line":"            )"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    @wsgi.extends"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_a325380b","line":59,"in_reply_to":"9a1a9d01_6f92a7f8","updated":"2015-10-01 19:40:17.000000000","message":"Oh, sorry, this is coming from the response above, I was thinking request.","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b043617ce46f0a7879980879ea3b555c1da39bf0","unresolved":false,"context_lines":[{"line_number":57,"context_line":"                db_instance,"},{"line_number":58,"context_line":"                req.api_version_request,"},{"line_number":59,"context_line":"                bdms[server[\u0027id\u0027]],"},{"line_number":60,"context_line":"            )"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    @wsgi.extends"},{"line_number":63,"context_line":"    def detail(self, req, resp_obj):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_0ca9b9df","line":60,"updated":"2015-10-01 17:41:50.000000000","message":"Same style argument for this call.","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b043617ce46f0a7879980879ea3b555c1da39bf0","unresolved":false,"context_lines":[{"line_number":76,"context_line":"                    db_instance,"},{"line_number":77,"context_line":"                    req.api_version_request,"},{"line_number":78,"context_line":"                    bdms[server[\u0027id\u0027]],"},{"line_number":79,"context_line":"                )"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def _bdms_by_instance_uuid(self, context, instance_uuids):"},{"line_number":82,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuids("}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_0c80d945","line":79,"updated":"2015-10-01 17:41:50.000000000","message":"...and here.","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"ba2491f06a8d57d683d1796eed57e63e943caf55","unresolved":false,"context_lines":[{"line_number":76,"context_line":"                    db_instance,"},{"line_number":77,"context_line":"                    req.api_version_request,"},{"line_number":78,"context_line":"                    bdms[server[\u0027id\u0027]],"},{"line_number":79,"context_line":"                )"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def _bdms_by_instance_uuid(self, context, instance_uuids):"},{"line_number":82,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuids("}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_0fba0b6f","line":79,"in_reply_to":"9a1a9d01_0c80d945","updated":"2015-10-01 18:33:39.000000000","message":"\u0027server\u0027 is a plain old python dict at this point, not an object","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b043617ce46f0a7879980879ea3b555c1da39bf0","unresolved":false,"context_lines":[{"line_number":83,"context_line":"            context, instance_uuids)"},{"line_number":84,"context_line":"        result \u003d collections.defaultdict(list)"},{"line_number":85,"context_line":"        for bdm in bdms:"},{"line_number":86,"context_line":"            result[bdm[\u0027instance_uuid\u0027]].append(bdm)"},{"line_number":87,"context_line":"        return result"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_0cf779f6","line":86,"range":{"start_line":86,"start_character":19,"end_line":86,"end_character":39},"updated":"2015-10-01 17:41:50.000000000","message":"bdm is an object here, so you should use attribute syntax:\n\n  result[bdm.instance_uuid].append(bdm)","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"ba2491f06a8d57d683d1796eed57e63e943caf55","unresolved":false,"context_lines":[{"line_number":83,"context_line":"            context, instance_uuids)"},{"line_number":84,"context_line":"        result \u003d collections.defaultdict(list)"},{"line_number":85,"context_line":"        for bdm in bdms:"},{"line_number":86,"context_line":"            result[bdm[\u0027instance_uuid\u0027]].append(bdm)"},{"line_number":87,"context_line":"        return result"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_cfac1338","line":86,"in_reply_to":"9a1a9d01_0cf779f6","updated":"2015-10-01 18:33:39.000000000","message":"Done","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"67d44c9b925038d52e5ecc3641a78590a2c65295","unresolved":false,"context_lines":[{"line_number":51,"context_line":"            # server[\u0027id\u0027] is guaranteed to be in the cache due to"},{"line_number":52,"context_line":"            # the core API adding it in its \u0027show\u0027 method."},{"line_number":53,"context_line":"            bdms \u003d self._bdms_by_instance_uuid(context, [server[\u0027id\u0027]])"},{"line_number":54,"context_line":"            self._extend_server("},{"line_number":55,"context_line":"                context,"},{"line_number":56,"context_line":"                server,"},{"line_number":57,"context_line":"                db_instance,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a1a9d01_033f44d7","line":54,"updated":"2015-10-01 19:54:26.000000000","message":"Can you fix this style too please? You only need to wrap a few lines for this call.","commit_id":"b9cfa9ae93881ac4e016223ffadd34c7c40c4775"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b5b52fbd18d82478c45a3f7433c2dea1121586d4","unresolved":false,"context_lines":[{"line_number":51,"context_line":"            # server[\u0027id\u0027] is guaranteed to be in the cache due to"},{"line_number":52,"context_line":"            # the core API adding it in its \u0027show\u0027 method."},{"line_number":53,"context_line":"            bdms \u003d self._bdms_by_instance_uuid(context, [server[\u0027id\u0027]])"},{"line_number":54,"context_line":"            self._extend_server("},{"line_number":55,"context_line":"                context,"},{"line_number":56,"context_line":"                server,"},{"line_number":57,"context_line":"                db_instance,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a1a9d01_3e161ba8","line":54,"in_reply_to":"9a1a9d01_033f44d7","updated":"2015-10-01 20:04:58.000000000","message":"Agree, don\u0027t need to change this from 2 lines to 6, just wrap to the next line and align with the other args, like this:\n\nself._extend_server(context, server, db_instance,\n                    req.api_version_request,\n                    bdms[server[\u0027id\u0027]])","commit_id":"b9cfa9ae93881ac4e016223ffadd34c7c40c4775"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"60a22e79678fec9a52419fdc16c8f746e6d79106","unresolved":false,"context_lines":[{"line_number":51,"context_line":"            # server[\u0027id\u0027] is guaranteed to be in the cache due to"},{"line_number":52,"context_line":"            # the core API adding it in its \u0027show\u0027 method."},{"line_number":53,"context_line":"            bdms \u003d self._bdms_by_instance_uuid(context, [server[\u0027id\u0027]])"},{"line_number":54,"context_line":"            self._extend_server("},{"line_number":55,"context_line":"                context,"},{"line_number":56,"context_line":"                server,"},{"line_number":57,"context_line":"                db_instance,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a1a9d01_95c70564","line":54,"in_reply_to":"9a1a9d01_033f44d7","updated":"2015-10-01 20:48:44.000000000","message":"Done","commit_id":"b9cfa9ae93881ac4e016223ffadd34c7c40c4775"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"417064019d05406a38e59c0463742ce354e82622","unresolved":false,"context_lines":[{"line_number":51,"context_line":"            # server[\u0027id\u0027] is guaranteed to be in the cache due to"},{"line_number":52,"context_line":"            # the core API adding it in its \u0027show\u0027 method."},{"line_number":53,"context_line":"            bdms \u003d self._bdms_by_instance_uuid(context, [server[\u0027id\u0027]])"},{"line_number":54,"context_line":"            self._extend_server("},{"line_number":55,"context_line":"                context,"},{"line_number":56,"context_line":"                server,"},{"line_number":57,"context_line":"                db_instance,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a1a9d01_bed78bbe","line":54,"in_reply_to":"9a1a9d01_033f44d7","updated":"2015-10-01 20:01:21.000000000","message":"Re: please \"fix\". I don\u0027t see how this is broken. Perhaps I\u0027m missing an additional style guide? Can you link it to me? I did think I read the conventions... and that this is in line with them.","commit_id":"b9cfa9ae93881ac4e016223ffadd34c7c40c4775"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"014981f7bfbbfe283f674bcc2f8bbfe2d95d363b","unresolved":false,"context_lines":[{"line_number":51,"context_line":"            # server[\u0027id\u0027] is guaranteed to be in the cache due to"},{"line_number":52,"context_line":"            # the core API adding it in its \u0027show\u0027 method."},{"line_number":53,"context_line":"            bdms \u003d self._bdms_by_instance_uuid(context, [server[\u0027id\u0027]])"},{"line_number":54,"context_line":"            self._extend_server("},{"line_number":55,"context_line":"                context,"},{"line_number":56,"context_line":"                server,"},{"line_number":57,"context_line":"                db_instance,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a1a9d01_cbdabee9","line":54,"in_reply_to":"9a1a9d01_bed78bbe","updated":"2015-10-01 21:40:54.000000000","message":"Let me try asking again. I really didn\u0027t mean for that to come out the way it did. My apologies if I offended anyone.\n\nIn a file like extended_volumes.py that had exactly 2 wrapped lines, each done differently (so no clearly established convention), is there some style guide that documents a preference? I figured since it was already inconsistent, and that I need to change those lines anyway, that I could just pick one of the many acceptable approaches to line wrapping, but if there\u0027s a global convention, then all the better -- I\u0027ll just follow that next time (unless the existing code differs, then I will do what it\u0027s currently doing).\n\nIn the mean time, I believe I have changed all the whitespace in this diff as suggested. Thanks for taking the time to review this patch.","commit_id":"b9cfa9ae93881ac4e016223ffadd34c7c40c4775"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b5b52fbd18d82478c45a3f7433c2dea1121586d4","unresolved":false,"context_lines":[{"line_number":70,"context_line":"                db_instance \u003d req.get_db_instance(server[\u0027id\u0027])"},{"line_number":71,"context_line":"                # server[\u0027id\u0027] is guaranteed to be in the cache due to"},{"line_number":72,"context_line":"                # the core API adding it in its \u0027detail\u0027 method."},{"line_number":73,"context_line":"                self._extend_server("},{"line_number":74,"context_line":"                    context,"},{"line_number":75,"context_line":"                    server,"},{"line_number":76,"context_line":"                    db_instance,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a1a9d01_de811fc5","line":73,"updated":"2015-10-01 20:04:58.000000000","message":"Same comment here.","commit_id":"b9cfa9ae93881ac4e016223ffadd34c7c40c4775"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"60a22e79678fec9a52419fdc16c8f746e6d79106","unresolved":false,"context_lines":[{"line_number":70,"context_line":"                db_instance \u003d req.get_db_instance(server[\u0027id\u0027])"},{"line_number":71,"context_line":"                # server[\u0027id\u0027] is guaranteed to be in the cache due to"},{"line_number":72,"context_line":"                # the core API adding it in its \u0027detail\u0027 method."},{"line_number":73,"context_line":"                self._extend_server("},{"line_number":74,"context_line":"                    context,"},{"line_number":75,"context_line":"                    server,"},{"line_number":76,"context_line":"                    db_instance,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a1a9d01_75a591b3","line":73,"in_reply_to":"9a1a9d01_de811fc5","updated":"2015-10-01 20:48:44.000000000","message":"Done","commit_id":"b9cfa9ae93881ac4e016223ffadd34c7c40c4775"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"73ed991509a678cabf98397f2f6527113b69265a","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        result \u003d collections.defaultdict(list)"},{"line_number":76,"context_line":"        for bdm in bdms:"},{"line_number":77,"context_line":"            result[bdm.instance_uuid].append(bdm)"},{"line_number":78,"context_line":"        return result"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"class ExtendedVolumes(extensions.V21APIExtensionBase):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a2fa921_092012f6","line":78,"updated":"2015-10-09 09:47:35.000000000","message":"So this would actually go inside the method as per my comment on the previous PS","commit_id":"67c7441cdd97c74d9f3d8d2ccc535b4aeb120f82"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"f0eb1b7a8eacccdc2d4c834fdd5c30da818b2098","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        result \u003d collections.defaultdict(list)"},{"line_number":76,"context_line":"        for bdm in bdms:"},{"line_number":77,"context_line":"            result[bdm.instance_uuid].append(bdm)"},{"line_number":78,"context_line":"        return result"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"class ExtendedVolumes(extensions.V21APIExtensionBase):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a2fa921_3f71800f","line":78,"in_reply_to":"7a2fa921_092012f6","updated":"2015-10-09 10:39:19.000000000","message":"Yup, I do that in that in patch set 6 \u0026 7. This patch set (8) just reverts everything back to how it was in patch set 5.","commit_id":"67c7441cdd97c74d9f3d8d2ccc535b4aeb120f82"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"f6ae4f8571d365a5c92cc81e7eeadce572d32d6f","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        result \u003d collections.defaultdict(list)"},{"line_number":76,"context_line":"        for bdm in bdms:"},{"line_number":77,"context_line":"            result[bdm.instance_uuid].append(bdm)"},{"line_number":78,"context_line":"        return result"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"class ExtendedVolumes(extensions.V21APIExtensionBase):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a2fa921_43257b59","line":78,"in_reply_to":"7a2fa921_3f71800f","updated":"2015-10-09 13:57:41.000000000","message":"yes as commented on PS 7 - we can\u0027t actually do that - disregard this comment pls","commit_id":"67c7441cdd97c74d9f3d8d2ccc535b4aeb120f82"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"a368335e3ece017370ce6666339cc634ec561edd","unresolved":false,"context_lines":[{"line_number":46,"context_line":"        if soft_authorize(context):"},{"line_number":47,"context_line":"            server \u003d resp_obj.obj[\u0027server\u0027]"},{"line_number":48,"context_line":"            bdms \u003d objects.BlockDeviceMappingList.bdms_by_instance_uuid("},{"line_number":49,"context_line":"                context, [server[\u0027id\u0027]])"},{"line_number":50,"context_line":"            instance_bdms \u003d self._get_instance_bdms(bdms, server)"},{"line_number":51,"context_line":"            self._extend_server(context, server, version, instance_bdms)"},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"da85f559_c1c2c984","line":49,"updated":"2015-11-12 13:36:26.000000000","message":"Nit: I quite liked the get instance object and do instance.uuid, but I guess this is safe and efficient.","commit_id":"4bf0024f0236d3e8073a4ce36b49575f49891478"}],"nova/api/openstack/compute/legacy_v2/contrib/extended_volumes.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b043617ce46f0a7879980879ea3b555c1da39bf0","unresolved":false,"context_lines":[{"line_number":63,"context_line":"            context, instance_uuids)"},{"line_number":64,"context_line":"        result \u003d collections.defaultdict(list)"},{"line_number":65,"context_line":"        for bdm in bdms:"},{"line_number":66,"context_line":"            result[bdm[\u0027instance_uuid\u0027]].append(bdm)"},{"line_number":67,"context_line":"        return result"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_cc1c2116","line":66,"range":{"start_line":66,"start_character":19,"end_line":66,"end_character":39},"updated":"2015-10-01 17:41:50.000000000","message":"Same attribute syntax for server and bdm here too.","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"ba2491f06a8d57d683d1796eed57e63e943caf55","unresolved":false,"context_lines":[{"line_number":63,"context_line":"            context, instance_uuids)"},{"line_number":64,"context_line":"        result \u003d collections.defaultdict(list)"},{"line_number":65,"context_line":"        for bdm in bdms:"},{"line_number":66,"context_line":"            result[bdm[\u0027instance_uuid\u0027]].append(bdm)"},{"line_number":67,"context_line":"        return result"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_8f54db21","line":66,"in_reply_to":"9a1a9d01_cc1c2116","updated":"2015-10-01 18:33:39.000000000","message":"\u0027server\u0027 is a plain old python dict at this point, not an object\n\nI fixed the \u0027bdm\u0027 attribute access though.","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"67d44c9b925038d52e5ecc3641a78590a2c65295","unresolved":false,"context_lines":[{"line_number":42,"context_line":"            # the core API adding it in its \u0027show\u0027 method."},{"line_number":43,"context_line":"            bdms \u003d self._bdms_by_instance_uuid(context, [server[\u0027id\u0027]])"},{"line_number":44,"context_line":"            self._extend_server("},{"line_number":45,"context_line":"                context, server, db_instance, bdms[server[\u0027id\u0027]])"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    @wsgi.extends"},{"line_number":48,"context_line":"    def detail(self, req, resp_obj):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a1a9d01_2317e855","line":45,"updated":"2015-10-01 19:54:26.000000000","message":"And this.","commit_id":"b9cfa9ae93881ac4e016223ffadd34c7c40c4775"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b5b52fbd18d82478c45a3f7433c2dea1121586d4","unresolved":false,"context_lines":[{"line_number":42,"context_line":"            # the core API adding it in its \u0027show\u0027 method."},{"line_number":43,"context_line":"            bdms \u003d self._bdms_by_instance_uuid(context, [server[\u0027id\u0027]])"},{"line_number":44,"context_line":"            self._extend_server("},{"line_number":45,"context_line":"                context, server, db_instance, bdms[server[\u0027id\u0027]])"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    @wsgi.extends"},{"line_number":48,"context_line":"    def detail(self, req, resp_obj):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a1a9d01_fe5d037a","line":45,"in_reply_to":"9a1a9d01_2317e855","updated":"2015-10-01 20:04:58.000000000","message":"Yeah could just drop a line here for the new parameter - not as bad a change as in the other module though.","commit_id":"b9cfa9ae93881ac4e016223ffadd34c7c40c4775"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"60a22e79678fec9a52419fdc16c8f746e6d79106","unresolved":false,"context_lines":[{"line_number":42,"context_line":"            # the core API adding it in its \u0027show\u0027 method."},{"line_number":43,"context_line":"            bdms \u003d self._bdms_by_instance_uuid(context, [server[\u0027id\u0027]])"},{"line_number":44,"context_line":"            self._extend_server("},{"line_number":45,"context_line":"                context, server, db_instance, bdms[server[\u0027id\u0027]])"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    @wsgi.extends"},{"line_number":48,"context_line":"    def detail(self, req, resp_obj):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a1a9d01_1581b5fb","line":45,"in_reply_to":"9a1a9d01_fe5d037a","updated":"2015-10-01 20:48:44.000000000","message":"Done","commit_id":"b9cfa9ae93881ac4e016223ffadd34c7c40c4775"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"67d44c9b925038d52e5ecc3641a78590a2c65295","unresolved":false,"context_lines":[{"line_number":56,"context_line":"                # server[\u0027id\u0027] is guaranteed to be in the cache due to"},{"line_number":57,"context_line":"                # the core API adding it in its \u0027detail\u0027 method."},{"line_number":58,"context_line":"                self._extend_server("},{"line_number":59,"context_line":"                    context, server, db_instance, bdms[server[\u0027id\u0027]])"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def _bdms_by_instance_uuid(self, context, instance_uuids):"},{"line_number":62,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuids("}],"source_content_type":"text/x-python","patch_set":3,"id":"9a1a9d01_c3086c30","line":59,"updated":"2015-10-01 19:54:26.000000000","message":"And this.","commit_id":"b9cfa9ae93881ac4e016223ffadd34c7c40c4775"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"60a22e79678fec9a52419fdc16c8f746e6d79106","unresolved":false,"context_lines":[{"line_number":56,"context_line":"                # server[\u0027id\u0027] is guaranteed to be in the cache due to"},{"line_number":57,"context_line":"                # the core API adding it in its \u0027detail\u0027 method."},{"line_number":58,"context_line":"                self._extend_server("},{"line_number":59,"context_line":"                    context, server, db_instance, bdms[server[\u0027id\u0027]])"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def _bdms_by_instance_uuid(self, context, instance_uuids):"},{"line_number":62,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuids("}],"source_content_type":"text/x-python","patch_set":3,"id":"9a1a9d01_b5724948","line":59,"in_reply_to":"9a1a9d01_c3086c30","updated":"2015-10-01 20:48:44.000000000","message":"Done","commit_id":"b9cfa9ae93881ac4e016223ffadd34c7c40c4775"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b5b52fbd18d82478c45a3f7433c2dea1121586d4","unresolved":false,"context_lines":[{"line_number":58,"context_line":"                self._extend_server("},{"line_number":59,"context_line":"                    context, server, db_instance, bdms[server[\u0027id\u0027]])"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def _bdms_by_instance_uuid(self, context, instance_uuids):"},{"line_number":62,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuids("},{"line_number":63,"context_line":"            context, instance_uuids)"},{"line_number":64,"context_line":"        result \u003d collections.defaultdict(list)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a1a9d01_5ee42fab","line":61,"updated":"2015-10-01 20:04:58.000000000","message":"This is duplicated in the other module - we could pull this into the common module as a helper method...","commit_id":"b9cfa9ae93881ac4e016223ffadd34c7c40c4775"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"db37a10e1ca32eebd4ccb0ead8832df50c3e64da","unresolved":false,"context_lines":[{"line_number":58,"context_line":"                self._extend_server("},{"line_number":59,"context_line":"                    context, server, db_instance, bdms[server[\u0027id\u0027]])"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def _bdms_by_instance_uuid(self, context, instance_uuids):"},{"line_number":62,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuids("},{"line_number":63,"context_line":"            context, instance_uuids)"},{"line_number":64,"context_line":"        result \u003d collections.defaultdict(list)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a1a9d01_32ce4fb5","line":61,"in_reply_to":"9a1a9d01_5ee42fab","updated":"2015-10-01 20:13:18.000000000","message":"Both of the files named extended_volumnes.py are pretty much entirely the same, so I figured they intentionally weren\u0027t sharing anything, perhaps with the goal of deleting one in some ideal world... Should they be refactored to share code?","commit_id":"b9cfa9ae93881ac4e016223ffadd34c7c40c4775"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"1fbcadc1c641c4844481ce6ea9b1d5d929eaf9f3","unresolved":false,"context_lines":[{"line_number":58,"context_line":"                self._extend_server("},{"line_number":59,"context_line":"                    context, server, db_instance, bdms[server[\u0027id\u0027]])"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def _bdms_by_instance_uuid(self, context, instance_uuids):"},{"line_number":62,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuids("},{"line_number":63,"context_line":"            context, instance_uuids)"},{"line_number":64,"context_line":"        result \u003d collections.defaultdict(list)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_d83e8327","line":61,"in_reply_to":"9a1a9d01_5ee42fab","updated":"2015-10-08 19:12:13.000000000","message":"I removed this duplication in patch set 6. The dictionary building is now done in BlockDeviceMappingList.get_by_instance_uuids().","commit_id":"b9cfa9ae93881ac4e016223ffadd34c7c40c4775"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"312a7c02a45edf92d97b20e07e1317b196465989","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":26,"context_line":"        super(ExtendedVolumesController, self).__init__(*args, **kwargs)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    def _extend_server(self, context, server, instance, bdms):"},{"line_number":29,"context_line":"        volume_ids \u003d [bdm.volume_id for bdm in bdms if bdm.volume_id]"},{"line_number":30,"context_line":"        key \u003d \"%s:volumes_attached\" % Extended_volumes.alias"},{"line_number":31,"context_line":"        server[key] \u003d [{\u0027id\u0027: volume_id} for volume_id in volume_ids]"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a2ca52d_4df51881","line":28,"updated":"2015-10-15 08:51:50.000000000","message":"instance parameter is not used anymore","commit_id":"82e7e8a096909259b2fe45ddbd86273557685016"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"4a086e2e487ce784f8c553f0cca32e7555d1797f","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":26,"context_line":"        super(ExtendedVolumesController, self).__init__(*args, **kwargs)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    def _extend_server(self, context, server, instance, bdms):"},{"line_number":29,"context_line":"        volume_ids \u003d [bdm.volume_id for bdm in bdms if bdm.volume_id]"},{"line_number":30,"context_line":"        key \u003d \"%s:volumes_attached\" % Extended_volumes.alias"},{"line_number":31,"context_line":"        server[key] \u003d [{\u0027id\u0027: volume_id} for volume_id in volume_ids]"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a2ca52d_fb42903d","line":28,"in_reply_to":"5a2ca52d_4df51881","updated":"2015-10-15 13:58:22.000000000","message":"Good catch, thanks!","commit_id":"82e7e8a096909259b2fe45ddbd86273557685016"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"9e6f6933ce3e422de31ed87758cca38ae536d1fa","unresolved":false,"context_lines":[{"line_number":41,"context_line":"            bdms \u003d objects.BlockDeviceMappingList.bdms_by_instance_uuid("},{"line_number":42,"context_line":"                context, [server[\u0027id\u0027]])"},{"line_number":43,"context_line":"            self._extend_server(context, server, db_instance,"},{"line_number":44,"context_line":"                                bdms[server[\u0027id\u0027]])"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    @wsgi.extends"},{"line_number":47,"context_line":"    def detail(self, req, resp_obj):"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a2ca52d_0aa70650","line":44,"updated":"2015-10-15 08:15:11.000000000","message":"Right - the race is here because we cache the instnce and it could be gone from the db by the time we get here so it will not show up in bdms.\n\nSo to keep old behaviour which is arguably wrong (it will show a server since it was cached even if it was deleted in the meantime, but will not show it\u0027s volumes, as the query would show up empty) I suggest we do a\n\n   bdms.get(server[\u0027id\u0027], [])\n\nhere.\n\nAlternatively - it might make sense to allow for read_deleted on the BDM query specifically for this case, although it would change (fix?) the API behaviour. This might actually deserve a bug report against the API.\n\nAlso a comment would be nice explaining why we do the get() there i.e. why can we be missing the id in the bdms dict.","commit_id":"82e7e8a096909259b2fe45ddbd86273557685016"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"4a086e2e487ce784f8c553f0cca32e7555d1797f","unresolved":false,"context_lines":[{"line_number":41,"context_line":"            bdms \u003d objects.BlockDeviceMappingList.bdms_by_instance_uuid("},{"line_number":42,"context_line":"                context, [server[\u0027id\u0027]])"},{"line_number":43,"context_line":"            self._extend_server(context, server, db_instance,"},{"line_number":44,"context_line":"                                bdms[server[\u0027id\u0027]])"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    @wsgi.extends"},{"line_number":47,"context_line":"    def detail(self, req, resp_obj):"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a2ca52d_9b783c91","line":44,"in_reply_to":"5a2ca52d_0aa70650","updated":"2015-10-15 13:58:22.000000000","message":"Thanks, I\u0027ll make it backwards compatible for now and then submit a bug.","commit_id":"82e7e8a096909259b2fe45ddbd86273557685016"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"312a7c02a45edf92d97b20e07e1317b196465989","unresolved":false,"context_lines":[{"line_number":41,"context_line":"            bdms \u003d objects.BlockDeviceMappingList.bdms_by_instance_uuid("},{"line_number":42,"context_line":"                context, [server[\u0027id\u0027]])"},{"line_number":43,"context_line":"            self._extend_server(context, server, db_instance,"},{"line_number":44,"context_line":"                                bdms[server[\u0027id\u0027]])"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    @wsgi.extends"},{"line_number":47,"context_line":"    def detail(self, req, resp_obj):"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a2ca52d_8d86d011","line":44,"in_reply_to":"5a2ca52d_0aa70650","updated":"2015-10-15 08:51:50.000000000","message":"Why not use the old call here? This is for a single instance right?\n\nHaving bdms \u003d objects.BlockDeviceMapping.get_by_instance_uuid() here would keep the old behavior and avoid the race.","commit_id":"82e7e8a096909259b2fe45ddbd86273557685016"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"4a086e2e487ce784f8c553f0cca32e7555d1797f","unresolved":false,"context_lines":[{"line_number":41,"context_line":"            bdms \u003d objects.BlockDeviceMappingList.bdms_by_instance_uuid("},{"line_number":42,"context_line":"                context, [server[\u0027id\u0027]])"},{"line_number":43,"context_line":"            self._extend_server(context, server, db_instance,"},{"line_number":44,"context_line":"                                bdms[server[\u0027id\u0027]])"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    @wsgi.extends"},{"line_number":47,"context_line":"    def detail(self, req, resp_obj):"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a2ca52d_ae4cd470","line":44,"in_reply_to":"5a2ca52d_8d86d011","updated":"2015-10-15 13:58:22.000000000","message":"Both methods (show \u0026 detail) are using BlockDeviceMappingList.bdms_by_instance_uuid() because in an earlier patch set they both went through some additional post-processing. This is also the case again in patch set 12 (although minor).","commit_id":"82e7e8a096909259b2fe45ddbd86273557685016"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"312a7c02a45edf92d97b20e07e1317b196465989","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        if authorize(context):"},{"line_number":50,"context_line":"            servers \u003d list(resp_obj.obj[\u0027servers\u0027])"},{"line_number":51,"context_line":"            instance_uuids \u003d [server[\u0027id\u0027] for server in servers]"},{"line_number":52,"context_line":"            bdms \u003d objects.BlockDeviceMappingList.bdms_by_instance_uuid("},{"line_number":53,"context_line":"                context, instance_uuids)"},{"line_number":54,"context_line":"            for server in servers:"},{"line_number":55,"context_line":"                db_instance \u003d req.get_db_instance(server[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a2ca52d_8d71f0e2","line":52,"updated":"2015-10-15 08:51:50.000000000","message":"maybe name this dict_of_bdms or bdms_by_instance to better explain it\u0027s contents.","commit_id":"82e7e8a096909259b2fe45ddbd86273557685016"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"4a086e2e487ce784f8c553f0cca32e7555d1797f","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        if authorize(context):"},{"line_number":50,"context_line":"            servers \u003d list(resp_obj.obj[\u0027servers\u0027])"},{"line_number":51,"context_line":"            instance_uuids \u003d [server[\u0027id\u0027] for server in servers]"},{"line_number":52,"context_line":"            bdms \u003d objects.BlockDeviceMappingList.bdms_by_instance_uuid("},{"line_number":53,"context_line":"                context, instance_uuids)"},{"line_number":54,"context_line":"            for server in servers:"},{"line_number":55,"context_line":"                db_instance \u003d req.get_db_instance(server[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a2ca52d_5b8704d7","line":52,"in_reply_to":"5a2ca52d_8d71f0e2","updated":"2015-10-15 13:58:22.000000000","message":"It\u0027s already 73 characters with all arguments pushed to the next line... neither of those naming suggestions work without making other reliability compromises. It does say `bdms_by_instance_uuid` on the right hand side, and the methods are small, so I think it\u0027s clear enough to the reader. I\u0027m happy to revisit this if you feel strongly about it though.","commit_id":"82e7e8a096909259b2fe45ddbd86273557685016"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"312a7c02a45edf92d97b20e07e1317b196465989","unresolved":false,"context_lines":[{"line_number":56,"context_line":"                # server[\u0027id\u0027] is guaranteed to be in the cache due to"},{"line_number":57,"context_line":"                # the core API adding it in its \u0027detail\u0027 method."},{"line_number":58,"context_line":"                self._extend_server(context, server, db_instance,"},{"line_number":59,"context_line":"                                    bdms[server[\u0027id\u0027]])"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"class Extended_volumes(extensions.ExtensionDescriptor):"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a2ca52d_adef0c86","line":59,"updated":"2015-10-15 08:51:50.000000000","message":"This might race as well since server[\u0027id\u0027] isn\u0027t guaranteed to exist in bdms.\n\nWe could instead iterate over bdms above since bdms is based on servers.","commit_id":"82e7e8a096909259b2fe45ddbd86273557685016"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"4a086e2e487ce784f8c553f0cca32e7555d1797f","unresolved":false,"context_lines":[{"line_number":56,"context_line":"                # server[\u0027id\u0027] is guaranteed to be in the cache due to"},{"line_number":57,"context_line":"                # the core API adding it in its \u0027detail\u0027 method."},{"line_number":58,"context_line":"                self._extend_server(context, server, db_instance,"},{"line_number":59,"context_line":"                                    bdms[server[\u0027id\u0027]])"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"class Extended_volumes(extensions.ExtensionDescriptor):"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a2ca52d_7b2d2045","line":59,"in_reply_to":"5a2ca52d_adef0c86","updated":"2015-10-15 13:58:22.000000000","message":"Yup, there are 4 places in 2 files where this could happen.\n\nI suspect iterating over bdms rather than servers is backwards incompatible in the instance-is-already-deleted case (those uuids will exist in `servers` but not `bdms`).","commit_id":"82e7e8a096909259b2fe45ddbd86273557685016"}],"nova/db/api.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"eeae6b7763f483f96b804a36a1f8ae79e364b99f","unresolved":false,"context_lines":[{"line_number":1124,"context_line":"    \"\"\"Roll back quota reservations.\"\"\""},{"line_number":1125,"context_line":"    return IMPL.reservation_rollback(context, reservations,"},{"line_number":1126,"context_line":"                                     project_id\u003dproject_id,"},{"line_number":1127,"context_line":"                                     user_id\u003duser_id)"},{"line_number":1128,"context_line":""},{"line_number":1129,"context_line":""},{"line_number":1130,"context_line":"def quota_destroy_all_by_project_and_user(context, project_id, user_id):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_038744d8","line":1127,"range":{"start_line":1127,"start_character":37,"end_line":1127,"end_character":52},"updated":"2015-10-01 19:40:17.000000000","message":"Here\u0027s an example.","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b043617ce46f0a7879980879ea3b555c1da39bf0","unresolved":false,"context_lines":[{"line_number":1196,"context_line":"    \"\"\"Get all block device mapping belonging to a list of instances.\"\"\""},{"line_number":1197,"context_line":"    return IMPL.block_device_mapping_get_all_by_instance_uuids(context,"},{"line_number":1198,"context_line":"                                                               instance_uuid,"},{"line_number":1199,"context_line":"                                                               use_slave)"},{"line_number":1200,"context_line":""},{"line_number":1201,"context_line":""},{"line_number":1202,"context_line":"def block_device_mapping_get_all_by_instance(context, instance_uuid,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_8cc4e95f","line":1199,"range":{"start_line":1199,"start_character":63,"end_line":1199,"end_character":72},"updated":"2015-10-01 17:41:50.000000000","message":"use_slave is a keyword argument in the function you\u0027re calling. We\u0027re not consistent about this, but many people (myself included) prefer calling the keyword arguments as keywords, which helps a lot when refactoring (and certainly when remoting) calls.","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"eeae6b7763f483f96b804a36a1f8ae79e364b99f","unresolved":false,"context_lines":[{"line_number":1196,"context_line":"    \"\"\"Get all block device mapping belonging to a list of instances.\"\"\""},{"line_number":1197,"context_line":"    return IMPL.block_device_mapping_get_all_by_instance_uuids(context,"},{"line_number":1198,"context_line":"                                                               instance_uuid,"},{"line_number":1199,"context_line":"                                                               use_slave)"},{"line_number":1200,"context_line":""},{"line_number":1201,"context_line":""},{"line_number":1202,"context_line":"def block_device_mapping_get_all_by_instance(context, instance_uuid,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_43cbfc1e","line":1199,"in_reply_to":"9a1a9d01_52c0a8e0","updated":"2015-10-01 19:40:17.000000000","message":"No, not in this patch for sure. Anyway, you don\u0027t have to do it, although I think it\u0027s worth doing it right, and it\u0027s not a visibly different style like my other complaint about wrapping lines.\n\nI\u0027ll not complain if you don\u0027t change it, but maybe something to keep in mind in the future.","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"ba2491f06a8d57d683d1796eed57e63e943caf55","unresolved":false,"context_lines":[{"line_number":1196,"context_line":"    \"\"\"Get all block device mapping belonging to a list of instances.\"\"\""},{"line_number":1197,"context_line":"    return IMPL.block_device_mapping_get_all_by_instance_uuids(context,"},{"line_number":1198,"context_line":"                                                               instance_uuid,"},{"line_number":1199,"context_line":"                                                               use_slave)"},{"line_number":1200,"context_line":""},{"line_number":1201,"context_line":""},{"line_number":1202,"context_line":"def block_device_mapping_get_all_by_instance(context, instance_uuid,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_52c0a8e0","line":1199,"in_reply_to":"9a1a9d01_8cc4e95f","updated":"2015-10-01 18:33:39.000000000","message":"None of the surrounding code does this... and I won\u0027t get away with changing them all in this section, will I?","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"eeae6b7763f483f96b804a36a1f8ae79e364b99f","unresolved":false,"context_lines":[{"line_number":1251,"context_line":"                               columns_to_join\u003dNone):"},{"line_number":1252,"context_line":"    \"\"\"Returns a security group with the specified name from a project.\"\"\""},{"line_number":1253,"context_line":"    return IMPL.security_group_get_by_name(context, project_id, group_name,"},{"line_number":1254,"context_line":"                                           columns_to_join\u003dNone)"},{"line_number":1255,"context_line":""},{"line_number":1256,"context_line":""},{"line_number":1257,"context_line":"def security_group_get_by_project(context, project_id):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_63764008","line":1254,"range":{"start_line":1254,"start_character":43,"end_line":1254,"end_character":63},"updated":"2015-10-01 19:40:17.000000000","message":"The next example","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"a368335e3ece017370ce6666339cc634ec561edd","unresolved":false,"context_lines":[{"line_number":1198,"context_line":""},{"line_number":1199,"context_line":""},{"line_number":1200,"context_line":"def block_device_mapping_get_all_by_instance_uuids(context, instance_uuid,"},{"line_number":1201,"context_line":"                                                   use_slave\u003dFalse):"},{"line_number":1202,"context_line":"    \"\"\"Get all block device mapping belonging to a list of instances.\"\"\""},{"line_number":1203,"context_line":"    return IMPL.block_device_mapping_get_all_by_instance_uuids(context,"},{"line_number":1204,"context_line":"                                                               instance_uuid,"}],"source_content_type":"text/x-python","patch_set":14,"id":"da85f559_a1fc65b8","line":1201,"updated":"2015-11-12 13:36:26.000000000","message":"Nit: I think we are in the process of dropping use_slave, maybe we should drop this.","commit_id":"4bf0024f0236d3e8073a4ce36b49575f49891478"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"4d08ce9d360f8844b473fca8878b75dad9ef2d7d","unresolved":false,"context_lines":[{"line_number":1198,"context_line":""},{"line_number":1199,"context_line":""},{"line_number":1200,"context_line":"def block_device_mapping_get_all_by_instance_uuids(context, instance_uuid,"},{"line_number":1201,"context_line":"                                                   use_slave\u003dFalse):"},{"line_number":1202,"context_line":"    \"\"\"Get all block device mapping belonging to a list of instances.\"\"\""},{"line_number":1203,"context_line":"    return IMPL.block_device_mapping_get_all_by_instance_uuids(context,"},{"line_number":1204,"context_line":"                                                               instance_uuid,"}],"source_content_type":"text/x-python","patch_set":14,"id":"da85f559_771abfc7","line":1201,"in_reply_to":"da85f559_a1fc65b8","updated":"2015-11-12 15:06:52.000000000","message":"Good to know, thanks! I\u0027ve dropped \u0027use_slave\u0027 from these new functions.","commit_id":"4bf0024f0236d3e8073a4ce36b49575f49891478"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"1d8f8389f35c2d2077beb46b02e574df7cb77fde","unresolved":false,"context_lines":[{"line_number":1197,"context_line":"    return IMPL.block_device_mapping_update_or_create(context, values, legacy)"},{"line_number":1198,"context_line":""},{"line_number":1199,"context_line":""},{"line_number":1200,"context_line":"def block_device_mapping_get_all_by_instance_uuids(context, instance_uuid):"},{"line_number":1201,"context_line":"    \"\"\"Get all block device mapping belonging to a list of instances.\"\"\""},{"line_number":1202,"context_line":"    return IMPL.block_device_mapping_get_all_by_instance_uuids(context,"},{"line_number":1203,"context_line":"                                                               instance_uuid)"}],"source_content_type":"text/x-python","patch_set":15,"id":"da85f559_b79d7e0b","line":1200,"updated":"2015-11-12 16:03:30.000000000","message":"Nit: the param name should probably be plural, no?","commit_id":"2e7f581a9379da5c1f8292a8a754bbf8f780aba8"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"7c9261f41434010e1eaaffbfc70e0b97083fce72","unresolved":false,"context_lines":[{"line_number":1197,"context_line":"    return IMPL.block_device_mapping_update_or_create(context, values, legacy)"},{"line_number":1198,"context_line":""},{"line_number":1199,"context_line":""},{"line_number":1200,"context_line":"def block_device_mapping_get_all_by_instance_uuids(context, instance_uuid):"},{"line_number":1201,"context_line":"    \"\"\"Get all block device mapping belonging to a list of instances.\"\"\""},{"line_number":1202,"context_line":"    return IMPL.block_device_mapping_get_all_by_instance_uuids(context,"},{"line_number":1203,"context_line":"                                                               instance_uuid)"}],"source_content_type":"text/x-python","patch_set":15,"id":"da85f559_52a46049","line":1200,"in_reply_to":"da85f559_b79d7e0b","updated":"2015-11-12 16:14:29.000000000","message":"Oh my! Yes, I\u0027ll fix that shortly. Thanks :)","commit_id":"2e7f581a9379da5c1f8292a8a754bbf8f780aba8"}],"nova/db/sqlalchemy/api.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b043617ce46f0a7879980879ea3b555c1da39bf0","unresolved":false,"context_lines":[{"line_number":3957,"context_line":""},{"line_number":3958,"context_line":"@require_context"},{"line_number":3959,"context_line":"def block_device_mapping_get_all_by_instance_uuids("},{"line_number":3960,"context_line":"        context, instance_uuids, use_slave\u003dFalse):"},{"line_number":3961,"context_line":"    if not instance_uuids:"},{"line_number":3962,"context_line":"        return []"},{"line_number":3963,"context_line":"    return _block_device_mapping_get_query("}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_0ce1b9fe","line":3960,"updated":"2015-10-01 17:41:50.000000000","message":"Same style thing here.","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"ba2491f06a8d57d683d1796eed57e63e943caf55","unresolved":false,"context_lines":[{"line_number":3957,"context_line":""},{"line_number":3958,"context_line":"@require_context"},{"line_number":3959,"context_line":"def block_device_mapping_get_all_by_instance_uuids("},{"line_number":3960,"context_line":"        context, instance_uuids, use_slave\u003dFalse):"},{"line_number":3961,"context_line":"    if not instance_uuids:"},{"line_number":3962,"context_line":"        return []"},{"line_number":3963,"context_line":"    return _block_device_mapping_get_query("}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_324c1476","line":3960,"in_reply_to":"9a1a9d01_0ce1b9fe","updated":"2015-10-01 18:33:39.000000000","message":"This is going to be painful. I think that way to soooo ugly ;(\n\nBut... Done.","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"}],"nova/exception.py":[{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"b3e57f7ad32b907dfe98d2d7f6cc22413357e0e8","unresolved":false,"context_lines":[{"line_number":575,"context_line":"    msg_fmt \u003d _(\"Volume %(volume_id)s could not be found.\")"},{"line_number":576,"context_line":""},{"line_number":577,"context_line":""},{"line_number":578,"context_line":"class UndefinedRootBDM(NovaException):"},{"line_number":579,"context_line":"    msg_fmt \u003d _(\"Undefined Block Device Mapping root: list contains\""},{"line_number":580,"context_line":"                \"Block Device Mappings from multiple instances.\")"},{"line_number":581,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"7a2fa921_cb0c153e","line":578,"updated":"2015-10-12 07:05:23.000000000","message":"I wonder if we should make it clearer somehow that this is a very Objects related exception - more for the readers of the code than anything else....","commit_id":"d618c4810db6ac268a2acdbd387f37be2007f18a"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"317693b0895d64090af300d31366f21d31c5fbd7","unresolved":false,"context_lines":[{"line_number":575,"context_line":"    msg_fmt \u003d _(\"Volume %(volume_id)s could not be found.\")"},{"line_number":576,"context_line":""},{"line_number":577,"context_line":""},{"line_number":578,"context_line":"class UndefinedRootBDM(NovaException):"},{"line_number":579,"context_line":"    msg_fmt \u003d _(\"Undefined Block Device Mapping root: list contains\""},{"line_number":580,"context_line":"                \"Block Device Mappings from multiple instances.\")"},{"line_number":581,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"7a2fa921_828837c8","line":578,"in_reply_to":"7a2fa921_cb0c153e","updated":"2015-10-12 12:53:26.000000000","message":"Does s/list/BlockDeviceMappingList help?\n\nBefore: Undefined Block Device Mapping root: list contains Block Device Mappings from multiple instances.\n\nAfter: Undefined Block Device Mapping root: BlockDeviceMappingList contains Block Device Mappings from multiple instances.","commit_id":"d618c4810db6ac268a2acdbd387f37be2007f18a"}],"nova/objects/block_device.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"341c7bf4cf8006cad8ead73b6a728860fa5607fc","unresolved":false,"context_lines":[{"line_number":274,"context_line":"    # Version 1.14: BlockDeviceMapping \u003c\u003d version 1.13"},{"line_number":275,"context_line":"    # Version 1.15: BlockDeviceMapping \u003c\u003d version 1.14"},{"line_number":276,"context_line":"    # Version 1.16: BlockDeviceMapping \u003c\u003d version 1.15"},{"line_number":277,"context_line":"    # Version 1.17: BlockDeviceMapping \u003c\u003d version 1.16"},{"line_number":278,"context_line":"    VERSION \u003d \u00271.17\u0027"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1a9d01_1449c58e","line":277,"updated":"2015-10-01 15:42:36.000000000","message":"This should describe the change you\u0027re making. The 1.16 version you copied was changed because the dependent object (BlockDeviceMapping) changed versions. In this change you\u0027re making, it\u0027s because you\u0027re adding a method. So, this should read:\n\n Version 1.17: Add get_by_instance_uuids()","commit_id":"ee9c7f3795e67151c00d6112bc8d81c4b7ae6f68"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"5bcd78671801278d2909a1933fbae20c5bcd9fe0","unresolved":false,"context_lines":[{"line_number":274,"context_line":"    # Version 1.14: BlockDeviceMapping \u003c\u003d version 1.13"},{"line_number":275,"context_line":"    # Version 1.15: BlockDeviceMapping \u003c\u003d version 1.14"},{"line_number":276,"context_line":"    # Version 1.16: BlockDeviceMapping \u003c\u003d version 1.15"},{"line_number":277,"context_line":"    # Version 1.17: BlockDeviceMapping \u003c\u003d version 1.16"},{"line_number":278,"context_line":"    VERSION \u003d \u00271.17\u0027"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1a9d01_6fbaac0c","line":277,"in_reply_to":"9a1a9d01_1449c58e","updated":"2015-10-01 15:48:50.000000000","message":"Thanks! I wondered what was actually supposed to be here... I just followed the existing pattern, not knowing. I\u0027ll fix this with the next patch.","commit_id":"ee9c7f3795e67151c00d6112bc8d81c4b7ae6f68"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6552e7006b1f91f9f60e7def8cbf69d279522788","unresolved":false,"context_lines":[{"line_number":286,"context_line":"                    (\u00271.6\u0027, \u00271.5\u0027), (\u00271.7\u0027, \u00271.6\u0027), (\u00271.8\u0027, \u00271.7\u0027),"},{"line_number":287,"context_line":"                    (\u00271.9\u0027, \u00271.8\u0027), (\u00271.10\u0027, \u00271.9\u0027), (\u00271.11\u0027, \u00271.10\u0027),"},{"line_number":288,"context_line":"                    (\u00271.12\u0027, \u00271.11\u0027), (\u00271.13\u0027, \u00271.12\u0027), (\u00271.14\u0027, \u00271.13\u0027),"},{"line_number":289,"context_line":"                    (\u00271.15\u0027, \u00271.14\u0027), (\u00271.16\u0027, \u00271.15\u0027)],"},{"line_number":290,"context_line":"    }"},{"line_number":291,"context_line":""},{"line_number":292,"context_line":"    @base.remotable_classmethod"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1a9d01_6f162cf3","line":289,"updated":"2015-10-01 15:49:18.000000000","message":"Shouldn\u0027t we add a new tuple (\u00271.17\u0027, \u00271.15\u0027) ?\n\nI know that all of that is just no longer necessary with Mitaka, so I wonder if it\u0027s worth asking for, but still a question.","commit_id":"ee9c7f3795e67151c00d6112bc8d81c4b7ae6f68"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"4d5999e778cc78f7f5e38c6ff19b5f9436825d85","unresolved":false,"context_lines":[{"line_number":286,"context_line":"                    (\u00271.6\u0027, \u00271.5\u0027), (\u00271.7\u0027, \u00271.6\u0027), (\u00271.8\u0027, \u00271.7\u0027),"},{"line_number":287,"context_line":"                    (\u00271.9\u0027, \u00271.8\u0027), (\u00271.10\u0027, \u00271.9\u0027), (\u00271.11\u0027, \u00271.10\u0027),"},{"line_number":288,"context_line":"                    (\u00271.12\u0027, \u00271.11\u0027), (\u00271.13\u0027, \u00271.12\u0027), (\u00271.14\u0027, \u00271.13\u0027),"},{"line_number":289,"context_line":"                    (\u00271.15\u0027, \u00271.14\u0027), (\u00271.16\u0027, \u00271.15\u0027)],"},{"line_number":290,"context_line":"    }"},{"line_number":291,"context_line":""},{"line_number":292,"context_line":"    @base.remotable_classmethod"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1a9d01_cfe5f8dc","line":289,"in_reply_to":"9a1a9d01_6f162cf3","updated":"2015-10-01 15:53:06.000000000","message":"Good question. I wondered the same thing.\n\nDan Smith says that no relationship change is required because nothing else has a BlockDeviceList.","commit_id":"ee9c7f3795e67151c00d6112bc8d81c4b7ae6f68"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1d4ab2c1fb09ef16156f911d04db2fdb89750a75","unresolved":false,"context_lines":[{"line_number":286,"context_line":"                    (\u00271.6\u0027, \u00271.5\u0027), (\u00271.7\u0027, \u00271.6\u0027), (\u00271.8\u0027, \u00271.7\u0027),"},{"line_number":287,"context_line":"                    (\u00271.9\u0027, \u00271.8\u0027), (\u00271.10\u0027, \u00271.9\u0027), (\u00271.11\u0027, \u00271.10\u0027),"},{"line_number":288,"context_line":"                    (\u00271.12\u0027, \u00271.11\u0027), (\u00271.13\u0027, \u00271.12\u0027), (\u00271.14\u0027, \u00271.13\u0027),"},{"line_number":289,"context_line":"                    (\u00271.15\u0027, \u00271.14\u0027), (\u00271.16\u0027, \u00271.15\u0027)],"},{"line_number":290,"context_line":"    }"},{"line_number":291,"context_line":""},{"line_number":292,"context_line":"    @base.remotable_classmethod"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1a9d01_0a119eaf","line":289,"in_reply_to":"9a1a9d01_cfe5f8dc","updated":"2015-10-01 16:19:38.000000000","message":"No, I don\u0027t think you need to do this because the sub-object hasn\u0027t changed. That means that we should still arrive at the proper version of the sub-object when asked to backport this to one of the previous versions.\n\nOn the next bump of the sub-object the test will calculate what changes need to be here so it can be figured out, but as you say, we\u0027re on the doorstep of removing this anyway, so I think we\u0027re okay.\n\nI won\u0027t *hurt* anything.","commit_id":"ee9c7f3795e67151c00d6112bc8d81c4b7ae6f68"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"341c7bf4cf8006cad8ead73b6a728860fa5607fc","unresolved":false,"context_lines":[{"line_number":299,"context_line":"    @base.remotable_classmethod"},{"line_number":300,"context_line":"    def get_by_instance_uuid(cls, context, instance_uuid, use_slave\u003dFalse):"},{"line_number":301,"context_line":"        db_bdms \u003d db.block_device_mapping_get_all_by_instance("},{"line_number":302,"context_line":"            context, instance_uuid, use_slave\u003duse_slave)"},{"line_number":303,"context_line":"        return base.obj_make_list("},{"line_number":304,"context_line":"            context, cls(), objects.BlockDeviceMapping, db_bdms or [])"},{"line_number":305,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1a9d01_7402810d","line":302,"updated":"2015-10-01 15:42:36.000000000","message":"This is unrelated whitespace damage, which isn\u0027t relevant to your change and thus should be removed. Above, you should follow the style of the code around the change you\u0027re making, which means indenting your continued line eight spaces instead of changing this existing one :)","commit_id":"ee9c7f3795e67151c00d6112bc8d81c4b7ae6f68"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"5bcd78671801278d2909a1933fbae20c5bcd9fe0","unresolved":false,"context_lines":[{"line_number":299,"context_line":"    @base.remotable_classmethod"},{"line_number":300,"context_line":"    def get_by_instance_uuid(cls, context, instance_uuid, use_slave\u003dFalse):"},{"line_number":301,"context_line":"        db_bdms \u003d db.block_device_mapping_get_all_by_instance("},{"line_number":302,"context_line":"            context, instance_uuid, use_slave\u003duse_slave)"},{"line_number":303,"context_line":"        return base.obj_make_list("},{"line_number":304,"context_line":"            context, cls(), objects.BlockDeviceMapping, db_bdms or [])"},{"line_number":305,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1a9d01_2fd7d424","line":302,"in_reply_to":"9a1a9d01_7402810d","updated":"2015-10-01 15:48:50.000000000","message":"Done in the next patch. Thanks!","commit_id":"ee9c7f3795e67151c00d6112bc8d81c4b7ae6f68"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"45b620c24e5adbeebd21c55f7f087ec09d4f901d","unresolved":false,"context_lines":[{"line_number":303,"context_line":"        return base.obj_make_list("},{"line_number":304,"context_line":"                context, cls(), objects.BlockDeviceMapping, db_bdms or [])"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"    def root_bdm(self):"},{"line_number":307,"context_line":"        try:"},{"line_number":308,"context_line":"            return next(bdm_obj for bdm_obj in self if bdm_obj.is_root)"},{"line_number":309,"context_line":"        except StopIteration:"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a2fa921_3a68ebb8","line":306,"updated":"2015-10-08 10:06:28.000000000","message":"So I worry about this method - it seems like it expects that all of the instances in a list object would belong to a single instance.\n\nMaybe we should guard against this in this method somehow (raise?) now that there is obviously a way to break this assumption?","commit_id":"309bb8cfcb7f6991c8441d8843c0569438c75cf9"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"c4b1ad896fffab8747e67df5d9805d29539ef2b9","unresolved":false,"context_lines":[{"line_number":303,"context_line":"        return base.obj_make_list("},{"line_number":304,"context_line":"                context, cls(), objects.BlockDeviceMapping, db_bdms or [])"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"    def root_bdm(self):"},{"line_number":307,"context_line":"        try:"},{"line_number":308,"context_line":"            return next(bdm_obj for bdm_obj in self if bdm_obj.is_root)"},{"line_number":309,"context_line":"        except StopIteration:"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a2fa921_b8d38703","line":306,"in_reply_to":"7a2fa921_3a68ebb8","updated":"2015-10-08 19:12:24.000000000","message":"Changed get_by_instance_uuids() in patch set 6 to return a dictionary of BlockDeviceMappingLists keyed by instance UUID.\n\nDoing that pulled in a bunch of \"plumbing\" similar to objects.base.obj_make_list() into get_by_instance_uuids(), like:\n\n        list_obj._context \u003d context\n        list_obj.obj_reset_changes()\n\nI wonder if I should add a new helper method to base.object that builds a dictionary of object lists keyed by some provided object id? Or just cross that bridge if we ever have a second list object that does something similar?\n\nAnyhoo, let me know if you\u0027re happier with patch set 6 or not. Thanks again for taking the time to review this.","commit_id":"309bb8cfcb7f6991c8441d8843c0569438c75cf9"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"b02352033fd4ef050696c1f310e1fa8de119446d","unresolved":false,"context_lines":[{"line_number":303,"context_line":"        return base.obj_make_list("},{"line_number":304,"context_line":"                context, cls(), objects.BlockDeviceMapping, db_bdms or [])"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"    def root_bdm(self):"},{"line_number":307,"context_line":"        try:"},{"line_number":308,"context_line":"            return next(bdm_obj for bdm_obj in self if bdm_obj.is_root)"},{"line_number":309,"context_line":"        except StopIteration:"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a2fa921_1ea863da","line":306,"in_reply_to":"7a2fa921_b8d38703","updated":"2015-10-08 19:52:12.000000000","message":"In patch set 7, I made a helper function in objects.base so that BlockDevice.get_by_instance_uuids() knows less about the \"plumbing\".\n\n    def obj_make_dict_of_lists(context, list_cls, item_cls, db_list, item_key):\n        obj_lists \u003d {}\n        for db_item in db_list:\n            item \u003d item_cls._from_db_object(context, item_cls(), db_item)\n            key \u003d getattr(item, item_key)\n            if key not in obj_lists:\n                obj_lists[key] \u003d list_cls()\n                obj_lists[key].objects \u003d []\n            obj_lists[key].objects.append(item)\n        for key in obj_lists:\n            obj_lists[key]._context \u003d context\n            obj_lists[key].obj_reset_changes()\n        return obj_lists","commit_id":"309bb8cfcb7f6991c8441d8843c0569438c75cf9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"fb8b4222542b853fbac83bbf0c36003efe2978da","unresolved":false,"context_lines":[{"line_number":295,"context_line":"                context, instance_uuids, use_slave\u003duse_slave)"},{"line_number":296,"context_line":"        return base.obj_make_dict_of_lists("},{"line_number":297,"context_line":"                context, cls, objects.BlockDeviceMapping, db_bdms or [],"},{"line_number":298,"context_line":"                \u0027instance_uuid\u0027)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    @base.remotable_classmethod"},{"line_number":301,"context_line":"    def get_by_instance_uuid(cls, context, instance_uuid, use_slave\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a2fa921_e124e6c5","line":298,"updated":"2015-10-08 20:30:51.000000000","message":"This ends up returning an unversioned structure from an RPC method, which is not okay, nor necessary.\n\nTo address Nikola\u0027s point, please either pre-filter the list before root_bdm() is called everywhere, or do the check in that method itself to raise an exception if there is some consistency problem with what it is passed.\n\nOptionally you could provide a non-remotable method that calls the remotable one and sorts the results by instance-uuid, and use that instead of the remotable method directly.","commit_id":"34bab8291fc506d0b3acdbbab92bfffb42e2451f"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"04de8b2594eb2c62ea696e25975963b7c7f2b469","unresolved":false,"context_lines":[{"line_number":295,"context_line":"                context, instance_uuids, use_slave\u003duse_slave)"},{"line_number":296,"context_line":"        return base.obj_make_dict_of_lists("},{"line_number":297,"context_line":"                context, cls, objects.BlockDeviceMapping, db_bdms or [],"},{"line_number":298,"context_line":"                \u0027instance_uuid\u0027)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    @base.remotable_classmethod"},{"line_number":301,"context_line":"    def get_by_instance_uuid(cls, context, instance_uuid, use_slave\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a2fa921_89af427c","line":298,"in_reply_to":"7a2fa921_2dbc0fdb","updated":"2015-10-09 09:45:29.000000000","message":"If only Python had type annotations... oh wait :)\n\nSo as for a) I would actually suggest to change the return type of get_by_instance_uuids that you add here from BlockDeviceMappingList to [BlockDeviceMappingList] (or even {uuid: BlockDeviceMappingList}). That way we keep the promise to make BlockDeviceMappingList instances that are constructed by classmethods OTHER than the __init__ only contain block devices of a single instance. This would mean that we do the \"filtering\" inside this method.\n\nIn addition to that I would do b) as well - we could also raise in the root_bdm method if we spot an incosistency with instance_uuids.\n\nAn alternative approach would be to give up on the promise that BlockDeviceMappingList instances contain only BDMs of a single instance, and move the root_bdm method out.\n\nWe should also probably add a docstring somewhere that states that this should really contain only BDMS from the same instance :)\n\nI hope it\u0027s clearer now.","commit_id":"34bab8291fc506d0b3acdbbab92bfffb42e2451f"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"0a18458a080da03a31df16b6037087bf02334e29","unresolved":false,"context_lines":[{"line_number":295,"context_line":"                context, instance_uuids, use_slave\u003duse_slave)"},{"line_number":296,"context_line":"        return base.obj_make_dict_of_lists("},{"line_number":297,"context_line":"                context, cls, objects.BlockDeviceMapping, db_bdms or [],"},{"line_number":298,"context_line":"                \u0027instance_uuid\u0027)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    @base.remotable_classmethod"},{"line_number":301,"context_line":"    def get_by_instance_uuid(cls, context, instance_uuid, use_slave\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a2fa921_557a0a89","line":298,"in_reply_to":"7a2fa921_31df6c3b","updated":"2015-10-09 16:27:04.000000000","message":"So, just to be clear, I should:\n\n1) Add a normal @classmethod to BlockDeviceMappingList that does something like the following, where get_by_instance_uuids is a @base.remotable_classmethod as implemented in patch set 8. \n\n    @classmethod\n    def bdms_by_instance_uuid(cls, context, instance_uuids):\n        bdms \u003d cls.get_by_instance_uuids(context, instance_uuids)\n        result \u003d collections.defaultdict(list)\n        for bdm in bdms:\n            result[bdm.instance_uuid].append(bdm)\n        return result\n\nThis just basically moves the indexing code from extended_volumnes.py to block_device.py. Note that some people might find it strange that this method lives on the BlockDeviceMappingList, but doesn\u0027t actually return any BlockDeviceMappingLists...\n\n2) Add an if-statement with a raise to root_bdm() on mixed-instance BlockDeviceMappingLists (and unit tests). Pseudo code:\n\n    def root_bdm(self): \n        if mixed_instance:\n            raise Exception(\"Boom!\")\n        try:\n            return next(bdm_obj for bdm_obj in self if bdm_obj.is_root)\n        except StopIteration:\n            return\n\nThanks!!","commit_id":"34bab8291fc506d0b3acdbbab92bfffb42e2451f"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"1c07e35a05717dba9d53aabf354e7e3b0b384275","unresolved":false,"context_lines":[{"line_number":295,"context_line":"                context, instance_uuids, use_slave\u003duse_slave)"},{"line_number":296,"context_line":"        return base.obj_make_dict_of_lists("},{"line_number":297,"context_line":"                context, cls, objects.BlockDeviceMapping, db_bdms or [],"},{"line_number":298,"context_line":"                \u0027instance_uuid\u0027)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    @base.remotable_classmethod"},{"line_number":301,"context_line":"    def get_by_instance_uuid(cls, context, instance_uuid, use_slave\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a2fa921_fc6f7ac1","line":298,"in_reply_to":"7a2fa921_557a0a89","updated":"2015-10-09 19:37:13.000000000","message":"Except that doesn\u0027t work either ;( Traversing the BDM records in the list, collecting the instance UUIDs to see if it\u0027s mixed or not, that is.\n\n    ObjectActionError: Object action obj_load_attr failed because: attribute instance_uuid not lazy-loadable\n\n    def root_bdm(self):\n        instance_uuids \u003d set([bdm_obj.instance_uuid for bdm_obj in self])\n        if len(instance_uuids) \u003e 1:\n            raise exception.UndefinedRootBDM()\n        try:\n            return next(bdm_obj for bdm_obj in self if bdm_obj.is_root)\n        except StopIteration:\n            return\n\nDiana clicks \u0027Abandon Change\u0027... just kidding. Kind of.","commit_id":"34bab8291fc506d0b3acdbbab92bfffb42e2451f"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"0dd63b420ddbb3335f47d025df35339c31d8a800","unresolved":false,"context_lines":[{"line_number":295,"context_line":"                context, instance_uuids, use_slave\u003duse_slave)"},{"line_number":296,"context_line":"        return base.obj_make_dict_of_lists("},{"line_number":297,"context_line":"                context, cls, objects.BlockDeviceMapping, db_bdms or [],"},{"line_number":298,"context_line":"                \u0027instance_uuid\u0027)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    @base.remotable_classmethod"},{"line_number":301,"context_line":"    def get_by_instance_uuid(cls, context, instance_uuid, use_slave\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a2fa921_dfc2cce8","line":298,"in_reply_to":"7a2fa921_89af427c","updated":"2015-10-09 10:47:03.000000000","message":"Re: \"I would actually suggest to change the return type of get_by_instance_uuids that you add here from BlockDeviceMappingList to [BlockDeviceMappingList] (or even {uuid: BlockDeviceMappingList}). \"\n\nThat\u0027s what this patch set (7) is already doing. The get_by_instance_uuids  method is returning a dictionary of BlockDeviceMappingLists, keyed by uuid, but that\u0027s what Dan said I couldn\u0027t do.... For example, this is what it currently would return using this patch set (7):\n\n    {\n        \u0027uuid_1\u0027: BlockDeviceMappingList_1,\n        \u0027uuid_2: BlockDeviceMappingList_2,\n        \u0027uuid_3\u0027: BlockDeviceMappingList_3,\n    }","commit_id":"34bab8291fc506d0b3acdbbab92bfffb42e2451f"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"bd6ad0fd6cc057eb90f2daa64ad54f95e81847a0","unresolved":false,"context_lines":[{"line_number":295,"context_line":"                context, instance_uuids, use_slave\u003duse_slave)"},{"line_number":296,"context_line":"        return base.obj_make_dict_of_lists("},{"line_number":297,"context_line":"                context, cls, objects.BlockDeviceMapping, db_bdms or [],"},{"line_number":298,"context_line":"                \u0027instance_uuid\u0027)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    @base.remotable_classmethod"},{"line_number":301,"context_line":"    def get_by_instance_uuid(cls, context, instance_uuid, use_slave\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a2fa921_1ce7efac","line":298,"in_reply_to":"7a2fa921_dfc2cce8","updated":"2015-10-09 13:58:13.000000000","message":"Ah Dan is right actually - you can\u0027t do that (unless you add an object which defeats the purpose).\n\nI am not sure TBH - I think the easiest thing to do is just raise in the method if used on a mixed instance list of bdms","commit_id":"34bab8291fc506d0b3acdbbab92bfffb42e2451f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9ff3e635b14e694268520a47a098f6f7f28a6118","unresolved":false,"context_lines":[{"line_number":295,"context_line":"                context, instance_uuids, use_slave\u003duse_slave)"},{"line_number":296,"context_line":"        return base.obj_make_dict_of_lists("},{"line_number":297,"context_line":"                context, cls, objects.BlockDeviceMapping, db_bdms or [],"},{"line_number":298,"context_line":"                \u0027instance_uuid\u0027)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    @base.remotable_classmethod"},{"line_number":301,"context_line":"    def get_by_instance_uuid(cls, context, instance_uuid, use_slave\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a2fa921_31df6c3b","line":298,"in_reply_to":"7a2fa921_dfc2cce8","updated":"2015-10-09 13:41:49.000000000","message":"It seems like having root_bdm on the object is helpful, so I\u0027m not sure I\u0027d want to remove it. But, doing a consistency check and raising an exception if we can\u0027t properly determine the root of a mixed set seems like it\u0027s an easy solution.\n\nYou do that, and then add a client-side-only wrapper call that does the index by uuid and we can all go home :)","commit_id":"34bab8291fc506d0b3acdbbab92bfffb42e2451f"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"a16aa9f6ca0a85c29910beefaee72961c9251959","unresolved":false,"context_lines":[{"line_number":295,"context_line":"                context, instance_uuids, use_slave\u003duse_slave)"},{"line_number":296,"context_line":"        return base.obj_make_dict_of_lists("},{"line_number":297,"context_line":"                context, cls, objects.BlockDeviceMapping, db_bdms or [],"},{"line_number":298,"context_line":"                \u0027instance_uuid\u0027)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    @base.remotable_classmethod"},{"line_number":301,"context_line":"    def get_by_instance_uuid(cls, context, instance_uuid, use_slave\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a2fa921_2dbc0fdb","line":298,"in_reply_to":"7a2fa921_e124e6c5","updated":"2015-10-08 21:50:34.000000000","message":"Hmmm... I clearly didn\u0027t know I couldn\u0027t return a container of versioned objects. But now I do. Thanks!\n\nI don\u0027t understand what you mean here though: \"To address Nikola\u0027s point, please either pre-filter the list before root_bdm() is called everywhere, or do the check in that method itself to raise an exception if there is some consistency problem with what it is passed.\".\n\nThis makes me suspect I don\u0027t actually understand the initial concern Nikola raised, so let me repeat it back to you in my own words to see if we\u0027re on the same page. Is the concern that a BlockDeviceMappingList created by patch set 5 would result in a list of BlockDeviceMapping objects from different instances mixed together in one list? Like the following pseudo code with block device mappings from instances A \u0026 B, together in one list? And that when you call block_device_mapping_list.root_bdm(), you might get the root BDM from instance A or the root BDM from instance B? That is, a call to block_device_mapping_list.root_bdm() only makes sense when the list contains BDMs for exactly one instance?\n\n    block_device_mapping_list  \u003d [\n        instance_A.block_device_1,\n        instance_A.block_device_2,\n        instance_B.block_device_1,\n        instance_B.block_device_2,\n    ]\n\nHave I restated the concern with patch set 5 correctly, or incorrectly? I think knowing that it the first step toward understanding what you mean by:\n\na) \"pre-filter the list before root_bdm() is called everywhere\"\n\nb) \"check in that method itself to raise an exception if there is some consistency problem with what it is passed\"\n\nFor case a: Do you perhaps mean find every caller of block_device_mapping_list.root_bdm() and make sure to filter block_device_mapping_list.objects() to just the BDMs for the instance in question before using that same block_device_mapping_list to call root_bdm()? Even though none of those callers could possibly have a mixed-instance list because I only just added that functionality? Remembering to do something before each call to something else is a pretty risky design choice (someone will eventually forget to), but I suspect that\u0027s not what you mean and I\u0027ve just misunderstood.\n\nFor case b: Do you perhaps mean raise an exception in root_bdm() if you attempt to call it from a BlockDeviceMappingList that has BDMs from more than one instance in it? I can totally do that. I was going to do that before patch sets 6 \u0026 7, where I opted to make a dictionary of BlockDeviceMappingLists instead, in case other situations like this came up (where mixed-instance block device mapping lists didn\u0027t make sense) because we\u0027d have to remember to do whatever we do for root_bdm() in those cases too.\n\nHope this is clear enough. Thanks!","commit_id":"34bab8291fc506d0b3acdbbab92bfffb42e2451f"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"c415a2d4fe4208321b702b011376ec7ad9313bdb","unresolved":false,"context_lines":[{"line_number":295,"context_line":"                context, instance_uuids, use_slave\u003duse_slave)"},{"line_number":296,"context_line":"        return base.obj_make_dict_of_lists("},{"line_number":297,"context_line":"                context, cls, objects.BlockDeviceMapping, db_bdms or [],"},{"line_number":298,"context_line":"                \u0027instance_uuid\u0027)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    @base.remotable_classmethod"},{"line_number":301,"context_line":"    def get_by_instance_uuid(cls, context, instance_uuid, use_slave\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a2fa921_9c70c60c","line":298,"in_reply_to":"7a2fa921_fc6f7ac1","updated":"2015-10-09 19:48:48.000000000","message":"Hmmm... actually, I think that\u0027s just poor test fixtures somewhere in the functional suite. I think this should actually work in real life.","commit_id":"34bab8291fc506d0b3acdbbab92bfffb42e2451f"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"b3e57f7ad32b907dfe98d2d7f6cc22413357e0e8","unresolved":false,"context_lines":[{"line_number":293,"context_line":""},{"line_number":294,"context_line":"    @property"},{"line_number":295,"context_line":"    def instance_uuids(self):"},{"line_number":296,"context_line":"        result \u003d []"},{"line_number":297,"context_line":"        for bdm in self:"},{"line_number":298,"context_line":"            try:"},{"line_number":299,"context_line":"                result.append(bdm.instance_uuid)"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a2fa921_9085ecae","line":296,"updated":"2015-10-12 07:05:23.000000000","message":"Why not just make this a set in the first place?","commit_id":"d618c4810db6ac268a2acdbd387f37be2007f18a"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"317693b0895d64090af300d31366f21d31c5fbd7","unresolved":false,"context_lines":[{"line_number":293,"context_line":""},{"line_number":294,"context_line":"    @property"},{"line_number":295,"context_line":"    def instance_uuids(self):"},{"line_number":296,"context_line":"        result \u003d []"},{"line_number":297,"context_line":"        for bdm in self:"},{"line_number":298,"context_line":"            try:"},{"line_number":299,"context_line":"                result.append(bdm.instance_uuid)"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a2fa921_022ae7e7","line":296,"in_reply_to":"7a2fa921_9085ecae","updated":"2015-10-12 12:53:26.000000000","message":"Done","commit_id":"d618c4810db6ac268a2acdbd387f37be2007f18a"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"b3e57f7ad32b907dfe98d2d7f6cc22413357e0e8","unresolved":false,"context_lines":[{"line_number":302,"context_line":"        return list(set(result))"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"    @classmethod"},{"line_number":305,"context_line":"    def bdms_by_instance_uuid(cls, context, instance_uuids):"},{"line_number":306,"context_line":"        bdms \u003d cls.get_by_instance_uuids(context, instance_uuids)"},{"line_number":307,"context_line":"        result \u003d collections.defaultdict(list)"},{"line_number":308,"context_line":"        for bdm in bdms:"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a2fa921_30a97825","line":305,"updated":"2015-10-12 07:05:23.000000000","message":"This method and the one above basically do the same thing - can we somehow re-use some of the code? Maybe make this one an instance method instead of a classmethod (not remotable though) and then for the price of an explicit call to get_by_instance_uuids at call sites - we can just use this one in root_bdm?\n\nI could be convinced that this is not necessary though.","commit_id":"d618c4810db6ac268a2acdbd387f37be2007f18a"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"317693b0895d64090af300d31366f21d31c5fbd7","unresolved":false,"context_lines":[{"line_number":302,"context_line":"        return list(set(result))"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"    @classmethod"},{"line_number":305,"context_line":"    def bdms_by_instance_uuid(cls, context, instance_uuids):"},{"line_number":306,"context_line":"        bdms \u003d cls.get_by_instance_uuids(context, instance_uuids)"},{"line_number":307,"context_line":"        result \u003d collections.defaultdict(list)"},{"line_number":308,"context_line":"        for bdm in bdms:"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a2fa921_adae83e5","line":305,"in_reply_to":"7a2fa921_30a97825","updated":"2015-10-12 12:53:26.000000000","message":"I don\u0027t think I quite follow... sorry. Let me know if you still want it changed in some way when you see patch set 11. Thanks!","commit_id":"d618c4810db6ac268a2acdbd387f37be2007f18a"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"b3e57f7ad32b907dfe98d2d7f6cc22413357e0e8","unresolved":false,"context_lines":[{"line_number":306,"context_line":"        bdms \u003d cls.get_by_instance_uuids(context, instance_uuids)"},{"line_number":307,"context_line":"        result \u003d collections.defaultdict(list)"},{"line_number":308,"context_line":"        for bdm in bdms:"},{"line_number":309,"context_line":"            result[bdm.instance_uuid].append(bdm)"},{"line_number":310,"context_line":"        return result"},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"    @base.remotable_classmethod"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a2fa921_705db047","line":309,"updated":"2015-10-12 07:05:23.000000000","message":"Would it maybe make sense to return a dict of BlockDeviceMappingList objects instead? Not really an issue just seems like it would make sense...","commit_id":"d618c4810db6ac268a2acdbd387f37be2007f18a"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"317693b0895d64090af300d31366f21d31c5fbd7","unresolved":false,"context_lines":[{"line_number":306,"context_line":"        bdms \u003d cls.get_by_instance_uuids(context, instance_uuids)"},{"line_number":307,"context_line":"        result \u003d collections.defaultdict(list)"},{"line_number":308,"context_line":"        for bdm in bdms:"},{"line_number":309,"context_line":"            result[bdm.instance_uuid].append(bdm)"},{"line_number":310,"context_line":"        return result"},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"    @base.remotable_classmethod"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a2fa921_c7d95e85","line":309,"in_reply_to":"7a2fa921_705db047","updated":"2015-10-12 12:53:26.000000000","message":"Good call. That way people will be able to call root_bdm() on the resulting lists in the dictionary if need be.\n\nPS. I added back base.obj_make_dict_of_lists from patch set 7 to do this \"plumbing\" (it doesn\u0027t come for free).","commit_id":"d618c4810db6ac268a2acdbd387f37be2007f18a"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"312a7c02a45edf92d97b20e07e1317b196465989","unresolved":false,"context_lines":[{"line_number":295,"context_line":"        for bdm in self:"},{"line_number":296,"context_line":"            try:"},{"line_number":297,"context_line":"                result.add(bdm.instance_uuid)"},{"line_number":298,"context_line":"            except exception.ObjectActionError:"},{"line_number":299,"context_line":"                pass"},{"line_number":300,"context_line":"        return list(result)"},{"line_number":301,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"5a2ca52d_8013f70c","line":298,"updated":"2015-10-15 08:51:50.000000000","message":"There is a base object method called obj_attr_is_set() that you can use here instead of having to catch an exception, like:\n\n set(bdm.instance_uuid for bdm in self if bdm.obj_attr_is_set(\u0027instance_uuid\u0027))","commit_id":"82e7e8a096909259b2fe45ddbd86273557685016"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"4a086e2e487ce784f8c553f0cca32e7555d1797f","unresolved":false,"context_lines":[{"line_number":295,"context_line":"        for bdm in self:"},{"line_number":296,"context_line":"            try:"},{"line_number":297,"context_line":"                result.add(bdm.instance_uuid)"},{"line_number":298,"context_line":"            except exception.ObjectActionError:"},{"line_number":299,"context_line":"                pass"},{"line_number":300,"context_line":"        return list(result)"},{"line_number":301,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"5a2ca52d_11cbad73","line":298,"in_reply_to":"5a2ca52d_8013f70c","updated":"2015-10-15 13:58:22.000000000","message":"I don\u0027t think these are identical. I was doing something similar in an earlier patch set (9 I think), but it was failing in some cases on python 3.4. I suspect there is something subtlety different about hasattr (which obj_attr_is_set also calls), but I wasn\u0027t able to track it down in the time-box I set aside to look. \n\n    http://paste.openstack.org/show/475956/","commit_id":"82e7e8a096909259b2fe45ddbd86273557685016"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"312a7c02a45edf92d97b20e07e1317b196465989","unresolved":false,"context_lines":[{"line_number":300,"context_line":"        return list(result)"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    @classmethod"},{"line_number":303,"context_line":"    def bdms_by_instance_uuid(cls, context, instance_uuids):"},{"line_number":304,"context_line":"        bdms \u003d cls.get_by_instance_uuids(context, instance_uuids)"},{"line_number":305,"context_line":"        return base.obj_make_dict_of_lists("},{"line_number":306,"context_line":"                context, cls, bdms, \u0027instance_uuid\u0027)"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a2ca52d_600a5b23","line":303,"updated":"2015-10-15 08:51:50.000000000","message":"How about we name this method get_dict_of_bdms_by_instance() to be more clear about what it does?","commit_id":"82e7e8a096909259b2fe45ddbd86273557685016"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"4a086e2e487ce784f8c553f0cca32e7555d1797f","unresolved":false,"context_lines":[{"line_number":300,"context_line":"        return list(result)"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    @classmethod"},{"line_number":303,"context_line":"    def bdms_by_instance_uuid(cls, context, instance_uuids):"},{"line_number":304,"context_line":"        bdms \u003d cls.get_by_instance_uuids(context, instance_uuids)"},{"line_number":305,"context_line":"        return base.obj_make_dict_of_lists("},{"line_number":306,"context_line":"                context, cls, bdms, \u0027instance_uuid\u0027)"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a2ca52d_2e3c44e1","line":303,"in_reply_to":"5a2ca52d_600a5b23","updated":"2015-10-15 13:58:22.000000000","message":"I played with a number of different names, but even with all arguments pushed to the next line, objects.BlockDeviceMappingList.get_dict_of_bdms_by_instance still doesn\u0027t fit in all 4 places where this method is called without making other readability compromises.","commit_id":"82e7e8a096909259b2fe45ddbd86273557685016"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1fc069d212147d2ebff7a3c9c920b5f971c8ba70","unresolved":false,"context_lines":[{"line_number":295,"context_line":"        for bdm in self:"},{"line_number":296,"context_line":"            try:"},{"line_number":297,"context_line":"                result.add(bdm.instance_uuid)"},{"line_number":298,"context_line":"            except exception.ObjectActionError:"},{"line_number":299,"context_line":"                pass"},{"line_number":300,"context_line":"        return list(result)"},{"line_number":301,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"5a2ca52d_4862cf3f","line":298,"updated":"2015-10-15 15:30:27.000000000","message":"As Hans said, you should be using obj_attr_is_set() for this, if you\u0027re trying to measure wether instance_uuid is set on the object, and not catch this exception. Depending on the implementation of this object now and going forward, this could later end up triggering a lazy load (i.e. rpc call and DB hit).\n\nIf you\u0027re not trying to detect whether it\u0027s set or not, then I\u0027m not sure where you\u0027re going with this. TBH, I\u0027m not sure why a BDM should ever have no instance_uuid. Are you doing this just because a unit test failed or something?","commit_id":"79c9bf1d2ccfd33e28658d8c0dd37d73d0c2e47b"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"bfdb29a543af3b12db29eed2c4369b0c4916342a","unresolved":false,"context_lines":[{"line_number":295,"context_line":"        for bdm in self:"},{"line_number":296,"context_line":"            try:"},{"line_number":297,"context_line":"                result.add(bdm.instance_uuid)"},{"line_number":298,"context_line":"            except exception.ObjectActionError:"},{"line_number":299,"context_line":"                pass"},{"line_number":300,"context_line":"        return list(result)"},{"line_number":301,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"5a2ca52d_f12863e4","line":298,"in_reply_to":"5a2ca52d_4862cf3f","updated":"2015-10-15 17:53:04.000000000","message":"Without this, I get a few hundred test failures in the unit tests (more if you count the functional tests). I had originally assumed it was just a test fixture issue, but the non-test compute_api.create_instance() code path appears to create block device mappings without instance_uuids in some cases (at least initially, perhaps before persistence happens). In particular, it appears to be BDMs that are created from these \"templates\": https://github.com/openstack/nova/blob/81fbb34e2544b26ae72f7eee3841b4024d22896f/nova/block_device.py#L265-L289\n\nI\u0027ll try obj_attr_is_set(), but I could have sworn that I had python 3.4 issues with it...","commit_id":"79c9bf1d2ccfd33e28658d8c0dd37d73d0c2e47b"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"fad672245310593510ca3f7d89393f7fd759a7bc","unresolved":false,"context_lines":[{"line_number":329,"context_line":"        \"\"\""},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"        if len(self.instance_uuids) \u003e 1:"},{"line_number":332,"context_line":"            raise exception.UndefinedRootBDM()"},{"line_number":333,"context_line":"        try:"},{"line_number":334,"context_line":"            return next(bdm_obj for bdm_obj in self if bdm_obj.is_root)"},{"line_number":335,"context_line":"        except StopIteration:"}],"source_content_type":"text/x-python","patch_set":13,"id":"5a2ca52d_5e7ca5b4","line":332,"updated":"2015-10-16 09:59:53.000000000","message":"this could have easily been a separate commit to keep changes more topical, but no big deal. Just pointing out that if there is a part of a change that can be separated into it\u0027s own commit (that on it\u0027s own passes CI) it\u0027s usually a good idea as it is easier to review, and in some cases makes backporting to previous versions easier.","commit_id":"c2e2fc3937e0fbb93e4cb829ed695fe4782bee4d"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"a368335e3ece017370ce6666339cc634ec561edd","unresolved":false,"context_lines":[{"line_number":290,"context_line":"    }"},{"line_number":291,"context_line":""},{"line_number":292,"context_line":"    @property"},{"line_number":293,"context_line":"    def instance_uuids(self):"},{"line_number":294,"context_line":"        return set("},{"line_number":295,"context_line":"            bdm.instance_uuid for bdm in self"},{"line_number":296,"context_line":"            if bdm.obj_attr_is_set(\u0027instance_uuid\u0027)"}],"source_content_type":"text/x-python","patch_set":14,"id":"da85f559_e19d8d0a","line":293,"updated":"2015-11-12 13:36:26.000000000","message":"I didn\u0027t see any tests for this, did I just miss them?\n\nI guess its tested as part of the root_bdm thing?","commit_id":"4bf0024f0236d3e8073a4ce36b49575f49891478"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"545c1fdd7775455d9f0a9433b9afcf85026e71a3","unresolved":false,"context_lines":[{"line_number":290,"context_line":"    }"},{"line_number":291,"context_line":""},{"line_number":292,"context_line":"    @property"},{"line_number":293,"context_line":"    def instance_uuids(self):"},{"line_number":294,"context_line":"        return set("},{"line_number":295,"context_line":"            bdm.instance_uuid for bdm in self"},{"line_number":296,"context_line":"            if bdm.obj_attr_is_set(\u0027instance_uuid\u0027)"}],"source_content_type":"text/x-python","patch_set":14,"id":"da85f559_360e42e6","line":293,"in_reply_to":"da85f559_32076583","updated":"2015-11-13 12:03:11.000000000","message":"ah, cool, thanks.","commit_id":"4bf0024f0236d3e8073a4ce36b49575f49891478"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"4d08ce9d360f8844b473fca8878b75dad9ef2d7d","unresolved":false,"context_lines":[{"line_number":290,"context_line":"    }"},{"line_number":291,"context_line":""},{"line_number":292,"context_line":"    @property"},{"line_number":293,"context_line":"    def instance_uuids(self):"},{"line_number":294,"context_line":"        return set("},{"line_number":295,"context_line":"            bdm.instance_uuid for bdm in self"},{"line_number":296,"context_line":"            if bdm.obj_attr_is_set(\u0027instance_uuid\u0027)"}],"source_content_type":"text/x-python","patch_set":14,"id":"da85f559_32076583","line":293,"in_reply_to":"da85f559_e19d8d0a","updated":"2015-11-12 15:06:52.000000000","message":"Yup: the one, none, and many cases are each tested via the root_bdm tests.\n\nAnd this method is exercised quite heavily via the test suite (every time an instance is created). A few hundred test fail when it\u0027s wrong.","commit_id":"4bf0024f0236d3e8073a4ce36b49575f49891478"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"a368335e3ece017370ce6666339cc634ec561edd","unresolved":false,"context_lines":[{"line_number":303,"context_line":"                context, cls, bdms, \u0027instance_uuid\u0027)"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"    @base.remotable_classmethod"},{"line_number":306,"context_line":"    def get_by_instance_uuids(cls, context, instance_uuids, use_slave\u003dFalse):"},{"line_number":307,"context_line":"        db_bdms \u003d db.block_device_mapping_get_all_by_instance_uuids("},{"line_number":308,"context_line":"                context, instance_uuids, use_slave\u003duse_slave)"},{"line_number":309,"context_line":"        return base.obj_make_list("}],"source_content_type":"text/x-python","patch_set":14,"id":"da85f559_a129c5e7","line":306,"updated":"2015-11-12 13:36:26.000000000","message":"I am not sure we want to add use_slave here is we are busy removing them now, as part of this effort:\nhttps://blueprints.launchpad.net/nova/+spec/new-oslodb-enginefacade","commit_id":"4bf0024f0236d3e8073a4ce36b49575f49891478"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"545c1fdd7775455d9f0a9433b9afcf85026e71a3","unresolved":false,"context_lines":[{"line_number":303,"context_line":"                context, cls, bdms, \u0027instance_uuid\u0027)"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"    @base.remotable_classmethod"},{"line_number":306,"context_line":"    def get_by_instance_uuids(cls, context, instance_uuids, use_slave\u003dFalse):"},{"line_number":307,"context_line":"        db_bdms \u003d db.block_device_mapping_get_all_by_instance_uuids("},{"line_number":308,"context_line":"                context, instance_uuids, use_slave\u003duse_slave)"},{"line_number":309,"context_line":"        return base.obj_make_list("}],"source_content_type":"text/x-python","patch_set":14,"id":"da85f559_b66b1285","line":306,"in_reply_to":"da85f559_172ed3fa","updated":"2015-11-13 12:03:11.000000000","message":"sorry about the distraction there.","commit_id":"4bf0024f0236d3e8073a4ce36b49575f49891478"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"4d08ce9d360f8844b473fca8878b75dad9ef2d7d","unresolved":false,"context_lines":[{"line_number":303,"context_line":"                context, cls, bdms, \u0027instance_uuid\u0027)"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"    @base.remotable_classmethod"},{"line_number":306,"context_line":"    def get_by_instance_uuids(cls, context, instance_uuids, use_slave\u003dFalse):"},{"line_number":307,"context_line":"        db_bdms \u003d db.block_device_mapping_get_all_by_instance_uuids("},{"line_number":308,"context_line":"                context, instance_uuids, use_slave\u003duse_slave)"},{"line_number":309,"context_line":"        return base.obj_make_list("}],"source_content_type":"text/x-python","patch_set":14,"id":"da85f559_172ed3fa","line":306,"in_reply_to":"da85f559_a129c5e7","updated":"2015-11-12 15:06:52.000000000","message":"Done","commit_id":"4bf0024f0236d3e8073a4ce36b49575f49891478"}],"nova/tests/functional/api_sample_tests/test_extended_volumes.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b043617ce46f0a7879980879ea3b555c1da39bf0","unresolved":false,"context_lines":[{"line_number":58,"context_line":"    def test_detail(self):"},{"line_number":59,"context_line":"        uuid \u003d self._post_server()"},{"line_number":60,"context_line":"        self.stubs.Set(db, \u0027block_device_mapping_get_all_by_instance_uuids\u0027,"},{"line_number":61,"context_line":"                       fakes.stub_bdm_get_all_by_instance_uuids)"},{"line_number":62,"context_line":"        response \u003d self._do_get(\u0027servers/detail\u0027)"},{"line_number":63,"context_line":"        subs \u003d self._get_regexes()"},{"line_number":64,"context_line":"        subs[\u0027id\u0027] \u003d uuid"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_8ca62952","line":61,"updated":"2015-10-01 17:41:50.000000000","message":"If you\u0027re going to do this, could you just make this use mock instead of self.Stubs? We\u0027re trying to get away from the latter.","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"}],"nova/tests/unit/api/openstack/compute/test_extended_volumes.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b043617ce46f0a7879980879ea3b555c1da39bf0","unresolved":false,"context_lines":[{"line_number":88,"context_line":"    def _setUp(self):"},{"line_number":89,"context_line":"        self.Controller \u003d extended_volumes_v21.ExtendedVolumesController()"},{"line_number":90,"context_line":"        self.stubs.Set(volume.cinder.API, \u0027get\u0027, fake_volume_get)"},{"line_number":91,"context_line":"        self.action_url \u003d \"/%s/action\" % UUID1"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    def _make_request(self, url, body\u003dNone):"},{"line_number":94,"context_line":"        req \u003d webob.Request.blank(\u0027/v2/fake/servers\u0027 + url)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_ec10055c","side":"PARENT","line":91,"updated":"2015-10-01 17:41:50.000000000","message":"How is this related to your change?","commit_id":"40159ac6d2cc87becb8f191bc2e4340eef6936f7"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"b692d145e22e07f88fd06b91b43096ddabf089ab","unresolved":false,"context_lines":[{"line_number":88,"context_line":"    def _setUp(self):"},{"line_number":89,"context_line":"        self.Controller \u003d extended_volumes_v21.ExtendedVolumesController()"},{"line_number":90,"context_line":"        self.stubs.Set(volume.cinder.API, \u0027get\u0027, fake_volume_get)"},{"line_number":91,"context_line":"        self.action_url \u003d \"/%s/action\" % UUID1"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    def _make_request(self, url, body\u003dNone):"},{"line_number":94,"context_line":"        req \u003d webob.Request.blank(\u0027/v2/fake/servers\u0027 + url)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_be9f8b4f","side":"PARENT","line":91,"in_reply_to":"9a1a9d01_c385ccad","updated":"2015-10-01 20:48:22.000000000","message":"Yup, I understand. I still decided to remove this line because I think confusing right now, trumps potentially confusing at some point in the future.","commit_id":"40159ac6d2cc87becb8f191bc2e4340eef6936f7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"eeae6b7763f483f96b804a36a1f8ae79e364b99f","unresolved":false,"context_lines":[{"line_number":88,"context_line":"    def _setUp(self):"},{"line_number":89,"context_line":"        self.Controller \u003d extended_volumes_v21.ExtendedVolumesController()"},{"line_number":90,"context_line":"        self.stubs.Set(volume.cinder.API, \u0027get\u0027, fake_volume_get)"},{"line_number":91,"context_line":"        self.action_url \u003d \"/%s/action\" % UUID1"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    def _make_request(self, url, body\u003dNone):"},{"line_number":94,"context_line":"        req \u003d webob.Request.blank(\u0027/v2/fake/servers\u0027 + url)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_c385ccad","side":"PARENT","line":91,"in_reply_to":"9a1a9d01_d2f4b888","updated":"2015-10-01 19:40:17.000000000","message":"I believe you that it\u0027s not used anymore, but its lack of use is not related to this patch. If we had to come along and revert your patch later, or were looking for details chasing a bug in a year, things like this are confusing.","commit_id":"40159ac6d2cc87becb8f191bc2e4340eef6936f7"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"ba2491f06a8d57d683d1796eed57e63e943caf55","unresolved":false,"context_lines":[{"line_number":88,"context_line":"    def _setUp(self):"},{"line_number":89,"context_line":"        self.Controller \u003d extended_volumes_v21.ExtendedVolumesController()"},{"line_number":90,"context_line":"        self.stubs.Set(volume.cinder.API, \u0027get\u0027, fake_volume_get)"},{"line_number":91,"context_line":"        self.action_url \u003d \"/%s/action\" % UUID1"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    def _make_request(self, url, body\u003dNone):"},{"line_number":94,"context_line":"        req \u003d webob.Request.blank(\u0027/v2/fake/servers\u0027 + url)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_d2f4b888","side":"PARENT","line":91,"in_reply_to":"9a1a9d01_ec10055c","updated":"2015-10-01 18:33:39.000000000","message":"I think this is a historical copy/paste error. Unless you can explain why it belongs here. In the mean time, it caused confusion because the same setup was being done for the list and show case, and the list case shouldn\u0027t  know about a particular uuid.","commit_id":"40159ac6d2cc87becb8f191bc2e4340eef6936f7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b043617ce46f0a7879980879ea3b555c1da39bf0","unresolved":false,"context_lines":[{"line_number":44,"context_line":"    db_list \u003d ["},{"line_number":45,"context_line":"        fakes.stub_instance(1, uuid\u003dUUID1),"},{"line_number":46,"context_line":"        fakes.stub_instance(2, uuid\u003dUUID2),"},{"line_number":47,"context_line":"    ]"},{"line_number":48,"context_line":"    fields \u003d instance_obj.INSTANCE_DEFAULT_FIELDS"},{"line_number":49,"context_line":"    return instance_obj._make_instance_list(args[1],"},{"line_number":50,"context_line":"                                            objects.InstanceList(),"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_4c4ed197","line":47,"updated":"2015-10-01 17:41:50.000000000","message":"Is this related to your change?","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"ba2491f06a8d57d683d1796eed57e63e943caf55","unresolved":false,"context_lines":[{"line_number":44,"context_line":"    db_list \u003d ["},{"line_number":45,"context_line":"        fakes.stub_instance(1, uuid\u003dUUID1),"},{"line_number":46,"context_line":"        fakes.stub_instance(2, uuid\u003dUUID2),"},{"line_number":47,"context_line":"    ]"},{"line_number":48,"context_line":"    fields \u003d instance_obj.INSTANCE_DEFAULT_FIELDS"},{"line_number":49,"context_line":"    return instance_obj._make_instance_list(args[1],"},{"line_number":50,"context_line":"                                            objects.InstanceList(),"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_d2c6f848","line":47,"in_reply_to":"9a1a9d01_4c4ed197","updated":"2015-10-01 18:33:39.000000000","message":"Yes. Unless you set the uuid (make the test data realistic), it defaults to something like aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa, and the bucketing will fail in: _bdms_by_instance_uuid() causing these tests to fail.","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"67d44c9b925038d52e5ecc3641a78590a2c65295","unresolved":false,"context_lines":[{"line_number":88,"context_line":"    def _setUp(self):"},{"line_number":89,"context_line":"        self.Controller \u003d extended_volumes_v21.ExtendedVolumesController()"},{"line_number":90,"context_line":"        self.stubs.Set(volume.cinder.API, \u0027get\u0027, fake_volume_get)"},{"line_number":91,"context_line":"        self.action_url \u003d \"/%s/action\" % UUID1"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    def _make_request(self, url, body\u003dNone):"},{"line_number":94,"context_line":"        req \u003d webob.Request.blank(\u0027/v2/fake/servers\u0027 + url)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a1a9d01_7ee33334","side":"PARENT","line":91,"updated":"2015-10-01 19:54:26.000000000","message":"Again, please don\u0027t remove this unrelated thing in this patch.","commit_id":"40159ac6d2cc87becb8f191bc2e4340eef6936f7"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"96dd9797d1ed73df72db32f753636361d19d0481","unresolved":false,"context_lines":[{"line_number":88,"context_line":"    def _setUp(self):"},{"line_number":89,"context_line":"        self.Controller \u003d extended_volumes_v21.ExtendedVolumesController()"},{"line_number":90,"context_line":"        self.stubs.Set(volume.cinder.API, \u0027get\u0027, fake_volume_get)"},{"line_number":91,"context_line":"        self.action_url \u003d \"/%s/action\" % UUID1"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    def _make_request(self, url, body\u003dNone):"},{"line_number":94,"context_line":"        req \u003d webob.Request.blank(\u0027/v2/fake/servers\u0027 + url)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a1a9d01_c1013f3b","side":"PARENT","line":91,"in_reply_to":"9a1a9d01_7ee33334","updated":"2015-10-01 22:37:21.000000000","message":"Done","commit_id":"40159ac6d2cc87becb8f191bc2e4340eef6936f7"}],"nova/tests/unit/api/openstack/fakes.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b043617ce46f0a7879980879ea3b555c1da39bf0","unresolved":false,"context_lines":[{"line_number":708,"context_line":""},{"line_number":709,"context_line":""},{"line_number":710,"context_line":"def stub_bdm_get_all_by_instance_uuids("},{"line_number":711,"context_line":"        context, instance_uuids, use_slave\u003dFalse):"},{"line_number":712,"context_line":"    i \u003d 1"},{"line_number":713,"context_line":"    result \u003d []"},{"line_number":714,"context_line":"    for instance_uuid in instance_uuids:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_cc08a130","line":711,"updated":"2015-10-01 17:41:50.000000000","message":"Style here.","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"ba2491f06a8d57d683d1796eed57e63e943caf55","unresolved":false,"context_lines":[{"line_number":708,"context_line":""},{"line_number":709,"context_line":""},{"line_number":710,"context_line":"def stub_bdm_get_all_by_instance_uuids("},{"line_number":711,"context_line":"        context, instance_uuids, use_slave\u003dFalse):"},{"line_number":712,"context_line":"    i \u003d 1"},{"line_number":713,"context_line":"    result \u003d []"},{"line_number":714,"context_line":"    for instance_uuid in instance_uuids:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_1239d041","line":711,"in_reply_to":"9a1a9d01_cc08a130","updated":"2015-10-01 18:33:39.000000000","message":"Done","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"402051ec327a315afaf012ba411f40aa4f9d4a24","unresolved":false,"context_lines":[{"line_number":709,"context_line":""},{"line_number":710,"context_line":"def stub_bdm_get_all_by_instance_uuids("},{"line_number":711,"context_line":"        context, instance_uuids, use_slave\u003dFalse):"},{"line_number":712,"context_line":"    i \u003d 1"},{"line_number":713,"context_line":"    result \u003d []"},{"line_number":714,"context_line":"    for instance_uuid in instance_uuids:"},{"line_number":715,"context_line":"        for x in range(2):  # add two BDMs per instance"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_ec248561","line":712,"updated":"2015-10-01 17:29:19.000000000","message":"Minor nit: using enumerate() would be cleaner than a separate counter.","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"eeae6b7763f483f96b804a36a1f8ae79e364b99f","unresolved":false,"context_lines":[{"line_number":709,"context_line":""},{"line_number":710,"context_line":"def stub_bdm_get_all_by_instance_uuids("},{"line_number":711,"context_line":"        context, instance_uuids, use_slave\u003dFalse):"},{"line_number":712,"context_line":"    i \u003d 1"},{"line_number":713,"context_line":"    result \u003d []"},{"line_number":714,"context_line":"    for instance_uuid in instance_uuids:"},{"line_number":715,"context_line":"        for x in range(2):  # add two BDMs per instance"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_c3acec37","line":712,"in_reply_to":"9a1a9d01_1210f0a7","updated":"2015-10-01 19:40:17.000000000","message":"Yep, I think what you have is fine.","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"ba2491f06a8d57d683d1796eed57e63e943caf55","unresolved":false,"context_lines":[{"line_number":709,"context_line":""},{"line_number":710,"context_line":"def stub_bdm_get_all_by_instance_uuids("},{"line_number":711,"context_line":"        context, instance_uuids, use_slave\u003dFalse):"},{"line_number":712,"context_line":"    i \u003d 1"},{"line_number":713,"context_line":"    result \u003d []"},{"line_number":714,"context_line":"    for instance_uuid in instance_uuids:"},{"line_number":715,"context_line":"        for x in range(2):  # add two BDMs per instance"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_1210f0a7","line":712,"in_reply_to":"9a1a9d01_ec248561","updated":"2015-10-01 18:33:39.000000000","message":"I started with enumerate but removed it because I needed to increment on the inner loop, not the outer loop. Unless I\u0027ve misunderstood. Show me?","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b5b52fbd18d82478c45a3f7433c2dea1121586d4","unresolved":false,"context_lines":[{"line_number":713,"context_line":"    result \u003d []"},{"line_number":714,"context_line":"    for instance_uuid in instance_uuids:"},{"line_number":715,"context_line":"        for x in range(2):  # add two BDMs per instance"},{"line_number":716,"context_line":"            result.append(fake_block_device.FakeDbBlockDeviceDict({"},{"line_number":717,"context_line":"                \u0027id\u0027: i,"},{"line_number":718,"context_line":"                \u0027source_type\u0027: \u0027volume\u0027,"},{"line_number":719,"context_line":"                \u0027destination_type\u0027: \u0027volume\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a1a9d01_9e8887c8","line":716,"updated":"2015-10-01 20:04:58.000000000","message":"This is actually nicer formatting IMO - when doing this kind of stuff with dicts, so I appreciate this cleanup while already in here making a change.","commit_id":"b9cfa9ae93881ac4e016223ffadd34c7c40c4775"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b5b52fbd18d82478c45a3f7433c2dea1121586d4","unresolved":false,"context_lines":[{"line_number":720,"context_line":"                \u0027volume_id\u0027: \u0027volume_id%d\u0027 % (i),"},{"line_number":721,"context_line":"                \u0027instance_uuid\u0027: instance_uuid,"},{"line_number":722,"context_line":"            }))"},{"line_number":723,"context_line":"            i +\u003d 1"},{"line_number":724,"context_line":"    return result"},{"line_number":725,"context_line":""},{"line_number":726,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9a1a9d01_7e8fb3cc","line":723,"updated":"2015-10-01 20:04:58.000000000","message":"Note you could use enumerate to get the index:\n\nfor index, instance_uuid in enumerate(instance_uuids):\n    ....","commit_id":"b9cfa9ae93881ac4e016223ffadd34c7c40c4775"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"60a22e79678fec9a52419fdc16c8f746e6d79106","unresolved":false,"context_lines":[{"line_number":720,"context_line":"                \u0027volume_id\u0027: \u0027volume_id%d\u0027 % (i),"},{"line_number":721,"context_line":"                \u0027instance_uuid\u0027: instance_uuid,"},{"line_number":722,"context_line":"            }))"},{"line_number":723,"context_line":"            i +\u003d 1"},{"line_number":724,"context_line":"    return result"},{"line_number":725,"context_line":""},{"line_number":726,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9a1a9d01_75083192","line":723,"in_reply_to":"9a1a9d01_7e8fb3cc","updated":"2015-10-01 20:48:44.000000000","message":"I originally had enumerate too, but I need to increment on the inner loop not the outer loop...","commit_id":"b9cfa9ae93881ac4e016223ffadd34c7c40c4775"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"b3e57f7ad32b907dfe98d2d7f6cc22413357e0e8","unresolved":false,"context_lines":[{"line_number":709,"context_line":""},{"line_number":710,"context_line":"def stub_bdm_get_all_by_instance_uuids(context, instance_uuids,"},{"line_number":711,"context_line":"                                       use_slave\u003dFalse):"},{"line_number":712,"context_line":"    i \u003d 1"},{"line_number":713,"context_line":"    result \u003d []"},{"line_number":714,"context_line":"    for instance_uuid in instance_uuids:"},{"line_number":715,"context_line":"        for x in range(2):  # add two BDMs per instance"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a2fa921_0b620dae","line":712,"updated":"2015-10-12 07:05:23.000000000","message":"nit: we could use enumerate(instance_uuids) below instead for maximum pythonicity :)","commit_id":"d618c4810db6ac268a2acdbd387f37be2007f18a"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"317693b0895d64090af300d31366f21d31c5fbd7","unresolved":false,"context_lines":[{"line_number":709,"context_line":""},{"line_number":710,"context_line":"def stub_bdm_get_all_by_instance_uuids(context, instance_uuids,"},{"line_number":711,"context_line":"                                       use_slave\u003dFalse):"},{"line_number":712,"context_line":"    i \u003d 1"},{"line_number":713,"context_line":"    result \u003d []"},{"line_number":714,"context_line":"    for instance_uuid in instance_uuids:"},{"line_number":715,"context_line":"        for x in range(2):  # add two BDMs per instance"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a2fa921_841604f0","line":712,"in_reply_to":"7a2fa921_0b620dae","updated":"2015-10-12 12:53:26.000000000","message":"I had enumerate until I added the second, inner loop. Note that `i` increments on the inner, not the outer loop. Using enumerate in this case will change the behaviour.","commit_id":"d618c4810db6ac268a2acdbd387f37be2007f18a"},{"author":{"_account_id":11803,"name":"Ryan Rossiter","email":"rlrossit@us.ibm.com","username":"rlrossiter"},"change_message_id":"dad7bfef939dc3e811c22c1daf4a6064b312a43f","unresolved":false,"context_lines":[{"line_number":684,"context_line":""},{"line_number":685,"context_line":"def stub_bdm_get_all_by_instance_uuids(context, instance_uuids,"},{"line_number":686,"context_line":"                                       use_slave\u003dFalse):"},{"line_number":687,"context_line":"    i \u003d 1"},{"line_number":688,"context_line":"    result \u003d []"},{"line_number":689,"context_line":"    for instance_uuid in instance_uuids:"},{"line_number":690,"context_line":"        for x in range(2):  # add two BDMs per instance"}],"source_content_type":"text/x-python","patch_set":15,"id":"da85f559_32c3dc39","line":687,"updated":"2015-11-12 16:14:34.000000000","message":"You don\u0027t need to use an index (see comment below)","commit_id":"2e7f581a9379da5c1f8292a8a754bbf8f780aba8"},{"author":{"_account_id":11803,"name":"Ryan Rossiter","email":"rlrossit@us.ibm.com","username":"rlrossiter"},"change_message_id":"dad7bfef939dc3e811c22c1daf4a6064b312a43f","unresolved":false,"context_lines":[{"line_number":687,"context_line":"    i \u003d 1"},{"line_number":688,"context_line":"    result \u003d []"},{"line_number":689,"context_line":"    for instance_uuid in instance_uuids:"},{"line_number":690,"context_line":"        for x in range(2):  # add two BDMs per instance"},{"line_number":691,"context_line":"            result.append(fake_block_device.FakeDbBlockDeviceDict({"},{"line_number":692,"context_line":"                \u0027id\u0027: i,"},{"line_number":693,"context_line":"                \u0027source_type\u0027: \u0027volume\u0027,"}],"source_content_type":"text/x-python","patch_set":15,"id":"da85f559_12bc98b9","line":690,"updated":"2015-11-12 16:14:34.000000000","message":"because you\u0027re using range, x in here will be 0 and 1, so you don\u0027t need to use i. If you want to keep using 1 and 2, you can just use x + 1.","commit_id":"2e7f581a9379da5c1f8292a8a754bbf8f780aba8"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"f8cc4f1e8f48a275f69b18106920b61bfda8fe27","unresolved":false,"context_lines":[{"line_number":687,"context_line":"    i \u003d 1"},{"line_number":688,"context_line":"    result \u003d []"},{"line_number":689,"context_line":"    for instance_uuid in instance_uuids:"},{"line_number":690,"context_line":"        for x in range(2):  # add two BDMs per instance"},{"line_number":691,"context_line":"            result.append(fake_block_device.FakeDbBlockDeviceDict({"},{"line_number":692,"context_line":"                \u0027id\u0027: i,"},{"line_number":693,"context_line":"                \u0027source_type\u0027: \u0027volume\u0027,"}],"source_content_type":"text/x-python","patch_set":15,"id":"da85f559_1d66e950","line":690,"in_reply_to":"da85f559_12bc98b9","updated":"2015-11-12 16:26:05.000000000","message":"I don\u0027t always want 1 \u0026 2. I want 1, 2, 3, 4, 5, 6... etc depending on what\u0027s in the uuid list.","commit_id":"2e7f581a9379da5c1f8292a8a754bbf8f780aba8"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"423234d443d22dc410183eff919b70416b804b58","unresolved":false,"context_lines":[{"line_number":687,"context_line":"    i \u003d 1"},{"line_number":688,"context_line":"    result \u003d []"},{"line_number":689,"context_line":"    for instance_uuid in instance_uuids:"},{"line_number":690,"context_line":"        for x in range(2):  # add two BDMs per instance"},{"line_number":691,"context_line":"            result.append(fake_block_device.FakeDbBlockDeviceDict({"},{"line_number":692,"context_line":"                \u0027id\u0027: i,"},{"line_number":693,"context_line":"                \u0027source_type\u0027: \u0027volume\u0027,"}],"source_content_type":"text/x-python","patch_set":15,"id":"da85f559_ec4c949f","line":690,"in_reply_to":"da85f559_1d37c9ff","updated":"2015-11-12 16:57:06.000000000","message":"Well, enumerate() is always an option, too. But yeah, you\u0027ll have to increment to start with 1 instead of 0.","commit_id":"2e7f581a9379da5c1f8292a8a754bbf8f780aba8"},{"author":{"_account_id":11803,"name":"Ryan Rossiter","email":"rlrossit@us.ibm.com","username":"rlrossiter"},"change_message_id":"d9b8f1d3ba49c98755036fc64365a6a862e72faa","unresolved":false,"context_lines":[{"line_number":687,"context_line":"    i \u003d 1"},{"line_number":688,"context_line":"    result \u003d []"},{"line_number":689,"context_line":"    for instance_uuid in instance_uuids:"},{"line_number":690,"context_line":"        for x in range(2):  # add two BDMs per instance"},{"line_number":691,"context_line":"            result.append(fake_block_device.FakeDbBlockDeviceDict({"},{"line_number":692,"context_line":"                \u0027id\u0027: i,"},{"line_number":693,"context_line":"                \u0027source_type\u0027: \u0027volume\u0027,"}],"source_content_type":"text/x-python","patch_set":15,"id":"da85f559_1d37c9ff","line":690,"in_reply_to":"da85f559_1d66e950","updated":"2015-11-12 16:29:06.000000000","message":"My bad! I missed the outer loop. Looks good then!","commit_id":"2e7f581a9379da5c1f8292a8a754bbf8f780aba8"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"3f02435b32ec1b84b69772a3be236f50255a812d","unresolved":false,"context_lines":[{"line_number":687,"context_line":"    i \u003d 1"},{"line_number":688,"context_line":"    result \u003d []"},{"line_number":689,"context_line":"    for instance_uuid in instance_uuids:"},{"line_number":690,"context_line":"        for x in range(2):  # add two BDMs per instance"},{"line_number":691,"context_line":"            result.append(fake_block_device.FakeDbBlockDeviceDict({"},{"line_number":692,"context_line":"                \u0027id\u0027: i,"},{"line_number":693,"context_line":"                \u0027source_type\u0027: \u0027volume\u0027,"}],"source_content_type":"text/x-python","patch_set":15,"id":"da85f559_df5a7e2a","line":690,"in_reply_to":"da85f559_2f484ecb","updated":"2015-11-13 01:54:12.000000000","message":"You could in theory enumerate the outer loop and then adjust the range call of the inner loop but this is probably better.","commit_id":"2e7f581a9379da5c1f8292a8a754bbf8f780aba8"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"b9ac7f745c44ca05c3a33afcb2b9ea181fb97eab","unresolved":false,"context_lines":[{"line_number":687,"context_line":"    i \u003d 1"},{"line_number":688,"context_line":"    result \u003d []"},{"line_number":689,"context_line":"    for instance_uuid in instance_uuids:"},{"line_number":690,"context_line":"        for x in range(2):  # add two BDMs per instance"},{"line_number":691,"context_line":"            result.append(fake_block_device.FakeDbBlockDeviceDict({"},{"line_number":692,"context_line":"                \u0027id\u0027: i,"},{"line_number":693,"context_line":"                \u0027source_type\u0027: \u0027volume\u0027,"}],"source_content_type":"text/x-python","patch_set":15,"id":"da85f559_2f484ecb","line":690,"in_reply_to":"da85f559_ec4c949f","updated":"2015-11-12 17:12:03.000000000","message":"Lots of people have suggested enumerate, but it doesn\u0027t quite work given the nested loops \u0026 the end goal. Unless I\u0027m missing something (which is always possible).\n\nAnyhoo, the number of enumerate suggestions is interesting - an indication of just how un-pythonic we all find manually incrementing loop counters is ;)","commit_id":"2e7f581a9379da5c1f8292a8a754bbf8f780aba8"}],"nova/tests/unit/db/test_db_api.py":[{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"402051ec327a315afaf012ba411f40aa4f9d4a24","unresolved":false,"context_lines":[{"line_number":5895,"context_line":"        uuid1 \u003d self.instance[\u0027uuid\u0027]"},{"line_number":5896,"context_line":"        uuid2 \u003d db.instance_create(self.ctxt, {})[\u0027uuid\u0027]"},{"line_number":5897,"context_line":""},{"line_number":5898,"context_line":"        bmds_values \u003d [{\u0027instance_uuid\u0027: uuid1,"},{"line_number":5899,"context_line":"                        \u0027device_name\u0027: \u0027/dev/vda\u0027},"},{"line_number":5900,"context_line":"                       {\u0027instance_uuid\u0027: uuid2,"},{"line_number":5901,"context_line":"                        \u0027device_name\u0027: \u0027/dev/vdb\u0027},"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_ac7bed28","line":5898,"updated":"2015-10-01 17:29:19.000000000","message":"I feel a bit dyslexic reading these variable names. Some are \u0027bdm\u0027, and others are \u0027bmd\u0027. Also, the result of the block_device_mapping_get_all_by_instance_uuids() calls below are lists, and typically have a plural variable name. These don\u0027t affect the test code, but if you respin, it would make the test easier to read.","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"ba2491f06a8d57d683d1796eed57e63e943caf55","unresolved":false,"context_lines":[{"line_number":5895,"context_line":"        uuid1 \u003d self.instance[\u0027uuid\u0027]"},{"line_number":5896,"context_line":"        uuid2 \u003d db.instance_create(self.ctxt, {})[\u0027uuid\u0027]"},{"line_number":5897,"context_line":""},{"line_number":5898,"context_line":"        bmds_values \u003d [{\u0027instance_uuid\u0027: uuid1,"},{"line_number":5899,"context_line":"                        \u0027device_name\u0027: \u0027/dev/vda\u0027},"},{"line_number":5900,"context_line":"                       {\u0027instance_uuid\u0027: uuid2,"},{"line_number":5901,"context_line":"                        \u0027device_name\u0027: \u0027/dev/vdb\u0027},"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_924ac0ab","line":5898,"in_reply_to":"9a1a9d01_0f0d2b37","updated":"2015-10-01 18:33:39.000000000","message":"These variable names are from the original surrounding code... if I change them all, am I going to get asked to revert the unnecessary changes?","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"eeae6b7763f483f96b804a36a1f8ae79e364b99f","unresolved":false,"context_lines":[{"line_number":5895,"context_line":"        uuid1 \u003d self.instance[\u0027uuid\u0027]"},{"line_number":5896,"context_line":"        uuid2 \u003d db.instance_create(self.ctxt, {})[\u0027uuid\u0027]"},{"line_number":5897,"context_line":""},{"line_number":5898,"context_line":"        bmds_values \u003d [{\u0027instance_uuid\u0027: uuid1,"},{"line_number":5899,"context_line":"                        \u0027device_name\u0027: \u0027/dev/vda\u0027},"},{"line_number":5900,"context_line":"                       {\u0027instance_uuid\u0027: uuid2,"},{"line_number":5901,"context_line":"                        \u0027device_name\u0027: \u0027/dev/vdb\u0027},"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_037184e0","line":5898,"in_reply_to":"9a1a9d01_924ac0ab","updated":"2015-10-01 19:40:17.000000000","message":"Yes, please don\u0027t change them all in this patch.","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"ba2491f06a8d57d683d1796eed57e63e943caf55","unresolved":false,"context_lines":[{"line_number":5895,"context_line":"        uuid1 \u003d self.instance[\u0027uuid\u0027]"},{"line_number":5896,"context_line":"        uuid2 \u003d db.instance_create(self.ctxt, {})[\u0027uuid\u0027]"},{"line_number":5897,"context_line":""},{"line_number":5898,"context_line":"        bmds_values \u003d [{\u0027instance_uuid\u0027: uuid1,"},{"line_number":5899,"context_line":"                        \u0027device_name\u0027: \u0027/dev/vda\u0027},"},{"line_number":5900,"context_line":"                       {\u0027instance_uuid\u0027: uuid2,"},{"line_number":5901,"context_line":"                        \u0027device_name\u0027: \u0027/dev/vdb\u0027},"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_12ec708e","line":5898,"in_reply_to":"9a1a9d01_ac7bed28","updated":"2015-10-01 18:33:39.000000000","message":"These variable names are from the original surrounding code. Sometimes when I try to \"fix\" things, I get asked to revert the unnecessary changes, but I\u0027ll try :)","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b043617ce46f0a7879980879ea3b555c1da39bf0","unresolved":false,"context_lines":[{"line_number":5895,"context_line":"        uuid1 \u003d self.instance[\u0027uuid\u0027]"},{"line_number":5896,"context_line":"        uuid2 \u003d db.instance_create(self.ctxt, {})[\u0027uuid\u0027]"},{"line_number":5897,"context_line":""},{"line_number":5898,"context_line":"        bmds_values \u003d [{\u0027instance_uuid\u0027: uuid1,"},{"line_number":5899,"context_line":"                        \u0027device_name\u0027: \u0027/dev/vda\u0027},"},{"line_number":5900,"context_line":"                       {\u0027instance_uuid\u0027: uuid2,"},{"line_number":5901,"context_line":"                        \u0027device_name\u0027: \u0027/dev/vdb\u0027},"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_0f0d2b37","line":5898,"in_reply_to":"9a1a9d01_ac7bed28","updated":"2015-10-01 17:41:50.000000000","message":"Yeah, this is confusing, agreed.","commit_id":"1777bcb957043b4aea769217e230be501ef8e530"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b5b52fbd18d82478c45a3f7433c2dea1121586d4","unresolved":false,"context_lines":[{"line_number":5895,"context_line":"        uuid1 \u003d self.instance[\u0027uuid\u0027]"},{"line_number":5896,"context_line":"        uuid2 \u003d db.instance_create(self.ctxt, {})[\u0027uuid\u0027]"},{"line_number":5897,"context_line":""},{"line_number":5898,"context_line":"        bmds_values \u003d [{\u0027instance_uuid\u0027: uuid1,"},{"line_number":5899,"context_line":"                        \u0027device_name\u0027: \u0027/dev/vda\u0027},"},{"line_number":5900,"context_line":"                       {\u0027instance_uuid\u0027: uuid2,"},{"line_number":5901,"context_line":"                        \u0027device_name\u0027: \u0027/dev/vdb\u0027},"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a1a9d01_de427f68","side":"PARENT","line":5898,"updated":"2015-10-01 20:04:58.000000000","message":"heh, bmd, good rename","commit_id":"40159ac6d2cc87becb8f191bc2e4340eef6936f7"}],"nova/tests/unit/objects/test_block_device.py":[{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"b3e57f7ad32b907dfe98d2d7f6cc22413357e0e8","unresolved":false,"context_lines":[{"line_number":349,"context_line":""},{"line_number":350,"context_line":"    @mock.patch.object(db, \u0027block_device_mapping_get_all_by_instance_uuids\u0027)"},{"line_number":351,"context_line":"    def test_get_by_instance_uuids(self, get_all_by_inst_uuids):"},{"line_number":352,"context_line":"        fakes \u003d [self.fake_bdm(123), self.fake_bdm(456)]"},{"line_number":353,"context_line":"        get_all_by_inst_uuids.return_value \u003d fakes"},{"line_number":354,"context_line":"        bdm_list \u003d objects.BlockDeviceMappingList.get_by_instance_uuids("},{"line_number":355,"context_line":"            self.context, [\u0027fake-instance\u0027])"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a2fa921_cbcdf5b8","line":352,"updated":"2015-10-12 07:05:23.000000000","message":"An id mock where a number other than 42 was used?! I question your heretic ways! (joke of course :) )","commit_id":"d618c4810db6ac268a2acdbd387f37be2007f18a"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"317693b0895d64090af300d31366f21d31c5fbd7","unresolved":false,"context_lines":[{"line_number":349,"context_line":""},{"line_number":350,"context_line":"    @mock.patch.object(db, \u0027block_device_mapping_get_all_by_instance_uuids\u0027)"},{"line_number":351,"context_line":"    def test_get_by_instance_uuids(self, get_all_by_inst_uuids):"},{"line_number":352,"context_line":"        fakes \u003d [self.fake_bdm(123), self.fake_bdm(456)]"},{"line_number":353,"context_line":"        get_all_by_inst_uuids.return_value \u003d fakes"},{"line_number":354,"context_line":"        bdm_list \u003d objects.BlockDeviceMappingList.get_by_instance_uuids("},{"line_number":355,"context_line":"            self.context, [\u0027fake-instance\u0027])"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a2fa921_a273f3e6","line":352,"in_reply_to":"7a2fa921_cbcdf5b8","updated":"2015-10-12 12:53:26.000000000","message":"I just followed the pattern set by the other tests in this class ;)","commit_id":"d618c4810db6ac268a2acdbd387f37be2007f18a"}]}