)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2b2e1b33c412544f8acc5ec4c14f901ae405456","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2015-11-06 11:54:57 -0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Make BlockDeviceMapping object support uuid"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I3b25debb0bcfd4e211734307c8d363f2b5dbc655"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"fa80f949_99af9829","line":8,"updated":"2015-11-06 19:53:52.000000000","message":"Partial-Bug: #1489581","commit_id":"d9c8fc77000cf35e50cc95fe84c654bf2b3b6eda"}],"nova/db/sqlalchemy/api.py":[{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"d96e0bed14ddc9d6287adf2d469404b879345a28","unresolved":false,"context_lines":[{"line_number":3734,"context_line":"    values \u003d _from_legacy_values(values, legacy)"},{"line_number":3735,"context_line":"    convert_objects_related_datetimes(values)"},{"line_number":3736,"context_line":""},{"line_number":3737,"context_line":"    # NOTE(mdbooth): For use in tests only, allow creation of a null uuid"},{"line_number":3738,"context_line":"    if not values.get(\u0027uuid\u0027) and not test_null_uuid:"},{"line_number":3739,"context_line":"        values[\u0027uuid\u0027] \u003d uuidutils.generate_uuid()"},{"line_number":3740,"context_line":""},{"line_number":3741,"context_line":"    bdm_ref \u003d models.BlockDeviceMapping()"}],"source_content_type":"text/x-python","patch_set":25,"id":"df87a7cf_5b679f26","line":3738,"range":{"start_line":3737,"start_character":0,"end_line":3738,"end_character":52},"updated":"2017-12-06 04:45:15.000000000","message":"IMO, it is not a good way to create a record whose \u0027uuid\u0027 is null.\n\nMy idea is:\nIn unit tests, once create a record whose \u0027uuid\u0027 is not null.\nThen update \u0027uuid\u0027 to null by \u0027block_device_mapping_update_or_create\u0027 method (line 3760).","commit_id":"a3d88a54b2576bf2e82b4eacf95716771e9ac381"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"572faa2c3ecad513617ca8db75d1064d9e0a4aff","unresolved":false,"context_lines":[{"line_number":3734,"context_line":"    values \u003d _from_legacy_values(values, legacy)"},{"line_number":3735,"context_line":"    convert_objects_related_datetimes(values)"},{"line_number":3736,"context_line":""},{"line_number":3737,"context_line":"    # NOTE(mdbooth): For use in tests only, allow creation of a null uuid"},{"line_number":3738,"context_line":"    if not values.get(\u0027uuid\u0027) and not test_null_uuid:"},{"line_number":3739,"context_line":"        values[\u0027uuid\u0027] \u003d uuidutils.generate_uuid()"},{"line_number":3740,"context_line":""},{"line_number":3741,"context_line":"    bdm_ref \u003d models.BlockDeviceMapping()"}],"source_content_type":"text/x-python","patch_set":25,"id":"df87a7cf_614bcd58","line":3738,"range":{"start_line":3737,"start_character":0,"end_line":3738,"end_character":52},"in_reply_to":"df87a7cf_5b679f26","updated":"2017-12-06 09:40:37.000000000","message":"That sounds worse, tbh.\n\nI\u0027ve done this. It\u0027s fine.","commit_id":"a3d88a54b2576bf2e82b4eacf95716771e9ac381"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ebc0ba2be9cb5b0340da5ff3b3ef73c366912f95","unresolved":false,"context_lines":[{"line_number":3734,"context_line":"    values \u003d _from_legacy_values(values, legacy)"},{"line_number":3735,"context_line":"    convert_objects_related_datetimes(values)"},{"line_number":3736,"context_line":""},{"line_number":3737,"context_line":"    # NOTE(mdbooth): For use in tests only, allow creation of a null uuid"},{"line_number":3738,"context_line":"    if not values.get(\u0027uuid\u0027) and not test_null_uuid:"},{"line_number":3739,"context_line":"        values[\u0027uuid\u0027] \u003d uuidutils.generate_uuid()"},{"line_number":3740,"context_line":""},{"line_number":3741,"context_line":"    bdm_ref \u003d models.BlockDeviceMapping()"}],"source_content_type":"text/x-python","patch_set":25,"id":"df87a7cf_8695be42","line":3738,"range":{"start_line":3737,"start_character":0,"end_line":3738,"end_character":52},"in_reply_to":"df87a7cf_614bcd58","updated":"2017-12-06 14:51:40.000000000","message":"Agree with Takashi that adding a kwarg just for injecting a test edge case condition isn\u0027t appropriate.\n\nWhen adding NULL-able fields like this, I\u0027ve just manually constructed a record that looks like what the record would look like without a populated uuid field and then verified the setting of the uuid field happens properly in the _from_db_object() method of the related object.\n\nFor instance, see this test here that does this:\n\nhttps://github.com/openstack/nova/blob/master/nova/tests/functional/db/test_resource_provider.py#L146-L168","commit_id":"a3d88a54b2576bf2e82b4eacf95716771e9ac381"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f35507098c35f8a287d8a22cd479d6c91d632ae0","unresolved":false,"context_lines":[{"line_number":3734,"context_line":"    values \u003d _from_legacy_values(values, legacy)"},{"line_number":3735,"context_line":"    convert_objects_related_datetimes(values)"},{"line_number":3736,"context_line":""},{"line_number":3737,"context_line":"    # NOTE(mdbooth): For use in tests only, allow creation of a null uuid"},{"line_number":3738,"context_line":"    if not values.get(\u0027uuid\u0027) and not test_null_uuid:"},{"line_number":3739,"context_line":"        values[\u0027uuid\u0027] \u003d uuidutils.generate_uuid()"},{"line_number":3740,"context_line":""},{"line_number":3741,"context_line":"    bdm_ref \u003d models.BlockDeviceMapping()"}],"source_content_type":"text/x-python","patch_set":25,"id":"df87a7cf_7db28596","line":3738,"range":{"start_line":3737,"start_character":0,"end_line":3738,"end_character":52},"in_reply_to":"df87a7cf_8695be42","updated":"2017-12-06 17:35:27.000000000","message":"Also agree, this is an anti-pattern.","commit_id":"a3d88a54b2576bf2e82b4eacf95716771e9ac381"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"f4ed553bfb18012f3c4d33b5ed482710d0241ffa","unresolved":false,"context_lines":[{"line_number":3734,"context_line":"    values \u003d _from_legacy_values(values, legacy)"},{"line_number":3735,"context_line":"    convert_objects_related_datetimes(values)"},{"line_number":3736,"context_line":""},{"line_number":3737,"context_line":"    # NOTE(mdbooth): For use in tests only, allow creation of a null uuid"},{"line_number":3738,"context_line":"    if not values.get(\u0027uuid\u0027) and not test_null_uuid:"},{"line_number":3739,"context_line":"        values[\u0027uuid\u0027] \u003d uuidutils.generate_uuid()"},{"line_number":3740,"context_line":""},{"line_number":3741,"context_line":"    bdm_ref \u003d models.BlockDeviceMapping()"}],"source_content_type":"text/x-python","patch_set":25,"id":"df87a7cf_b809fb2c","line":3738,"range":{"start_line":3737,"start_character":0,"end_line":3738,"end_character":52},"in_reply_to":"df87a7cf_8695be42","updated":"2017-12-06 17:41:21.000000000","message":"As discussed on IRC, I still personally prefer this approach as it doesn\u0027t involve any code duplication. Given that it\u0027s only a few lines, though, I\u0027ll change it anyway. Also, given that it\u0027s explicitly for creating a legacy object, it\u0027s not important for future changes to be reflected in the duplicate test code.","commit_id":"a3d88a54b2576bf2e82b4eacf95716771e9ac381"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"f11e12c1b5994135f9ce9b3805668160874e3c2c","unresolved":false,"context_lines":[{"line_number":3734,"context_line":"    values \u003d _from_legacy_values(values, legacy)"},{"line_number":3735,"context_line":"    convert_objects_related_datetimes(values)"},{"line_number":3736,"context_line":""},{"line_number":3737,"context_line":"    # NOTE(mdbooth): For use in tests only, allow creation of a null uuid"},{"line_number":3738,"context_line":"    if not values.get(\u0027uuid\u0027) and not test_null_uuid:"},{"line_number":3739,"context_line":"        values[\u0027uuid\u0027] \u003d uuidutils.generate_uuid()"},{"line_number":3740,"context_line":""},{"line_number":3741,"context_line":"    bdm_ref \u003d models.BlockDeviceMapping()"}],"source_content_type":"text/x-python","patch_set":25,"id":"df87a7cf_0cd78825","line":3738,"range":{"start_line":3737,"start_character":0,"end_line":3738,"end_character":52},"in_reply_to":"df87a7cf_b809fb2c","updated":"2017-12-06 18:55:52.000000000","message":"Done","commit_id":"a3d88a54b2576bf2e82b4eacf95716771e9ac381"}],"nova/objects/block_device.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2b2e1b33c412544f8acc5ec4c14f901ae405456","unresolved":false,"context_lines":[{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    fields \u003d {"},{"line_number":66,"context_line":"        \u0027id\u0027: fields.IntegerField(),"},{"line_number":67,"context_line":"        \u0027uuid\u0027: fields.UUIDField(nullable\u003dTrue),"},{"line_number":68,"context_line":"        \u0027instance_uuid\u0027: fields.UUIDField(),"},{"line_number":69,"context_line":"        \u0027instance\u0027: fields.ObjectField(\u0027Instance\u0027, nullable\u003dTrue),"},{"line_number":70,"context_line":"        \u0027source_type\u0027: fields.BlockDeviceSourceTypeField(nullable\u003dTrue),"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa80f949_597cd0b4","line":67,"updated":"2015-11-06 19:53:52.000000000","message":"Shouldn\u0027t we enforce non-nullable in the object even if not in the DB?  Or is this to handle backward compat with existing BDMs in the database that would now have a null uuid column value?","commit_id":"d9c8fc77000cf35e50cc95fe84c654bf2b3b6eda"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2b2e1b33c412544f8acc5ec4c14f901ae405456","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        super(BlockDeviceMapping, self).obj_make_compatible(primitive,"},{"line_number":89,"context_line":"                                                            target_version)"},{"line_number":90,"context_line":"        target_version \u003d utils.convert_version_to_tuple(target_version)"},{"line_number":91,"context_line":"        if target_version \u003c (1, 14) and \u0027uuid\u0027 in primitive:"},{"line_number":92,"context_line":"            del primitive[\u0027uuid\u0027]"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa80f949_3956ac2a","line":91,"updated":"2015-11-06 19:53:52.000000000","message":"(1, 16)","commit_id":"d9c8fc77000cf35e50cc95fe84c654bf2b3b6eda"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"888157fb39b098c748314ef4ab7cd6d4cf301411","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        super(BlockDeviceMapping, self).obj_make_compatible(primitive,"},{"line_number":91,"context_line":"                                                            target_version)"},{"line_number":92,"context_line":"        target_version \u003d utils.convert_version_to_tuple(target_version)"},{"line_number":93,"context_line":"        if target_version \u003c (1, 14) and \u0027uuid\u0027 in primitive:"},{"line_number":94,"context_line":"            del primitive[\u0027uuid\u0027]"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":3,"id":"da85f559_34b65e3e","line":93,"updated":"2015-11-06 21:16:16.000000000","message":"Shouldn\u0027t this be \u003c (1, 16) which is when uuid was added.","commit_id":"898952a0b8557e518646b7d097c992bdd6d1061a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"002251af9e952f6c7f39924f96189987aff9f071","unresolved":false,"context_lines":[{"line_number":104,"context_line":"            if key \u003d\u003d \u0027uuid\u0027 and not db_block_device.get(key):"},{"line_number":105,"context_line":"                # NOTE(danms): While the records could be nullable,"},{"line_number":106,"context_line":"                # generate a UUID on read since the object requires it"},{"line_number":107,"context_line":"                db_block_device[key] \u003d str(uuid.uuid4())"},{"line_number":108,"context_line":"            block_device_obj[key] \u003d db_block_device[key]"},{"line_number":109,"context_line":"        if \u0027instance\u0027 in expected_attrs:"},{"line_number":110,"context_line":"            my_inst \u003d objects.Instance(context)"}],"source_content_type":"text/x-python","patch_set":7,"id":"da85f559_f5589e58","line":107,"updated":"2015-11-09 14:44:30.000000000","message":"Seems we can\u0027t do this if we\u0027re in the API cell since we try to do an update_or_create and since the bdm has a uuid but it hasn\u0027t been created yet (in the API cell), we blow up:\n\nhttp://logs.openstack.org/03/242603/7/check/gate-tempest-dsvm-cells/d13935b/logs/screen-n-cell-region.txt.gz?level\u003dTRACE\n\nI wonder if we need to filter this out based on whether or not we\u0027re in the compute cell, or if we need to update cells.messaging.bdm_update_or_create_at_top in this change.","commit_id":"a92b95c2d3f06a4029593361a9740f89e3f3a694"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"bd2c922edc0e4272ca2e9c0e1f5885ec90970e69","unresolved":false,"context_lines":[{"line_number":91,"context_line":"                                                            target_version)"},{"line_number":92,"context_line":"        target_version \u003d utils.convert_version_to_tuple(target_version)"},{"line_number":93,"context_line":"        if target_version \u003c (1, 16) and \u0027uuid\u0027 in primitive:"},{"line_number":94,"context_line":"            del primitive[\u0027uuid\u0027]"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    @staticmethod"},{"line_number":97,"context_line":"    def _from_db_object(context, block_device_obj,"}],"source_content_type":"text/x-python","patch_set":8,"id":"da85f559_3e083591","line":94,"updated":"2015-11-09 19:12:14.000000000","message":"Don\u0027t forget to cover this in a later respin before we merge","commit_id":"347cda824d3651e925c0e1776b97d5065a7b7b5d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"22ac64fce0dc55b9043a695cc6c4e5c347fe40b7","unresolved":false,"context_lines":[{"line_number":211,"context_line":"            cells_api.bdm_update_or_create_at_top(self._context, self,"},{"line_number":212,"context_line":"                    create\u003dcreate)"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    @base.remotable_classmethod"},{"line_number":215,"context_line":"    def get_by_volume_id(cls, context, volume_id,"},{"line_number":216,"context_line":"                         instance_uuid\u003dNone, expected_attrs\u003dNone):"},{"line_number":217,"context_line":"        if expected_attrs is None:"}],"source_content_type":"text/x-python","patch_set":8,"id":"da85f559_8e1258b3","line":214,"updated":"2015-11-09 16:27:18.000000000","message":"Maybe not necessary, but we could have a get_by_uuid method now that calls db.block_device_mapping_get_by_uuid.","commit_id":"347cda824d3651e925c0e1776b97d5065a7b7b5d"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"09741ae9ea51d0027709e8e14b1abc8e9870bd36","unresolved":false,"context_lines":[{"line_number":150,"context_line":"            db_bdm \u003d db.block_device_mapping_update_or_create("},{"line_number":151,"context_line":"                    context, updates, legacy\u003dFalse)"},{"line_number":152,"context_line":"        else:"},{"line_number":153,"context_line":"            db_bdm \u003d db.block_device_mapping_create("},{"line_number":154,"context_line":"                    context, updates, legacy\u003dFalse)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        self._from_db_object(context, self, db_bdm)"}],"source_content_type":"text/x-python","patch_set":9,"id":"da85f559_2af19301","line":153,"updated":"2015-11-10 19:01:16.000000000","message":"It seems that we should ensure that there is a uuid at this point.  If not already set on the object then one should be set here.  The uuid being set in _from_db_object should only apply to bdms that already exist, not those created after this.\n\nUltimately I would expect the uuid to be created by whomever creates the bdm to create it, in the same manner that it\u0027s handled for instances.","commit_id":"5297d44640432d99679b6c6101c2f13f6a2768ab"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4f496191e48bc0b6b423af21ffc4ce58a303dced","unresolved":false,"context_lines":[{"line_number":150,"context_line":"            db_bdm \u003d db.block_device_mapping_update_or_create("},{"line_number":151,"context_line":"                    context, updates, legacy\u003dFalse)"},{"line_number":152,"context_line":"        else:"},{"line_number":153,"context_line":"            db_bdm \u003d db.block_device_mapping_create("},{"line_number":154,"context_line":"                    context, updates, legacy\u003dFalse)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        self._from_db_object(context, self, db_bdm)"}],"source_content_type":"text/x-python","patch_set":9,"id":"da85f559_f5ccd06b","line":153,"in_reply_to":"da85f559_2af19301","updated":"2015-11-10 19:31:02.000000000","message":"It is:\n\nhttps://review.openstack.org/#/c/242602/7/nova/db/sqlalchemy/api.py","commit_id":"5297d44640432d99679b6c6101c2f13f6a2768ab"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"69edf20f5e10940d5e47dce11756d3b7ea63f69b","unresolved":false,"context_lines":[{"line_number":150,"context_line":"            db_bdm \u003d db.block_device_mapping_update_or_create("},{"line_number":151,"context_line":"                    context, updates, legacy\u003dFalse)"},{"line_number":152,"context_line":"        else:"},{"line_number":153,"context_line":"            db_bdm \u003d db.block_device_mapping_create("},{"line_number":154,"context_line":"                    context, updates, legacy\u003dFalse)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        self._from_db_object(context, self, db_bdm)"}],"source_content_type":"text/x-python","patch_set":9,"id":"da85f559_bba87721","line":153,"in_reply_to":"da85f559_f5ccd06b","updated":"2015-11-10 20:17:53.000000000","message":"Ahh.  I wasn\u0027t expecting it in the db layer.","commit_id":"5297d44640432d99679b6c6101c2f13f6a2768ab"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"09741ae9ea51d0027709e8e14b1abc8e9870bd36","unresolved":false,"context_lines":[{"line_number":193,"context_line":"            raise exception.ObjectActionError(action\u003d\u0027save\u0027,"},{"line_number":194,"context_line":"                                              reason\u003d\u0027instance changed\u0027)"},{"line_number":195,"context_line":"        updates.pop(\u0027id\u0027, None)"},{"line_number":196,"context_line":"        updated \u003d db.block_device_mapping_update(self._context, self.id,"},{"line_number":197,"context_line":"                                                 updates, legacy\u003dFalse)"},{"line_number":198,"context_line":"        if not updated:"},{"line_number":199,"context_line":"            raise exception.BDMNotFound(id\u003dself.id)"}],"source_content_type":"text/x-python","patch_set":9,"id":"da85f559_aabca306","line":196,"updated":"2015-11-10 19:01:16.000000000","message":"Here too, I think we should make sure that \u0027uuid\u0027 is always set at this point whether it needs to be created or not.  _from_db_object should only create it for the _get_by_* methods.","commit_id":"5297d44640432d99679b6c6101c2f13f6a2768ab"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4f496191e48bc0b6b423af21ffc4ce58a303dced","unresolved":false,"context_lines":[{"line_number":193,"context_line":"            raise exception.ObjectActionError(action\u003d\u0027save\u0027,"},{"line_number":194,"context_line":"                                              reason\u003d\u0027instance changed\u0027)"},{"line_number":195,"context_line":"        updates.pop(\u0027id\u0027, None)"},{"line_number":196,"context_line":"        updated \u003d db.block_device_mapping_update(self._context, self.id,"},{"line_number":197,"context_line":"                                                 updates, legacy\u003dFalse)"},{"line_number":198,"context_line":"        if not updated:"},{"line_number":199,"context_line":"            raise exception.BDMNotFound(id\u003dself.id)"}],"source_content_type":"text/x-python","patch_set":9,"id":"da85f559_35e578db","line":196,"in_reply_to":"da85f559_aabca306","updated":"2015-11-10 19:31:02.000000000","message":"Yeah I think I agree with this one, we shouldn\u0027t inject a uuid on update.","commit_id":"5297d44640432d99679b6c6101c2f13f6a2768ab"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c765c9437453e03fb17b49547d2cb9f7ed05ee2b","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            raise exception.ObjectActionError(action\u003d\u0027create\u0027,"},{"line_number":146,"context_line":"                                              reason\u003d\u0027instance assigned\u0027)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"        if \u0027uuid\u0027 not in updates:"},{"line_number":149,"context_line":"            updates[\u0027uuid\u0027] \u003d str(uuid.uuid4())"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"        cells_create \u003d update_or_create or None"}],"source_content_type":"text/x-python","patch_set":10,"id":"da85f559_3b176249","line":148,"updated":"2015-11-12 01:53:21.000000000","message":"We don\u0027t really need this in _create since the DB API does it for us, like with instance create. Is there a reason for adding it?","commit_id":"a92da7a00a8ef1b42a6e6d013bbb92778ab380b3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c765c9437453e03fb17b49547d2cb9f7ed05ee2b","unresolved":false,"context_lines":[{"line_number":149,"context_line":"            updates[\u0027uuid\u0027] \u003d str(uuid.uuid4())"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"        cells_create \u003d update_or_create or None"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"        if update_or_create:"},{"line_number":154,"context_line":"            db_bdm \u003d db.block_device_mapping_update_or_create("},{"line_number":155,"context_line":"                    context, updates, legacy\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":10,"id":"da85f559_5b3906d6","line":152,"updated":"2015-11-12 01:53:21.000000000","message":"Unrelated whitespace change.","commit_id":"a92da7a00a8ef1b42a6e6d013bbb92778ab380b3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"17c1306340d711088e52675fb3926d99b734d882","unresolved":false,"context_lines":[{"line_number":63,"context_line":"    # Version 1.16: Deprecate get_by_volume_id(), add"},{"line_number":64,"context_line":"    #               get_by_volume() and get_by_volume_and_instance()"},{"line_number":65,"context_line":"    # Version 1.17: Added tag field"},{"line_number":66,"context_line":"    # Version 1.18: Added uuid"},{"line_number":67,"context_line":"    VERSION \u003d \u00271.18\u0027"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":21,"id":"ba2be162_30e597db","line":66,"updated":"2017-03-03 17:12:20.000000000","message":"The DB API change before this went to the work of creating a block_device_mapping_get_by_uuid method, shouldn\u0027t we expose that up through the object here? We could do it later but then it\u0027s another version bump.","commit_id":"bea1925771e40d6b52b0d5dc7bb209a0766eeff0"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"b257f4dafd95f98c33efc590c21d155aa34b3973","unresolved":false,"context_lines":[{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    fields \u003d {"},{"line_number":70,"context_line":"        \u0027id\u0027: fields.IntegerField(),"},{"line_number":71,"context_line":"        \u0027uuid\u0027: fields.UUIDField(),"},{"line_number":72,"context_line":"        \u0027instance_uuid\u0027: fields.UUIDField(),"},{"line_number":73,"context_line":"        \u0027instance\u0027: fields.ObjectField(\u0027Instance\u0027, nullable\u003dTrue),"},{"line_number":74,"context_line":"        \u0027source_type\u0027: fields.BlockDeviceSourceTypeField(nullable\u003dTrue),"}],"source_content_type":"text/x-python","patch_set":23,"id":"3a1ff146_0be8e80f","line":71,"updated":"2017-03-15 17:34:00.000000000","message":"In response to your question on the previous patch: yes, I did :)","commit_id":"6b2384a629742c729a12eaf64b85f41e91386aff"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"b257f4dafd95f98c33efc590c21d155aa34b3973","unresolved":false,"context_lines":[{"line_number":107,"context_line":"            if key \u003d\u003d \u0027uuid\u0027 and not db_block_device.get(key):"},{"line_number":108,"context_line":"                # NOTE(danms): While the records could be nullable,"},{"line_number":109,"context_line":"                # generate a UUID on read since the object requires it"},{"line_number":110,"context_line":"                db_block_device[key] \u003d uuidutils.generate_uuid()"},{"line_number":111,"context_line":"            block_device_obj[key] \u003d db_block_device[key]"},{"line_number":112,"context_line":"        if \u0027instance\u0027 in expected_attrs:"},{"line_number":113,"context_line":"            my_inst \u003d objects.Instance(context)"}],"source_content_type":"text/x-python","patch_set":23,"id":"3a1ff146_8bfe9869","line":110,"updated":"2017-03-15 17:34:00.000000000","message":"This looks dangerous unless you force a save before returning the object. If the caller sends this uuid elsewhere without saving the object (and how would the caller know it needs to?) we\u0027ve just propagated a random ephemeral uuid.","commit_id":"6b2384a629742c729a12eaf64b85f41e91386aff"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"67c2eff2c600b70406f4c87fbd39f47b6704a072","unresolved":false,"context_lines":[{"line_number":107,"context_line":"            if key \u003d\u003d \u0027uuid\u0027 and not db_block_device.get(key):"},{"line_number":108,"context_line":"                # NOTE(danms): While the records could be nullable,"},{"line_number":109,"context_line":"                # generate a UUID on read since the object requires it"},{"line_number":110,"context_line":"                db_block_device[key] \u003d uuidutils.generate_uuid()"},{"line_number":111,"context_line":"            block_device_obj[key] \u003d db_block_device[key]"},{"line_number":112,"context_line":"        if \u0027instance\u0027 in expected_attrs:"},{"line_number":113,"context_line":"            my_inst \u003d objects.Instance(context)"}],"source_content_type":"text/x-python","patch_set":23,"id":"3a1ff146_3ae4e239","line":110,"in_reply_to":"3a1ff146_8bfe9869","updated":"2017-03-16 15:44:38.000000000","message":"I wanted to see what we did for the ComputeNode.uuid field and in that case we set the uuid on create which will be persisted:\n\nhttps://github.com/openstack/nova/blob/master/nova/objects/compute_node.py#L294\n\nSo it\u0027s handled differently there. I\u0027m not sure why we wouldn\u0027t do similar in _create below, except the update_or_create call worries me in case we would end up updating and overwriting an existing uuid on the bdm, but in the case of update I\u0027d think the uuid would already be set if we pulled the bdm from the database.","commit_id":"6b2384a629742c729a12eaf64b85f41e91386aff"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"10ffd2448e1c0f03c744c1e216d6d01b96573b26","unresolved":false,"context_lines":[{"line_number":111,"context_line":"            if key \u003d\u003d \u0027uuid\u0027 and not db_block_device.get(key):"},{"line_number":112,"context_line":"                # NOTE(danms): While the records could be nullable,"},{"line_number":113,"context_line":"                # generate a UUID on read since the object requires it"},{"line_number":114,"context_line":"                db_block_device[key] \u003d uuidutils.generate_uuid()"},{"line_number":115,"context_line":"            block_device_obj[key] \u003d db_block_device[key]"},{"line_number":116,"context_line":"        if \u0027instance\u0027 in expected_attrs:"},{"line_number":117,"context_line":"            my_inst \u003d objects.Instance(context)"}],"source_content_type":"text/x-python","patch_set":24,"id":"df87a7cf_8e91959c","line":114,"updated":"2017-12-04 15:37:08.000000000","message":"As noted in previous review, we need to persist this immediately (and atomically), or risk a race. The impact would be potentially persisting a reference to the wrong uuid.","commit_id":"685cae4f0cf00d37c440c56c788eeec481ef003d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"9393754690404ee68f6035d86e4a342a0672b252","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        if target_version \u003c (1, 17) and \u0027tag\u0027 in primitive:"},{"line_number":105,"context_line":"            del primitive[\u0027tag\u0027]"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    @staticmethod"},{"line_number":108,"context_line":"    @oslo_db_api.wrap_db_retry(max_retries\u003d1, retry_on_deadlock\u003dTrue)"},{"line_number":109,"context_line":"    def _create_uuid(context, bdm_id):"},{"line_number":110,"context_line":"        # NOTE(mdbooth): This method is only required until uuid is made"},{"line_number":111,"context_line":"        # non-nullable in a future release."},{"line_number":112,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"df87a7cf_b246fc7b","line":109,"range":{"start_line":107,"start_character":0,"end_line":109,"end_character":38},"updated":"2017-12-06 17:06:10.000000000","message":"As mentioned on IRC, I realize now there are no enginefacade decorators on any of the BDM object methods like create()/save() etc.\n\nInstead of doing the inner transaction context manager on lines 124-125, you should just be able to do this instead:\n\n @staticmethod\n @db.api_context_manager.writer\n @oslo_db_api.wrap_db_retry(...)\n def _create_uuid(...):","commit_id":"a3d88a54b2576bf2e82b4eacf95716771e9ac381"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"6916c7c1f2ebbc606e1ee007df7643e4b66d7dff","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        if target_version \u003c (1, 17) and \u0027tag\u0027 in primitive:"},{"line_number":105,"context_line":"            del primitive[\u0027tag\u0027]"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    @staticmethod"},{"line_number":108,"context_line":"    @oslo_db_api.wrap_db_retry(max_retries\u003d1, retry_on_deadlock\u003dTrue)"},{"line_number":109,"context_line":"    def _create_uuid(context, bdm_id):"},{"line_number":110,"context_line":"        # NOTE(mdbooth): This method is only required until uuid is made"},{"line_number":111,"context_line":"        # non-nullable in a future release."},{"line_number":112,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"df87a7cf_bdb24de5","line":109,"range":{"start_line":107,"start_character":0,"end_line":109,"end_character":38},"in_reply_to":"df87a7cf_b246fc7b","updated":"2017-12-06 17:23:03.000000000","message":"As discussed on IRC, I think this would be broken unless we can guarantee that we\u0027ve got a top-level transaction scope. There are 2 reasons for this:\n\n1. If the transaction isn\u0027t at this scope, the decorator won\u0027t create us a new transaction on entry (it uses the existing one), which means we won\u0027t get a new transaction if we retry.\n\n2. If the transaction isn\u0027t at this scope and we anticipate a recoverable failure, by re-using the enclosing scope we fail the whole transaction anyway if our bit fails.\n\nIf I\u0027m right, though, I think this means most/all of our existing uses of wrap_db_retry() are broken unless they\u0027re guaranteed to be top-level scoped both now and in the future. If that\u0027s correct, though, I guess we can clean this and other uses up together at some later date.\n\nI\u0027m happy to change this to be in line with other callers. A retry here is astonishingly unlikely, and failure is still safe (a bdm only ever gets 1 uuid).","commit_id":"a3d88a54b2576bf2e82b4eacf95716771e9ac381"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"f11e12c1b5994135f9ce9b3805668160874e3c2c","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        if target_version \u003c (1, 17) and \u0027tag\u0027 in primitive:"},{"line_number":105,"context_line":"            del primitive[\u0027tag\u0027]"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    @staticmethod"},{"line_number":108,"context_line":"    @oslo_db_api.wrap_db_retry(max_retries\u003d1, retry_on_deadlock\u003dTrue)"},{"line_number":109,"context_line":"    def _create_uuid(context, bdm_id):"},{"line_number":110,"context_line":"        # NOTE(mdbooth): This method is only required until uuid is made"},{"line_number":111,"context_line":"        # non-nullable in a future release."},{"line_number":112,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"df87a7cf_ecdd8414","line":109,"range":{"start_line":107,"start_character":0,"end_line":109,"end_character":38},"in_reply_to":"df87a7cf_bdb24de5","updated":"2017-12-06 18:55:52.000000000","message":"I made this change locally.\n\nThen I noticed that in addition to needing an independent transaction in the event of a retry, we would also need an independent transaction in the event that we were called from an enclosing read transaction scope. This wouldn\u0027t be an unreasonable thing to do: a caller of _from_db_object isn\u0027t typically considering that it might also write to the db. We don\u0027t currently have such a transaction scope, but in case anybody unwittingly added one in the future I wrote a comment explaining what to do about it, and to discourage promoting the entire scope to a write transaction.\n\nHaving written this, though, I realised that the comment describing the potential problem and its fix was longer than the fix, which I\u0027ve already written :)\n\nI know it looks a little unusual, but backfilling a uuid during a read is kinda unusual. We\u0027ve done it before more simplistically in other objects, but I think this code covers all the bases, and it\u0027s written already.","commit_id":"a3d88a54b2576bf2e82b4eacf95716771e9ac381"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ebc0ba2be9cb5b0340da5ff3b3ef73c366912f95","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        values \u003d {\u0027uuid\u0027: uuid}"},{"line_number":120,"context_line":"        compare \u003d db_models.BlockDeviceMapping(id\u003dbdm_id, uuid\u003dNone)"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        # We need an independent transaction context because we\u0027re in a"},{"line_number":123,"context_line":"        # retry loop."},{"line_number":124,"context_line":"        tctxt \u003d db_api.get_context_manager(context).writer.independent"},{"line_number":125,"context_line":"        with tctxt.using(context):"},{"line_number":126,"context_line":"            query \u003d context.session.query(db_models.BlockDeviceMapping).\\"}],"source_content_type":"text/x-python","patch_set":25,"id":"df87a7cf_66c19a2e","line":123,"range":{"start_line":122,"start_character":8,"end_line":123,"end_character":21},"updated":"2017-12-06 14:51:40.000000000","message":"Not sure I agree with this... I\u0027ve never seen the need to do this before. The oslo_db retry decorator catches a deadlock exception, which would have ROLLBACK\u0027d the outer transaction and restarted a new transaction context.","commit_id":"a3d88a54b2576bf2e82b4eacf95716771e9ac381"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"eb5aeb9d07bfb008f72b6333155d8e5bb93794c2","unresolved":false,"context_lines":[{"line_number":122,"context_line":"        # NOTE(mdbooth): We explicitly use an independent transaction context"},{"line_number":123,"context_line":"        # here so as not to fail if:"},{"line_number":124,"context_line":"        # 1. We retry."},{"line_number":125,"context_line":"        # 2. We\u0027re in a read transaction.This is an edge case of what\u0027s"},{"line_number":126,"context_line":"        #    normally a read operation. Forcing everything (transitively)"},{"line_number":127,"context_line":"        #    which reads a BDM to be in a write transaction for a narrow"},{"line_number":128,"context_line":"        #    temporary edge case is undesirable."}],"source_content_type":"text/x-python","patch_set":33,"id":"bf8cb3f7_94ec18f4","line":125,"updated":"2017-12-27 10:48:24.000000000","message":"nit: missing space after \u0027.\u0027","commit_id":"8df136dab5861caa6289cc5a254e8b401736e079"}],"nova/tests/unit/objects/test_block_device.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ca41aa41fad1d8680a505781caabb2949b33974a","unresolved":false,"context_lines":[{"line_number":395,"context_line":"                          bdm.save)"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"    @mock.patch.object(db, \u0027block_device_mapping_create\u0027)"},{"line_number":398,"context_line":"    def test_save_saves_uuid(self, mock_create):"},{"line_number":399,"context_line":"        mock_create.return_value \u003d self.fake_bdm(1)"},{"line_number":400,"context_line":"        with mock.patch(\u0027uuid.uuid4\u0027) as mock_uuid:"},{"line_number":401,"context_line":"            mock_uuid.return_value \u003d \u0027foo\u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"da85f559_36174746","line":398,"updated":"2015-11-12 18:38:15.000000000","message":"Did you mean to call this:\n\ntest_create_saves_uuid?","commit_id":"d268a663444666ecdc30a117a62f9024dee2c2f7"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"973b38cde5bc6e7551d07f408a1469a24d7137ee","unresolved":false,"context_lines":[{"line_number":373,"context_line":""},{"line_number":374,"context_line":"    def test_load_with_null_uuid(self):"},{"line_number":375,"context_line":"        bdm \u003d self.fake_bdm()"},{"line_number":376,"context_line":"        bdm[\u0027uuid\u0027] \u003d None"},{"line_number":377,"context_line":"        bdm_obj \u003d objects.BlockDeviceMapping()"},{"line_number":378,"context_line":"        objects.BlockDeviceMapping._from_db_object(self.context,"},{"line_number":379,"context_line":"                                                   bdm_obj,"}],"source_content_type":"text/x-python","patch_set":19,"id":"da36d5c6_bd02e909","line":376,"range":{"start_line":376,"start_character":0,"end_line":376,"end_character":26},"updated":"2017-02-27 20:44:17.000000000","message":"Do we also want a test where we del(bdm[\u0027uuid\u0027])?","commit_id":"b3772fa13332a184f67d53012be99b9277a8909c"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8104cb3e1c2b7633c1892187ee808c6f27ddc03d","unresolved":false,"context_lines":[{"line_number":373,"context_line":""},{"line_number":374,"context_line":"    def test_load_with_null_uuid(self):"},{"line_number":375,"context_line":"        bdm \u003d self.fake_bdm()"},{"line_number":376,"context_line":"        bdm[\u0027uuid\u0027] \u003d None"},{"line_number":377,"context_line":"        bdm_obj \u003d objects.BlockDeviceMapping()"},{"line_number":378,"context_line":"        objects.BlockDeviceMapping._from_db_object(self.context,"},{"line_number":379,"context_line":"                                                   bdm_obj,"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba2be162_6fa2d74c","line":376,"range":{"start_line":376,"start_character":0,"end_line":376,"end_character":26},"in_reply_to":"ba2be162_1fac458b","updated":"2017-03-03 15:29:16.000000000","message":"Okay.  Wasn\u0027t sure if it was possible in a version-mismatch scenario.","commit_id":"b3772fa13332a184f67d53012be99b9277a8909c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"136e064fd7d9554c6069baefec2f9ed6c96c377a","unresolved":false,"context_lines":[{"line_number":373,"context_line":""},{"line_number":374,"context_line":"    def test_load_with_null_uuid(self):"},{"line_number":375,"context_line":"        bdm \u003d self.fake_bdm()"},{"line_number":376,"context_line":"        bdm[\u0027uuid\u0027] \u003d None"},{"line_number":377,"context_line":"        bdm_obj \u003d objects.BlockDeviceMapping()"},{"line_number":378,"context_line":"        objects.BlockDeviceMapping._from_db_object(self.context,"},{"line_number":379,"context_line":"                                                   bdm_obj,"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba2be162_1fac458b","line":376,"range":{"start_line":376,"start_character":0,"end_line":376,"end_character":26},"in_reply_to":"da36d5c6_bd02e909","updated":"2017-03-03 01:29:34.000000000","message":"That would mean uuid isn\u0027t set on the bdm record in the database, which I don\u0027t think can happen since it\u0027s a column in the table based on the patch below this one in the series.","commit_id":"b3772fa13332a184f67d53012be99b9277a8909c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"17525219ec90c18c7ffd03b52a5dfe977bf20546","unresolved":false,"context_lines":[{"line_number":363,"context_line":"                                         destination_type\u003d\u0027local\u0027)"},{"line_number":364,"context_line":"        self.assertFalse(bdm.is_volume)"},{"line_number":365,"context_line":""},{"line_number":366,"context_line":"    def test_obj_make_compatible_pre_1_17(self):"},{"line_number":367,"context_line":"        values \u003d {\u0027source_type\u0027: \u0027volume\u0027, \u0027volume_id\u0027: \u0027fake-vol-id\u0027,"},{"line_number":368,"context_line":"                  \u0027destination_type\u0027: \u0027volume\u0027,"},{"line_number":369,"context_line":"                  \u0027instance_uuid\u0027: uuids.instance}"}],"source_content_type":"text/x-python","patch_set":20,"id":"ba2be162_ff7bf125","line":366,"updated":"2017-03-03 01:28:40.000000000","message":"Would be nice to have a test like this for 1.18.","commit_id":"b4eca9d5649de95613ceb3a99b0daf3ac507bf52"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"c6e17363d5a2b82070e69ec9f49a7e839edc9d0d","unresolved":false,"context_lines":[{"line_number":456,"context_line":"        self.assertEqual(1, mock_create_uuid.call_count)"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"    @mock.patch.object(objects.BlockDeviceMapping, \u0027_create_uuid\u0027)"},{"line_number":459,"context_line":"    def test_populate_race(self, mock_create_uuid):"},{"line_number":460,"context_line":"        uuid \u003d [None]"},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"        def race(*args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":31,"id":"bf8cb3f7_039a4913","line":459,"updated":"2017-12-21 21:38:00.000000000","message":"Does this really test the race though? It\u0027s all happening sequentially in a single thread, right? I made a pseudo-sequence diagram of what I understand is happening [1], and while the test does pass (obviously), I\u0027m not sure where the value of all this complexity is.\n\n[1] http://paste.openstack.org/show/629605/","commit_id":"7f7f6fbed9b2276c085ce7c21aa5eacc1ced27b5"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"d8b1a40e77cac1f9d9fc458938956bc1e8e8a996","unresolved":false,"context_lines":[{"line_number":456,"context_line":"        self.assertEqual(1, mock_create_uuid.call_count)"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"    @mock.patch.object(objects.BlockDeviceMapping, \u0027_create_uuid\u0027)"},{"line_number":459,"context_line":"    def test_populate_race(self, mock_create_uuid):"},{"line_number":460,"context_line":"        uuid \u003d [None]"},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"        def race(*args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":31,"id":"bf8cb3f7_d72410c9","line":459,"in_reply_to":"bf8cb3f7_039a4913","updated":"2017-12-22 13:44:55.000000000","message":"Yes, we\u0027re testing a version of the race. If you found this hard to read, you probably don\u0027t want comprehensive tests for all versions of the race ;)\n\nThis one is the most likely version. We\u0027re testing what happens when 2 threads read the same BDM object with no uuid, and they both end up attempting to populate it in the belief that it is not currently set.\n\nRather than try to contrive that with 2 separate threads (I\u0027ve had to do that for other concurrency unit tests before, and it\u0027s *really* hard to read), I\u0027ve done it in a single thread. I\u0027ve hijacked the entry of the first call to _create_uuid to do another, separate one, before continuing with the first one.\n\nSo, the first \u0027thread\u0027 gets part way through its execution into a \u0027critical section\u0027 when it\u0027s interrupted and another execution context does the same thing. That completed, it continues running. In fact, as python doesn\u0027t have interpreter concurrency, this is exactly what\u0027s happening in the threaded/eventlet case anyway.","commit_id":"7f7f6fbed9b2276c085ce7c21aa5eacc1ced27b5"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"c6e17363d5a2b82070e69ec9f49a7e839edc9d0d","unresolved":false,"context_lines":[{"line_number":457,"context_line":""},{"line_number":458,"context_line":"    @mock.patch.object(objects.BlockDeviceMapping, \u0027_create_uuid\u0027)"},{"line_number":459,"context_line":"    def test_populate_race(self, mock_create_uuid):"},{"line_number":460,"context_line":"        uuid \u003d [None]"},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"        def race(*args, **kwargs):"},{"line_number":463,"context_line":"            # First fetch the bdms again, including uuid"}],"source_content_type":"text/x-python","patch_set":31,"id":"bf8cb3f7_d16c2a0a","line":460,"updated":"2017-12-21 21:38:00.000000000","message":"What\u0027s the point of making this a list?","commit_id":"7f7f6fbed9b2276c085ce7c21aa5eacc1ced27b5"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"d8b1a40e77cac1f9d9fc458938956bc1e8e8a996","unresolved":false,"context_lines":[{"line_number":457,"context_line":""},{"line_number":458,"context_line":"    @mock.patch.object(objects.BlockDeviceMapping, \u0027_create_uuid\u0027)"},{"line_number":459,"context_line":"    def test_populate_race(self, mock_create_uuid):"},{"line_number":460,"context_line":"        uuid \u003d [None]"},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"        def race(*args, **kwargs):"},{"line_number":463,"context_line":"            # First fetch the bdms again, including uuid"}],"source_content_type":"text/x-python","patch_set":31,"id":"bf8cb3f7_77f50457","line":460,"in_reply_to":"bf8cb3f7_d16c2a0a","updated":"2017-12-22 13:44:55.000000000","message":"It\u0027s a python quirk. Notice that it\u0027s updated from inside the closure. Closed variables are read-only in python. If we did \u0027uuid \u003d bdms[0].uuid\u0027 below, then that would create a new shadow variable called uuid in the closure\u0027s local stack, and this one would remain unmodified. However, by modifying the contents of a list, but not changing the reference to the list itself, we can have the closure set data visible outside its own scope.\n\nI believe this is a pretty common idiom for doing this.","commit_id":"7f7f6fbed9b2276c085ce7c21aa5eacc1ced27b5"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"c6e17363d5a2b82070e69ec9f49a7e839edc9d0d","unresolved":false,"context_lines":[{"line_number":468,"context_line":"            # Then continue calling _create_uuid"},{"line_number":469,"context_line":"            return self.orig_create_uuid(*args, **kwargs)"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":"        executed \u003d [False]"},{"line_number":472,"context_line":""},{"line_number":473,"context_line":"        def flip(*args, **kwargs):"},{"line_number":474,"context_line":"            # Execute our explicit race first time only"}],"source_content_type":"text/x-python","patch_set":31,"id":"bf8cb3f7_d19e6a06","line":471,"updated":"2017-12-21 21:38:00.000000000","message":"Ditto.","commit_id":"7f7f6fbed9b2276c085ce7c21aa5eacc1ced27b5"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"d8b1a40e77cac1f9d9fc458938956bc1e8e8a996","unresolved":false,"context_lines":[{"line_number":468,"context_line":"            # Then continue calling _create_uuid"},{"line_number":469,"context_line":"            return self.orig_create_uuid(*args, **kwargs)"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":"        executed \u003d [False]"},{"line_number":472,"context_line":""},{"line_number":473,"context_line":"        def flip(*args, **kwargs):"},{"line_number":474,"context_line":"            # Execute our explicit race first time only"}],"source_content_type":"text/x-python","patch_set":31,"id":"bf8cb3f7_d703b041","line":471,"in_reply_to":"bf8cb3f7_d19e6a06","updated":"2017-12-22 13:44:55.000000000","message":"Ditto ;)","commit_id":"7f7f6fbed9b2276c085ce7c21aa5eacc1ced27b5"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"9c1cf8af06789bf66c98675bd3990422b08d8f89","unresolved":false,"context_lines":[{"line_number":461,"context_line":"        # calling _create_uuid multiple times. Assert that calling _create_uuid"},{"line_number":462,"context_line":"        # multiple times yields the same uuid."},{"line_number":463,"context_line":""},{"line_number":464,"context_line":"        # NOTE(mdbooth): _create_uuid handles all forms of race, including any"},{"line_number":465,"context_line":"        # amount of overlapping. I have not attempted to write unit tests for"},{"line_number":466,"context_line":"        # all potential execution orders. This test is sufficient to"},{"line_number":467,"context_line":"        # demonstrate that the compare-and-swap works correctly, and we trust"},{"line_number":468,"context_line":"        # the correctness of the database for the rest."},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"        db_bdm \u003d self._create_legacy_bdm(self.context)"},{"line_number":471,"context_line":"        uuid1 \u003d objects.BlockDeviceMapping._create_uuid(self.context,"}],"source_content_type":"text/x-python","patch_set":33,"id":"bf8cb3f7_4c9387a4","line":468,"range":{"start_line":464,"start_character":9,"end_line":468,"end_character":55},"updated":"2017-12-24 11:46:29.000000000","message":"ack.","commit_id":"8df136dab5861caa6289cc5a254e8b401736e079"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"9c1cf8af06789bf66c98675bd3990422b08d8f89","unresolved":false,"context_lines":[{"line_number":479,"context_line":"        # This is also testing that the compare-and-swap doesn\u0027t overwrite an"},{"line_number":480,"context_line":"        # existing uuid if we hit that race."},{"line_number":481,"context_line":"        uuid2 \u003d objects.BlockDeviceMapping._create_uuid(self.context,"},{"line_number":482,"context_line":"                                                        bdm[\u0027id\u0027])"},{"line_number":483,"context_line":""},{"line_number":484,"context_line":"        self.assertEqual(uuid1, uuid2)"},{"line_number":485,"context_line":""}],"source_content_type":"text/x-python","patch_set":33,"id":"bf8cb3f7_0c8d7f7a","line":482,"range":{"start_line":482,"start_character":59,"end_line":482,"end_character":65},"updated":"2017-12-24 11:46:29.000000000","message":"prefer to use bdm.id here since bdm is an object and db_bdm is a dict...","commit_id":"8df136dab5861caa6289cc5a254e8b401736e079"}]}