)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3b8706ec9cb9a3e06fcdad22366272b8a41c3189","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add {get_,set_}aggregates to objects.ResourceProvider"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The Placement API will provider support for associating a list of"},{"line_number":10,"context_line":"aggregate uuids with a resource provider. This is a way of stating"},{"line_number":11,"context_line":"that, for example, a shared storage provider can be used by"},{"line_number":12,"context_line":"compute hosts in aggregates A, B and C."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"fa7ab95a_740b097e","line":9,"range":{"start_line":9,"start_character":23,"end_line":9,"end_character":31},"updated":"2016-08-31 14:27:50.000000000","message":"Provide","commit_id":"e3ae0cdfdbbbfb0f94779d9b4da005d685952f9c"}],"nova/objects/resource_provider.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"7ddb837c0d207874ff6bfbf8d2cbfb4d0e75ec2a","unresolved":false,"context_lines":[{"line_number":260,"context_line":"        \u0027id\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":261,"context_line":"        \u0027uuid\u0027: fields.UUIDField(nullable\u003dFalse),"},{"line_number":262,"context_line":"        \u0027name\u0027: fields.StringField(nullable\u003dFalse),"},{"line_number":263,"context_line":"        \u0027generation\u0027: fields.IntegerField(nullable\u003dFalse),"},{"line_number":264,"context_line":"    }"},{"line_number":265,"context_line":""},{"line_number":266,"context_line":"    @base.remotable"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ac06dbe_cb32b2f1","line":263,"updated":"2016-08-23 14:36:02.000000000","message":"I was half expecting a list of strings field for the associated aggregates?","commit_id":"de121afb171d6bdb5535ba8ca8be923413cdffe4"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"cec8d1ec9b5051a8aa4dc4e15cd295995a748641","unresolved":false,"context_lines":[{"line_number":260,"context_line":"        \u0027id\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":261,"context_line":"        \u0027uuid\u0027: fields.UUIDField(nullable\u003dFalse),"},{"line_number":262,"context_line":"        \u0027name\u0027: fields.StringField(nullable\u003dFalse),"},{"line_number":263,"context_line":"        \u0027generation\u0027: fields.IntegerField(nullable\u003dFalse),"},{"line_number":264,"context_line":"    }"},{"line_number":265,"context_line":""},{"line_number":266,"context_line":"    @base.remotable"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ac06dbe_ac309508","line":263,"in_reply_to":"1ac06dbe_3ad620f1","updated":"2016-08-25 13:00:08.000000000","message":"I mean we have done lazy load fields in the past for that thing, but I guess I am not against a method, its just doesn\u0027t feel very object-ey I guess. We could always return to that later I guess.","commit_id":"de121afb171d6bdb5535ba8ca8be923413cdffe4"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"11963ec72658a892e410c59174b19318399d4471","unresolved":false,"context_lines":[{"line_number":260,"context_line":"        \u0027id\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":261,"context_line":"        \u0027uuid\u0027: fields.UUIDField(nullable\u003dFalse),"},{"line_number":262,"context_line":"        \u0027name\u0027: fields.StringField(nullable\u003dFalse),"},{"line_number":263,"context_line":"        \u0027generation\u0027: fields.IntegerField(nullable\u003dFalse),"},{"line_number":264,"context_line":"    }"},{"line_number":265,"context_line":""},{"line_number":266,"context_line":"    @base.remotable"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ac06dbe_3ad620f1","line":263,"in_reply_to":"1ac06dbe_cb32b2f1","updated":"2016-08-23 16:46:36.000000000","message":"I discussed that with Jay and we decided that straight methods made more sense based on the way in which the resource providers were being managed in the api: when you create or update a resource provider you set its name and maybe uuid, or you update its name.\n\nManaging aggregates are a separate API call, and so are managing aggregates so it makes sense to make them separate methods (as already done with set_inventory). If we had aggregates as a field then we would need to decide what would happen with them when we did save() or create() and the interface between the HTTP API and the objects get weird.\n\nThere was a version with a field for aggregates back in the beginning of the year, but it was unwieldy and would be more so with the two different databases issue below.","commit_id":"de121afb171d6bdb5535ba8ca8be923413cdffe4"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"7ddb837c0d207874ff6bfbf8d2cbfb4d0e75ec2a","unresolved":false,"context_lines":[{"line_number":408,"context_line":"        # needed. When using the API database, where the \"real\""},{"line_number":409,"context_line":"        # aggregates table is being used, presumably we do not want"},{"line_number":410,"context_line":"        # to be creating new aggregates, instead checking for their"},{"line_number":411,"context_line":"        # existence. In that context, this method is insufficient."},{"line_number":412,"context_line":"        provided_aggregates \u003d set(provided_aggregates)"},{"line_number":413,"context_line":"        existing_aggregates \u003d set(cls._get_aggregates(context, _id))"},{"line_number":414,"context_line":"        to_add \u003d provided_aggregates - existing_aggregates"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ac06dbe_4607d917","line":411,"updated":"2016-08-23 14:36:02.000000000","message":"This feels a lot like quota an tenant_ids, where we don\u0027t validate the uuid we get told about.\n\nI am not sure we have a good solution in either case. The usability of auto creating the uuids is a bit unfortunate from a typo situation.","commit_id":"de121afb171d6bdb5535ba8ca8be923413cdffe4"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"cec8d1ec9b5051a8aa4dc4e15cd295995a748641","unresolved":false,"context_lines":[{"line_number":408,"context_line":"        # needed. When using the API database, where the \"real\""},{"line_number":409,"context_line":"        # aggregates table is being used, presumably we do not want"},{"line_number":410,"context_line":"        # to be creating new aggregates, instead checking for their"},{"line_number":411,"context_line":"        # existence. In that context, this method is insufficient."},{"line_number":412,"context_line":"        provided_aggregates \u003d set(provided_aggregates)"},{"line_number":413,"context_line":"        existing_aggregates \u003d set(cls._get_aggregates(context, _id))"},{"line_number":414,"context_line":"        to_add \u003d provided_aggregates - existing_aggregates"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ac06dbe_5767eab6","line":411,"in_reply_to":"1ac06dbe_4607d917","updated":"2016-08-25 13:00:08.000000000","message":"So alaski made a good point, I forgot long term the aggregates will move to this DB, so really we want this to fail if the aggregate is not known.\n\nThis API kinda sucks if we have to keep this autocreate behaviour.\n\nIs there a short term thing we can do? like look into nova DB to validate, and fail if its missing? That feels less bad here. Just to get us across the bump?\n\nIdeally I think we move the aggregates and API into the placement API first, but this doesn\u0027t seem possible in the next week, I guess?","commit_id":"de121afb171d6bdb5535ba8ca8be923413cdffe4"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"80ac502020e48c948028b58bb498d1afdf5aac9b","unresolved":false,"context_lines":[{"line_number":396,"context_line":"    @staticmethod"},{"line_number":397,"context_line":"    @db_api.placement_context_manager.reader"},{"line_number":398,"context_line":"    def _get_aggregates(context, _id):"},{"line_number":399,"context_line":"        query \u003d context.session.query(models.PlacementAggregate.uuid).\\"},{"line_number":400,"context_line":"            join(\u0027resource_providers\u0027).\\"},{"line_number":401,"context_line":"            filter(models.ResourceProvider.id \u003d\u003d _id)"},{"line_number":402,"context_line":"        return [row.uuid for row in query.all()]"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ac06dbe_126e162f","line":399,"range":{"start_line":399,"start_character":45,"end_line":399,"end_character":63},"updated":"2016-08-25 15:18:42.000000000","message":"So I think we should change the PlacementAggregate into a table that is totally separate from Nova\u0027s aggregates table.\n\nAt that point, it just contains id and uuid, and we can auto create whenever we want to, if we are in the api db or the placement db.\n\nThat means changing the mapping table to point to the correct place, but thats OK, as its empty right now anyways.","commit_id":"05d135b8a15b626e0c264fea0cf8baf66583d5ac"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ccb7448316a0fa4a730670cd4b1f4408a4b9b475","unresolved":false,"context_lines":[{"line_number":396,"context_line":"    @staticmethod"},{"line_number":397,"context_line":"    @db_api.placement_context_manager.reader"},{"line_number":398,"context_line":"    def _get_aggregates(context, _id):"},{"line_number":399,"context_line":"        query \u003d context.session.query(models.PlacementAggregate.uuid).\\"},{"line_number":400,"context_line":"            join(\u0027resource_providers\u0027).\\"},{"line_number":401,"context_line":"            filter(models.ResourceProvider.id \u003d\u003d _id)"},{"line_number":402,"context_line":"        return [row.uuid for row in query.all()]"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ac06dbe_75ec9429","line":399,"range":{"start_line":399,"start_character":45,"end_line":399,"end_character":63},"in_reply_to":"1ac06dbe_126e162f","updated":"2016-08-25 15:25:08.000000000","message":"Lets refine that:\n\nkill the current PlacementAggregate model that uses the aggregates table:\nhttps://review.openstack.org/#/c/349021/13/nova/db/sqlalchemy/api_models.py@357\n\ncreate a new table, that is just a uuid and id, called PlacementAggregate, that is in a separate table. I understand this is useful to help index performance/size.\n\nThis now means we need to update this to refer to the new PlacementAggregate table, i.e. drop aggreate_id, and move to placement_aggregate_id:\nhttps://review.openstack.org/#/c/349021/13/nova/db/sqlalchemy/api_models.py@354\n\nThis means: when the placement API auto adds an aggregate, it will never appear in the Nova API.\n\nIn both the api db and placement db usage case, the data is in a separate table, and must always we auto added as needed.\n\nA nit: we could use provider_group instead of provider_aggregate to stop confusion here, but whatever.","commit_id":"05d135b8a15b626e0c264fea0cf8baf66583d5ac"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"e982ac8bfa1ab770be70e2f32a7fcff582024762","unresolved":false,"context_lines":[{"line_number":396,"context_line":"    @staticmethod"},{"line_number":397,"context_line":"    @db_api.placement_context_manager.reader"},{"line_number":398,"context_line":"    def _get_aggregates(context, _id):"},{"line_number":399,"context_line":"        query \u003d context.session.query(models.PlacementAggregate.uuid).\\"},{"line_number":400,"context_line":"            join(\u0027resource_providers\u0027).\\"},{"line_number":401,"context_line":"            filter(models.ResourceProvider.id \u003d\u003d _id)"},{"line_number":402,"context_line":"        return [row.uuid for row in query.all()]"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ac06dbe_8263cb19","line":399,"range":{"start_line":399,"start_character":45,"end_line":399,"end_character":63},"in_reply_to":"1ac06dbe_75ec9429","updated":"2016-08-25 19:17:21.000000000","message":"FWIW, +1 separate table","commit_id":"05d135b8a15b626e0c264fea0cf8baf66583d5ac"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"d93caa9cee2bc72f9d795805ebf9d7c8a19dcc28","unresolved":false,"context_lines":[{"line_number":396,"context_line":"    @staticmethod"},{"line_number":397,"context_line":"    @db_api.placement_context_manager.reader"},{"line_number":398,"context_line":"    def _get_aggregates(context, _id):"},{"line_number":399,"context_line":"        query \u003d context.session.query(models.PlacementAggregate.uuid).\\"},{"line_number":400,"context_line":"            join(\u0027resource_providers\u0027).\\"},{"line_number":401,"context_line":"            filter(models.ResourceProvider.id \u003d\u003d _id)"},{"line_number":402,"context_line":"        return [row.uuid for row in query.all()]"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ac06dbe_98c7c511","line":399,"range":{"start_line":399,"start_character":45,"end_line":399,"end_character":63},"in_reply_to":"1ac06dbe_75ec9429","updated":"2016-08-25 15:43:51.000000000","message":"So I was thinking that as the ResourceProvider stays in the API DB and the Aggregate table as well, we just need to add a new mapping table for associating both.\n\nFor performance reasons, it\u0027s totally acceptable to add indexes on the aggregate uuids for sure.","commit_id":"05d135b8a15b626e0c264fea0cf8baf66583d5ac"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"21440699ca998f3f9cb3e0366291b35fd37934f6","unresolved":false,"context_lines":[{"line_number":396,"context_line":"    @staticmethod"},{"line_number":397,"context_line":"    @db_api.placement_context_manager.reader"},{"line_number":398,"context_line":"    def _get_aggregates(context, _id):"},{"line_number":399,"context_line":"        query \u003d context.session.query(models.PlacementAggregate.uuid).\\"},{"line_number":400,"context_line":"            join(\u0027resource_providers\u0027).\\"},{"line_number":401,"context_line":"            filter(models.ResourceProvider.id \u003d\u003d _id)"},{"line_number":402,"context_line":"        return [row.uuid for row in query.all()]"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa7ab95a_6a5ed946","line":399,"range":{"start_line":399,"start_character":45,"end_line":399,"end_character":63},"in_reply_to":"1ac06dbe_8263cb19","updated":"2016-08-26 07:44:19.000000000","message":"So post the revert: https://review.openstack.org/#/c/360638/ we clearly need a bit more work now, but I think we could use the above design in the new version of the above. It shouldn\u0027t be too bad to change that up, hopefully.","commit_id":"05d135b8a15b626e0c264fea0cf8baf66583d5ac"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"eefca8ed3f65b21354c08a5fdffb623b277e4db7","unresolved":false,"context_lines":[{"line_number":404,"context_line":"    @classmethod"},{"line_number":405,"context_line":"    @db_api.placement_context_manager.writer"},{"line_number":406,"context_line":"    def _set_aggregates(cls, context, _id, provided_aggregates):"},{"line_number":407,"context_line":"        # TODO(cdent): This method currently works okay with the"},{"line_number":408,"context_line":"        # placement db where we can arbitrarily create new aggregates as"},{"line_number":409,"context_line":"        # needed. When using the API database, where the \"real\""},{"line_number":410,"context_line":"        # aggregates table is being used, presumably we do not want"},{"line_number":411,"context_line":"        # to be creating new aggregates, instead checking for their"},{"line_number":412,"context_line":"        # existence. In that context, this method is insufficient."},{"line_number":413,"context_line":"        provided_aggregates \u003d set(provided_aggregates)"},{"line_number":414,"context_line":"        existing_aggregates \u003d set(cls._get_aggregates(context, _id))"},{"line_number":415,"context_line":"        to_add \u003d provided_aggregates - existing_aggregates"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa7ab95a_8daed4e5","line":412,"range":{"start_line":407,"start_character":8,"end_line":412,"end_character":66},"updated":"2016-08-31 10:23:11.000000000","message":"I think this comment is not required any more.\n\nIt might be worth leaving a note about the validation of the uuids being left to the client that consumes the placement info we return.","commit_id":"bab12903526c23c5c7c1eed2447362f49ed10419"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"0621dd252a8c04f82471a5271abc136ce7e1a0e0","unresolved":false,"context_lines":[{"line_number":409,"context_line":"        # needed. When using the API database, where the \"real\""},{"line_number":410,"context_line":"        # aggregates table is being used, presumably we do not want"},{"line_number":411,"context_line":"        # to be creating new aggregates, instead checking for their"},{"line_number":412,"context_line":"        # existence. In that context, this method is insufficient."},{"line_number":413,"context_line":"        provided_aggregates \u003d set(provided_aggregates)"},{"line_number":414,"context_line":"        existing_aggregates \u003d set(cls._get_aggregates(context, _id))"},{"line_number":415,"context_line":"        to_add \u003d provided_aggregates - existing_aggregates"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa7ab95a_4d1121af","line":412,"in_reply_to":"fa7ab95a_8daed4e5","updated":"2016-08-31 12:56:02.000000000","message":"Done","commit_id":"bab12903526c23c5c7c1eed2447362f49ed10419"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"8831f59560c2366d82f2f45e72329d3ee04bab1b","unresolved":false,"context_lines":[{"line_number":425,"context_line":"                # this new aggregate."},{"line_number":426,"context_line":"                new_aggregate \u003d models.PlacementAggregate(uuid\u003dagg_uuid)"},{"line_number":427,"context_line":"                context.session.add(new_aggregate)"},{"line_number":428,"context_line":"            target_aggregates.append(agg_uuid)"},{"line_number":429,"context_line":"        # Flush here to ensure aggregate ids used below."},{"line_number":430,"context_line":"        context.session.flush()"},{"line_number":431,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa7ab95a_9ea999e2","line":428,"range":{"start_line":428,"start_character":12,"end_line":428,"end_character":46},"updated":"2016-08-31 11:36:21.000000000","message":"So this is the bit that is confusing me.\n\nYou could just do:\n  target_aggregates \u003d list(provided_aggregates)\n\non line 417, and remove this line, and you get the same effect.","commit_id":"bab12903526c23c5c7c1eed2447362f49ed10419"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"0621dd252a8c04f82471a5271abc136ce7e1a0e0","unresolved":false,"context_lines":[{"line_number":425,"context_line":"                # this new aggregate."},{"line_number":426,"context_line":"                new_aggregate \u003d models.PlacementAggregate(uuid\u003dagg_uuid)"},{"line_number":427,"context_line":"                context.session.add(new_aggregate)"},{"line_number":428,"context_line":"            target_aggregates.append(agg_uuid)"},{"line_number":429,"context_line":"        # Flush here to ensure aggregate ids used below."},{"line_number":430,"context_line":"        context.session.flush()"},{"line_number":431,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa7ab95a_2d469dbf","line":428,"in_reply_to":"fa7ab95a_9ea999e2","updated":"2016-08-31 12:56:02.000000000","message":"Done","commit_id":"bab12903526c23c5c7c1eed2447362f49ed10419"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"eefca8ed3f65b21354c08a5fdffb623b277e4db7","unresolved":false,"context_lines":[{"line_number":431,"context_line":""},{"line_number":432,"context_line":"        # Remove all aggregate assocations so we can refresh them"},{"line_number":433,"context_line":"        # below. This mean that any association to aggregate uuids"},{"line_number":434,"context_line":"        # not in to_add or to_keep will be removed, but the"},{"line_number":435,"context_line":"        # aggregates themselves stay around."},{"line_number":436,"context_line":"        context.session.query(models.ResourceProviderAggregate).filter_by("},{"line_number":437,"context_line":"            resource_provider_id\u003d_id).delete()"},{"line_number":438,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa7ab95a_4daf6c7d","line":435,"range":{"start_line":434,"start_character":52,"end_line":435,"end_character":44},"updated":"2016-08-31 10:23:11.000000000","message":"we probably want to delete the unused PlacementAggregates now (after all associations are deleted)","commit_id":"bab12903526c23c5c7c1eed2447362f49ed10419"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"0621dd252a8c04f82471a5271abc136ce7e1a0e0","unresolved":false,"context_lines":[{"line_number":432,"context_line":"        # Remove all aggregate assocations so we can refresh them"},{"line_number":433,"context_line":"        # below. This mean that any association to aggregate uuids"},{"line_number":434,"context_line":"        # not in to_add or to_keep will be removed, but the"},{"line_number":435,"context_line":"        # aggregates themselves stay around."},{"line_number":436,"context_line":"        context.session.query(models.ResourceProviderAggregate).filter_by("},{"line_number":437,"context_line":"            resource_provider_id\u003d_id).delete()"},{"line_number":438,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa7ab95a_ad10eda9","line":435,"in_reply_to":"fa7ab95a_4daf6c7d","updated":"2016-08-31 12:56:02.000000000","message":"Something else might be associated with them, so we\u0027d have to go looking for them. Since they are cheap and we don\u0027t how the concept of a PlacementAggregate is going to get used, seems reasonable to let them linger?","commit_id":"bab12903526c23c5c7c1eed2447362f49ed10419"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"eefca8ed3f65b21354c08a5fdffb623b277e4db7","unresolved":false,"context_lines":[{"line_number":439,"context_line":"        # Set resource_provider_id, aggregate_id pairs to"},{"line_number":440,"context_line":"        # ResourceProviderAggregate table."},{"line_number":441,"context_line":"        select_agg_id \u003d sa.select([_id, models.PlacementAggregate.id]).\\"},{"line_number":442,"context_line":"            where(models.PlacementAggregate.uuid.in_(target_aggregates))"},{"line_number":443,"context_line":"        insert_aggregates \u003d models.ResourceProviderAggregate.__table__.\\"},{"line_number":444,"context_line":"            insert().from_select([\u0027resource_provider_id\u0027, \u0027aggregate_id\u0027],"},{"line_number":445,"context_line":"                                 select_agg_id)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa7ab95a_b0368fe5","line":442,"range":{"start_line":442,"start_character":53,"end_line":442,"end_character":70},"updated":"2016-08-31 10:23:11.000000000","message":"Is this correct, doesn\u0027t this mean we don\u0027t ever \"drop\" aggregates?\n\nI assumed we would have list(provided_aggregates) here?","commit_id":"bab12903526c23c5c7c1eed2447362f49ed10419"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"0621dd252a8c04f82471a5271abc136ce7e1a0e0","unresolved":false,"context_lines":[{"line_number":439,"context_line":"        # Set resource_provider_id, aggregate_id pairs to"},{"line_number":440,"context_line":"        # ResourceProviderAggregate table."},{"line_number":441,"context_line":"        select_agg_id \u003d sa.select([_id, models.PlacementAggregate.id]).\\"},{"line_number":442,"context_line":"            where(models.PlacementAggregate.uuid.in_(target_aggregates))"},{"line_number":443,"context_line":"        insert_aggregates \u003d models.ResourceProviderAggregate.__table__.\\"},{"line_number":444,"context_line":"            insert().from_select([\u0027resource_provider_id\u0027, \u0027aggregate_id\u0027],"},{"line_number":445,"context_line":"                                 select_agg_id)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa7ab95a_cdaff129","line":442,"in_reply_to":"fa7ab95a_30f27f70","updated":"2016-08-31 12:56:02.000000000","message":"Done","commit_id":"bab12903526c23c5c7c1eed2447362f49ed10419"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ebefa8875b2ee158d155ddc1cda827302a91b4e0","unresolved":false,"context_lines":[{"line_number":439,"context_line":"        # Set resource_provider_id, aggregate_id pairs to"},{"line_number":440,"context_line":"        # ResourceProviderAggregate table."},{"line_number":441,"context_line":"        select_agg_id \u003d sa.select([_id, models.PlacementAggregate.id]).\\"},{"line_number":442,"context_line":"            where(models.PlacementAggregate.uuid.in_(target_aggregates))"},{"line_number":443,"context_line":"        insert_aggregates \u003d models.ResourceProviderAggregate.__table__.\\"},{"line_number":444,"context_line":"            insert().from_select([\u0027resource_provider_id\u0027, \u0027aggregate_id\u0027],"},{"line_number":445,"context_line":"                                 select_agg_id)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa7ab95a_30f27f70","line":442,"range":{"start_line":442,"start_character":53,"end_line":442,"end_character":70},"in_reply_to":"fa7ab95a_b0368fe5","updated":"2016-08-31 10:30:12.000000000","message":"Hmm, so we want [] to empty the list (we should test that), this works.\n\nI have a feeling this should really be to_keep + to_add, i.e. put patch what was there, and add in the new ones.","commit_id":"bab12903526c23c5c7c1eed2447362f49ed10419"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"0621dd252a8c04f82471a5271abc136ce7e1a0e0","unresolved":false,"context_lines":[{"line_number":439,"context_line":"        # Set resource_provider_id, aggregate_id pairs to"},{"line_number":440,"context_line":"        # ResourceProviderAggregate table."},{"line_number":441,"context_line":"        select_agg_id \u003d sa.select([_id, models.PlacementAggregate.id]).\\"},{"line_number":442,"context_line":"            where(models.PlacementAggregate.uuid.in_(target_aggregates))"},{"line_number":443,"context_line":"        insert_aggregates \u003d models.ResourceProviderAggregate.__table__.\\"},{"line_number":444,"context_line":"            insert().from_select([\u0027resource_provider_id\u0027, \u0027aggregate_id\u0027],"},{"line_number":445,"context_line":"                                 select_agg_id)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa7ab95a_ad71cde2","line":442,"in_reply_to":"fa7ab95a_b0368fe5","updated":"2016-08-31 12:56:02.000000000","message":"right, aggregates themselves persist, uuids are good for that.","commit_id":"bab12903526c23c5c7c1eed2447362f49ed10419"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"e62059a25f8708fd9946834af3247b05a74c5518","unresolved":false,"context_lines":[{"line_number":428,"context_line":"        # below. This mean that all ssociations are added, but the"},{"line_number":429,"context_line":"        # aggregates themselves stay around."},{"line_number":430,"context_line":"        context.session.query(models.ResourceProviderAggregate).filter_by("},{"line_number":431,"context_line":"            resource_provider_id\u003d_id).delete()"},{"line_number":432,"context_line":""},{"line_number":433,"context_line":"        # Set resource_provider_id, aggregate_id pairs to"},{"line_number":434,"context_line":"        # ResourceProviderAggregate table."}],"source_content_type":"text/x-python","patch_set":9,"id":"fa7ab95a_e5c3e936","line":431,"updated":"2016-08-31 13:34:58.000000000","message":"Nit: I still fancy deleting the PlacementAggregates that are not in use. They are cheap, but it feels bad to slowly bloat the db. Given the timing, I would settle for a TODO added in here to decide what to do about them?","commit_id":"8f1838d07d20a380137ec6dc1c92ce6584697e61"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3b8706ec9cb9a3e06fcdad22366272b8a41c3189","unresolved":false,"context_lines":[{"line_number":430,"context_line":"        context.session.flush()"},{"line_number":431,"context_line":""},{"line_number":432,"context_line":"        # Remove all aggregate assocations so we can refresh them"},{"line_number":433,"context_line":"        # below. This mean that all ssociations are added, but the"},{"line_number":434,"context_line":"        # aggregates themselves stay around."},{"line_number":435,"context_line":"        context.session.query(models.ResourceProviderAggregate).filter_by("},{"line_number":436,"context_line":"            resource_provider_id\u003d_id).delete()"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa7ab95a_34b981ed","line":433,"range":{"start_line":433,"start_character":36,"end_line":433,"end_character":47},"updated":"2016-08-31 14:27:50.000000000","message":"spelling","commit_id":"e3ae0cdfdbbbfb0f94779d9b4da005d685952f9c"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"1f4e1b26e9f57f2173b452641dbb7a13add0fe22","unresolved":false,"context_lines":[{"line_number":395,"context_line":""},{"line_number":396,"context_line":"    @staticmethod"},{"line_number":397,"context_line":"    @db_api.api_context_manager.reader"},{"line_number":398,"context_line":"    def _get_aggregates(context, _id):"},{"line_number":399,"context_line":"        query \u003d context.session.query(models.PlacementAggregate.uuid).\\"},{"line_number":400,"context_line":"            join(\u0027resource_providers\u0027).\\"},{"line_number":401,"context_line":"            filter(models.ResourceProvider.id \u003d\u003d _id)"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa7ab95a_049408e0","line":398,"range":{"start_line":398,"start_character":33,"end_line":398,"end_character":36},"updated":"2016-09-01 13:08:09.000000000","message":"nit, why underscored, from what I see it\u0027s not shadowing any other variable.","commit_id":"4575dc4779ff65a67c02cdbfdf26334207c25f4e"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"1f4e1b26e9f57f2173b452641dbb7a13add0fe22","unresolved":false,"context_lines":[{"line_number":403,"context_line":""},{"line_number":404,"context_line":"    @classmethod"},{"line_number":405,"context_line":"    @db_api.api_context_manager.writer"},{"line_number":406,"context_line":"    def _set_aggregates(cls, context, _id, provided_aggregates):"},{"line_number":407,"context_line":"        # When aggregate uuids are persisted no validation is done"},{"line_number":408,"context_line":"        # to insure that they refer to something that has meaning"},{"line_number":409,"context_line":"        # elsewhere. It is assumed that code which makes use of the"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa7ab95a_846d7806","line":406,"range":{"start_line":406,"start_character":38,"end_line":406,"end_character":41},"updated":"2016-09-01 13:08:09.000000000","message":"same nit","commit_id":"4575dc4779ff65a67c02cdbfdf26334207c25f4e"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"1f4e1b26e9f57f2173b452641dbb7a13add0fe22","unresolved":false,"context_lines":[{"line_number":415,"context_line":"        # Not doing now to move things along."},{"line_number":416,"context_line":"        provided_aggregates \u003d set(provided_aggregates)"},{"line_number":417,"context_line":"        existing_aggregates \u003d set(cls._get_aggregates(context, _id))"},{"line_number":418,"context_line":"        to_add \u003d provided_aggregates - existing_aggregates"},{"line_number":419,"context_line":"        target_aggregates \u003d list(provided_aggregates)"},{"line_number":420,"context_line":""},{"line_number":421,"context_line":"        # Create any aggregates that do not yet existing in"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa7ab95a_e707261b","line":418,"range":{"start_line":418,"start_character":8,"end_line":418,"end_character":58},"updated":"2016-09-01 13:08:09.000000000","message":"nit: I\u0027d add check after that,smth like:\nif not to_add:\n    return","commit_id":"4575dc4779ff65a67c02cdbfdf26334207c25f4e"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"94780afe11c265a231c25e8eb645537cc5daffc6","unresolved":false,"context_lines":[{"line_number":443,"context_line":"        # Create any aggregates that do not yet existing in"},{"line_number":444,"context_line":"        # PlacementAggregates."},{"line_number":445,"context_line":"        for agg_uuid in to_add:"},{"line_number":446,"context_line":"            found_agg \u003d context.session.query(models.PlacementAggregate.uuid).\\"},{"line_number":447,"context_line":"                filter_by(uuid\u003dagg_uuid).first()"},{"line_number":448,"context_line":"            if not found_agg:"},{"line_number":449,"context_line":"                new_aggregate \u003d models.PlacementAggregate(uuid\u003dagg_uuid)"},{"line_number":450,"context_line":"                context.session.add(new_aggregate)"},{"line_number":451,"context_line":"        # Flush here to ensure aggregate ids used below."}],"source_content_type":"text/x-python","patch_set":19,"id":"5a8bc5a2_c2571bc8","line":448,"range":{"start_line":446,"start_character":0,"end_line":448,"end_character":29},"updated":"2016-09-26 22:41:14.000000000","message":"Why is this needed? If there was a record, it would have been in existing_aggregates above, no? And thus not in to_add. What am I missing?","commit_id":"b5897a0f1372228e1e9636efc3d6745e7f35e1b7"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"fa6ab1b77ba5e6248c2e7327d8065ccf217c760a","unresolved":false,"context_lines":[{"line_number":445,"context_line":"        for agg_uuid in to_add:"},{"line_number":446,"context_line":"            found_agg \u003d context.session.query(models.PlacementAggregate.uuid).\\"},{"line_number":447,"context_line":"                filter_by(uuid\u003dagg_uuid).first()"},{"line_number":448,"context_line":"            if not found_agg:"},{"line_number":449,"context_line":"                new_aggregate \u003d models.PlacementAggregate(uuid\u003dagg_uuid)"},{"line_number":450,"context_line":"                context.session.add(new_aggregate)"},{"line_number":451,"context_line":"        # Flush here to ensure aggregate ids used below."}],"source_content_type":"text/x-python","patch_set":19,"id":"5a8bc5a2_90438f12","line":448,"in_reply_to":"5a8bc5a2_c2571bc8","updated":"2016-09-27 08:19:15.000000000","message":"I reckon you\u0027re not missing anything. I suspect what happened here is that in the gaps between when I started on this and when I made some next steps I merged two different mental models and didn\u0027t clean up effectively. Ought to be able to skip the block you highlighted. I\u0027ll provide a new version later today.","commit_id":"b5897a0f1372228e1e9636efc3d6745e7f35e1b7"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"0d1fe360488629c64f7078e2f53fd895d50ed5b5","unresolved":false,"context_lines":[{"line_number":473,"context_line":"                insert().from_select([\u0027resource_provider_id\u0027, \u0027aggregate_id\u0027],"},{"line_number":474,"context_line":"                                     select_agg_id)"},{"line_number":475,"context_line":"            conn \u003d context.session.connection()"},{"line_number":476,"context_line":"            conn.execute(insert_aggregates)"},{"line_number":477,"context_line":""},{"line_number":478,"context_line":""},{"line_number":479,"context_line":"@base.NovaObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":22,"id":"1a95cdbc_1f7d5ee7","line":476,"updated":"2016-10-05 18:24:49.000000000","message":"Maybe it\u0027s just me, but I can\u0027t imagine anyone thinking that ORM code is \"simpler\" than SQL.","commit_id":"2f6dc8686d9b0f796413b8be794f123f1c633993"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"812636a2244c11488c62449561768c11fe2871ef","unresolved":false,"context_lines":[{"line_number":473,"context_line":"                insert().from_select([\u0027resource_provider_id\u0027, \u0027aggregate_id\u0027],"},{"line_number":474,"context_line":"                                     select_agg_id)"},{"line_number":475,"context_line":"            conn \u003d context.session.connection()"},{"line_number":476,"context_line":"            conn.execute(insert_aggregates)"},{"line_number":477,"context_line":""},{"line_number":478,"context_line":""},{"line_number":479,"context_line":"@base.NovaObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":22,"id":"1a95cdbc_858cbc80","line":476,"in_reply_to":"1a95cdbc_1f7d5ee7","updated":"2016-10-05 21:18:28.000000000","message":"I find neither to be particularly lovely. If you\u0027ve got some ideas for improvement in this chunk please hack away with abandon.","commit_id":"2f6dc8686d9b0f796413b8be794f123f1c633993"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"e27b2613cc62ff33aa11ae879d0d3f6613bd6a81","unresolved":false,"context_lines":[{"line_number":473,"context_line":"                insert().from_select([\u0027resource_provider_id\u0027, \u0027aggregate_id\u0027],"},{"line_number":474,"context_line":"                                     select_agg_id)"},{"line_number":475,"context_line":"            conn \u003d context.session.connection()"},{"line_number":476,"context_line":"            conn.execute(insert_aggregates)"},{"line_number":477,"context_line":""},{"line_number":478,"context_line":""},{"line_number":479,"context_line":"@base.NovaObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":22,"id":"1a95cdbc_c830f558","line":476,"in_reply_to":"1a95cdbc_858cbc80","updated":"2016-10-10 17:45:22.000000000","message":"Oh, there\u0027s nothing wrong with your ORM code. It\u0027s just that I\u0027ve always found it to be so much more confusing than SQL.","commit_id":"2f6dc8686d9b0f796413b8be794f123f1c633993"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"068482d81ff55b2880bdf06b635b6a28b8843a9a","unresolved":false,"context_lines":[{"line_number":419,"context_line":"    def _get_aggregates(context, rp_id):"},{"line_number":420,"context_line":"        query \u003d context.session.query(models.PlacementAggregate.uuid).\\"},{"line_number":421,"context_line":"            join(\u0027resource_providers\u0027).\\"},{"line_number":422,"context_line":"            filter(models.ResourceProvider.id \u003d\u003d rp_id)"},{"line_number":423,"context_line":"        return [row.uuid for row in query.all()]"},{"line_number":424,"context_line":""},{"line_number":425,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":27,"id":"fa6399be_4f9c63c0","line":422,"updated":"2016-10-12 23:51:45.000000000","message":"The problem with the above is that the SQL query it will construct contains more joins/complexity than is needed. What the above will produce is the following:\n\n SELECT placement_aggregates.uuid\n FROM placement_aggregates\n JOIN resource_provider_aggregates\n ON placement_aggregates.id \u003d resource_provider_aggregates.aggregate_id\n JOIN resource_providers\n ON resource_provider_aggregates.resource_provider_id \u003d resource_providers.id\n WHERE resource_providers.id \u003d $RP_ID;\n\nThere is no reason to construct the above though, when you can do the simpler:\n\n SELECT placement_aggregates.uuid\n FROM placement_aggregates\n JOIN resource_provider_aggregates\n ON placement_aggregates.id \u003d resource_provider_aggregates.aggregate_id\n AND resource_provider_aggregates.resource_provider_id \u003d $RP_ID;\n\nYou could accomplish the latter query most efficiently with the following, which would mean you wouldn\u0027t need to use the ORM relationship() mess added in the previous patch to the api_models.py:\n\n _AGG_TBL \u003d api_models.PlacementAggregate.__table__\n _RP_AGG_TBL \u003d api_models.ResourceProviderAggregate.__table__\n ...\n\n def _get_aggregates(context, rp_id):\n     conn \u003d context.session.connection()\n     j \u003d sa.join(_AGG_TBL, _RP_AGG_TBL,\n                 sa.and_(_AGG_TBL.c.id \u003d\u003d _RP_AGG_TBL.c.aggregate_id,\n                         _RP_AGG_TBL.c.resource_provider_id \u003d\u003d rp_id)))\n     sel \u003d sa.select([_AGG_TBL.c.uuid]).select_from(j)\n     return [r[0] for r in conn.execute(sel).fetchall()]","commit_id":"0821971e9136ebb7bbe5dc53c11d83363716501a"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"c8edb5e359ae4b67c29c3b8e0f95d25f798f7880","unresolved":false,"context_lines":[{"line_number":419,"context_line":"    def _get_aggregates(context, rp_id):"},{"line_number":420,"context_line":"        query \u003d context.session.query(models.PlacementAggregate.uuid).\\"},{"line_number":421,"context_line":"            join(\u0027resource_providers\u0027).\\"},{"line_number":422,"context_line":"            filter(models.ResourceProvider.id \u003d\u003d rp_id)"},{"line_number":423,"context_line":"        return [row.uuid for row in query.all()]"},{"line_number":424,"context_line":""},{"line_number":425,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":27,"id":"fa6399be_50f15400","line":422,"in_reply_to":"fa6399be_4f9c63c0","updated":"2016-10-13 09:49:50.000000000","message":"I can\u0027t remember now, because it is has been months, but I think I was borrowing use of the relationship \"mess\" from something that RomanP had done at some point. But yeah, this makes sense, will tweak.\n\nI continue to laugh, however, about our efforts to optimize performance in such a tiny database doing so little work :P\n(sure it\u0027s good hygiene, but still)","commit_id":"0821971e9136ebb7bbe5dc53c11d83363716501a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"068482d81ff55b2880bdf06b635b6a28b8843a9a","unresolved":false,"context_lines":[{"line_number":426,"context_line":"    @db_api.api_context_manager.writer"},{"line_number":427,"context_line":"    def _set_aggregates(cls, context, rp_id, provided_aggregates):"},{"line_number":428,"context_line":"        # When aggregate uuids are persisted no validation is done"},{"line_number":429,"context_line":"        # to insure that they refer to something that has meaning"},{"line_number":430,"context_line":"        # elsewhere. It is assumed that code which makes use of the"},{"line_number":431,"context_line":"        # aggregates, later, will validate their fitness."},{"line_number":432,"context_line":"        # TODO(cdent): At the moment we do not delete"}],"source_content_type":"text/x-python","patch_set":27,"id":"fa6399be_0fc0ab7a","line":429,"updated":"2016-10-12 23:51:45.000000000","message":"s/insure/ensure/ :)","commit_id":"0821971e9136ebb7bbe5dc53c11d83363716501a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"068482d81ff55b2880bdf06b635b6a28b8843a9a","unresolved":false,"context_lines":[{"line_number":470,"context_line":"                where(models.PlacementAggregate.uuid.in_(target_aggregates))"},{"line_number":471,"context_line":"            insert_aggregates \u003d models.ResourceProviderAggregate.__table__.\\"},{"line_number":472,"context_line":"                insert().from_select([\u0027resource_provider_id\u0027, \u0027aggregate_id\u0027],"},{"line_number":473,"context_line":"                                     select_agg_id)"},{"line_number":474,"context_line":"            conn \u003d context.session.connection()"},{"line_number":475,"context_line":"            conn.execute(insert_aggregates)"},{"line_number":476,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"fa6399be_0fc6eb47","line":473,"updated":"2016-10-12 23:51:45.000000000","message":"++, good solution to use the INSERT SELECT FROM here.","commit_id":"0821971e9136ebb7bbe5dc53c11d83363716501a"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"c8edb5e359ae4b67c29c3b8e0f95d25f798f7880","unresolved":false,"context_lines":[{"line_number":470,"context_line":"                where(models.PlacementAggregate.uuid.in_(target_aggregates))"},{"line_number":471,"context_line":"            insert_aggregates \u003d models.ResourceProviderAggregate.__table__.\\"},{"line_number":472,"context_line":"                insert().from_select([\u0027resource_provider_id\u0027, \u0027aggregate_id\u0027],"},{"line_number":473,"context_line":"                                     select_agg_id)"},{"line_number":474,"context_line":"            conn \u003d context.session.connection()"},{"line_number":475,"context_line":"            conn.execute(insert_aggregates)"},{"line_number":476,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"fa6399be_30d7804c","line":473,"in_reply_to":"fa6399be_0fc6eb47","updated":"2016-10-13 09:49:50.000000000","message":"Either stolen from something you did elsewhere, or roman, I can\u0027t take credit for this.","commit_id":"0821971e9136ebb7bbe5dc53c11d83363716501a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a66ae812e74b6b005e298b5ccf84c3c3d3a2baf3","unresolved":false,"context_lines":[{"line_number":384,"context_line":"        return self._get_aggregates(self._context, self.id)"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"    @base.remotable"},{"line_number":387,"context_line":"    def set_aggregates(self, aggregate_uuids):"},{"line_number":388,"context_line":"        \"\"\"Set the aggregate uuids associated with this resource provider."},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"        If an aggregate does not exist, one will be created using the"}],"source_content_type":"text/x-python","patch_set":29,"id":"da6895a0_3b0168af","line":387,"updated":"2016-10-23 08:34:44.000000000","message":"This doesn\u0027t look like what I would expect from a remotable method. It doesn\u0027t change anything on self, it just uses self.id.\n\nI would expect this to be a remotable_classmethod, I think, which takes both the id and the list of aggregates. Otherwise you\u0027re passing the whole object for no real reason. If we stored aggregates on the object, then this would/could be kinda like save(), but we don\u0027t.","commit_id":"4ebede3e490b19b14a45cda9c82898ee659cb88e"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"14af96888b5027c28c5fd55e8278fb113975b346","unresolved":false,"context_lines":[{"line_number":384,"context_line":"        return self._get_aggregates(self._context, self.id)"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"    @base.remotable"},{"line_number":387,"context_line":"    def set_aggregates(self, aggregate_uuids):"},{"line_number":388,"context_line":"        \"\"\"Set the aggregate uuids associated with this resource provider."},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"        If an aggregate does not exist, one will be created using the"}],"source_content_type":"text/x-python","patch_set":29,"id":"da6895a0_526eec96","line":387,"in_reply_to":"da6895a0_3b0168af","updated":"2016-10-24 13:41:13.000000000","message":"I did it this way to duplicate the calling convention already established with set_inventory (and the other inventory methods).It preserves the pattern and it also means that if we choose to update the rp generation on aggregate associations (I forget where that landed) we\u0027ve already got the generation in hand.\n\nSo I guess we need to resolve (probably re-resolve) that question about generation.","commit_id":"4ebede3e490b19b14a45cda9c82898ee659cb88e"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"ff3d0faeaaa89ed2f2520d1acd84d5294e81d414","unresolved":false,"context_lines":[{"line_number":491,"context_line":"                filter_by(uuid\u003dagg_uuid).first()"},{"line_number":492,"context_line":"            if not found_agg:"},{"line_number":493,"context_line":"                new_aggregate \u003d models.PlacementAggregate(uuid\u003dagg_uuid)"},{"line_number":494,"context_line":"                context.session.add(new_aggregate)"},{"line_number":495,"context_line":"        # Flush here to ensure aggregate ids used below."},{"line_number":496,"context_line":"        context.session.flush()"},{"line_number":497,"context_line":""}],"source_content_type":"text/x-python","patch_set":33,"id":"7a77a97e_7a6543b3","line":494,"updated":"2016-11-17 10:10:51.000000000","message":"So, we may have race at here I guess?\n\nTwo calls, both found the agg_uuid isn\u0027t in the db, then add it into the db. Then there should be an exception at here, we should catch it, and ignore it?","commit_id":"8c69a08930c929f2ae98cfd5098946a7e403dbb7"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"900c4c26616e78a632d12c4a65b3dca8ecf3e667","unresolved":false,"context_lines":[{"line_number":491,"context_line":"                filter_by(uuid\u003dagg_uuid).first()"},{"line_number":492,"context_line":"            if not found_agg:"},{"line_number":493,"context_line":"                new_aggregate \u003d models.PlacementAggregate(uuid\u003dagg_uuid)"},{"line_number":494,"context_line":"                context.session.add(new_aggregate)"},{"line_number":495,"context_line":"        # Flush here to ensure aggregate ids used below."},{"line_number":496,"context_line":"        context.session.flush()"},{"line_number":497,"context_line":""}],"source_content_type":"text/x-python","patch_set":33,"id":"7a77a97e_098fa775","line":494,"in_reply_to":"7a77a97e_7a6543b3","updated":"2016-11-17 11:55:00.000000000","message":"Yeah, you\u0027re right, nice catch.\n\nIgnoring is the right thing too. I\u0027ll push up a change.","commit_id":"8c69a08930c929f2ae98cfd5098946a7e403dbb7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"98c99f50df5216afbc2a9248babad678ff53c434","unresolved":false,"context_lines":[{"line_number":474,"context_line":"        # a PlacementAggregate that no longer has any associations"},{"line_number":475,"context_line":"        # with at least one resource provider. We may wish to do that"},{"line_number":476,"context_line":"        # to avoid bloat if it turns out we\u0027re creating a lot of noise."},{"line_number":477,"context_line":"        # Not doing now to move things along."},{"line_number":478,"context_line":"        provided_aggregates \u003d set(provided_aggregates)"},{"line_number":479,"context_line":"        existing_aggregates \u003d set(cls._get_aggregates(context, rp_id))"},{"line_number":480,"context_line":"        to_add \u003d provided_aggregates - existing_aggregates"}],"source_content_type":"text/x-python","patch_set":34,"id":"5a74a57a_c8ca7b4f","line":477,"updated":"2016-11-23 15:38:59.000000000","message":"Thats fair enough I think.\n\nYou could have someone fill up the database, in theory by posting a load of uuids, but this is an admin only API right now, so I am OK with that.","commit_id":"d5e6a29be1b62f988e15f387b03c75e049d75e2f"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"4fcecad3426310ddc87f35c82c927c35d3bd9294","unresolved":false,"context_lines":[{"line_number":502,"context_line":"                    # also makes sure that the new aggregates have"},{"line_number":503,"context_line":"                    # ids when the SELECT below happens."},{"line_number":504,"context_line":"                    context.session.flush()"},{"line_number":505,"context_line":"                except db_exc.DBDuplicateEntry:"},{"line_number":506,"context_line":"                    # Something else has already added this agg_uuid"},{"line_number":507,"context_line":"                    pass"},{"line_number":508,"context_line":""}],"source_content_type":"text/x-python","patch_set":34,"id":"7a77a97e_1bb0ce89","line":505,"updated":"2016-11-17 12:35:33.000000000","message":"I\u0027m not sure how to test this race case also. Hope other people can give some suggestion.","commit_id":"d5e6a29be1b62f988e15f387b03c75e049d75e2f"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"98c99f50df5216afbc2a9248babad678ff53c434","unresolved":false,"context_lines":[{"line_number":502,"context_line":"                    # also makes sure that the new aggregates have"},{"line_number":503,"context_line":"                    # ids when the SELECT below happens."},{"line_number":504,"context_line":"                    context.session.flush()"},{"line_number":505,"context_line":"                except db_exc.DBDuplicateEntry:"},{"line_number":506,"context_line":"                    # Something else has already added this agg_uuid"},{"line_number":507,"context_line":"                    pass"},{"line_number":508,"context_line":""}],"source_content_type":"text/x-python","patch_set":34,"id":"5a74a57a_c8951b41","line":505,"in_reply_to":"7a77a97e_1bb0ce89","updated":"2016-11-23 15:38:59.000000000","message":"You can do evil mocking, but honestly, it doesn\u0027t totally seem worth it.","commit_id":"d5e6a29be1b62f988e15f387b03c75e049d75e2f"}],"nova/tests/functional/db/test_resource_provider.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"e62059a25f8708fd9946834af3247b05a74c5518","unresolved":false,"context_lines":[{"line_number":481,"context_line":"        re_read_aggregate_uuids \u003d read_rp.get_aggregates()"},{"line_number":482,"context_line":"        self.assertItemsEqual(aggregate_uuids, re_read_aggregate_uuids)"},{"line_number":483,"context_line":""},{"line_number":484,"context_line":"    # TODO(cdent): For the moment this test doesn\u0027t actually test"},{"line_number":485,"context_line":"    # anything new because the aggregate is created in the api"},{"line_number":486,"context_line":"    # database and set_aggregates currently only talks the placement"},{"line_number":487,"context_line":"    # database. In the real world this could be the same database,"},{"line_number":488,"context_line":"    # but under testing situations it is a different in ram databases."},{"line_number":489,"context_line":"    def test_set_and_get_existing_aggregates(self):"},{"line_number":490,"context_line":"        agg_uuid \u003d uuidsentinel.agg_uuid"},{"line_number":491,"context_line":"        aggregate \u003d objects.Aggregate("},{"line_number":492,"context_line":"            context\u003dself.context,"},{"line_number":493,"context_line":"            uuid\u003dagg_uuid,"},{"line_number":494,"context_line":"            name\u003d\u0027an aggregate\u0027"},{"line_number":495,"context_line":"        )"},{"line_number":496,"context_line":"        aggregate.create()"},{"line_number":497,"context_line":"        rp \u003d objects.ResourceProvider("},{"line_number":498,"context_line":"            context\u003dself.context,"},{"line_number":499,"context_line":"            uuid\u003duuidsentinel.rp_uuid,"},{"line_number":500,"context_line":"            name\u003duuidsentinel.rp_name"},{"line_number":501,"context_line":"        )"},{"line_number":502,"context_line":"        rp.create()"},{"line_number":503,"context_line":"        rp.set_aggregates([agg_uuid])"},{"line_number":504,"context_line":"        read_aggregate_uuids \u003d rp.get_aggregates()"},{"line_number":505,"context_line":"        self.assertItemsEqual([agg_uuid], read_aggregate_uuids)"},{"line_number":506,"context_line":""},{"line_number":507,"context_line":"    def test_set_aggregates_is_replace(self):"},{"line_number":508,"context_line":"        rp \u003d objects.ResourceProvider("}],"source_content_type":"text/x-python","patch_set":9,"id":"fa7ab95a_05efedb0","line":505,"range":{"start_line":484,"start_character":4,"end_line":505,"end_character":63},"updated":"2016-08-31 13:34:58.000000000","message":"Hmm, now we have decided should we just delete this one. Sorry, I overlooked that last time.","commit_id":"8f1838d07d20a380137ec6dc1c92ce6584697e61"}]}