)]}'
{"nova/objects/resource_provider.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"34fc20616829fa65fc4ba641c9c836eda343a5b5","unresolved":false,"context_lines":[{"line_number":311,"context_line":"    @base.remotable"},{"line_number":312,"context_line":"    def resources(self):"},{"line_number":313,"context_line":"        \"\"\"Join inventories with allocations to get used.\"\"\""},{"line_number":314,"context_line":"        # NOTE(cdent): Does this mean we need yet another object?"},{"line_number":315,"context_line":"        resource_data \u003d self._get_resource_data(self._context,"},{"line_number":316,"context_line":"                                                self.resource_provider)"},{"line_number":317,"context_line":"        return resource_data"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa0719c6_05c6f866","line":314,"updated":"2016-03-21 14:52:18.000000000","message":"In tests and in the next patch up the stack I\u0027ve just been working with the provided list of resources, and that works fine enough, but is it idiomatic.","commit_id":"206b12774c265bba9a36d0af140ced0ed66ae857"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"eea16eacc02385f8b6b2a0eefd156aef0ebda13b","unresolved":false,"context_lines":[{"line_number":314,"context_line":"        # NOTE(cdent): Does this mean we need yet another object?"},{"line_number":315,"context_line":"        resource_data \u003d self._get_resource_data(self._context,"},{"line_number":316,"context_line":"                                                self.resource_provider)"},{"line_number":317,"context_line":"        return resource_data"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"    @base.remotable"},{"line_number":320,"context_line":"    def create(self):"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa0719c6_e782deff","line":317,"updated":"2016-03-21 18:08:21.000000000","message":"This is a list of dicts, but in use it tends to get used a nest dicts, keyed on resource class.","commit_id":"206b12774c265bba9a36d0af140ced0ed66ae857"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"34fc20616829fa65fc4ba641c9c836eda343a5b5","unresolved":false,"context_lines":[{"line_number":319,"context_line":"    @base.remotable"},{"line_number":320,"context_line":"    def create(self):"},{"line_number":321,"context_line":"        \"\"\"Associate the resource_provider with the aggregates.\"\"\""},{"line_number":322,"context_line":"        self.obj_set_defaults()"},{"line_number":323,"context_line":"        try:"},{"line_number":324,"context_line":"            self._make_resource_provider_aggregates(self._context,"},{"line_number":325,"context_line":"                                                    self.resource_provider,"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa0719c6_8514c8f5","line":322,"updated":"2016-03-21 14:52:18.000000000","message":"Should we force a ResourcePool create to require aggregates to be something other than a []?","commit_id":"206b12774c265bba9a36d0af140ced0ed66ae857"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"323a52fb9eba52607e525e75de36246b10c04a86","unresolved":false,"context_lines":[{"line_number":336,"context_line":"            models.Inventory,"},{"line_number":337,"context_line":"            func.coalesce(func.sum(models.Allocation.used),"},{"line_number":338,"context_line":"                          0).label(\u0027used\u0027))."},{"line_number":339,"context_line":"            outerjoin(models.Allocation,"},{"line_number":340,"context_line":"                 models.Inventory.resource_provider_id \u003d\u003d  # noqa"},{"line_number":341,"context_line":"                 models.Allocation.resource_provider_id)."},{"line_number":342,"context_line":"            filter(models.Inventory.resource_provider_id \u003d\u003d  # noqa"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa0719c6_cd11bbae","line":339,"updated":"2016-03-21 18:51:24.000000000","message":"This join is incorrect. It\u0027s fixed in the code I\u0027ve got going now.","commit_id":"206b12774c265bba9a36d0af140ced0ed66ae857"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"34fc20616829fa65fc4ba641c9c836eda343a5b5","unresolved":false,"context_lines":[{"line_number":344,"context_line":"            group_by(models.Allocation.resource_class_id))"},{"line_number":345,"context_line":"        # TODO(cdent): This is noisy and annoying and should be"},{"line_number":346,"context_line":"        # changed before we finish this process, but doing it this"},{"line_number":347,"context_line":"        # way for now to keep moving."},{"line_number":348,"context_line":"        return [dict(total\u003dinventory.total,"},{"line_number":349,"context_line":"                     resource_class\u003dfields.ResourceClass.from_index("},{"line_number":350,"context_line":"                         inventory.resource_class_id),"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa0719c6_a5262c66","line":347,"updated":"2016-03-21 14:52:18.000000000","message":"Yeah, so crufty","commit_id":"206b12774c265bba9a36d0af140ced0ed66ae857"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"34fc20616829fa65fc4ba641c9c836eda343a5b5","unresolved":false,"context_lines":[{"line_number":374,"context_line":"    @db_api.main_context_manager.reader"},{"line_number":375,"context_line":"    def _build_from_resource_provider_uuid(context, resource_pool, rp_uuid):"},{"line_number":376,"context_line":"        # TODO(cdent): Way to collapse this into a single sql query"},{"line_number":377,"context_line":"        # the result of which is then fed to _from_db_object?"},{"line_number":378,"context_line":"        resource_pool.resource_provider \u003d objects.ResourceProvider.get_by_uuid("},{"line_number":379,"context_line":"            context, rp_uuid)"},{"line_number":380,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"fa0719c6_c5924057","line":377,"updated":"2016-03-21 14:52:18.000000000","message":"Yeah, this doesn\u0027t seem as sql-awesome as perhaps it could be?","commit_id":"206b12774c265bba9a36d0af140ced0ed66ae857"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5dde5548cf404a93b023d399b2a8b5ca3dda2ed2","unresolved":false,"context_lines":[{"line_number":293,"context_line":""},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"@base.NovaObjectRegistry.register"},{"line_number":296,"context_line":"class ResourcePool(base.NovaObject):"},{"line_number":297,"context_line":"    # Version 1.0: Initial Version"},{"line_number":298,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":299,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"fa0719c6_b55bb086","line":296,"updated":"2016-03-22 20:51:44.000000000","message":"How about inheriting from ResourceProvider, since ResourcePool is simply a specialization of ResourceProvider? Really, the *only* difference between a ResourceProvider and a ResourcePool is the create() method, which, in the case of a resource pool MUST indicate an initial aggregate to which the resource provider must be associated.","commit_id":"8348262237988882dafe537269e1305d6a87fdff"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5dde5548cf404a93b023d399b2a8b5ca3dda2ed2","unresolved":false,"context_lines":[{"line_number":309,"context_line":"            \u0027AggregateList\u0027, nullable\u003dFalse,"},{"line_number":310,"context_line":"            default\u003dobjects.AggregateList(objects\u003d[])),"},{"line_number":311,"context_line":"    }"},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"    @property"},{"line_number":314,"context_line":"    @base.remotable"},{"line_number":315,"context_line":"    def resources(self):"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa0719c6_955e6c74","line":312,"updated":"2016-03-22 20:51:44.000000000","message":"This should be on ResourceProvider.","commit_id":"8348262237988882dafe537269e1305d6a87fdff"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5dde5548cf404a93b023d399b2a8b5ca3dda2ed2","unresolved":false,"context_lines":[{"line_number":330,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":331,"context_line":"            raise exception.ObjectActionError("},{"line_number":332,"context_line":"                action\u003d\u0027create\u0027, reason\u003d\u0027aggregate already associated\u0027)"},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"    @staticmethod"},{"line_number":335,"context_line":"    @db_api.main_context_manager.reader"},{"line_number":336,"context_line":"    def _get_resource_data(context, resource_provider):"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa0719c6_f5552850","line":333,"updated":"2016-03-22 20:51:44.000000000","message":"This should be on ResourceProvider.","commit_id":"8348262237988882dafe537269e1305d6a87fdff"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5dde5548cf404a93b023d399b2a8b5ca3dda2ed2","unresolved":false,"context_lines":[{"line_number":360,"context_line":"                     step_size\u003dinventory.step_size,"},{"line_number":361,"context_line":"                     used\u003dused)"},{"line_number":362,"context_line":"                for inventory, used in query.all()]"},{"line_number":363,"context_line":""},{"line_number":364,"context_line":"    @staticmethod"},{"line_number":365,"context_line":"    @db_api.main_context_manager.writer"},{"line_number":366,"context_line":"    def _make_resource_provider_aggregates(context, resource_provider,"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa0719c6_d550e460","line":363,"updated":"2016-03-22 20:51:44.000000000","message":"This should be on ResourceProvider.","commit_id":"8348262237988882dafe537269e1305d6a87fdff"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5dde5548cf404a93b023d399b2a8b5ca3dda2ed2","unresolved":false,"context_lines":[{"line_number":370,"context_line":"                resource_provider_id\u003dresource_provider.id,"},{"line_number":371,"context_line":"                aggregate_id\u003daggregate.id)"},{"line_number":372,"context_line":"            context.session.add(db_rpa)"},{"line_number":373,"context_line":""},{"line_number":374,"context_line":"    @base.remotable_classmethod"},{"line_number":375,"context_line":"    def get_by_resource_provider_uuid(cls, context, rp_uuid):"},{"line_number":376,"context_line":"        return cls._build_from_resource_provider_uuid(context, cls(), rp_uuid)"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa0719c6_3550405d","line":373,"updated":"2016-03-22 20:51:44.000000000","message":"This should go away and just be ResourceProvider.get_by_uuid() once ResourcePool inherits from ResourceProvider.","commit_id":"8348262237988882dafe537269e1305d6a87fdff"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5dde5548cf404a93b023d399b2a8b5ca3dda2ed2","unresolved":false,"context_lines":[{"line_number":374,"context_line":"    @base.remotable_classmethod"},{"line_number":375,"context_line":"    def get_by_resource_provider_uuid(cls, context, rp_uuid):"},{"line_number":376,"context_line":"        return cls._build_from_resource_provider_uuid(context, cls(), rp_uuid)"},{"line_number":377,"context_line":""},{"line_number":378,"context_line":"    @staticmethod"},{"line_number":379,"context_line":"    @db_api.main_context_manager.reader"},{"line_number":380,"context_line":"    def _build_from_resource_provider_uuid(context, resource_pool, rp_uuid):"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa0719c6_154b7cb1","line":377,"updated":"2016-03-22 20:51:44.000000000","message":"How about just modify the ResourceProvider.get_by_uuid() to take an expected_attrs argument and, for resource pools, load aggregates by making the get_by_uuid() default to expected_attrs\u003d[\u0027aggregates\u0027]?","commit_id":"8348262237988882dafe537269e1305d6a87fdff"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5dde5548cf404a93b023d399b2a8b5ca3dda2ed2","unresolved":false,"context_lines":[{"line_number":379,"context_line":"    @db_api.main_context_manager.reader"},{"line_number":380,"context_line":"    def _build_from_resource_provider_uuid(context, resource_pool, rp_uuid):"},{"line_number":381,"context_line":"        # TODO(cdent): Way to collapse this into a single sql query"},{"line_number":382,"context_line":"        # the result of which is then fed to _from_db_object?"},{"line_number":383,"context_line":"        resource_pool.resource_provider \u003d objects.ResourceProvider.get_by_uuid("},{"line_number":384,"context_line":"            context, rp_uuid)"},{"line_number":385,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"fa0719c6_75c8f822","line":382,"updated":"2016-03-22 20:51:44.000000000","message":"Do that using expected_attrs and building a join to resource_provider_aggregates table when expected_attrs contains \u0027aggregates\u0027.","commit_id":"8348262237988882dafe537269e1305d6a87fdff"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5dde5548cf404a93b023d399b2a8b5ca3dda2ed2","unresolved":false,"context_lines":[{"line_number":438,"context_line":"        # ResourcePool one by one using that uuid. That seems rather"},{"line_number":439,"context_line":"        # clumsy but its still not entirely clear the best way to"},{"line_number":440,"context_line":"        # build a ResourcePool from available data, whether one or"},{"line_number":441,"context_line":"        # many."},{"line_number":442,"context_line":"        resource_provider_uuids \u003d cls._get_all_by_resource_provider_uuids("},{"line_number":443,"context_line":"            context)"},{"line_number":444,"context_line":"        resource_pools \u003d [objects.ResourcePool.get_by_resource_provider_uuid("}],"source_content_type":"text/x-python","patch_set":17,"id":"fa0719c6_55cb341e","line":441,"updated":"2016-03-22 20:51:44.000000000","message":"Hmm, so actually, all we want to return here (at least for now) is any resource provider that *isn\u0027t* a compute node. And the easiest way to do that actually has nothing to do with aggregates, but rather, simply outerjoin with an IS NULL condition from resource_providers to compute_nodes. The following SQL query will give you the ID of all resource pools, which at this stage means all resource providers that are not compute nodes:\n\n SELECT rp.id\n FROM resource_providers AS rp\n LEFT JOIN compute_nodes AS cn\n  ON rp.uuid \u003d cn.uuid\n WHERE rp.uuid IS NULL;","commit_id":"8348262237988882dafe537269e1305d6a87fdff"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"43fcdc8e23204d724aca1cbe1a00d4a1186dcf0e","unresolved":false,"context_lines":[{"line_number":349,"context_line":"    fields \u003d {"},{"line_number":350,"context_line":"        # TODO(cdent): What is the correct way to say that this"},{"line_number":351,"context_line":"        # field must be a non-empty list of Aggregates?"},{"line_number":352,"context_line":"        \u0027aggregates\u0027: fields.ObjectField("},{"line_number":353,"context_line":"            \u0027AggregateList\u0027, nullable\u003dFalse,"},{"line_number":354,"context_line":"            default\u003dobjects.AggregateList(objects\u003d[])),"},{"line_number":355,"context_line":"    }"}],"source_content_type":"text/x-python","patch_set":29,"id":"9a061dce_ea0fa034","line":352,"updated":"2016-04-07 15:00:58.000000000","message":"I would not do this, but rather go for a ListfOfObjectsField(\u0027Aggregate\u0027). There are a couple of reasons for that, the simplest of which is that going with the AggregateList container increases the serialization footprint a bit. It also complicates adding and removing things from the list, as well as confuses the issue a little since the nested object has its own query method, etc. Finally, and maybe the most compelling, is that it\u0027s harder to tell if something in the nested object has changed, versus just that the list itself has been touched.\n\nSo, I would go that route.","commit_id":"ffb6e2764f3b138b26a5084b20984a34f965280f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"43fcdc8e23204d724aca1cbe1a00d4a1186dcf0e","unresolved":false,"context_lines":[{"line_number":351,"context_line":"        # field must be a non-empty list of Aggregates?"},{"line_number":352,"context_line":"        \u0027aggregates\u0027: fields.ObjectField("},{"line_number":353,"context_line":"            \u0027AggregateList\u0027, nullable\u003dFalse,"},{"line_number":354,"context_line":"            default\u003dobjects.AggregateList(objects\u003d[])),"},{"line_number":355,"context_line":"    }"},{"line_number":356,"context_line":""},{"line_number":357,"context_line":"    @base.remotable"}],"source_content_type":"text/x-python","patch_set":29,"id":"9a061dce_cd3f56b2","line":354,"updated":"2016-04-07 15:00:58.000000000","message":"Unless you use obj_set_defaults() this doesn\u0027t really do anything, and I think you should remove it. For something like this, we want to be extra sure we never wipe out the list of aggregates for this pool in the database because you took an empty default.","commit_id":"ffb6e2764f3b138b26a5084b20984a34f965280f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"fdd4ad57b946a140a9e28c71265393bdf4519964","unresolved":false,"context_lines":[{"line_number":276,"context_line":"    fields \u003d {"},{"line_number":277,"context_line":"        \u0027id\u0027: fields.IntegerField(),"},{"line_number":278,"context_line":"        \u0027resource_provider\u0027: fields.ObjectField(\u0027ResourceProvider\u0027,"},{"line_number":279,"context_line":"                                                subclasses\u003dTrue),"},{"line_number":280,"context_line":"        \u0027consumer_id\u0027: fields.UUIDField(),"},{"line_number":281,"context_line":"        \u0027resource_class\u0027: fields.ResourceClassField(),"},{"line_number":282,"context_line":"        \u0027used\u0027: fields.IntegerField(),"}],"source_content_type":"text/x-python","patch_set":31,"id":"5a18252c_4a7443fc","line":279,"updated":"2016-04-13 14:33:08.000000000","message":"Hmm, this seems like it should require a version bump, but I\u0027m guessing you didn\u0027t because it didn\u0027t complain. I\u0027m trying to think about whether that is a bug or not...","commit_id":"ff3e6ec532c1a60ada840b20b67174f26251e5ee"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"9df0e869f1a0a6cf8a732b29f4826beba0f5687a","unresolved":false,"context_lines":[{"line_number":276,"context_line":"    fields \u003d {"},{"line_number":277,"context_line":"        \u0027id\u0027: fields.IntegerField(),"},{"line_number":278,"context_line":"        \u0027resource_provider\u0027: fields.ObjectField(\u0027ResourceProvider\u0027,"},{"line_number":279,"context_line":"                                                subclasses\u003dTrue),"},{"line_number":280,"context_line":"        \u0027consumer_id\u0027: fields.UUIDField(),"},{"line_number":281,"context_line":"        \u0027resource_class\u0027: fields.ResourceClassField(),"},{"line_number":282,"context_line":"        \u0027used\u0027: fields.IntegerField(),"}],"source_content_type":"text/x-python","patch_set":31,"id":"5a18252c_8ad1365a","line":279,"in_reply_to":"5a18252c_4a7443fc","updated":"2016-04-13 15:34:56.000000000","message":"I was surprised too, but yeah, it didn\u0027t complain.","commit_id":"ff3e6ec532c1a60ada840b20b67174f26251e5ee"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"fdd4ad57b946a140a9e28c71265393bdf4519964","unresolved":false,"context_lines":[{"line_number":342,"context_line":""},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"@base.NovaObjectRegistry.register"},{"line_number":345,"context_line":"class ResourcePool(ResourceProvider):"},{"line_number":346,"context_line":"    # Version 1.0: Initial Version"},{"line_number":347,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":348,"context_line":""}],"source_content_type":"text/x-python","patch_set":31,"id":"5a18252c_47f9de4c","line":345,"updated":"2016-04-13 14:33:08.000000000","message":"Hmm, this seems strange to me. A ResourcePool is not a specialization of a ResourceProvider is it? When is there a ResourceProvider that doesn\u0027t need to be associated with a set of aggregates? If there is a case, can\u0027t we just have the list be empty or None?","commit_id":"ff3e6ec532c1a60ada840b20b67174f26251e5ee"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"9df0e869f1a0a6cf8a732b29f4826beba0f5687a","unresolved":false,"context_lines":[{"line_number":342,"context_line":""},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"@base.NovaObjectRegistry.register"},{"line_number":345,"context_line":"class ResourcePool(ResourceProvider):"},{"line_number":346,"context_line":"    # Version 1.0: Initial Version"},{"line_number":347,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":348,"context_line":""}],"source_content_type":"text/x-python","patch_set":31,"id":"5a18252c_6abe5ad5","line":345,"in_reply_to":"5a18252c_47f9de4c","updated":"2016-04-13 15:34:56.000000000","message":"It was Jay\u0027s idea (see fairly extensive comments from him on earlier patchset 17).\n\nBasically yes, a ResourcePool is a ResourceProvider that is not a compute node. A compute node is^wcould be a ResourceProvider that doesn\u0027t have aggregates.","commit_id":"ff3e6ec532c1a60ada840b20b67174f26251e5ee"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"feea9ebbbb2f9bbaf1acfcaae5de69dd0a77a332","unresolved":false,"context_lines":[{"line_number":342,"context_line":""},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"@base.NovaObjectRegistry.register"},{"line_number":345,"context_line":"class ResourcePool(ResourceProvider):"},{"line_number":346,"context_line":"    # Version 1.0: Initial Version"},{"line_number":347,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":348,"context_line":""}],"source_content_type":"text/x-python","patch_set":31,"id":"5a18252c_c084afd5","line":345,"in_reply_to":"5a18252c_6abe5ad5","updated":"2016-04-13 17:48:12.000000000","message":"I just don\u0027t see the point in representing that by a subclass relationship, which just makes things more complicated.\n\nIf a compute node is a provider of resources with no aggregates, then just set aggregates to None or empty.","commit_id":"ff3e6ec532c1a60ada840b20b67174f26251e5ee"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"fdd4ad57b946a140a9e28c71265393bdf4519964","unresolved":false,"context_lines":[{"line_number":353,"context_line":"    @base.remotable"},{"line_number":354,"context_line":"    def create(self):"},{"line_number":355,"context_line":"        \"\"\"Associate the resource_provider with the aggregates.\"\"\""},{"line_number":356,"context_line":"        self.obj_set_defaults()"},{"line_number":357,"context_line":"        try:"},{"line_number":358,"context_line":"            super(ResourcePool, self).create()"},{"line_number":359,"context_line":"            self._make_resource_provider_aggregates(self._context,"}],"source_content_type":"text/x-python","patch_set":31,"id":"5a18252c_aa835755","line":356,"updated":"2016-04-13 14:33:08.000000000","message":"I\u0027m not completely opposed to this, but it seems like it\u0027s something that the caller should do if they want it. Alternately, maybe a create_with_defaults() method that does that an then calls create() would be better?\n\nBecause this is remotable, it means that someone on the remote side will call create() and get the defaults that are set on the local (i.e. conductor) side. That may be surprising later, especially if we were to remove the automatic default-setting, or change a default. Putting this inside the remotable method basically means that we can\u0027t (or shouldn\u0027t) change it later because it\u0027s part of the RPC API.","commit_id":"ff3e6ec532c1a60ada840b20b67174f26251e5ee"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"9df0e869f1a0a6cf8a732b29f4826beba0f5687a","unresolved":false,"context_lines":[{"line_number":353,"context_line":"    @base.remotable"},{"line_number":354,"context_line":"    def create(self):"},{"line_number":355,"context_line":"        \"\"\"Associate the resource_provider with the aggregates.\"\"\""},{"line_number":356,"context_line":"        self.obj_set_defaults()"},{"line_number":357,"context_line":"        try:"},{"line_number":358,"context_line":"            super(ResourcePool, self).create()"},{"line_number":359,"context_line":"            self._make_resource_provider_aggregates(self._context,"}],"source_content_type":"text/x-python","patch_set":31,"id":"5a18252c_8a893629","line":356,"in_reply_to":"5a18252c_aa835755","updated":"2016-04-13 15:34:56.000000000","message":"Actually that\u0027s a carry over from the previous way of managing aggregates and not something I meant to leave in, so I\u0027ll just kill it.","commit_id":"ff3e6ec532c1a60ada840b20b67174f26251e5ee"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"fdd4ad57b946a140a9e28c71265393bdf4519964","unresolved":false,"context_lines":[{"line_number":355,"context_line":"        \"\"\"Associate the resource_provider with the aggregates.\"\"\""},{"line_number":356,"context_line":"        self.obj_set_defaults()"},{"line_number":357,"context_line":"        try:"},{"line_number":358,"context_line":"            super(ResourcePool, self).create()"},{"line_number":359,"context_line":"            self._make_resource_provider_aggregates(self._context,"},{"line_number":360,"context_line":"                                                    self.id,"},{"line_number":361,"context_line":"                                                    self.aggregates)"}],"source_content_type":"text/x-python","patch_set":31,"id":"5a18252c_0a6feb92","line":358,"updated":"2016-04-13 14:33:08.000000000","message":"Yeah, this is just very odd and I don\u0027t think it makes sense other than just outright code reuse. But, answer the question above.","commit_id":"ff3e6ec532c1a60ada840b20b67174f26251e5ee"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"9df0e869f1a0a6cf8a732b29f4826beba0f5687a","unresolved":false,"context_lines":[{"line_number":355,"context_line":"        \"\"\"Associate the resource_provider with the aggregates.\"\"\""},{"line_number":356,"context_line":"        self.obj_set_defaults()"},{"line_number":357,"context_line":"        try:"},{"line_number":358,"context_line":"            super(ResourcePool, self).create()"},{"line_number":359,"context_line":"            self._make_resource_provider_aggregates(self._context,"},{"line_number":360,"context_line":"                                                    self.id,"},{"line_number":361,"context_line":"                                                    self.aggregates)"}],"source_content_type":"text/x-python","patch_set":31,"id":"5a18252c_eacd4a5e","line":358,"in_reply_to":"5a18252c_0a6feb92","updated":"2016-04-13 15:34:56.000000000","message":"a) see jay\u0027s comments on patchset 17\n\nb) it\u0027s working nicely in the change above this one (the placement api wip)","commit_id":"ff3e6ec532c1a60ada840b20b67174f26251e5ee"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"8e0cfe0e1e4dbc18879bf4050306b08c3d24ff17","unresolved":false,"context_lines":[{"line_number":375,"context_line":"    @base.remotable"},{"line_number":376,"context_line":"    def associate_aggregates(self, aggregates):"},{"line_number":377,"context_line":"        aggregates \u003d [aggregate for aggregate in aggregates"},{"line_number":378,"context_line":"                      if aggregate not in self.aggregates]"},{"line_number":379,"context_line":"        try:"},{"line_number":380,"context_line":"            self._make_resource_provider_aggregates(self._context,"},{"line_number":381,"context_line":"                                                    self.id,"}],"source_content_type":"text/x-python","patch_set":31,"id":"5a18252c_34ee85ec","line":378,"updated":"2016-04-13 17:06:17.000000000","message":"oops this doesn\u0027t actually work (or below) because Aggregate objects are not comparable. Will fix it later/tomorrow.","commit_id":"ff3e6ec532c1a60ada840b20b67174f26251e5ee"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"fdd4ad57b946a140a9e28c71265393bdf4519964","unresolved":false,"context_lines":[{"line_number":510,"context_line":"        return resource_pools"},{"line_number":511,"context_line":""},{"line_number":512,"context_line":"    @base.remotable_classmethod"},{"line_number":513,"context_line":"    def get_all(cls, context):"},{"line_number":514,"context_line":"        resource_pools \u003d cls._get_all_from_db(context)"},{"line_number":515,"context_line":"        return base.obj_make_list(context, cls(context),"},{"line_number":516,"context_line":"                                  objects.ResourcePool, resource_pools)"}],"source_content_type":"text/x-python","patch_set":31,"id":"5a18252c_eae8afb6","line":513,"updated":"2016-04-13 14:33:08.000000000","message":"No filtering capability? At least be filterable by resource class or something as an option, I would think.","commit_id":"ff3e6ec532c1a60ada840b20b67174f26251e5ee"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"feea9ebbbb2f9bbaf1acfcaae5de69dd0a77a332","unresolved":false,"context_lines":[{"line_number":510,"context_line":"        return resource_pools"},{"line_number":511,"context_line":""},{"line_number":512,"context_line":"    @base.remotable_classmethod"},{"line_number":513,"context_line":"    def get_all(cls, context):"},{"line_number":514,"context_line":"        resource_pools \u003d cls._get_all_from_db(context)"},{"line_number":515,"context_line":"        return base.obj_make_list(context, cls(context),"},{"line_number":516,"context_line":"                                  objects.ResourcePool, resource_pools)"}],"source_content_type":"text/x-python","patch_set":31,"id":"5a18252c_8fbadf6c","line":513,"in_reply_to":"5a18252c_0a79a6c8","updated":"2016-04-13 17:48:12.000000000","message":"Yeah, I just don\u0027t want to add this and then have to add a filter before it\u0027s usable.","commit_id":"ff3e6ec532c1a60ada840b20b67174f26251e5ee"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"9df0e869f1a0a6cf8a732b29f4826beba0f5687a","unresolved":false,"context_lines":[{"line_number":510,"context_line":"        return resource_pools"},{"line_number":511,"context_line":""},{"line_number":512,"context_line":"    @base.remotable_classmethod"},{"line_number":513,"context_line":"    def get_all(cls, context):"},{"line_number":514,"context_line":"        resource_pools \u003d cls._get_all_from_db(context)"},{"line_number":515,"context_line":"        return base.obj_make_list(context, cls(context),"},{"line_number":516,"context_line":"                                  objects.ResourcePool, resource_pools)"}],"source_content_type":"text/x-python","patch_set":31,"id":"5a18252c_0a79a6c8","line":513,"in_reply_to":"5a18252c_eae8afb6","updated":"2016-04-13 15:34:56.000000000","message":"a) any given resource pool can support more than one resource class, so filtering on that, at least thus far, is weird\n\nb) to throw something I\u0027ve heard from you back at you: There isn\u0027t yet any code that is doing anything involving filtering with this, so it would be premature to guess what kind of filtering will be needed. dstepanenko is going to start on the resource tracker side of this work very soon so that should reveal the filtering requirements, probably before this gets merged, so if that proves to show we need filter we can get it in without a version bump.\n\nThe HTTP API described in the spec doesn\u0027t provide any plans for filtering, as yet, so this just hits what\u0027s been described over discovered thus far.","commit_id":"ff3e6ec532c1a60ada840b20b67174f26251e5ee"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"4b96f4c28158e29544bf4a1e59df8eacfda294a3","unresolved":false,"context_lines":[{"line_number":113,"context_line":"    @base.remotable"},{"line_number":114,"context_line":"    def disassociate_aggregates(self, aggregates):"},{"line_number":115,"context_line":"        # NOTE(cdent): This expects the incoming list of aggregates"},{"line_number":116,"context_line":"        # to be already associated and errors otherwise if any of"},{"line_number":117,"context_line":"        # them are not."},{"line_number":118,"context_line":"        provided_aggregate_uuids \u003d [aggregate.uuid for aggregate in aggregates]"},{"line_number":119,"context_line":"        self._delete_resource_provider_aggregates(self._context,"},{"line_number":120,"context_line":"                                                  self.id,"}],"source_content_type":"text/x-python","patch_set":45,"id":"1a122d0e_3c4dfac4","line":117,"range":{"start_line":116,"start_character":39,"end_line":117,"end_character":22},"updated":"2016-05-03 14:55:15.000000000","message":"The test in https://review.openstack.org/#/c/284963/45/nova/tests/unit/objects/test_resource_provider.py@379 only proves that if all of the aggregates are disassociated, error will be raised.\n\nAnd I don\u0027t think this logic will conform the statement that: \"error will be raised if any of them are not associated\". Maybe that test needs to be improved.","commit_id":"284e9ee3c1ca73e8d7350fe681df9c117081856b"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"14dacf7b1817615838199fba779a847ca5c5d4b3","unresolved":false,"context_lines":[{"line_number":114,"context_line":"    def disassociate_aggregates(self, aggregates):"},{"line_number":115,"context_line":"        # NOTE(cdent): This expects the incoming list of aggregates"},{"line_number":116,"context_line":"        # to be already associated and errors otherwise if any of"},{"line_number":117,"context_line":"        # them are not."},{"line_number":118,"context_line":"        provided_aggregate_uuids \u003d [aggregate.uuid for aggregate in aggregates]"},{"line_number":119,"context_line":"        self._delete_resource_provider_aggregates(self._context,"},{"line_number":120,"context_line":"                                                  self.id,"}],"source_content_type":"text/x-python","patch_set":45,"id":"1a122d0e_a7256d0b","line":117,"in_reply_to":"1a122d0e_3c4dfac4","updated":"2016-05-03 18:30:41.000000000","message":"Yes, definitely, thanks for noticing. There are quite a few areas where the tests need to be improved.","commit_id":"284e9ee3c1ca73e8d7350fe681df9c117081856b"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"4b96f4c28158e29544bf4a1e59df8eacfda294a3","unresolved":false,"context_lines":[{"line_number":195,"context_line":"                  filter_by(id\u003dresource_provider_id)).delete()"},{"line_number":196,"context_line":"        if not result:"},{"line_number":197,"context_line":"            raise exception.NotFound()"},{"line_number":198,"context_line":"        for query in (rpa_query, inv_query, allocation_query):"},{"line_number":199,"context_line":"            query.delete()"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    @staticmethod"},{"line_number":202,"context_line":"    def _from_db_object(context, resource_provider, db_resource_provider):"}],"source_content_type":"text/x-python","patch_set":45,"id":"1a122d0e_2fcdcb86","line":199,"range":{"start_line":198,"start_character":8,"end_line":199,"end_character":26},"updated":"2016-05-03 14:55:15.000000000","message":"The better way is to delete child rows first.\n\nIf there\u0027s a failure during deletion, there won\u0027t be any orphan child records because the parent row is not deleted yet.\n\nAnd if foreign key is configured, it is also safer to delete child rows first.","commit_id":"284e9ee3c1ca73e8d7350fe681df9c117081856b"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"9d5260586cd293bdb9ad4628c442f9c28e8d4e93","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        if not result:"},{"line_number":197,"context_line":"            raise exception.NotFound()"},{"line_number":198,"context_line":"        for query in (rpa_query, inv_query, allocation_query):"},{"line_number":199,"context_line":"            query.delete()"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    @staticmethod"},{"line_number":202,"context_line":"    def _from_db_object(context, resource_provider, db_resource_provider):"}],"source_content_type":"text/x-python","patch_set":45,"id":"1a122d0e_9971a40f","line":199,"in_reply_to":"1a122d0e_074c219c","updated":"2016-05-04 05:36:33.000000000","message":"OK, it\u0027s wrapped by a transaction, still learning :)","commit_id":"284e9ee3c1ca73e8d7350fe681df9c117081856b"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"14dacf7b1817615838199fba779a847ca5c5d4b3","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        if not result:"},{"line_number":197,"context_line":"            raise exception.NotFound()"},{"line_number":198,"context_line":"        for query in (rpa_query, inv_query, allocation_query):"},{"line_number":199,"context_line":"            query.delete()"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    @staticmethod"},{"line_number":202,"context_line":"    def _from_db_object(context, resource_provider, db_resource_provider):"}],"source_content_type":"text/x-python","patch_set":45,"id":"1a122d0e_074c219c","line":199,"in_reply_to":"1a122d0e_2fcdcb86","updated":"2016-05-03 18:30:41.000000000","message":"Aren\u0027t we in a transaction here, such that any failure will cause a rollback?\n\nI\u0027ve done the parent delete first to avoid running the child queries (which would do nothing) at all in the (rather unlikely) event that the resource provider has already been deleted since the request came in. Is that not a reasonable thing to do?","commit_id":"284e9ee3c1ca73e8d7350fe681df9c117081856b"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"4b96f4c28158e29544bf4a1e59df8eacfda294a3","unresolved":false,"context_lines":[{"line_number":238,"context_line":"                          models.Allocation.resource_provider_id,"},{"line_number":239,"context_line":"                          models.Inventory.resource_class_id \u003d\u003d"},{"line_number":240,"context_line":"                          models.Allocation.resource_class_id))."},{"line_number":241,"context_line":"            filter(models.Inventory.resource_provider_id \u003d\u003d"},{"line_number":242,"context_line":"                   resource_provider_id)."},{"line_number":243,"context_line":"            group_by(models.Inventory.resource_class_id))"},{"line_number":244,"context_line":"        # TODO(cdent): This is noisy and annoying and should be"},{"line_number":245,"context_line":"        # changed before we finish this process, but doing it this"}],"source_content_type":"text/x-python","patch_set":45,"id":"1a122d0e_1b280333","line":242,"range":{"start_line":241,"start_character":12,"end_line":242,"end_character":41},"updated":"2016-05-03 14:55:15.000000000","message":"Just asking: does execution order matters here?\n\nBy nature I think if filter operation comes first, this query will be faster because the outerjoin for large tables is much more expensive.","commit_id":"284e9ee3c1ca73e8d7350fe681df9c117081856b"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"14dacf7b1817615838199fba779a847ca5c5d4b3","unresolved":false,"context_lines":[{"line_number":239,"context_line":"                          models.Inventory.resource_class_id \u003d\u003d"},{"line_number":240,"context_line":"                          models.Allocation.resource_class_id))."},{"line_number":241,"context_line":"            filter(models.Inventory.resource_provider_id \u003d\u003d"},{"line_number":242,"context_line":"                   resource_provider_id)."},{"line_number":243,"context_line":"            group_by(models.Inventory.resource_class_id))"},{"line_number":244,"context_line":"        # TODO(cdent): This is noisy and annoying and should be"},{"line_number":245,"context_line":"        # changed before we finish this process, but doing it this"}],"source_content_type":"text/x-python","patch_set":45,"id":"1a122d0e_c729899d","line":242,"in_reply_to":"1a122d0e_1b280333","updated":"2016-05-03 18:30:41.000000000","message":"Very good question. I\u0027ll experiment and see.","commit_id":"284e9ee3c1ca73e8d7350fe681df9c117081856b"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"4b96f4c28158e29544bf4a1e59df8eacfda294a3","unresolved":false,"context_lines":[{"line_number":241,"context_line":"            filter(models.Inventory.resource_provider_id \u003d\u003d"},{"line_number":242,"context_line":"                   resource_provider_id)."},{"line_number":243,"context_line":"            group_by(models.Inventory.resource_class_id))"},{"line_number":244,"context_line":"        # TODO(cdent): This is noisy and annoying and should be"},{"line_number":245,"context_line":"        # changed before we finish this process, but doing it this"},{"line_number":246,"context_line":"        # way for now to keep moving. Perhaps some kind of method on"},{"line_number":247,"context_line":"        # ResourceUse itself?"},{"line_number":248,"context_line":"        results \u003d [ResourceUse(total\u003dinventory.total,"},{"line_number":249,"context_line":"                     resource_class\u003dfields.ResourceClass.from_index("},{"line_number":250,"context_line":"                         inventory.resource_class_id),"}],"source_content_type":"text/x-python","patch_set":45,"id":"1a122d0e_9bd533e7","line":247,"range":{"start_line":244,"start_character":23,"end_line":247,"end_character":29},"updated":"2016-05-03 14:55:15.000000000","message":"Maybe a solution here is to accumulate the `used` field of Allocation tables *only*, filtered by resource_provider_id and grouped by resource_class_id. Then read the related inventory tables and construct ResourceUse objects without the outerjoin, agree?","commit_id":"284e9ee3c1ca73e8d7350fe681df9c117081856b"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"14dacf7b1817615838199fba779a847ca5c5d4b3","unresolved":false,"context_lines":[{"line_number":244,"context_line":"        # TODO(cdent): This is noisy and annoying and should be"},{"line_number":245,"context_line":"        # changed before we finish this process, but doing it this"},{"line_number":246,"context_line":"        # way for now to keep moving. Perhaps some kind of method on"},{"line_number":247,"context_line":"        # ResourceUse itself?"},{"line_number":248,"context_line":"        results \u003d [ResourceUse(total\u003dinventory.total,"},{"line_number":249,"context_line":"                     resource_class\u003dfields.ResourceClass.from_index("},{"line_number":250,"context_line":"                         inventory.resource_class_id),"}],"source_content_type":"text/x-python","patch_set":45,"id":"1a122d0e_fa4ed86c","line":247,"in_reply_to":"1a122d0e_9bd533e7","updated":"2016-05-03 18:30:41.000000000","message":"The noise I was concerned with is the mapping between the db objects and nova objects. It\u0027s more verbose than seems ought to be necessary, a loop across Inventory.fields might do the trick.","commit_id":"284e9ee3c1ca73e8d7350fe681df9c117081856b"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"4fd76c486ad27ef9aca3a8419b35116c8721290f","unresolved":false,"context_lines":[{"line_number":244,"context_line":"        # TODO(cdent): This is noisy and annoying and should be"},{"line_number":245,"context_line":"        # changed before we finish this process, but doing it this"},{"line_number":246,"context_line":"        # way for now to keep moving. Perhaps some kind of method on"},{"line_number":247,"context_line":"        # ResourceUse itself?"},{"line_number":248,"context_line":"        results \u003d [ResourceUse(total\u003dinventory.total,"},{"line_number":249,"context_line":"                     resource_class\u003dfields.ResourceClass.from_index("},{"line_number":250,"context_line":"                         inventory.resource_class_id),"}],"source_content_type":"text/x-python","patch_set":45,"id":"1a122d0e_2c349048","line":247,"in_reply_to":"1a122d0e_fa4ed86c","updated":"2016-05-04 06:29:35.000000000","message":"Maybe this static method is better to be put in `ResourceUse` class.\n\nThe usage is like `ResourceUse.get_all(context, rp_id)`","commit_id":"284e9ee3c1ca73e8d7350fe681df9c117081856b"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"26b97810c1c46564e0712f52f1ffb3013fb5d50e","unresolved":false,"context_lines":[{"line_number":251,"context_line":"        # TODO(cdent): This is noisy and annoying and should be"},{"line_number":252,"context_line":"        # changed before we finish this process, but doing it this"},{"line_number":253,"context_line":"        # way for now to keep moving. Perhaps some kind of method on"},{"line_number":254,"context_line":"        # ResourceUse itself?"},{"line_number":255,"context_line":"        results \u003d [ResourceUse(total\u003dinventory.total,"},{"line_number":256,"context_line":"                     resource_class\u003dfields.ResourceClass.from_index("},{"line_number":257,"context_line":"                         inventory.resource_class_id),"}],"source_content_type":"text/x-python","patch_set":46,"id":"1a122d0e_e9a9c17d","line":254,"updated":"2016-05-04 20:06:49.000000000","message":"Yeah, it\u0027s ugly, but all you\u0027d probably do is move the ugly to that class. If you were to make the parameter simpler:\n\n    resource_class\u003dinventory.resource_class_id\n\nyou\u0027d have to modify ResourceClass to detect that it got an ID and not an object, and create the object there. Either way it\u0027s ugly.","commit_id":"ba7a9a4eb090b02ca708fbd8bc575eb4662e2251"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"ab85cf2a059cbd828bf8357d813d27959d949c10","unresolved":false,"context_lines":[{"line_number":486,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":487,"context_line":""},{"line_number":488,"context_line":"    fields \u003d {"},{"line_number":489,"context_line":"        \u0027resource_class\u0027: fields.ResourceClassField(read_only\u003dTrue),"},{"line_number":490,"context_line":"        \u0027total\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":491,"context_line":"        \u0027reserved\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":492,"context_line":"        \u0027min_unit\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":493,"context_line":"        \u0027max_unit\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":494,"context_line":"        \u0027step_size\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":495,"context_line":"        \u0027allocation_ratio\u0027: fields.FloatField(read_only\u003dTrue),"},{"line_number":496,"context_line":"        \u0027used\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":497,"context_line":"    }"},{"line_number":498,"context_line":""},{"line_number":499,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"1a122d0e_398170fc","line":496,"range":{"start_line":489,"start_character":8,"end_line":496,"end_character":52},"updated":"2016-05-04 05:18:35.000000000","message":"It is much more useful to provide a new property \u0027available\u0027 in this object that represents `(total*allocation_ratio)-reserved-used`.\n\nThe logic to calculate available resources is redundant in current scheduler:\n\nhttps://github.com/openstack/nova/blob/master/nova/scheduler/filters/disk_filter.py#L46-L48\n\nhttps://github.com/openstack/nova/blob/master/nova/scheduler/filters/ram_filter.py#L51-L53\n\nhttps://github.com/openstack/nova/blob/master/nova/scheduler/filters/core_filter.py#L59","commit_id":"ba7a9a4eb090b02ca708fbd8bc575eb4662e2251"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"bbf7297db821fd2b25991e8307857bb86f75a72a","unresolved":false,"context_lines":[{"line_number":493,"context_line":"        \u0027max_unit\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":494,"context_line":"        \u0027step_size\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":495,"context_line":"        \u0027allocation_ratio\u0027: fields.FloatField(read_only\u003dTrue),"},{"line_number":496,"context_line":"        \u0027used\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":497,"context_line":"    }"},{"line_number":498,"context_line":""},{"line_number":499,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"dab17558_307441ce","line":496,"in_reply_to":"1a122d0e_0355df4e","updated":"2016-05-06 02:38:06.000000000","message":"I tend to agree with replacing the \u0027used\u0027 field with \u0027available\u0027 field, as well as replacing \u0027total\u0027 field, \u0027reserved\u0027 field and \u0027allocation_ratio\u0027.\n\nIf this class is only used in scheduler service, there\u0027s actually no need to expose \u0027total\u0027, \u0027reserved\u0027, \u0027used\u0027 nor \u0027allocation_ratio\u0027 fields according to my current understanding during scheduling.\n\n\nThe only important fields are:\n\n1) \u0027available\u0027 field for CoreFilter, DiskFilter, RamFilter, DiskWeigher, RAMWeigher and scheduler cache consumption.\n\n2) \u0027exact_available\u0027 field for ExactCoreFilter, ExactDiskFilter, ExactRamFilter and scheduler cache consumption.\n\n3) \u0027min_unit\u0027, \u0027max_unit\u0027 and \u0027step_size\u0027 are necessary to check resource consumption in scheduler.","commit_id":"ba7a9a4eb090b02ca708fbd8bc575eb4662e2251"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"d33675e0b7d11bb71a8edeb082a0aeca7f68560f","unresolved":false,"context_lines":[{"line_number":486,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":487,"context_line":""},{"line_number":488,"context_line":"    fields \u003d {"},{"line_number":489,"context_line":"        \u0027resource_class\u0027: fields.ResourceClassField(read_only\u003dTrue),"},{"line_number":490,"context_line":"        \u0027total\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":491,"context_line":"        \u0027reserved\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":492,"context_line":"        \u0027min_unit\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":493,"context_line":"        \u0027max_unit\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":494,"context_line":"        \u0027step_size\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":495,"context_line":"        \u0027allocation_ratio\u0027: fields.FloatField(read_only\u003dTrue),"},{"line_number":496,"context_line":"        \u0027used\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":497,"context_line":"    }"},{"line_number":498,"context_line":""},{"line_number":499,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"1a122d0e_580a3921","line":496,"range":{"start_line":489,"start_character":8,"end_line":496,"end_character":52},"in_reply_to":"1a122d0e_09099d95","updated":"2016-05-04 22:06:48.000000000","message":"Elaborating: a common need with this object is to see how much is available. Knowing how much is used may be of interest in some cases, but in many more the concern is \"Are there enough Resource X\". So what Yingxin is proposing (if I read it correctly) would be to add a read-only property to this object that will \"return\" the result of the calculation in his second comment above. As things currently are proposed, any code that uses this object would have to duplicate this calculation if it needed to know how much of Resource X was available. By providing a property that handles that, you eliminate the need for duplicated code.","commit_id":"ba7a9a4eb090b02ca708fbd8bc575eb4662e2251"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"26b97810c1c46564e0712f52f1ffb3013fb5d50e","unresolved":false,"context_lines":[{"line_number":486,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":487,"context_line":""},{"line_number":488,"context_line":"    fields \u003d {"},{"line_number":489,"context_line":"        \u0027resource_class\u0027: fields.ResourceClassField(read_only\u003dTrue),"},{"line_number":490,"context_line":"        \u0027total\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":491,"context_line":"        \u0027reserved\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":492,"context_line":"        \u0027min_unit\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":493,"context_line":"        \u0027max_unit\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":494,"context_line":"        \u0027step_size\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":495,"context_line":"        \u0027allocation_ratio\u0027: fields.FloatField(read_only\u003dTrue),"},{"line_number":496,"context_line":"        \u0027used\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":497,"context_line":"    }"},{"line_number":498,"context_line":""},{"line_number":499,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"1a122d0e_09099d95","line":496,"range":{"start_line":489,"start_character":8,"end_line":496,"end_character":52},"in_reply_to":"1a122d0e_1aec4bf2","updated":"2016-05-04 20:06:49.000000000","message":"Yes, I agree that having a calculated property on the object would be cleaner.","commit_id":"ba7a9a4eb090b02ca708fbd8bc575eb4662e2251"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"cec1a27e3553a23624be60a03119e0172c9b99d1","unresolved":false,"context_lines":[{"line_number":486,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":487,"context_line":""},{"line_number":488,"context_line":"    fields \u003d {"},{"line_number":489,"context_line":"        \u0027resource_class\u0027: fields.ResourceClassField(read_only\u003dTrue),"},{"line_number":490,"context_line":"        \u0027total\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":491,"context_line":"        \u0027reserved\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":492,"context_line":"        \u0027min_unit\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":493,"context_line":"        \u0027max_unit\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":494,"context_line":"        \u0027step_size\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":495,"context_line":"        \u0027allocation_ratio\u0027: fields.FloatField(read_only\u003dTrue),"},{"line_number":496,"context_line":"        \u0027used\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":497,"context_line":"    }"},{"line_number":498,"context_line":""},{"line_number":499,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"1a122d0e_1aec4bf2","line":496,"range":{"start_line":489,"start_character":8,"end_line":496,"end_character":52},"in_reply_to":"1a122d0e_398170fc","updated":"2016-05-04 15:14:45.000000000","message":"Sorry, it\u0027s `(total - reserved) * allocation_ratio - used`","commit_id":"ba7a9a4eb090b02ca708fbd8bc575eb4662e2251"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"a7e932a523ef5a31bc823e8984b2f82c50b4c6c2","unresolved":false,"context_lines":[{"line_number":493,"context_line":"        \u0027max_unit\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":494,"context_line":"        \u0027step_size\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":495,"context_line":"        \u0027allocation_ratio\u0027: fields.FloatField(read_only\u003dTrue),"},{"line_number":496,"context_line":"        \u0027used\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":497,"context_line":"    }"},{"line_number":498,"context_line":""},{"line_number":499,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"1a122d0e_0355df4e","line":496,"in_reply_to":"1a122d0e_580a3921","updated":"2016-05-05 12:09:13.000000000","message":"Thanks for the clarification Ed, my confusion yesterday seems to stem from me thinking that the suggestion was to replace the \u0027used\u0027 field with an \u0027available\u0027 field, not add it.\n\nAdding it as a property makes good sense.","commit_id":"ba7a9a4eb090b02ca708fbd8bc575eb4662e2251"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"ed04e43364b605fa04dca1c997780d2967845ccc","unresolved":false,"context_lines":[{"line_number":493,"context_line":"        \u0027max_unit\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":494,"context_line":"        \u0027step_size\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":495,"context_line":"        \u0027allocation_ratio\u0027: fields.FloatField(read_only\u003dTrue),"},{"line_number":496,"context_line":"        \u0027used\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":497,"context_line":"    }"},{"line_number":498,"context_line":""},{"line_number":499,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"dab17558_3b187d1d","line":496,"in_reply_to":"dab17558_307441ce","updated":"2016-05-06 19:38:33.000000000","message":"The primary purpose of the class is provide a way to encapsulate the representation of resources in the placement API: https://review.openstack.org/#/c/293104/\n\nWhen you GET a resource pool from the api it includes a resources element that is a dictionary of resource class keys, with values that come from the ResourceUse object.\n\nIn that sense all the fields are important, because all the fields are needed in the representation as they are there for information purposes, not calculation.\n\nIn the scheduler, the corresponding fields are expected to be used in SQL calls, not via nova objects.","commit_id":"ba7a9a4eb090b02ca708fbd8bc575eb4662e2251"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"b01b77c24547064b07cedab7ee9a0fe9202d3f6f","unresolved":false,"context_lines":[{"line_number":266,"context_line":"                for inventory, used in query.all()]"},{"line_number":267,"context_line":"        return results"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    @staticmethod"},{"line_number":270,"context_line":"    def _update_aggregates(context, resource_provider_id,"},{"line_number":271,"context_line":"                                           aggregates):"},{"line_number":272,"context_line":"        for aggregate in aggregates:"},{"line_number":273,"context_line":"            db_rpa \u003d models.ResourceProviderAggregate("},{"line_number":274,"context_line":"                resource_provider_id\u003dresource_provider_id,"}],"source_content_type":"text/x-python","patch_set":50,"id":"dab17558_df24deb3","line":271,"range":{"start_line":269,"start_character":4,"end_line":271,"end_character":55},"updated":"2016-05-12 11:28:12.000000000","message":"The 2nd argument can be treated as \u0027self.id\u0027 and this method is not necessarily a staticmethod.","commit_id":"ace2feea2305301c16676250d4630e1759e63092"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"4cdf1dac788aeca1be349cc2fb73ff516caccfa3","unresolved":false,"context_lines":[{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    @staticmethod"},{"line_number":270,"context_line":"    def _update_aggregates(context, resource_provider_id,"},{"line_number":271,"context_line":"                                           aggregates):"},{"line_number":272,"context_line":"        for aggregate in aggregates:"},{"line_number":273,"context_line":"            db_rpa \u003d models.ResourceProviderAggregate("},{"line_number":274,"context_line":"                resource_provider_id\u003dresource_provider_id,"}],"source_content_type":"text/x-python","patch_set":50,"id":"dab17558_59d592b7","line":271,"in_reply_to":"dab17558_df24deb3","updated":"2016-05-12 19:59:22.000000000","message":"The preponderance of staticmethod is the result of following examples in other objects where a lot of the of methods that go to the database or otherwise do internal data-mashing are made static, presumably to ease some aspects of unit-testing.\n\nIn the absence of other feedback I\u0027ve been following that pattern. I agree it is cumbersome and feels a bit wrong. Since the style of testing I\u0027ve been doing hasn\u0027t required the staticmethods I should probably get rid of them.","commit_id":"ace2feea2305301c16676250d4630e1759e63092"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"b01b77c24547064b07cedab7ee9a0fe9202d3f6f","unresolved":false,"context_lines":[{"line_number":285,"context_line":"        if not result:"},{"line_number":286,"context_line":"            raise exception.NotFound()"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"    @staticmethod"},{"line_number":289,"context_line":"    def _update_resources(context, resource_provider, resources,"},{"line_number":290,"context_line":"                          existing_resources, existing_inventories):"},{"line_number":291,"context_line":"        \"\"\"Set the resources associated with this resource provider"},{"line_number":292,"context_line":""},{"line_number":293,"context_line":"        This means creating, updating or deleting Inventory objects to"}],"source_content_type":"text/x-python","patch_set":50,"id":"dab17558_84020b0a","line":290,"range":{"start_line":288,"start_character":4,"end_line":290,"end_character":68},"updated":"2016-05-12 11:28:12.000000000","message":"The 2nd argument can be treated as \u0027self\u0027 and this method is not necessarily a staticmethod.","commit_id":"ace2feea2305301c16676250d4630e1759e63092"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"b01b77c24547064b07cedab7ee9a0fe9202d3f6f","unresolved":false,"context_lines":[{"line_number":300,"context_line":"        for resource in resources:"},{"line_number":301,"context_line":"            if resource.resource_class in existing_resources:"},{"line_number":302,"context_line":"                current_resource \u003d existing_resources[resource.resource_class]"},{"line_number":303,"context_line":"                if current_resource.used \u003e ("},{"line_number":304,"context_line":"                        (resource.total - resource.reserved)"},{"line_number":305,"context_line":"                        * resource.allocation_ratio):"},{"line_number":306,"context_line":"                    raise exception.ObjectActionError("},{"line_number":307,"context_line":"                        action\u003d\u0027save\u0027,"},{"line_number":308,"context_line":"                        reason\u003d\u0027resource capacity mismatch for %s\u0027"}],"source_content_type":"text/x-python","patch_set":50,"id":"dab17558_7f3052e9","line":305,"range":{"start_line":303,"start_character":19,"end_line":305,"end_character":52},"updated":"2016-05-12 11:28:12.000000000","message":"current_resource.available \u003c 0","commit_id":"ace2feea2305301c16676250d4630e1759e63092"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"4cdf1dac788aeca1be349cc2fb73ff516caccfa3","unresolved":false,"context_lines":[{"line_number":302,"context_line":"                current_resource \u003d existing_resources[resource.resource_class]"},{"line_number":303,"context_line":"                if current_resource.used \u003e ("},{"line_number":304,"context_line":"                        (resource.total - resource.reserved)"},{"line_number":305,"context_line":"                        * resource.allocation_ratio):"},{"line_number":306,"context_line":"                    raise exception.ObjectActionError("},{"line_number":307,"context_line":"                        action\u003d\u0027save\u0027,"},{"line_number":308,"context_line":"                        reason\u003d\u0027resource capacity mismatch for %s\u0027"}],"source_content_type":"text/x-python","patch_set":50,"id":"dab17558_b9011e3c","line":305,"in_reply_to":"dab17558_7f3052e9","updated":"2016-05-12 19:59:22.000000000","message":"ah, of course, thanks","commit_id":"ace2feea2305301c16676250d4630e1759e63092"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"6905bcbc9044aecf66c8dac6411319ae58636fa4","unresolved":false,"context_lines":[{"line_number":302,"context_line":"                current_resource \u003d existing_resources[resource.resource_class]"},{"line_number":303,"context_line":"                if current_resource.used \u003e ("},{"line_number":304,"context_line":"                        (resource.total - resource.reserved)"},{"line_number":305,"context_line":"                        * resource.allocation_ratio):"},{"line_number":306,"context_line":"                    raise exception.ObjectActionError("},{"line_number":307,"context_line":"                        action\u003d\u0027save\u0027,"},{"line_number":308,"context_line":"                        reason\u003d\u0027resource capacity mismatch for %s\u0027"}],"source_content_type":"text/x-python","patch_set":50,"id":"dab17558_cf47f049","line":305,"in_reply_to":"dab17558_b9011e3c","updated":"2016-05-12 20:12:32.000000000","message":"actually no, that\u0027s not an \"of course\"\n\nWe\u0027re comparing two different resource representations here: The `used` created from the on-disk allocations versus the newly provided resource data in the not-yet-persisted object.\n\nWhat we are checking for here is that the new constraints would not be the existing allocations into an impossible conflict.\n\nThis comparison is reason one of why this method is so long: we\u0027re comparing presumptive resource constraints with stored resource values (allocations).\n\n(The other reason it is so long is because I\u0027ve yet to do any extract method refactoring).","commit_id":"ace2feea2305301c16676250d4630e1759e63092"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"7bb57f69284c937d1c04a2dba9b22d5586038ff2","unresolved":false,"context_lines":[{"line_number":426,"context_line":""},{"line_number":427,"context_line":"    fields \u003d {"},{"line_number":428,"context_line":"        \u0027id\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":429,"context_line":"        \u0027resource_provider\u0027: fields.ObjectField(\u0027ResourceProvider\u0027),"},{"line_number":430,"context_line":"        \u0027resource_class\u0027: fields.ResourceClassField(read_only\u003dTrue),"},{"line_number":431,"context_line":"        \u0027total\u0027: fields.NonNegativeIntegerField(),"},{"line_number":432,"context_line":"        \u0027reserved\u0027: fields.NonNegativeIntegerField(default\u003d0),"}],"source_content_type":"text/x-python","patch_set":50,"id":"dab17558_02cf2ab4","line":429,"range":{"start_line":429,"start_character":8,"end_line":429,"end_character":68},"updated":"2016-05-12 12:23:01.000000000","message":"The logical meaning here is \"Inventory has a resource provider\", but it makes more sense to me that \"A resource provider has multiple Resources, and each Resource has an Inventory and multiple Allocations\".","commit_id":"ace2feea2305301c16676250d4630e1759e63092"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"4cdf1dac788aeca1be349cc2fb73ff516caccfa3","unresolved":false,"context_lines":[{"line_number":426,"context_line":""},{"line_number":427,"context_line":"    fields \u003d {"},{"line_number":428,"context_line":"        \u0027id\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":429,"context_line":"        \u0027resource_provider\u0027: fields.ObjectField(\u0027ResourceProvider\u0027),"},{"line_number":430,"context_line":"        \u0027resource_class\u0027: fields.ResourceClassField(read_only\u003dTrue),"},{"line_number":431,"context_line":"        \u0027total\u0027: fields.NonNegativeIntegerField(),"},{"line_number":432,"context_line":"        \u0027reserved\u0027: fields.NonNegativeIntegerField(default\u003d0),"}],"source_content_type":"text/x-python","patch_set":50,"id":"dab17558_995a5a4f","line":429,"in_reply_to":"dab17558_02cf2ab4","updated":"2016-05-12 19:59:22.000000000","message":"The structure of the objects is a bit weird, but one of the issues we have is that Inventory, InventoryList and the initial ResourceProvider object was merged back in February, which means it is out in the wild and whatever we do now needs to be an evolution from there and cannot be a a wholesale change.","commit_id":"ace2feea2305301c16676250d4630e1759e63092"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"7bb57f69284c937d1c04a2dba9b22d5586038ff2","unresolved":false,"context_lines":[{"line_number":509,"context_line":""},{"line_number":510,"context_line":"    fields \u003d {"},{"line_number":511,"context_line":"        \u0027id\u0027: fields.IntegerField(),"},{"line_number":512,"context_line":"        \u0027resource_provider\u0027: fields.ObjectField(\u0027ResourceProvider\u0027),"},{"line_number":513,"context_line":"        \u0027consumer_id\u0027: fields.UUIDField(),"},{"line_number":514,"context_line":"        \u0027resource_class\u0027: fields.ResourceClassField(),"},{"line_number":515,"context_line":"        \u0027used\u0027: fields.IntegerField(),"}],"source_content_type":"text/x-python","patch_set":50,"id":"dab17558_c26b6284","line":512,"range":{"start_line":512,"start_character":8,"end_line":512,"end_character":68},"updated":"2016-05-12 12:23:01.000000000","message":"Same to @429","commit_id":"ace2feea2305301c16676250d4630e1759e63092"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"b01b77c24547064b07cedab7ee9a0fe9202d3f6f","unresolved":false,"context_lines":[{"line_number":581,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":582,"context_line":""},{"line_number":583,"context_line":"    fields \u003d {"},{"line_number":584,"context_line":"        \u0027resource_class\u0027: fields.ResourceClassField(read_only\u003dTrue),"},{"line_number":585,"context_line":"        \u0027total\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":586,"context_line":"        \u0027reserved\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":587,"context_line":"        \u0027min_unit\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":588,"context_line":"        \u0027max_unit\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":589,"context_line":"        \u0027step_size\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":590,"context_line":"        \u0027allocation_ratio\u0027: fields.FloatField(read_only\u003dTrue),"},{"line_number":591,"context_line":"        \u0027used\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":592,"context_line":"    }"},{"line_number":593,"context_line":""}],"source_content_type":"text/x-python","patch_set":50,"id":"dab17558_bf663a27","line":590,"range":{"start_line":584,"start_character":8,"end_line":590,"end_character":62},"updated":"2016-05-12 11:28:12.000000000","message":"All these fields are duplicated with Inventory class, why not just provide a field `\u0027inventory\u0027: fields.ObjectField(\u0027Inventory\u0027, nullable\u003dFalse)` instead?\n\nAnd the code from @289 are also trying to update/add Inventory to db through `ResourceUse` rather than the actual `Inventory` class, it looks weird.","commit_id":"ace2feea2305301c16676250d4630e1759e63092"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"4cdf1dac788aeca1be349cc2fb73ff516caccfa3","unresolved":false,"context_lines":[{"line_number":587,"context_line":"        \u0027min_unit\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":588,"context_line":"        \u0027max_unit\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":589,"context_line":"        \u0027step_size\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":590,"context_line":"        \u0027allocation_ratio\u0027: fields.FloatField(read_only\u003dTrue),"},{"line_number":591,"context_line":"        \u0027used\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":592,"context_line":"    }"},{"line_number":593,"context_line":""}],"source_content_type":"text/x-python","patch_set":50,"id":"dab17558_6c56d207","line":590,"in_reply_to":"dab17558_1f866513","updated":"2016-05-12 19:59:22.000000000","message":"This was generated as a convenience object to make aspects of dealing with the resource-pool concept at the HTTP API a bit easier. There the initial design (and jsonschema) spoke of resources which weren\u0027t quite the same thing as an Inventory, but a mapping over a single Inventory and all its AllocationS.\n\nIt does feel crufty and weird. The state things are in right now is \"what I was able to make work with the information I had available\". Now that feedback has started we can hopefully get it to something more clean and correct.","commit_id":"ace2feea2305301c16676250d4630e1759e63092"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"7bb57f69284c937d1c04a2dba9b22d5586038ff2","unresolved":false,"context_lines":[{"line_number":581,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":582,"context_line":""},{"line_number":583,"context_line":"    fields \u003d {"},{"line_number":584,"context_line":"        \u0027resource_class\u0027: fields.ResourceClassField(read_only\u003dTrue),"},{"line_number":585,"context_line":"        \u0027total\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":586,"context_line":"        \u0027reserved\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":587,"context_line":"        \u0027min_unit\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":588,"context_line":"        \u0027max_unit\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":589,"context_line":"        \u0027step_size\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":590,"context_line":"        \u0027allocation_ratio\u0027: fields.FloatField(read_only\u003dTrue),"},{"line_number":591,"context_line":"        \u0027used\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":592,"context_line":"    }"},{"line_number":593,"context_line":""}],"source_content_type":"text/x-python","patch_set":50,"id":"dab17558_1f866513","line":590,"range":{"start_line":584,"start_character":8,"end_line":590,"end_character":62},"in_reply_to":"dab17558_bf663a27","updated":"2016-05-12 12:23:01.000000000","message":"Like i said, if the fields in Inventory class are changed, it is easy to forget to update the corresponding fields in this class. Because they are duplicated.\n\nSee fields #L431-L436","commit_id":"ace2feea2305301c16676250d4630e1759e63092"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"40588338183346749e28ebdca55f01295fe0ab90","unresolved":false,"context_lines":[{"line_number":593,"context_line":"    @property"},{"line_number":594,"context_line":"    def available(self):"},{"line_number":595,"context_line":"        \"\"\"Convenience property to calculate availability.\"\"\""},{"line_number":596,"context_line":"        return self.capacity - self.used"},{"line_number":597,"context_line":""},{"line_number":598,"context_line":""},{"line_number":599,"context_line":"@base.NovaObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":51,"id":"dab17558_03cd27b7","line":596,"updated":"2016-05-13 15:31:01.000000000","message":"Like it!","commit_id":"1749c49010cfc80326f257c65471fc9e19f7ac2d"}],"nova/tests/functional/db/test_resource_provider.py":[{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"8117e7138ce213a46442e9cf655e6b1a0e9d0bdd","unresolved":false,"context_lines":[{"line_number":136,"context_line":"                                                    aggregate_2])"},{"line_number":137,"context_line":"        resource_provider \u003d objects.ResourceProvider("},{"line_number":138,"context_line":"            self.context, name\u003duuidsentinel.resource_pool_name,"},{"line_number":139,"context_line":"            uuid\u003duuidsentinel.resource_pool_uuid)"},{"line_number":140,"context_line":"        resource_provider.create()"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        rp \u003d objects.ResourcePool(self.context,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9aed3d3a_5d6433e4","line":139,"range":{"start_line":139,"start_character":39,"end_line":139,"end_character":43},"updated":"2016-02-26 15:02:43.000000000","message":"This (and the line above) should be named:\n    s/pool/provider","commit_id":"dc7dda042bd826779d39ba5a3c5f56bec9eb5778"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"d916850ef83087816ec664ec78b27d6954a1a2ff","unresolved":false,"context_lines":[{"line_number":136,"context_line":"                                                    aggregate_2])"},{"line_number":137,"context_line":"        resource_provider \u003d objects.ResourceProvider("},{"line_number":138,"context_line":"            self.context, name\u003duuidsentinel.resource_pool_name,"},{"line_number":139,"context_line":"            uuid\u003duuidsentinel.resource_pool_uuid)"},{"line_number":140,"context_line":"        resource_provider.create()"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        rp \u003d objects.ResourcePool(self.context,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9aed3d3a_43f731f5","line":139,"in_reply_to":"9aed3d3a_5d6433e4","updated":"2016-02-26 16:41:39.000000000","message":"Done","commit_id":"dc7dda042bd826779d39ba5a3c5f56bec9eb5778"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"8117e7138ce213a46442e9cf655e6b1a0e9d0bdd","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        self.assertIn(aggregate_2, rp.aggregates)"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"        retrieved_rp \u003d objects.ResourcePool.get_by_resource_provider_uuid("},{"line_number":153,"context_line":"            self.context, uuidsentinel.resource_pool_uuid)"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"        self.assertEqual(resource_provider.name,"},{"line_number":156,"context_line":"                         retrieved_rp.resource_provider.name)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9aed3d3a_bd6d7f9a","line":153,"range":{"start_line":153,"start_character":48,"end_line":153,"end_character":52},"updated":"2016-02-26 15:02:43.000000000","message":"Yeah, it seems odd to have \u0027get_by_resource_provider_uuid\u0027, and then pass in a thing named \u0027resource_pool_id\u0027.","commit_id":"dc7dda042bd826779d39ba5a3c5f56bec9eb5778"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"d916850ef83087816ec664ec78b27d6954a1a2ff","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        self.assertIn(aggregate_2, rp.aggregates)"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"        retrieved_rp \u003d objects.ResourcePool.get_by_resource_provider_uuid("},{"line_number":153,"context_line":"            self.context, uuidsentinel.resource_pool_uuid)"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"        self.assertEqual(resource_provider.name,"},{"line_number":156,"context_line":"                         retrieved_rp.resource_provider.name)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9aed3d3a_83d93986","line":153,"in_reply_to":"9aed3d3a_bd6d7f9a","updated":"2016-02-26 16:41:39.000000000","message":"Done","commit_id":"dc7dda042bd826779d39ba5a3c5f56bec9eb5778"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"8117e7138ce213a46442e9cf655e6b1a0e9d0bdd","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        # NOTE(cdent): I was hoping objects themselves would be"},{"line_number":163,"context_line":"        # comparable, but it seems not, so comparing contents."},{"line_number":164,"context_line":"        self.assertEqual(aggregate_1.uuid, retrieved_rp.aggregates[0].uuid)"},{"line_number":165,"context_line":"        self.assertEqual(aggregate_2.uuid, retrieved_rp.aggregates[1].uuid)"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"        # try to create the same rp again"},{"line_number":168,"context_line":"        rp \u003d objects.ResourcePool(self.context,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9aed3d3a_fdba67e0","line":165,"updated":"2016-02-26 15:02:43.000000000","message":"If you\u0027re going to test this way, maybe name the aggs \u0027aggregate_0\u0027 and \u0027aggregate_1\u0027?","commit_id":"dc7dda042bd826779d39ba5a3c5f56bec9eb5778"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"d916850ef83087816ec664ec78b27d6954a1a2ff","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        # NOTE(cdent): I was hoping objects themselves would be"},{"line_number":163,"context_line":"        # comparable, but it seems not, so comparing contents."},{"line_number":164,"context_line":"        self.assertEqual(aggregate_1.uuid, retrieved_rp.aggregates[0].uuid)"},{"line_number":165,"context_line":"        self.assertEqual(aggregate_2.uuid, retrieved_rp.aggregates[1].uuid)"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"        # try to create the same rp again"},{"line_number":168,"context_line":"        rp \u003d objects.ResourcePool(self.context,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9aed3d3a_e3e80513","line":165,"in_reply_to":"9aed3d3a_fdba67e0","updated":"2016-02-26 16:41:39.000000000","message":"Done","commit_id":"dc7dda042bd826779d39ba5a3c5f56bec9eb5778"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"34fc20616829fa65fc4ba641c9c836eda343a5b5","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        self.assertEqual(0, len(allocations))"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"class ResourcePoolTestCase(test.NoDBTestCase):"},{"line_number":171,"context_line":"    \"\"\"Test resource-pool objects\u0027 lifecycles.\"\"\""},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa0719c6_652b74ec","line":170,"updated":"2016-03-21 14:52:18.000000000","message":"At this stage these are \"let\u0027s go \u0027xplorin!\" tests.","commit_id":"206b12774c265bba9a36d0af140ced0ed66ae857"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"ab85cf2a059cbd828bf8357d813d27959d949c10","unresolved":false,"context_lines":[{"line_number":197,"context_line":"        self.assertEqual([], retrieved_rp.resources)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"        retrieved_rp.aggregates.sort(key\u003dlambda x: x.id)"},{"line_number":200,"context_line":"        # NOTE(cdent): I was hoping objects themselves would be"},{"line_number":201,"context_line":"        # comparable, but it seems not, so comparing contents."},{"line_number":202,"context_line":"        self.assertEqual(aggregate_0.uuid, retrieved_rp.aggregates[0].uuid)"},{"line_number":203,"context_line":"        self.assertEqual(aggregate_1.uuid, retrieved_rp.aggregates[1].uuid)"},{"line_number":204,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"1a122d0e_624336b0","line":201,"range":{"start_line":200,"start_character":23,"end_line":201,"end_character":62},"updated":"2016-05-04 05:18:35.000000000","message":"A solution here is to use the mixin of oslo_versionedobject `base.ComparableVersionedObject`. Basically it provides an built-in implementation for `__eq__` to compare primitive contents of objects.","commit_id":"ba7a9a4eb090b02ca708fbd8bc575eb4662e2251"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"40588338183346749e28ebdca55f01295fe0ab90","unresolved":false,"context_lines":[{"line_number":211,"context_line":""},{"line_number":212,"context_line":"    def test_resource_provider_inventories(self):"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"        aggregate \u003d objects.Aggregate(self.context, name\u003d\u0027myagg\u0027)"},{"line_number":215,"context_line":"        aggregate.create()"},{"line_number":216,"context_line":"        aggregates \u003d [aggregate]"},{"line_number":217,"context_line":""}],"source_content_type":"text/x-python","patch_set":51,"id":"dab17558_005d1d79","line":214,"updated":"2016-05-13 15:31:01.000000000","message":"Odd blank line above this one, making the method declaration appear to be floating unattached in space...","commit_id":"1749c49010cfc80326f257c65471fc9e19f7ac2d"}],"nova/tests/unit/objects/test_resource_provider.py":[{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"fb5dc2b0726b381aa802e1c4e5993901d78bf735","unresolved":false,"context_lines":[{"line_number":10,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":11,"context_line":"#    under the License."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"import mock"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"from nova import exception"}],"source_content_type":"text/x-python","patch_set":37,"id":"1a122d0e_d3b8cb77","line":13,"updated":"2016-04-20 03:12:41.000000000","message":"It is an unnecessary change.","commit_id":"f49efe3f572e60598f957c7307f040bad3991e14"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"42c8e73ddf69fad9248446caa1d3c229ff1c848d","unresolved":false,"context_lines":[{"line_number":10,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":11,"context_line":"#    under the License."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"import mock"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"from nova import exception"}],"source_content_type":"text/x-python","patch_set":37,"id":"1a122d0e_b3538238","line":13,"in_reply_to":"1a122d0e_d3b8cb77","updated":"2016-04-20 09:15:14.000000000","message":"Done","commit_id":"f49efe3f572e60598f957c7307f040bad3991e14"}]}