)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ee533090ea8761ff72e994369295042b8cf972bb","unresolved":false,"context_lines":[{"line_number":68,"context_line":"and UsageSpec objects introduced in this patch."},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"Change-Id: I0261bf3a252fac657496c62790818d24871678e1"},{"line_number":71,"context_line":"Partially-implements: resource-objects"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"7a8c0949_d5d18efc","line":71,"updated":"2014-11-03 23:34:25.000000000","message":"nit, should be \n\nPartially Implements blueprint resource-objects","commit_id":"2048a1118dc867a3f27d089557906a7289e99bd3"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"de98ab8a374e1ec4c55aa968092b4b4ebae946d9","unresolved":false,"context_lines":[{"line_number":68,"context_line":"and UsageSpec objects introduced in this patch."},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"Change-Id: I0261bf3a252fac657496c62790818d24871678e1"},{"line_number":71,"context_line":"Partially-implements: resource-objects"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"5a890539_5bcd05ba","line":71,"updated":"2014-12-05 12:41:17.000000000","message":"Nit-pick should be  \u0027Partially-implements: blueprint resource-objects\u0027 otherwise gerrit doesn\u0027t linkify the blueprint name","commit_id":"260133b5b6c901307bd2c98d2b8d7a80d9aa7db0"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"bae4051337afa33edd20797d4978bd0e97b32c23","unresolved":false,"context_lines":[{"line_number":54,"context_line":"  way of calculating and storing usage records: in a JSON blob that is"},{"line_number":55,"context_line":"  passed from one plugin to another, with the plugin expected to"},{"line_number":56,"context_line":"  update the supplied dictionary with a usage record that only it"},{"line_number":57,"context_line":"  understands."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"This patch adds the basic object models that will enable us to have a"},{"line_number":60,"context_line":"single interface for comparing resource amounts and usages.  It adds"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"3a50d1a3_b08100c6","line":57,"updated":"2015-07-23 21:34:22.000000000","message":"While I think the above explanation is good for a BP, maybe it\u0027s just not needed for a commit msg :-)\n\nI don\u0027t want to -1 for that, but if you need to provide a new patch, maybe removing the above is fine to me.","commit_id":"0e4d22d55b701d49b36624062cf772e833f333a6"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"c9fdc5bc28b8f4b300094fc85f164ff714cc1ad2","unresolved":false,"context_lines":[{"line_number":54,"context_line":"  way of calculating and storing usage records: in a JSON blob that is"},{"line_number":55,"context_line":"  passed from one plugin to another, with the plugin expected to"},{"line_number":56,"context_line":"  update the supplied dictionary with a usage record that only it"},{"line_number":57,"context_line":"  understands."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"This patch adds the basic object models that will enable us to have a"},{"line_number":60,"context_line":"single interface for comparing resource amounts and usages.  It adds"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":22,"id":"3a50d1a3_decd2832","line":57,"updated":"2015-07-28 09:37:34.000000000","message":"see PS19, I would appreciate a shorter commit msg","commit_id":"f6ff91a6bdecbdd3ac687ad8f82c237c19b82e2e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"01e67ddba36f238c31075159c491489ac4793e80","unresolved":false,"context_lines":[{"line_number":54,"context_line":"  way of calculating and storing usage records: in a JSON blob that is"},{"line_number":55,"context_line":"  passed from one plugin to another, with the plugin expected to"},{"line_number":56,"context_line":"  update the supplied dictionary with a usage record that only it"},{"line_number":57,"context_line":"  understands."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"This patch adds the basic object models that will enable us to have a"},{"line_number":60,"context_line":"single interface for comparing resource amounts and usages.  It adds"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"3a50d1a3_362d3a46","line":57,"updated":"2015-07-28 16:48:17.000000000","message":"Erm, that\u0027s just 4 PSs that I\u0027m nitpicking the lenght of that commit msg. Lemme just again yakshaving that :-)\n\n(nit, nit, nit)","commit_id":"ff63e8f9098ed918272a8fd89e062c52ac4da707"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"85d3bb5716e5babb905cd1c59b2ff142bd7cf14c","unresolved":false,"context_lines":[{"line_number":15,"context_line":"  For example: {\"local_gb\": X, \"local_gb_used\": Y}"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"- NUMA topology amounts and usages are stored as a serialized"},{"line_number":18,"context_line":"  nova.virt.hardware.VirtNUMAInstanceTopology/VirtNUMAHostTopology"},{"line_number":19,"context_line":"  pair in the old_resources dictionary under the \u0027numa_topology\u0027"},{"line_number":20,"context_line":"  key: {\"numa_topology\": JSON-BLOB}"},{"line_number":21,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":28,"id":"fa80f949_6344f373","line":18,"updated":"2015-11-04 13:26:17.000000000","message":"seems this was moved to objects.instance_numa_topology?","commit_id":"61940a97c2345a2deeccd43f5a562c98243593f4"},{"author":{"_account_id":10618,"name":"Pavel Kholkin","email":"p.v.holkin@mail.ru","username":"pkholkin"},"change_message_id":"7927bb6280f9bff50ac7d3c04733c15cb809824f","unresolved":false,"context_lines":[{"line_number":68,"context_line":""},{"line_number":69,"context_line":"Co-Authored-By: Alexis Lee \u003calexisl@hp.com\u003e"},{"line_number":70,"context_line":"Change-Id: I0261bf3a252fac657496c62790818d24871678e1"},{"line_number":71,"context_line":"Partially-implements: resource-objects"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":28,"id":"7a740942_246b06f3","line":71,"updated":"2015-12-07 12:50:28.000000000","message":"no direct link to bp here","commit_id":"61940a97c2345a2deeccd43f5a562c98243593f4"}],"nova/objects/__init__.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"9ccb8545302fdb9c0b92e72910777a49330c6242","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    __import__(\u0027nova.objects.security_group\u0027)"},{"line_number":56,"context_line":"    __import__(\u0027nova.objects.security_group_rule\u0027)"},{"line_number":57,"context_line":"    __import__(\u0027nova.objects.service\u0027)"},{"line_number":58,"context_line":"    __import__(\u0027nova.objects.tag\u0027)"},{"line_number":59,"context_line":"    __import__(\u0027nova.objects.virt_cpu_topology\u0027)"},{"line_number":60,"context_line":"    __import__(\u0027nova.objects.virtual_interface\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_1f14613f","line":58,"updated":"2015-01-18 15:58:20.000000000","message":"this looks like it can be done in a separate patch - but that is nit picking","commit_id":"83030092eb719eef00042d944b5b1a3eaf03ed0a"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"1e1551c218df560cfbbf5192f1e31c1843137dbe","unresolved":false,"context_lines":[{"line_number":53,"context_line":"    __import__(\u0027nova.objects.numa\u0027)"},{"line_number":54,"context_line":"    __import__(\u0027nova.objects.pci_device\u0027)"},{"line_number":55,"context_line":"    __import__(\u0027nova.objects.pci_device_pool\u0027)"},{"line_number":56,"context_line":"    __import__(\u0027nova.objects.tag\u0027)"},{"line_number":57,"context_line":"    __import__(\u0027nova.objects.quotas\u0027)"},{"line_number":58,"context_line":"    __import__(\u0027nova.objects.security_group\u0027)"},{"line_number":59,"context_line":"    __import__(\u0027nova.objects.security_group_rule\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3afb71cf_aa5dce7a","side":"PARENT","line":56,"updated":"2015-06-08 15:03:12.000000000","message":"This smells like a merge error","commit_id":"5a1a1b7b71f88969f68bf498d1e2b594cd34c164"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3699e856ca64eb5872647fbe20507e276b6fb896","unresolved":false,"context_lines":[{"line_number":53,"context_line":"    __import__(\u0027nova.objects.numa\u0027)"},{"line_number":54,"context_line":"    __import__(\u0027nova.objects.pci_device\u0027)"},{"line_number":55,"context_line":"    __import__(\u0027nova.objects.pci_device_pool\u0027)"},{"line_number":56,"context_line":"    __import__(\u0027nova.objects.tag\u0027)"},{"line_number":57,"context_line":"    __import__(\u0027nova.objects.quotas\u0027)"},{"line_number":58,"context_line":"    __import__(\u0027nova.objects.security_group\u0027)"},{"line_number":59,"context_line":"    __import__(\u0027nova.objects.security_group_rule\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_5dc44c3f","side":"PARENT","line":56,"in_reply_to":"3afb71cf_aa5dce7a","updated":"2015-06-13 16:28:18.000000000","message":"Done","commit_id":"5a1a1b7b71f88969f68bf498d1e2b594cd34c164"}],"nova/objects/fields.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"04141830a14abf918c2605f4f154f14766717376","unresolved":false,"context_lines":[{"line_number":548,"context_line":"        if v \u003c 0:"},{"line_number":549,"context_line":"            raise ValueError(_(\u0027Value must be \u003e\u003d 0 for field %s\u0027) % attr)"},{"line_number":550,"context_line":"        return v"},{"line_number":551,"context_line":""},{"line_number":552,"context_line":""},{"line_number":553,"context_line":"class AutoTypedField(fields.Field):"},{"line_number":554,"context_line":"    AUTO_TYPE \u003d None"}],"source_content_type":"text/x-python","patch_set":20,"id":"3a50d1a3_1ba5fe02","line":551,"updated":"2015-07-27 20:12:27.000000000","message":"Uh, you probably missed that, but you need to provide a separate change for providing new field types in case someone wants to backport them to o.vo.","commit_id":"a5f5656f10fb07cf0b465d816fcc94ffec13973c"}],"nova/objects/numa.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"bae4051337afa33edd20797d4978bd0e97b32c23","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        \"\"\"Defined so that boolean testing works the same as for lists.\"\"\""},{"line_number":203,"context_line":"        return len(self.cells)"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    def __eq__(self, other):"},{"line_number":206,"context_line":"        if other is None:"},{"line_number":207,"context_line":"            return False"},{"line_number":208,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"3a50d1a3_b06b40f1","line":205,"updated":"2015-07-23 21:34:22.000000000","message":"I\u0027m a bit worried to see the __eq__ operator having different behaviour depending on the object. I\u0027m just wondering if we should rather provide a different method name for the below implementation so it would be clear that it would be different from what the other __eq__ methods do. Thoughts ?","commit_id":"0e4d22d55b701d49b36624062cf772e833f333a6"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"528376052ca1ffbe6171ffe670296ea14a8c8a68","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        \"\"\"Defined so that boolean testing works the same as for lists.\"\"\""},{"line_number":203,"context_line":"        return len(self.cells)"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    def __eq__(self, other):"},{"line_number":206,"context_line":"        if other is None:"},{"line_number":207,"context_line":"            return False"},{"line_number":208,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"3a50d1a3_a0c5b25c","line":205,"in_reply_to":"3a50d1a3_943483fb","updated":"2015-07-27 20:12:41.000000000","message":"I leave that to others, I\u0027m not really opiniated","commit_id":"0e4d22d55b701d49b36624062cf772e833f333a6"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"84784f446c0e2696c4630977aa06358d862e3416","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        \"\"\"Defined so that boolean testing works the same as for lists.\"\"\""},{"line_number":203,"context_line":"        return len(self.cells)"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    def __eq__(self, other):"},{"line_number":206,"context_line":"        if other is None:"},{"line_number":207,"context_line":"            return False"},{"line_number":208,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"3a50d1a3_943483fb","line":205,"in_reply_to":"3a50d1a3_b06b40f1","updated":"2015-07-27 17:44:13.000000000","message":"The intent and contract are preserved, as far as I can tell? The means to get there is necessarily different because NUMA topologies are a bit more complicated than integers.","commit_id":"0e4d22d55b701d49b36624062cf772e833f333a6"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"bae4051337afa33edd20797d4978bd0e97b32c23","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        }"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"    def __eq__(self, other):"},{"line_number":251,"context_line":"        return all_things_equal(self, other)"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def to_dict_legacy(self, host_topology):"},{"line_number":254,"context_line":"        cells \u003d []"}],"source_content_type":"text/x-python","patch_set":19,"id":"3a50d1a3_b0b4a082","line":251,"updated":"2015-07-23 21:34:22.000000000","message":"That\u0027s fair to use a method already provided in the same module but now that we have base.obj_equal_prims(), maybe it would be worth using it ?\n\nhttp://git.openstack.org/cgit/openstack/nova/tree/nova/objects/base.py#n756","commit_id":"0e4d22d55b701d49b36624062cf772e833f333a6"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"84784f446c0e2696c4630977aa06358d862e3416","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        }"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"    def __eq__(self, other):"},{"line_number":251,"context_line":"        return all_things_equal(self, other)"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def to_dict_legacy(self, host_topology):"},{"line_number":254,"context_line":"        cells \u003d []"}],"source_content_type":"text/x-python","patch_set":19,"id":"3a50d1a3_f995c53f","line":251,"in_reply_to":"3a50d1a3_b0b4a082","updated":"2015-07-27 17:44:13.000000000","message":"Sounds like a follow-on patch to me; it\u0027s nice to be consistent in this patch","commit_id":"0e4d22d55b701d49b36624062cf772e833f333a6"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"528376052ca1ffbe6171ffe670296ea14a8c8a68","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        }"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"    def __eq__(self, other):"},{"line_number":251,"context_line":"        return all_things_equal(self, other)"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def to_dict_legacy(self, host_topology):"},{"line_number":254,"context_line":"        cells \u003d []"}],"source_content_type":"text/x-python","patch_set":19,"id":"3a50d1a3_e0e89ae4","line":251,"in_reply_to":"3a50d1a3_f995c53f","updated":"2015-07-27 20:12:41.000000000","message":"Mmm, not sure I agree with that. Why should we add a new method if we know it\u0027s bad to use it because it duplicates a base one ?","commit_id":"0e4d22d55b701d49b36624062cf772e833f333a6"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"04141830a14abf918c2605f4f154f14766717376","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        }"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"    def __eq__(self, other):"},{"line_number":251,"context_line":"        return all_things_equal(self, other)"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def to_dict_legacy(self, host_topology):"},{"line_number":254,"context_line":"        cells \u003d []"}],"source_content_type":"text/x-python","patch_set":20,"id":"3a50d1a3_00e6e614","line":251,"updated":"2015-07-27 20:12:27.000000000","message":"see my comment on PS19","commit_id":"a5f5656f10fb07cf0b465d816fcc94ffec13973c"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"5e70cfe4a329ba2b1fdf49aca8fe64cdd2050e09","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        }"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"    def __eq__(self, other):"},{"line_number":251,"context_line":"        return all_things_equal(self, other)"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def to_dict_legacy(self, host_topology):"},{"line_number":254,"context_line":"        cells \u003d []"}],"source_content_type":"text/x-python","patch_set":20,"id":"3a50d1a3_c8d49221","line":251,"in_reply_to":"3a50d1a3_00e6e614","updated":"2015-07-28 09:19:08.000000000","message":"We\u0027re not adding a new method, we\u0027re using an existing one. Tidying up the fact that there are two pre-existing methods with duplicate functionality is a separate patch.","commit_id":"a5f5656f10fb07cf0b465d816fcc94ffec13973c"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b3ce62e31e0c3082897b93e1d649e640d4b87150","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        }"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"    def __eq__(self, other):"},{"line_number":251,"context_line":"        return all_things_equal(self, other)"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def to_dict_legacy(self, host_topology):"},{"line_number":254,"context_line":"        cells \u003d []"}],"source_content_type":"text/x-python","patch_set":20,"id":"3a50d1a3_9eb6c0f7","line":251,"in_reply_to":"3a50d1a3_c8d49221","updated":"2015-07-28 09:30:45.000000000","message":"You\u0027re implementing __eq__, right ?\n\nWhy then calling a legacy method (targeted to be removed) when we know that something else provided in the base module exactly does the same job ?","commit_id":"a5f5656f10fb07cf0b465d816fcc94ffec13973c"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"c9fdc5bc28b8f4b300094fc85f164ff714cc1ad2","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        }"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"    def __eq__(self, other):"},{"line_number":251,"context_line":"        return all_things_equal(self, other)"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def to_dict_legacy(self, host_topology):"},{"line_number":254,"context_line":"        cells \u003d []"}],"source_content_type":"text/x-python","patch_set":22,"id":"3a50d1a3_de668850","line":251,"updated":"2015-07-28 09:37:34.000000000","message":"see PS20, here I\u0027m considering using base.obj_equal_prims()","commit_id":"f6ff91a6bdecbdd3ac687ad8f82c237c19b82e2e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"01e67ddba36f238c31075159c491489ac4793e80","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        }"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"    def __eq__(self, other):"},{"line_number":251,"context_line":"        return all_things_equal(self, other)"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def to_dict_legacy(self, host_topology):"},{"line_number":254,"context_line":"        cells \u003d []"}],"source_content_type":"text/x-python","patch_set":23,"id":"3a50d1a3_96c20e52","line":251,"updated":"2015-07-28 16:48:17.000000000","message":"Grmblblbl, I still put my thought on using obj_equal_prims() \n\n(nit, nit, nit again)","commit_id":"ff63e8f9098ed918272a8fd89e062c52ac4da707"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"79160b09f4504740409f2b506f0775916266ea6c","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        \"\"\"Defined so that boolean testing works the same as for lists.\"\"\""},{"line_number":203,"context_line":"        return len(self.cells)"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    def __eq__(self, other):"},{"line_number":206,"context_line":"        if other is None:"},{"line_number":207,"context_line":"            return False"},{"line_number":208,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"3a50d1a3_282fb9a8","line":205,"updated":"2015-07-30 10:35:09.000000000","message":"It occured to me that the way we could make this a bit nicer is to have a mixin class that makes the object a compare-able resource and requires that __eq__ is implemented. that way it\u0027ll be clear why some of them have these and some don\u0027t.\n\nCan easily be a follow up patch though","commit_id":"8d2979a09ac9ce72bda54fa254aa12b2a0773fae"}],"nova/objects/resources.py":[{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"023340f0b502fd1c54c5851b94dfe42f8781edf5","unresolved":false,"context_lines":[{"line_number":31,"context_line":"from nova.virt import hardware as hw"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"class IntegerAmount(base.NovaObject):"},{"line_number":35,"context_line":"    # Version 1.0: Initial version"},{"line_number":36,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":37,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"fa81d914_53996e6d","line":34,"updated":"2015-01-27 20:13:20.000000000","message":"What are the benefits of modelling an int as an IntegerAmount?","commit_id":"83030092eb719eef00042d944b5b1a3eaf03ed0a"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"9ccb8545302fdb9c0b92e72910777a49330c6242","unresolved":false,"context_lines":[{"line_number":46,"context_line":"        :param amount: integer amount requested of the resource."},{"line_number":47,"context_line":"        :raises ValueError if amount is not a non-zero integer."},{"line_number":48,"context_line":"        \"\"\""},{"line_number":49,"context_line":"        if not isinstance(amount, (int, long)) or (amount \u003d\u003d 0):"},{"line_number":50,"context_line":"            msg \u003d _(\"amount must be a non-zero integer.\")"},{"line_number":51,"context_line":"            raise ValueError(msg)"},{"line_number":52,"context_line":"        super(IntegerAmount, self).__init__(amount\u003damount,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_3f267dba","line":49,"updated":"2015-01-18 15:58:20.000000000","message":"why is 0 invalid?","commit_id":"83030092eb719eef00042d944b5b1a3eaf03ed0a"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"023340f0b502fd1c54c5851b94dfe42f8781edf5","unresolved":false,"context_lines":[{"line_number":193,"context_line":"                not isinstance(used, (int, long)) or"},{"line_number":194,"context_line":"                (total \u003c 0) or"},{"line_number":195,"context_line":"                (used \u003c 0)):"},{"line_number":196,"context_line":"            msg \u003d _(\"total and used cannot be negative integers\")"},{"line_number":197,"context_line":"            raise ValueError(msg)"},{"line_number":198,"context_line":"        if (overcommit_ratio is not None and ("},{"line_number":199,"context_line":"                not isinstance(overcommit_ratio, (float, int)) or"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa81d914_d361ded4","line":196,"updated":"2015-01-27 20:13:20.000000000","message":"message should be \"total and used must be positive integers\", to cover the type checks (longs are integers, outside crazy coding world).","commit_id":"83030092eb719eef00042d944b5b1a3eaf03ed0a"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"023340f0b502fd1c54c5851b94dfe42f8781edf5","unresolved":false,"context_lines":[{"line_number":196,"context_line":"            msg \u003d _(\"total and used cannot be negative integers\")"},{"line_number":197,"context_line":"            raise ValueError(msg)"},{"line_number":198,"context_line":"        if (overcommit_ratio is not None and ("},{"line_number":199,"context_line":"                not isinstance(overcommit_ratio, (float, int)) or"},{"line_number":200,"context_line":"                (overcommit_ratio \u003c 0))):"},{"line_number":201,"context_line":"            msg \u003d _(\"Overcommit must be a positive float or integer\")"},{"line_number":202,"context_line":"            raise ValueError(msg)"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa81d914_13e9b61a","line":199,"updated":"2015-01-27 20:13:20.000000000","message":"why not long?","commit_id":"83030092eb719eef00042d944b5b1a3eaf03ed0a"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"023340f0b502fd1c54c5851b94dfe42f8781edf5","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        if (overcommit_ratio is not None and ("},{"line_number":199,"context_line":"                not isinstance(overcommit_ratio, (float, int)) or"},{"line_number":200,"context_line":"                (overcommit_ratio \u003c 0))):"},{"line_number":201,"context_line":"            msg \u003d _(\"Overcommit must be a positive float or integer\")"},{"line_number":202,"context_line":"            raise ValueError(msg)"},{"line_number":203,"context_line":"        if overcommit_ratio is None:"},{"line_number":204,"context_line":"            overcommit_ratio \u003d 1.0"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa81d914_735d92e9","line":201,"updated":"2015-01-27 20:13:20.000000000","message":"So if overcommit_ratio \u003d\u003d 0, the request will always be denied? That doesn\u0027t seem useful. What about \u003c1, for an OVERcommit ratio?","commit_id":"83030092eb719eef00042d944b5b1a3eaf03ed0a"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"023340f0b502fd1c54c5851b94dfe42f8781edf5","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        if not isinstance(amount_spec, IntegerAmount):"},{"line_number":230,"context_line":"            msg \u003d _(\"amount_spec must be an IntegerAmount\")"},{"line_number":231,"context_line":"            raise ValueError(msg)"},{"line_number":232,"context_line":"        self.used +\u003d amount_spec.amount"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"    def has_room_for(self, amount_spec):"},{"line_number":235,"context_line":"        \"\"\"Determine if there is room to fit the supplied amount of resources."}],"source_content_type":"text/x-python","patch_set":5,"id":"fa81d914_f3eee2b3","line":232,"updated":"2015-01-27 20:13:20.000000000","message":"shouldn\u0027t we call has_room_for and error if it fails?","commit_id":"83030092eb719eef00042d944b5b1a3eaf03ed0a"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"023340f0b502fd1c54c5851b94dfe42f8781edf5","unresolved":false,"context_lines":[{"line_number":238,"context_line":"        :returns True if the supplied requested amount of resources is able"},{"line_number":239,"context_line":"                 to be consumed on the node, False otherwise."},{"line_number":240,"context_line":""},{"line_number":241,"context_line":"                 If the supplied `amount_spec` is negative, returns False."},{"line_number":242,"context_line":"        :raises ValueError if amount_spec is not an `IntegerAmount`."},{"line_number":243,"context_line":"        \"\"\""},{"line_number":244,"context_line":"        if not isinstance(amount_spec, IntegerAmount):"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa81d914_d31f1eb6","line":241,"updated":"2015-01-27 20:13:20.000000000","message":"If I have total\u003d50, used\u003d20, then shouldn\u0027t amount_spec\u003d-50 \u003d\u003e False and amount_spec\u003d-15 \u003d\u003e True ?","commit_id":"83030092eb719eef00042d944b5b1a3eaf03ed0a"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"023340f0b502fd1c54c5851b94dfe42f8781edf5","unresolved":false,"context_lines":[{"line_number":264,"context_line":"    }"},{"line_number":265,"context_line":""},{"line_number":266,"context_line":"    obj_relationships \u003d {"},{"line_number":267,"context_line":"        \u0027topology\u0027: [(\u00271.0\u0027, \u00271.2\u0027)]"},{"line_number":268,"context_line":"    }"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"    def __init__(self, topology, limits\u003dNone):"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa81d914_73ed928b","line":267,"updated":"2015-01-27 20:13:20.000000000","message":"I don\u0027t really know what this means, but I see you had (\u00271.0\u0027, \u00271.1\u0027) above for the NumaAmount. Shouldn\u0027t these be consistent?","commit_id":"83030092eb719eef00042d944b5b1a3eaf03ed0a"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"7d56deea13ea72f73b0512cb06cbc41259d18cc8","unresolved":false,"context_lines":[{"line_number":31,"context_line":"from nova.virt import hardware as hw"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"class IntegerAmount(base.NovaObject):"},{"line_number":35,"context_line":"    # Version 1.0: Initial version"},{"line_number":36,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":37,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3afb71cf_17725b11","line":34,"updated":"2015-06-12 13:55:06.000000000","message":"Why no abstract base class like AmountSpec please?","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3699e856ca64eb5872647fbe20507e276b6fb896","unresolved":false,"context_lines":[{"line_number":31,"context_line":"from nova.virt import hardware as hw"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"class IntegerAmount(base.NovaObject):"},{"line_number":35,"context_line":"    # Version 1.0: Initial version"},{"line_number":36,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":37,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_1ddd6414","line":34,"in_reply_to":"3afb71cf_17725b11","updated":"2015-06-13 16:28:18.000000000","message":"Done","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"7d56deea13ea72f73b0512cb06cbc41259d18cc8","unresolved":false,"context_lines":[{"line_number":36,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    fields \u003d {"},{"line_number":39,"context_line":"        \u0027is_negative\u0027: fields.BooleanField(),"},{"line_number":40,"context_line":"        \u0027amount\u0027: fields.IntegerField(),"},{"line_number":41,"context_line":"    }"},{"line_number":42,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3afb71cf_f77c5fed","line":39,"updated":"2015-06-12 13:55:06.000000000","message":"Why have this as a real field please? We could offer a property instead","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3699e856ca64eb5872647fbe20507e276b6fb896","unresolved":false,"context_lines":[{"line_number":36,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    fields \u003d {"},{"line_number":39,"context_line":"        \u0027is_negative\u0027: fields.BooleanField(),"},{"line_number":40,"context_line":"        \u0027amount\u0027: fields.IntegerField(),"},{"line_number":41,"context_line":"    }"},{"line_number":42,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_3de2a8d6","line":39,"in_reply_to":"3afb71cf_f77c5fed","updated":"2015-06-13 16:28:18.000000000","message":"Done","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"5da71a821b80bc88ad656201856b5fe73d9359fc","unresolved":false,"context_lines":[{"line_number":46,"context_line":"        :param amount: integer amount requested of the resource."},{"line_number":47,"context_line":"        :raises ValueError if amount is not a non-zero integer."},{"line_number":48,"context_line":"        \"\"\""},{"line_number":49,"context_line":"        if not isinstance(amount, (int, long)) or (amount \u003d\u003d 0):"},{"line_number":50,"context_line":"            msg \u003d _(\"amount must be a non-zero integer.\")"},{"line_number":51,"context_line":"            raise ValueError(msg)"},{"line_number":52,"context_line":"        super(IntegerAmount, self).__init__(amount\u003damount,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3afb71cf_e5ead00e","line":49,"updated":"2015-06-08 17:45:00.000000000","message":"It would be better to `import six`, and then change this to:\n\n    if not isinstance(amount, six.integer_types) or ...","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3699e856ca64eb5872647fbe20507e276b6fb896","unresolved":false,"context_lines":[{"line_number":46,"context_line":"        :param amount: integer amount requested of the resource."},{"line_number":47,"context_line":"        :raises ValueError if amount is not a non-zero integer."},{"line_number":48,"context_line":"        \"\"\""},{"line_number":49,"context_line":"        if not isinstance(amount, (int, long)) or (amount \u003d\u003d 0):"},{"line_number":50,"context_line":"            msg \u003d _(\"amount must be a non-zero integer.\")"},{"line_number":51,"context_line":"            raise ValueError(msg)"},{"line_number":52,"context_line":"        super(IntegerAmount, self).__init__(amount\u003damount,"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_3d094896","line":49,"in_reply_to":"3afb71cf_e5ead00e","updated":"2015-06-13 16:28:18.000000000","message":"Done","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"7d56deea13ea72f73b0512cb06cbc41259d18cc8","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        \"\"\""},{"line_number":49,"context_line":"        if not isinstance(amount, (int, long)) or (amount \u003d\u003d 0):"},{"line_number":50,"context_line":"            msg \u003d _(\"amount must be a non-zero integer.\")"},{"line_number":51,"context_line":"            raise ValueError(msg)"},{"line_number":52,"context_line":"        super(IntegerAmount, self).__init__(amount\u003damount,"},{"line_number":53,"context_line":"                                            is_negative\u003d(amount \u003c 0))"},{"line_number":54,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3afb71cf_976f6bfb","line":51,"updated":"2015-06-12 13:55:06.000000000","message":"I\u0027m not sure it buys us a lot to specifically exclude zero. If we end up doing math with these, it could be useful to have zero as a potential result.","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3699e856ca64eb5872647fbe20507e276b6fb896","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        \"\"\""},{"line_number":49,"context_line":"        if not isinstance(amount, (int, long)) or (amount \u003d\u003d 0):"},{"line_number":50,"context_line":"            msg \u003d _(\"amount must be a non-zero integer.\")"},{"line_number":51,"context_line":"            raise ValueError(msg)"},{"line_number":52,"context_line":"        super(IntegerAmount, self).__init__(amount\u003damount,"},{"line_number":53,"context_line":"                                            is_negative\u003d(amount \u003c 0))"},{"line_number":54,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_bdf55884","line":51,"in_reply_to":"3afb71cf_976f6bfb","updated":"2015-06-13 16:28:18.000000000","message":"Done","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"7d56deea13ea72f73b0512cb06cbc41259d18cc8","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        return self.amount \u003d\u003d other.amount"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    def __hash__(self):"},{"line_number":61,"context_line":"        return None"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    def __ne__(self, other):"},{"line_number":64,"context_line":"        return not self \u003d\u003d other"}],"source_content_type":"text/x-python","patch_set":6,"id":"3afb71cf_f707bf7f","line":61,"updated":"2015-06-12 13:55:06.000000000","message":"Why not just return self.amount?","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3699e856ca64eb5872647fbe20507e276b6fb896","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        return self.amount \u003d\u003d other.amount"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    def __hash__(self):"},{"line_number":61,"context_line":"        return None"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    def __ne__(self, other):"},{"line_number":64,"context_line":"        return not self \u003d\u003d other"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_dd34dc4b","line":61,"in_reply_to":"3afb71cf_f707bf7f","updated":"2015-06-13 16:28:18.000000000","message":"Done","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"7d56deea13ea72f73b0512cb06cbc41259d18cc8","unresolved":false,"context_lines":[{"line_number":111,"context_line":"        return hw.topology_is_same(self.topology, other.topology)"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"    def __hash__(self):"},{"line_number":114,"context_line":"        return None"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    def __ne__(self, other):"},{"line_number":117,"context_line":"        return not self \u003d\u003d other"}],"source_content_type":"text/x-python","patch_set":6,"id":"3afb71cf_f7157f9b","line":114,"updated":"2015-06-12 13:55:06.000000000","message":"self.topology.__hash__() is probably near enough. is_negative is the only difference. We could negate the topology hash if it\u0027s of a negateable type.\n\nAlternatively we could return repr(self).__hash__() which is a bit inefficient but clearly meets the contract.","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3699e856ca64eb5872647fbe20507e276b6fb896","unresolved":false,"context_lines":[{"line_number":111,"context_line":"        return hw.topology_is_same(self.topology, other.topology)"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"    def __hash__(self):"},{"line_number":114,"context_line":"        return None"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    def __ne__(self, other):"},{"line_number":117,"context_line":"        return not self \u003d\u003d other"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_7d46f0c0","line":114,"in_reply_to":"3afb71cf_f7157f9b","updated":"2015-06-13 16:28:18.000000000","message":"Done","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"7d56deea13ea72f73b0512cb06cbc41259d18cc8","unresolved":false,"context_lines":[{"line_number":129,"context_line":"    resource that has been used/consumed."},{"line_number":130,"context_line":"    \"\"\""},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    def update(self, amount_spec):"},{"line_number":133,"context_line":"        \"\"\"Update the used amount of resources by the supplied amount."},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        :param amount_spec: `AmountSpec` to modify the usage with."}],"source_content_type":"text/x-python","patch_set":6,"id":"1af86dd1_215a6b84","line":132,"updated":"2015-06-12 13:55:06.000000000","message":"This method name is unclear about the result when passed positive and negative amounts.","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3699e856ca64eb5872647fbe20507e276b6fb896","unresolved":false,"context_lines":[{"line_number":129,"context_line":"    resource that has been used/consumed."},{"line_number":130,"context_line":"    \"\"\""},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    def update(self, amount_spec):"},{"line_number":133,"context_line":"        \"\"\"Update the used amount of resources by the supplied amount."},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        :param amount_spec: `AmountSpec` to modify the usage with."}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_fd8760d5","line":132,"in_reply_to":"1af86dd1_215a6b84","updated":"2015-06-13 16:28:18.000000000","message":"Changed the class to InventorySpec and I added a docstring that describes the contract -- i.e. the resources are freed up if an amount_spec that is negative is passed.","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"7d56deea13ea72f73b0512cb06cbc41259d18cc8","unresolved":false,"context_lines":[{"line_number":156,"context_line":"    fields \u003d {"},{"line_number":157,"context_line":"        \u0027total\u0027: fields.IntegerField(),"},{"line_number":158,"context_line":"        \u0027used\u0027: fields.IntegerField(),"},{"line_number":159,"context_line":"        \u0027overcommit_ratio\u0027: fields.FloatField(),"},{"line_number":160,"context_line":"    }"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"    def __init__(self, total, used, overcommit_ratio\u003dNone):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3afb71cf_57da13e9","line":159,"updated":"2015-06-12 13:55:06.000000000","message":"nit: This is probably fine but I usually try to avoid floats. We could consider this a percentage, with 100 meaning no overcommit. I don\u0027t expect anyone to need finer granularity than that?","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3699e856ca64eb5872647fbe20507e276b6fb896","unresolved":false,"context_lines":[{"line_number":156,"context_line":"    fields \u003d {"},{"line_number":157,"context_line":"        \u0027total\u0027: fields.IntegerField(),"},{"line_number":158,"context_line":"        \u0027used\u0027: fields.IntegerField(),"},{"line_number":159,"context_line":"        \u0027overcommit_ratio\u0027: fields.FloatField(),"},{"line_number":160,"context_line":"    }"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"    def __init__(self, total, used, overcommit_ratio\u003dNone):"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_9d541455","line":159,"in_reply_to":"3afb71cf_57da13e9","updated":"2015-06-13 16:28:18.000000000","message":"No, overcommit can be something like 1.5:1 (the default RAM overcommit ratio in Nova). So this does need to be a float.","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"0c5326480fc53779fb35c87bec4f36a604d149de","unresolved":false,"context_lines":[{"line_number":156,"context_line":"    fields \u003d {"},{"line_number":157,"context_line":"        \u0027total\u0027: fields.IntegerField(),"},{"line_number":158,"context_line":"        \u0027used\u0027: fields.IntegerField(),"},{"line_number":159,"context_line":"        \u0027overcommit_ratio\u0027: fields.FloatField(),"},{"line_number":160,"context_line":"    }"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"    def __init__(self, total, used, overcommit_ratio\u003dNone):"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_6d81454a","line":159,"in_reply_to":"fa32b979_9d541455","updated":"2015-06-15 08:36:10.000000000","message":"Wouldn\u0027t that simply be 150?","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"5da71a821b80bc88ad656201856b5fe73d9359fc","unresolved":false,"context_lines":[{"line_number":189,"context_line":"              total RAM (16 \u003c\u003d 16), and the (total * overcommit) minus used is"},{"line_number":190,"context_line":"              \u003e\u003d requested ((16 * 2.0) - 8 \u003e\u003d 16)."},{"line_number":191,"context_line":"        \"\"\""},{"line_number":192,"context_line":"        if (not isinstance(total, (int, long)) or"},{"line_number":193,"context_line":"                not isinstance(used, (int, long)) or"},{"line_number":194,"context_line":"                (total \u003c 0) or"},{"line_number":195,"context_line":"                (used \u003c 0)):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3afb71cf_05ee0cec","line":192,"updated":"2015-06-08 17:45:00.000000000","message":"Same comment re: integer_types as noted above in IntegerAmount.","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3699e856ca64eb5872647fbe20507e276b6fb896","unresolved":false,"context_lines":[{"line_number":189,"context_line":"              total RAM (16 \u003c\u003d 16), and the (total * overcommit) minus used is"},{"line_number":190,"context_line":"              \u003e\u003d requested ((16 * 2.0) - 8 \u003e\u003d 16)."},{"line_number":191,"context_line":"        \"\"\""},{"line_number":192,"context_line":"        if (not isinstance(total, (int, long)) or"},{"line_number":193,"context_line":"                not isinstance(used, (int, long)) or"},{"line_number":194,"context_line":"                (total \u003c 0) or"},{"line_number":195,"context_line":"                (used \u003c 0)):"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_bd51d864","line":192,"in_reply_to":"3afb71cf_05ee0cec","updated":"2015-06-13 16:28:18.000000000","message":"Done","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"7d56deea13ea72f73b0512cb06cbc41259d18cc8","unresolved":false,"context_lines":[{"line_number":193,"context_line":"                not isinstance(used, (int, long)) or"},{"line_number":194,"context_line":"                (total \u003c 0) or"},{"line_number":195,"context_line":"                (used \u003c 0)):"},{"line_number":196,"context_line":"            msg \u003d _(\"total and used cannot be negative integers\")"},{"line_number":197,"context_line":"            raise ValueError(msg)"},{"line_number":198,"context_line":"        if (overcommit_ratio is not None and ("},{"line_number":199,"context_line":"                not isinstance(overcommit_ratio, (float, int)) or"}],"source_content_type":"text/x-python","patch_set":6,"id":"1af86dd1_52d09353","line":196,"updated":"2015-06-12 13:55:06.000000000","message":"this seems pretty confusing if you pass a float, better to say\n\n    Total and used must be positive ints or longs\n\nIt\u0027d also be better to test each var separately so you get a more precise error","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3699e856ca64eb5872647fbe20507e276b6fb896","unresolved":false,"context_lines":[{"line_number":193,"context_line":"                not isinstance(used, (int, long)) or"},{"line_number":194,"context_line":"                (total \u003c 0) or"},{"line_number":195,"context_line":"                (used \u003c 0)):"},{"line_number":196,"context_line":"            msg \u003d _(\"total and used cannot be negative integers\")"},{"line_number":197,"context_line":"            raise ValueError(msg)"},{"line_number":198,"context_line":"        if (overcommit_ratio is not None and ("},{"line_number":199,"context_line":"                not isinstance(overcommit_ratio, (float, int)) or"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_5d4e8cc2","line":196,"in_reply_to":"1af86dd1_52d09353","updated":"2015-06-13 16:28:18.000000000","message":"Done","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"7d56deea13ea72f73b0512cb06cbc41259d18cc8","unresolved":false,"context_lines":[{"line_number":201,"context_line":"            msg \u003d _(\"Overcommit must be a positive float or integer\")"},{"line_number":202,"context_line":"            raise ValueError(msg)"},{"line_number":203,"context_line":"        if overcommit_ratio is None:"},{"line_number":204,"context_line":"            overcommit_ratio \u003d 1.0"},{"line_number":205,"context_line":"        super(IntegerUsage, self).__init__(total\u003dtotal,"},{"line_number":206,"context_line":"                                           used\u003dused,"},{"line_number":207,"context_line":"                                           overcommit_ratio\u003dovercommit_ratio)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1af86dd1_32d87753","line":204,"updated":"2015-06-12 13:55:06.000000000","message":"if you move this above the type/range test, you can stop testing for None twice","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3699e856ca64eb5872647fbe20507e276b6fb896","unresolved":false,"context_lines":[{"line_number":201,"context_line":"            msg \u003d _(\"Overcommit must be a positive float or integer\")"},{"line_number":202,"context_line":"            raise ValueError(msg)"},{"line_number":203,"context_line":"        if overcommit_ratio is None:"},{"line_number":204,"context_line":"            overcommit_ratio \u003d 1.0"},{"line_number":205,"context_line":"        super(IntegerUsage, self).__init__(total\u003dtotal,"},{"line_number":206,"context_line":"                                           used\u003dused,"},{"line_number":207,"context_line":"                                           overcommit_ratio\u003dovercommit_ratio)"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_7de270a1","line":204,"in_reply_to":"1af86dd1_32d87753","updated":"2015-06-13 16:28:18.000000000","message":"Done","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"7d56deea13ea72f73b0512cb06cbc41259d18cc8","unresolved":false,"context_lines":[{"line_number":245,"context_line":"            msg \u003d _(\"amount_spec must be an IntegerAmount\")"},{"line_number":246,"context_line":"            raise ValueError(msg)"},{"line_number":247,"context_line":"        if amount_spec.is_negative:"},{"line_number":248,"context_line":"            return False"},{"line_number":249,"context_line":"        # We must always check to ensure that the raw requested amount is"},{"line_number":250,"context_line":"        # not greater than the amount of non-overcommit-adjusted resources."},{"line_number":251,"context_line":"        if self.total \u003c amount_spec.amount:"}],"source_content_type":"text/x-python","patch_set":6,"id":"1af86dd1_92adeb8a","line":248,"updated":"2015-06-12 13:55:06.000000000","message":"We could check that a negative amount_spec \u003c\u003d self.used","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3699e856ca64eb5872647fbe20507e276b6fb896","unresolved":false,"context_lines":[{"line_number":245,"context_line":"            msg \u003d _(\"amount_spec must be an IntegerAmount\")"},{"line_number":246,"context_line":"            raise ValueError(msg)"},{"line_number":247,"context_line":"        if amount_spec.is_negative:"},{"line_number":248,"context_line":"            return False"},{"line_number":249,"context_line":"        # We must always check to ensure that the raw requested amount is"},{"line_number":250,"context_line":"        # not greater than the amount of non-overcommit-adjusted resources."},{"line_number":251,"context_line":"        if self.total \u003c amount_spec.amount:"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_dde7fcaa","line":248,"in_reply_to":"1af86dd1_92adeb8a","updated":"2015-06-13 16:28:18.000000000","message":"No, has_room_for() is not designed to be used for negative amount specs, thus this check. This is only for checking if there is room in the inventory for a requested positive amount. There is never any need to check for capacity when we are freeing up resources in inventory.","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"0c5326480fc53779fb35c87bec4f36a604d149de","unresolved":false,"context_lines":[{"line_number":245,"context_line":"            msg \u003d _(\"amount_spec must be an IntegerAmount\")"},{"line_number":246,"context_line":"            raise ValueError(msg)"},{"line_number":247,"context_line":"        if amount_spec.is_negative:"},{"line_number":248,"context_line":"            return False"},{"line_number":249,"context_line":"        # We must always check to ensure that the raw requested amount is"},{"line_number":250,"context_line":"        # not greater than the amount of non-overcommit-adjusted resources."},{"line_number":251,"context_line":"        if self.total \u003c amount_spec.amount:"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_0d73d981","line":248,"in_reply_to":"fa32b979_dde7fcaa","updated":"2015-06-15 08:36:10.000000000","message":"What about a bug that caused a resource to be freed twice? We\u0027d catch that faster if we ensured \u0027used\u0027 couldn\u0027t go negative","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"7d56deea13ea72f73b0512cb06cbc41259d18cc8","unresolved":false,"context_lines":[{"line_number":321,"context_line":"        sl \u003d self.limits"},{"line_number":322,"context_line":"        ol \u003d other.limits"},{"line_number":323,"context_line":"        return (sl.cpu_allocation_ratio \u003d\u003d ol.cpu_allocation_ratio and"},{"line_number":324,"context_line":"                sl.ram_allocation_ratio \u003d\u003d ol.ram_allocation_ratio)"},{"line_number":325,"context_line":""},{"line_number":326,"context_line":"    def __ne__(self, other):"},{"line_number":327,"context_line":"        return not self \u003d\u003d other"}],"source_content_type":"text/x-python","patch_set":6,"id":"1af86dd1_b437b7e2","line":324,"updated":"2015-06-12 13:55:06.000000000","message":"I think lines 314-324 are clearer as:\n\n    if self.limits is None and other.limits is None:\n        return True\n    if self.limits is None or other.limits is None:\n        return False\n    return self.limits \u003d\u003d other.limits","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3699e856ca64eb5872647fbe20507e276b6fb896","unresolved":false,"context_lines":[{"line_number":321,"context_line":"        sl \u003d self.limits"},{"line_number":322,"context_line":"        ol \u003d other.limits"},{"line_number":323,"context_line":"        return (sl.cpu_allocation_ratio \u003d\u003d ol.cpu_allocation_ratio and"},{"line_number":324,"context_line":"                sl.ram_allocation_ratio \u003d\u003d ol.ram_allocation_ratio)"},{"line_number":325,"context_line":""},{"line_number":326,"context_line":"    def __ne__(self, other):"},{"line_number":327,"context_line":"        return not self \u003d\u003d other"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_9d02d43d","line":324,"in_reply_to":"1af86dd1_b437b7e2","updated":"2015-06-13 16:28:18.000000000","message":"nova.objects.NUMATopologyLimits doesn\u0027t implement __eq__, which is why I did this. But, you\u0027re right. So, I implemented __eq__ in the next revision :)","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"5da71a821b80bc88ad656201856b5fe73d9359fc","unresolved":false,"context_lines":[{"line_number":359,"context_line":"        if amount_spec.is_negative:"},{"line_number":360,"context_line":"            return False"},{"line_number":361,"context_line":"        requested_topology \u003d amount_spec.topology"},{"line_number":362,"context_line":"        return hw.numa_fit_instance_to_host("},{"line_number":363,"context_line":"                    self.topology, requested_topology,"},{"line_number":364,"context_line":"                    limits\u003dself.limits) is not None"}],"source_content_type":"text/x-python","patch_set":6,"id":"3afb71cf_f74cc371","line":362,"updated":"2015-06-08 17:45:00.000000000","message":"Super nit-picky: if you\u0027re not using a visual indentation for a continued line, it should be 2 levels (8 spaces).","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3699e856ca64eb5872647fbe20507e276b6fb896","unresolved":false,"context_lines":[{"line_number":359,"context_line":"        if amount_spec.is_negative:"},{"line_number":360,"context_line":"            return False"},{"line_number":361,"context_line":"        requested_topology \u003d amount_spec.topology"},{"line_number":362,"context_line":"        return hw.numa_fit_instance_to_host("},{"line_number":363,"context_line":"                    self.topology, requested_topology,"},{"line_number":364,"context_line":"                    limits\u003dself.limits) is not None"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_dd9e1c07","line":362,"in_reply_to":"3afb71cf_f74cc371","updated":"2015-06-13 16:28:18.000000000","message":"it is :)","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"79be3c3ab0a0020ea7a3b5471d873d376b28867b","unresolved":false,"context_lines":[{"line_number":33,"context_line":"from nova.virt import hardware as hw"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"class AmountSpec(object):"},{"line_number":37,"context_line":"    pass"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_b2680cf7","line":36,"updated":"2015-06-15 08:36:02.000000000","message":"A docstring explaining the expectations wouldn\u0027t go amiss. EG eq, ne, hash, is_negative","commit_id":"20257b4783cf3c90a1c2012a5c15e192dfcde143"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3ec4f8c2b4d789d180c6ee63e6de4faaf5cb4461","unresolved":false,"context_lines":[{"line_number":33,"context_line":"from nova.virt import hardware as hw"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"class AmountSpec(object):"},{"line_number":37,"context_line":"    pass"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_fe097a32","line":36,"in_reply_to":"fa32b979_b2680cf7","updated":"2015-06-15 11:18:56.000000000","message":"Done","commit_id":"20257b4783cf3c90a1c2012a5c15e192dfcde143"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"79be3c3ab0a0020ea7a3b5471d873d376b28867b","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        :raises ValueError if amount is not a non-zero integer."},{"line_number":54,"context_line":"        \"\"\""},{"line_number":55,"context_line":"        if not isinstance(amount, six.integer_types):"},{"line_number":56,"context_line":"            msg \u003d _(\"amount must be an integer greater than or equal to zero.\")"},{"line_number":57,"context_line":"            raise ValueError(msg)"},{"line_number":58,"context_line":"        super(IntegerAmount, self).__init__(amount\u003damount)"},{"line_number":59,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_6d25c5a9","line":56,"updated":"2015-06-15 08:36:02.000000000","message":"amount may be less than zero","commit_id":"20257b4783cf3c90a1c2012a5c15e192dfcde143"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3ec4f8c2b4d789d180c6ee63e6de4faaf5cb4461","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        :raises ValueError if amount is not a non-zero integer."},{"line_number":54,"context_line":"        \"\"\""},{"line_number":55,"context_line":"        if not isinstance(amount, six.integer_types):"},{"line_number":56,"context_line":"            msg \u003d _(\"amount must be an integer greater than or equal to zero.\")"},{"line_number":57,"context_line":"            raise ValueError(msg)"},{"line_number":58,"context_line":"        super(IntegerAmount, self).__init__(amount\u003damount)"},{"line_number":59,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_de0cb644","line":56,"in_reply_to":"fa32b979_6d25c5a9","updated":"2015-06-15 11:18:56.000000000","message":"Done","commit_id":"20257b4783cf3c90a1c2012a5c15e192dfcde143"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"79be3c3ab0a0020ea7a3b5471d873d376b28867b","unresolved":false,"context_lines":[{"line_number":139,"context_line":"    resource that has been used/consumed."},{"line_number":140,"context_line":"    \"\"\""},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def update(self, amount_spec):"},{"line_number":143,"context_line":"        \"\"\"Update the used amount of resources by the supplied amount."},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        :param amount_spec: `AmountSpec` to modify the usage with."}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_8d9b69c9","line":142,"updated":"2015-06-15 08:36:02.000000000","message":"nit: I\u0027d prefer \"consume\" but the note does make it clearer, thank you","commit_id":"20257b4783cf3c90a1c2012a5c15e192dfcde143"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"94eebfb370c6ce7fe571376c2b071b48c25ef928","unresolved":false,"context_lines":[{"line_number":139,"context_line":"    resource that has been used/consumed."},{"line_number":140,"context_line":"    \"\"\""},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def update(self, amount_spec):"},{"line_number":143,"context_line":"        \"\"\"Update the used amount of resources by the supplied amount."},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        :param amount_spec: `AmountSpec` to modify the usage with."}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_6f4db271","line":142,"in_reply_to":"fa32b979_1938f8d2","updated":"2015-06-15 12:07:18.000000000","message":"I like that consume implies a direction. IMO you /can/ consume a negative amount and there\u0027s only one possible meaning of that - to free.\n\nUpdate is less clear. Some might say updating an inventory by a positive amount is naturally a free, whereas updating by a negative amount is naturally a consumption. You have it the other way around, which might strike a newcomer as odd.","commit_id":"20257b4783cf3c90a1c2012a5c15e192dfcde143"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3ec4f8c2b4d789d180c6ee63e6de4faaf5cb4461","unresolved":false,"context_lines":[{"line_number":139,"context_line":"    resource that has been used/consumed."},{"line_number":140,"context_line":"    \"\"\""},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def update(self, amount_spec):"},{"line_number":143,"context_line":"        \"\"\"Update the used amount of resources by the supplied amount."},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        :param amount_spec: `AmountSpec` to modify the usage with."}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_1938f8d2","line":142,"in_reply_to":"fa32b979_8d9b69c9","updated":"2015-06-15 11:18:56.000000000","message":"\"consume\" kind of implies positive amounts, no? You can\u0027t \"consume\" a negative amount. You can \"free\" a negative amount from the inventory. \"update\" seemed to be a fair balance between consume and free...","commit_id":"20257b4783cf3c90a1c2012a5c15e192dfcde143"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"e1370deb30ef40ebde92ce1ce90dde66903cd366","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"class AmountSpec(object):"},{"line_number":37,"context_line":"    \"\"\"Abstract base class for a quantity of some resource."},{"line_number":38,"context_line":"    "},{"line_number":39,"context_line":"    Has no method interface, but should have an attribute or property"},{"line_number":40,"context_line":"    called \"is_negative\" that returns whether the amount represents"},{"line_number":41,"context_line":"    a negative value."}],"source_content_type":"text/x-python","patch_set":8,"id":"fa32b979_54b965e8","line":38,"updated":"2015-06-15 12:07:11.000000000","message":"trailing space of death! Not sure what the convention here is so I\u0027m going to +1 anyway","commit_id":"f46bf4377459f81cf51affbe61846b4fa08a369a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c5044f0a4916e32dfa4b8b9c7ff0c976d57cb3ca","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"class AmountSpec(object):"},{"line_number":37,"context_line":"    \"\"\"Abstract base class for a quantity of some resource."},{"line_number":38,"context_line":"    "},{"line_number":39,"context_line":"    Has no method interface, but should have an attribute or property"},{"line_number":40,"context_line":"    called \"is_negative\" that returns whether the amount represents"},{"line_number":41,"context_line":"    a negative value."}],"source_content_type":"text/x-python","patch_set":8,"id":"fa32b979_8a3104a5","line":38,"in_reply_to":"fa32b979_54b965e8","updated":"2015-06-15 12:37:52.000000000","message":"Done","commit_id":"f46bf4377459f81cf51affbe61846b4fa08a369a"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"7643f7eeb9ec7f4be914bcde3d5996f7c67776d9","unresolved":false,"context_lines":[{"line_number":33,"context_line":"from nova.virt import hardware as hw"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"class AmountSpec(object):"},{"line_number":37,"context_line":"    \"\"\"Abstract base class for a quantity of some resource."},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    Has no method interface, but should have an attribute or property"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa32b979_a149868d","line":36,"updated":"2015-06-15 15:13:44.000000000","message":"Why isn\u0027t this simply an \"Amount\" please?","commit_id":"6e96798a00a6b78ed4477c78341b74cc9d59d03b"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"7643f7eeb9ec7f4be914bcde3d5996f7c67776d9","unresolved":false,"context_lines":[{"line_number":137,"context_line":"            self.topology, self.is_negative)"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"class InventorySpec(object):"},{"line_number":141,"context_line":"    \"\"\"Represents the capacity and used amount of a resource."},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    The InventorySpec subclasses enable different types of resources to express"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa32b979_a140a657","line":140,"updated":"2015-06-15 15:13:44.000000000","message":"Why isn\u0027t this simply an Inventory please?","commit_id":"6e96798a00a6b78ed4477c78341b74cc9d59d03b"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"154209cf188195ff9e9fb5b9eada497f820733b2","unresolved":false,"context_lines":[{"line_number":140,"context_line":"class Inventory(object):"},{"line_number":141,"context_line":"    \"\"\"Represents the capacity and used amount of a resource."},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    The InventorySpec subclasses enable different types of resources to express"},{"line_number":144,"context_line":"    a total capacity of that resource type, along with the amount of the"},{"line_number":145,"context_line":"    resource that has been used/consumed."},{"line_number":146,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":12,"id":"fa32b979_e09437e3","line":143,"updated":"2015-06-16 13:12:50.000000000","message":"nit: says Spec","commit_id":"a67b2cbdea9ff10b9ccc48dc7477c9d4126573b4"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"154209cf188195ff9e9fb5b9eada497f820733b2","unresolved":false,"context_lines":[{"line_number":263,"context_line":"        :raises ValueError if amountis not an `IntegerAmount`."},{"line_number":264,"context_line":"        \"\"\""},{"line_number":265,"context_line":"        if not isinstance(amount, IntegerAmount):"},{"line_number":266,"context_line":"            msg \u003d _(\"amount_spec must be an IntegerAmount\")"},{"line_number":267,"context_line":"            raise ValueError(msg)"},{"line_number":268,"context_line":"        if amount.is_negative:"},{"line_number":269,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa32b979_2079bfc8","line":266,"updated":"2015-06-16 13:12:50.000000000","message":"nit: says spec","commit_id":"a67b2cbdea9ff10b9ccc48dc7477c9d4126573b4"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"626b0aee824bc4527569d3664f55b60eec10ca9d","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        # of RAM, it *would* be able to fit on the host, since the requested"},{"line_number":158,"context_line":"        # RAM \u003c\u003d total RAM (16 \u003c\u003d 16), and the (total * overcommit) minus used"},{"line_number":159,"context_line":"        # is \u003e\u003d requested ((16 * 2.0) - 8 \u003e\u003d 16)."},{"line_number":160,"context_line":"        \u0027overcommit_ratio\u0027: fields.FloatField(default\u003dNone, nullable\u003dNone),"},{"line_number":161,"context_line":"    }"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    def __init__(self, total, used, overcommit_ratio\u003dNone):"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a50d1a3_4ec62669","line":160,"updated":"2015-07-22 21:17:07.000000000","message":"seems like we could default to 1.0, but ^^","commit_id":"998fe5a8fa923899bfc95dbfd44792e091d1504d"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"bae4051337afa33edd20797d4978bd0e97b32c23","unresolved":false,"context_lines":[{"line_number":39,"context_line":"    called \"is_negative\" that returns whether the amount represents"},{"line_number":40,"context_line":"    a negative value."},{"line_number":41,"context_line":"    \"\"\""},{"line_number":42,"context_line":"    pass"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"@base.NovaObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":19,"id":"3a50d1a3_4b5ed113","line":42,"updated":"2015-07-23 21:34:22.000000000","message":"I probably missed some conversation, so don\u0027t be afraid by my silly question, but why are you needing an ABC if you don\u0027t provide an abstract interface for it?\n\nI can see your docstring, maybe just providing an abstract property called is_negative would be good, no ?","commit_id":"0e4d22d55b701d49b36624062cf772e833f333a6"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"84784f446c0e2696c4630977aa06358d862e3416","unresolved":false,"context_lines":[{"line_number":39,"context_line":"    called \"is_negative\" that returns whether the amount represents"},{"line_number":40,"context_line":"    a negative value."},{"line_number":41,"context_line":"    \"\"\""},{"line_number":42,"context_line":"    pass"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"@base.NovaObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":19,"id":"3a50d1a3_f902a53d","line":42,"in_reply_to":"3a50d1a3_4b5ed113","updated":"2015-07-27 17:44:13.000000000","message":"I\u0027ll add some methods here","commit_id":"0e4d22d55b701d49b36624062cf772e833f333a6"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"84784f446c0e2696c4630977aa06358d862e3416","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        return self.amount \u003d\u003d other.amount"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    def __hash__(self):"},{"line_number":64,"context_line":"        return self.amount"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def __ne__(self, other):"},{"line_number":67,"context_line":"        return not self \u003d\u003d other"}],"source_content_type":"text/x-python","patch_set":19,"id":"3a50d1a3_25dd3b42","line":64,"updated":"2015-07-27 17:44:13.000000000","message":"I noticed that hashes must not change and therefore can only be taken on immutable objects. While Amounts are not meant to change, they can, and Inventories are mutable by design. So rather than promote __hash__ to the ABC as I intended, I\u0027ve removed it from all these classes.","commit_id":"0e4d22d55b701d49b36624062cf772e833f333a6"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e936adf71c1b3f07988bc6ea72dc7cdac9df93eb","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        return self.amount \u003d\u003d other.amount"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    def __hash__(self):"},{"line_number":64,"context_line":"        return self.amount"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def __ne__(self, other):"},{"line_number":67,"context_line":"        return not self \u003d\u003d other"}],"source_content_type":"text/x-python","patch_set":19,"id":"3a50d1a3_800af688","line":64,"in_reply_to":"3a50d1a3_203e828b","updated":"2015-07-27 20:16:30.000000000","message":"Uh, my proof is not exactly clear, you\u0027ll see that an object providing only __eq__ is not keyable while the same object adding __hash__ is keyable.","commit_id":"0e4d22d55b701d49b36624062cf772e833f333a6"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"528376052ca1ffbe6171ffe670296ea14a8c8a68","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        return self.amount \u003d\u003d other.amount"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    def __hash__(self):"},{"line_number":64,"context_line":"        return self.amount"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def __ne__(self, other):"},{"line_number":67,"context_line":"        return not self \u003d\u003d other"}],"source_content_type":"text/x-python","patch_set":19,"id":"3a50d1a3_203e828b","line":64,"in_reply_to":"3a50d1a3_25dd3b42","updated":"2015-07-27 20:12:41.000000000","message":"Well, hashes are for keying iterables, hence why you speak about immutables. Nevertheless, you can provide an hash method for mutable objects in case you want to sort/key against them.\nsee https://docs.python.org/2/library/stdtypes.html#mapping-types-dict and https://docs.python.org/2/glossary.html#term-hashable\n\nProof here : http://paste.openstack.org/show/405965/\n\nSo, keeping an implementation of the builtin hash() sounds correct if we need to provide an iterable for IntegerAmount objects or other resources","commit_id":"0e4d22d55b701d49b36624062cf772e833f333a6"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"84784f446c0e2696c4630977aa06358d862e3416","unresolved":false,"context_lines":[{"line_number":118,"context_line":"              is decreased. In other words, the resources are freed up in"},{"line_number":119,"context_line":"              the inventory."},{"line_number":120,"context_line":"        \"\"\""},{"line_number":121,"context_line":"        raise NotImplementedError(self.update)"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"    def can_provide(self, amount):"},{"line_number":124,"context_line":"        \"\"\"Determine if there is room to fit the supplied amount of resources."}],"source_content_type":"text/x-python","patch_set":19,"id":"3a50d1a3_c5ec97f5","line":121,"updated":"2015-07-27 17:44:13.000000000","message":"This and below seem to have the wrong method names? Fixed in next PS.","commit_id":"0e4d22d55b701d49b36624062cf772e833f333a6"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"bae4051337afa33edd20797d4978bd0e97b32c23","unresolved":false,"context_lines":[{"line_number":159,"context_line":"        \u0027overcommit_ratio\u0027: fields.FloatField(nullable\u003dFalse),"},{"line_number":160,"context_line":"    }"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"    def __init__(self, total, used, overcommit_ratio\u003dNone):"},{"line_number":163,"context_line":"        \"\"\"Constructs an amount from a requested integer amount."},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        :param total: non-negative integer representing the total capacity of"}],"source_content_type":"text/x-python","patch_set":19,"id":"3a50d1a3_0bd76924","line":162,"updated":"2015-07-23 21:34:22.000000000","message":"not sure if it\u0027s good to provide an init method for a NovaObject needing to provide arguments. I would prefer to leave args and kwargs IMHO","commit_id":"0e4d22d55b701d49b36624062cf772e833f333a6"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"84784f446c0e2696c4630977aa06358d862e3416","unresolved":false,"context_lines":[{"line_number":159,"context_line":"        \u0027overcommit_ratio\u0027: fields.FloatField(nullable\u003dFalse),"},{"line_number":160,"context_line":"    }"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"    def __init__(self, total, used, overcommit_ratio\u003dNone):"},{"line_number":163,"context_line":"        \"\"\"Constructs an amount from a requested integer amount."},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        :param total: non-negative integer representing the total capacity of"}],"source_content_type":"text/x-python","patch_set":19,"id":"3a50d1a3_71a1c9a8","line":162,"in_reply_to":"3a50d1a3_0bd76924","updated":"2015-07-27 17:44:13.000000000","message":"I like that this makes it mandatory to supply total and used. Almost put something about compile-time warning but this is Python haha.","commit_id":"0e4d22d55b701d49b36624062cf772e833f333a6"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"bae4051337afa33edd20797d4978bd0e97b32c23","unresolved":false,"context_lines":[{"line_number":173,"context_line":"                either is a negative integer."},{"line_number":174,"context_line":"        \"\"\""},{"line_number":175,"context_line":"        # TODO(jaypipes): Remove this custom constructor when o.vo 3.0"},{"line_number":176,"context_line":"        # is release which adds validator callables."},{"line_number":177,"context_line":"        if not isinstance(total, six.integer_types) or (total \u003c 0):"},{"line_number":178,"context_line":"            msg \u003d _(\"total must be a positive integer\")"},{"line_number":179,"context_line":"            raise ValueError(msg)"}],"source_content_type":"text/x-python","patch_set":19,"id":"3a50d1a3_8b1cd914","line":176,"updated":"2015-07-23 21:34:22.000000000","message":"I totally understand the problem, but why is it better to put some validation within the init method when you can just provide a new type of field for coercing correctly the fields ? Of course, you would mention that TODO at the top of the nova needed type of field.\n\nHere, I\u0027m a bit worried that anyone can just update the field by putting a negative integer which wouldn\u0027t be fenced.\n\nsomething like :\n\n obj \u003d IntegerInventory(1, 1)\n obj.total \u003d -1","commit_id":"0e4d22d55b701d49b36624062cf772e833f333a6"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"84784f446c0e2696c4630977aa06358d862e3416","unresolved":false,"context_lines":[{"line_number":173,"context_line":"                either is a negative integer."},{"line_number":174,"context_line":"        \"\"\""},{"line_number":175,"context_line":"        # TODO(jaypipes): Remove this custom constructor when o.vo 3.0"},{"line_number":176,"context_line":"        # is release which adds validator callables."},{"line_number":177,"context_line":"        if not isinstance(total, six.integer_types) or (total \u003c 0):"},{"line_number":178,"context_line":"            msg \u003d _(\"total must be a positive integer\")"},{"line_number":179,"context_line":"            raise ValueError(msg)"}],"source_content_type":"text/x-python","patch_set":19,"id":"3a50d1a3_d1445db8","line":176,"in_reply_to":"3a50d1a3_8b1cd914","updated":"2015-07-27 17:44:13.000000000","message":"Created NonNegativeFloatField and NonNegativeIntegerField.","commit_id":"0e4d22d55b701d49b36624062cf772e833f333a6"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"04141830a14abf918c2605f4f154f14766717376","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    @property"},{"line_number":37,"context_line":"    def is_negative(self):"},{"line_number":38,"context_line":"        raise NotImplementedError(self.is_negative)"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def __eq__(self, other):"},{"line_number":41,"context_line":"        raise NotImplementedError(self.__eq__)"}],"source_content_type":"text/x-python","patch_set":20,"id":"3a50d1a3_b5c2d251","line":38,"updated":"2015-07-27 20:12:27.000000000","message":"nit: NotImplementedError accepts an optional string argument but I think it\u0027s not really needed here since it\u0027s an ABC.\n\n(same comment for the 2 other below methods)","commit_id":"a5f5656f10fb07cf0b465d816fcc94ffec13973c"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"5e70cfe4a329ba2b1fdf49aca8fe64cdd2050e09","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    @property"},{"line_number":37,"context_line":"    def is_negative(self):"},{"line_number":38,"context_line":"        raise NotImplementedError(self.is_negative)"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def __eq__(self, other):"},{"line_number":41,"context_line":"        raise NotImplementedError(self.__eq__)"}],"source_content_type":"text/x-python","patch_set":20,"id":"3a50d1a3_08efdae9","line":38,"in_reply_to":"3a50d1a3_b5c2d251","updated":"2015-07-28 09:19:08.000000000","message":"I defer to Jay on this one; Python manual is totally inadequate here","commit_id":"a5f5656f10fb07cf0b465d816fcc94ffec13973c"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"04141830a14abf918c2605f4f154f14766717376","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    def __eq__(self, other):"},{"line_number":61,"context_line":"        if not isinstance(other, IntegerAmount):"},{"line_number":62,"context_line":"            return NotImplemented"},{"line_number":63,"context_line":"        return self.amount \u003d\u003d other.amount"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def __ne__(self, other):"},{"line_number":66,"context_line":"        return not self \u003d\u003d other"}],"source_content_type":"text/x-python","patch_set":20,"id":"3a50d1a3_205522c1","line":63,"updated":"2015-07-27 20:12:27.000000000","message":"like I said in PS19, you should keep __hash__ IMHO","commit_id":"a5f5656f10fb07cf0b465d816fcc94ffec13973c"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b3ce62e31e0c3082897b93e1d649e640d4b87150","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    def __eq__(self, other):"},{"line_number":61,"context_line":"        if not isinstance(other, IntegerAmount):"},{"line_number":62,"context_line":"            return NotImplemented"},{"line_number":63,"context_line":"        return self.amount \u003d\u003d other.amount"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def __ne__(self, other):"},{"line_number":66,"context_line":"        return not self \u003d\u003d other"}],"source_content_type":"text/x-python","patch_set":20,"id":"3a50d1a3_1e6710b8","line":63,"in_reply_to":"3a50d1a3_08c6fa67","updated":"2015-07-28 09:30:45.000000000","message":"Sounds like we\u0027re yakshaving with nits. Here, my thought is that if two IntegerAmount objects are having the same internal \u0027amount\u0027 field value, then those 2 objects should be compared as equal and would be considered as duplicates for a set.\n\nProof here :\nhttp://paste.openstack.org/show/406035/\n\nI\u0027m not saying that what you explain is wrong, here I\u0027m debating on the expected behaviour we want to have. Since Jay is the one who wrote that, I suspect he added the hash() override for that specific purpose, and that\u0027s why I want to keep it back.","commit_id":"a5f5656f10fb07cf0b465d816fcc94ffec13973c"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"5e70cfe4a329ba2b1fdf49aca8fe64cdd2050e09","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    def __eq__(self, other):"},{"line_number":61,"context_line":"        if not isinstance(other, IntegerAmount):"},{"line_number":62,"context_line":"            return NotImplemented"},{"line_number":63,"context_line":"        return self.amount \u003d\u003d other.amount"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def __ne__(self, other):"},{"line_number":66,"context_line":"        return not self \u003d\u003d other"}],"source_content_type":"text/x-python","patch_set":20,"id":"3a50d1a3_08c6fa67","line":63,"in_reply_to":"3a50d1a3_205522c1","updated":"2015-07-28 09:19:08.000000000","message":"https://docs.python.org/3.0/glossary.html#term-hashable\n\nThere are two constraints on a class to be properly hashable:\n* The hash value must never change over time\n* Two objects which compare equal must have the same hash\n\nNovaObject inherits __eq__ and __hash__ from object. __eq__ always returns False while __hash__ returns id(). These meet the constraints, although arguably __eq__ is lying a bit.\n\nIf we implement __eq__ according to attributes, we break the second constraint, unless we also implement __hash__ according to attributes. However, we then break the first constraint unless all those attributes are immutable.","commit_id":"a5f5656f10fb07cf0b465d816fcc94ffec13973c"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"04141830a14abf918c2605f4f154f14766717376","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        \u0027overcommit_ratio\u0027: fields.NonNegativeFloatField(nullable\u003dFalse),"},{"line_number":162,"context_line":"    }"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def __init__(self, total, used, overcommit_ratio\u003d1.0):"},{"line_number":165,"context_line":"        \"\"\"Constructs an amount from a requested integer amount."},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"        :param total: non-negative integer representing the total capacity of"}],"source_content_type":"text/x-python","patch_set":20,"id":"3a50d1a3_602d6a45","line":164,"updated":"2015-07-27 20:12:27.000000000","message":"same comment as in PS19, I still think we should leave a NovaObject initialization just done without needing some parameters IMHO.\n\nWhy ? Because you know, we have the fields already non nullable, so it means a proper NovaObject needs anyway to have the fields set.","commit_id":"a5f5656f10fb07cf0b465d816fcc94ffec13973c"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b3ce62e31e0c3082897b93e1d649e640d4b87150","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        \u0027overcommit_ratio\u0027: fields.NonNegativeFloatField(nullable\u003dFalse),"},{"line_number":162,"context_line":"    }"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def __init__(self, total, used, overcommit_ratio\u003d1.0):"},{"line_number":165,"context_line":"        \"\"\"Constructs an amount from a requested integer amount."},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"        :param total: non-negative integer representing the total capacity of"}],"source_content_type":"text/x-python","patch_set":20,"id":"3a50d1a3_bed4dc1f","line":164,"in_reply_to":"3a50d1a3_5bafea29","updated":"2015-07-28 09:30:45.000000000","message":"In general, I explicitely prefix my comments by \u0027nit: \u0027 if so. No, here, I seriously think we shouldn\u0027t constraint by a list of parameters and rather use args and kwargs for a proper initialization.","commit_id":"a5f5656f10fb07cf0b465d816fcc94ffec13973c"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"5e70cfe4a329ba2b1fdf49aca8fe64cdd2050e09","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        \u0027overcommit_ratio\u0027: fields.NonNegativeFloatField(nullable\u003dFalse),"},{"line_number":162,"context_line":"    }"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def __init__(self, total, used, overcommit_ratio\u003d1.0):"},{"line_number":165,"context_line":"        \"\"\"Constructs an amount from a requested integer amount."},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"        :param total: non-negative integer representing the total capacity of"}],"source_content_type":"text/x-python","patch_set":20,"id":"3a50d1a3_5bafea29","line":164,"in_reply_to":"3a50d1a3_602d6a45","updated":"2015-07-28 09:19:08.000000000","message":"is this a nit or a -1?","commit_id":"a5f5656f10fb07cf0b465d816fcc94ffec13973c"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"c9fdc5bc28b8f4b300094fc85f164ff714cc1ad2","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    @property"},{"line_number":37,"context_line":"    def is_negative(self):"},{"line_number":38,"context_line":"        raise NotImplementedError(self.is_negative)"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def __eq__(self, other):"},{"line_number":41,"context_line":"        raise NotImplementedError(self.__eq__)"}],"source_content_type":"text/x-python","patch_set":22,"id":"3a50d1a3_9e8560d0","line":38,"updated":"2015-07-28 09:37:34.000000000","message":"see PS20, I think it\u0027s rather better to use a string or nothing instead of stringifying the method name","commit_id":"f6ff91a6bdecbdd3ac687ad8f82c237c19b82e2e"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"afdca4af0633bd10ff4d187f1bee1ea705cd12dc","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    @property"},{"line_number":37,"context_line":"    def is_negative(self):"},{"line_number":38,"context_line":"        raise NotImplementedError(self.is_negative)"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def __eq__(self, other):"},{"line_number":41,"context_line":"        raise NotImplementedError(self.__eq__)"}],"source_content_type":"text/x-python","patch_set":22,"id":"3a50d1a3_b4963bee","line":38,"in_reply_to":"3a50d1a3_9e8560d0","updated":"2015-07-28 10:24:37.000000000","message":"This is a nit imo; I don\u0027t care either way but I don\u0027t think it\u0027s worth a -1","commit_id":"f6ff91a6bdecbdd3ac687ad8f82c237c19b82e2e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"c9fdc5bc28b8f4b300094fc85f164ff714cc1ad2","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    def __eq__(self, other):"},{"line_number":61,"context_line":"        if not isinstance(other, IntegerAmount):"},{"line_number":62,"context_line":"            return NotImplemented"},{"line_number":63,"context_line":"        return self.amount \u003d\u003d other.amount"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def __ne__(self, other):"},{"line_number":66,"context_line":"        return not self \u003d\u003d other"}],"source_content_type":"text/x-python","patch_set":22,"id":"3a50d1a3_5e6cb834","line":63,"updated":"2015-07-28 09:37:34.000000000","message":"see PS20, I\u0027m worried by removing the __hash__() implementation","commit_id":"f6ff91a6bdecbdd3ac687ad8f82c237c19b82e2e"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"afdca4af0633bd10ff4d187f1bee1ea705cd12dc","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    def __eq__(self, other):"},{"line_number":61,"context_line":"        if not isinstance(other, IntegerAmount):"},{"line_number":62,"context_line":"            return NotImplemented"},{"line_number":63,"context_line":"        return self.amount \u003d\u003d other.amount"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def __ne__(self, other):"},{"line_number":66,"context_line":"        return not self \u003d\u003d other"}],"source_content_type":"text/x-python","patch_set":22,"id":"3a50d1a3_74e513a2","line":63,"in_reply_to":"3a50d1a3_5e6cb834","updated":"2015-07-28 10:24:37.000000000","message":"I can\u0027t accept an implementation of __hash__ which breaks the contract stated in the Python manual. It\u0027s worse than useless. Paste illustrating the problem:\n\nhttp://paste.openstack.org/show/406040/","commit_id":"f6ff91a6bdecbdd3ac687ad8f82c237c19b82e2e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"41ae7b5a70bca95f72e80819ece1f3730bc12021","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    def __eq__(self, other):"},{"line_number":61,"context_line":"        if not isinstance(other, IntegerAmount):"},{"line_number":62,"context_line":"            return NotImplemented"},{"line_number":63,"context_line":"        return self.amount \u003d\u003d other.amount"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def __ne__(self, other):"},{"line_number":66,"context_line":"        return not self \u003d\u003d other"}],"source_content_type":"text/x-python","patch_set":22,"id":"3a50d1a3_3748e3b2","line":63,"in_reply_to":"3a50d1a3_74e513a2","updated":"2015-07-28 13:40:35.000000000","message":"okay, let me say that it becomes a nitpicking then... We already discussed for like 40 mins on IRC without having a consensus so I don\u0027t want to block this.\n\nMy view here is that it\u0027s okay to have 2 objects being identical using hash() but seems you don\u0027t like having some problems with set(), let\u0027s put out the convo.","commit_id":"f6ff91a6bdecbdd3ac687ad8f82c237c19b82e2e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"c9fdc5bc28b8f4b300094fc85f164ff714cc1ad2","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        \u0027overcommit_ratio\u0027: fields.NonNegativeFloatField(nullable\u003dFalse),"},{"line_number":162,"context_line":"    }"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def __init__(self, total, used, overcommit_ratio\u003d1.0):"},{"line_number":165,"context_line":"        \"\"\"Constructs an amount from a requested integer amount."},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"        :param total: non-negative integer representing the total capacity of"}],"source_content_type":"text/x-python","patch_set":22,"id":"3a50d1a3_1e72b011","line":164,"updated":"2015-07-28 09:37:34.000000000","message":"see PS20, I\u0027d prefer args and kwargs, not something explicit","commit_id":"f6ff91a6bdecbdd3ac687ad8f82c237c19b82e2e"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"afdca4af0633bd10ff4d187f1bee1ea705cd12dc","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        \u0027overcommit_ratio\u0027: fields.NonNegativeFloatField(nullable\u003dFalse),"},{"line_number":162,"context_line":"    }"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def __init__(self, total, used, overcommit_ratio\u003d1.0):"},{"line_number":165,"context_line":"        \"\"\"Constructs an amount from a requested integer amount."},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"        :param total: non-negative integer representing the total capacity of"}],"source_content_type":"text/x-python","patch_set":22,"id":"3a50d1a3_2f6ad8b3","line":164,"in_reply_to":"3a50d1a3_1e72b011","updated":"2015-07-28 10:24:37.000000000","message":"What impact does this have please? One person wants one thing, one person wants another... without facts how can we choose?","commit_id":"f6ff91a6bdecbdd3ac687ad8f82c237c19b82e2e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"41ae7b5a70bca95f72e80819ece1f3730bc12021","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        \u0027overcommit_ratio\u0027: fields.NonNegativeFloatField(nullable\u003dFalse),"},{"line_number":162,"context_line":"    }"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def __init__(self, total, used, overcommit_ratio\u003d1.0):"},{"line_number":165,"context_line":"        \"\"\"Constructs an amount from a requested integer amount."},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"        :param total: non-negative integer representing the total capacity of"}],"source_content_type":"text/x-python","patch_set":22,"id":"3a50d1a3_973777fd","line":164,"in_reply_to":"3a50d1a3_2f6ad8b3","updated":"2015-07-28 13:40:35.000000000","message":"Well, having some parameters just proxying down to the parent object sounds a bit weirdo and useless, nope ?\nIn particular when the NovaObject init method accepts a list of kwargs and when the Inventory mixin doesn\u0027t have those parameters within it.","commit_id":"f6ff91a6bdecbdd3ac687ad8f82c237c19b82e2e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"3702665e743c6deac21182adb66397671e081580","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        \u0027overcommit_ratio\u0027: fields.NonNegativeFloatField(nullable\u003dFalse),"},{"line_number":162,"context_line":"    }"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def __init__(self, total, used, overcommit_ratio\u003d1.0):"},{"line_number":165,"context_line":"        \"\"\"Constructs an amount from a requested integer amount."},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"        :param total: non-negative integer representing the total capacity of"}],"source_content_type":"text/x-python","patch_set":22,"id":"3a50d1a3_3a668abb","line":164,"in_reply_to":"3a50d1a3_973777fd","updated":"2015-07-28 13:47:49.000000000","message":"Oh, I thought my view was unclear, you should rather do this way IMHO :\n\n#1: \u0027overcommit_ratio\u0027: fields.NonNegativeFloatField(default\u003d1.0)\n\n#2: def __init__(*args, **kwargs):\n      super().__init__()\n      self.obj_set_defaults()","commit_id":"f6ff91a6bdecbdd3ac687ad8f82c237c19b82e2e"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"a1b353948d447d7d9fcfa07fe6275971c8e8240a","unresolved":false,"context_lines":[{"line_number":158,"context_line":"        # of RAM, it *would* be able to fit on the host, since the requested"},{"line_number":159,"context_line":"        # RAM \u003c\u003d total RAM (16 \u003c\u003d 16), and the (total * overcommit) minus used"},{"line_number":160,"context_line":"        # is \u003e\u003d requested ((16 * 2.0) - 8 \u003e\u003d 16)."},{"line_number":161,"context_line":"        \u0027overcommit_ratio\u0027: fields.NonNegativeFloatField(nullable\u003dFalse),"},{"line_number":162,"context_line":"    }"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def __eq__(self, other):"}],"source_content_type":"text/x-python","patch_set":23,"id":"3a50d1a3_86cc278a","line":161,"updated":"2015-07-28 15:06:19.000000000","message":"Isn\u0027t there supposed to be a \u0027default\u003d1.0\u0027 parameter here? And a call to obj_set_defaults() somewhere?","commit_id":"ff63e8f9098ed918272a8fd89e062c52ac4da707"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"0c6b304db66e7a2020bc754f0d56246424fa7257","unresolved":false,"context_lines":[{"line_number":158,"context_line":"        # of RAM, it *would* be able to fit on the host, since the requested"},{"line_number":159,"context_line":"        # RAM \u003c\u003d total RAM (16 \u003c\u003d 16), and the (total * overcommit) minus used"},{"line_number":160,"context_line":"        # is \u003e\u003d requested ((16 * 2.0) - 8 \u003e\u003d 16)."},{"line_number":161,"context_line":"        \u0027overcommit_ratio\u0027: fields.NonNegativeFloatField(nullable\u003dFalse),"},{"line_number":162,"context_line":"    }"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def __eq__(self, other):"}],"source_content_type":"text/x-python","patch_set":23,"id":"3a50d1a3_5039caa2","line":161,"in_reply_to":"3a50d1a3_36069ace","updated":"2015-07-29 08:46:49.000000000","message":"hmm, thought I\u0027d put that back - done","commit_id":"ff63e8f9098ed918272a8fd89e062c52ac4da707"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"01e67ddba36f238c31075159c491489ac4793e80","unresolved":false,"context_lines":[{"line_number":158,"context_line":"        # of RAM, it *would* be able to fit on the host, since the requested"},{"line_number":159,"context_line":"        # RAM \u003c\u003d total RAM (16 \u003c\u003d 16), and the (total * overcommit) minus used"},{"line_number":160,"context_line":"        # is \u003e\u003d requested ((16 * 2.0) - 8 \u003e\u003d 16)."},{"line_number":161,"context_line":"        \u0027overcommit_ratio\u0027: fields.NonNegativeFloatField(nullable\u003dFalse),"},{"line_number":162,"context_line":"    }"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def __eq__(self, other):"}],"source_content_type":"text/x-python","patch_set":23,"id":"3a50d1a3_36069ace","line":161,"in_reply_to":"3a50d1a3_86cc278a","updated":"2015-07-28 16:48:17.000000000","message":"That\u0027s something we discussed and we agreed to not use since obj_set_defaults() is a bit tricky if the IntegerInventory becomes persisted.\n\nThat said, I think we should keep the default value for the overcommit_ratio even if not calling obj_set_defaults()","commit_id":"ff63e8f9098ed918272a8fd89e062c52ac4da707"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"05b77174584f90a40ab0f4246ec1f6a050b2a60b","unresolved":false,"context_lines":[{"line_number":97,"context_line":"        return not self \u003d\u003d other"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"class Inventory(object):"},{"line_number":101,"context_line":"    \"\"\"Represents the capacity and used amount of a resource."},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"    The Inventory subclasses enable different types of resources to express"}],"source_content_type":"text/x-python","patch_set":24,"id":"3a50d1a3_f12815e4","line":100,"updated":"2015-07-29 11:44:05.000000000","message":"Technically we could call this and Amount a Mixin - just a comment, no action needed.","commit_id":"8d2979a09ac9ce72bda54fa254aa12b2a0773fae"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"05b77174584f90a40ab0f4246ec1f6a050b2a60b","unresolved":false,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"@base.NovaObjectRegistry.register"},{"line_number":138,"context_line":"class IntegerInventory(base.NovaObject, Inventory):"},{"line_number":139,"context_line":"    \"\"\"Represents the total and used amount of a resource with integers.\"\"\""},{"line_number":140,"context_line":"    # Version 1.0: Initial version"},{"line_number":141,"context_line":"    VERSION \u003d \u00271.0\u0027"}],"source_content_type":"text/x-python","patch_set":24,"id":"3a50d1a3_2cb2a606","line":138,"updated":"2015-07-29 11:44:05.000000000","message":"I wonder why these need to be objects - we do build them from objects - yes, but they don\u0027t really have any remotable methods and are basically purely code containers.\n\nI guess we will eventually want to pass them over RPC, as I assume compute nodes will advertize their capacity using these - but even that seems stretched to me. These are really purely wrappers around something that is already versioned as part of the compute_node object - it feels to me we are introducing a parallel branch of versioning here.\n\nSeeing how version bumps trickle down to the root - It might be wiser to not make thse objects until we are certain they need to be?","commit_id":"8d2979a09ac9ce72bda54fa254aa12b2a0773fae"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"51c91f56831d4a133da060eddde52219a2a64470","unresolved":false,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"@base.NovaObjectRegistry.register"},{"line_number":138,"context_line":"class IntegerInventory(base.NovaObject, Inventory):"},{"line_number":139,"context_line":"    \"\"\"Represents the total and used amount of a resource with integers.\"\"\""},{"line_number":140,"context_line":"    # Version 1.0: Initial version"},{"line_number":141,"context_line":"    VERSION \u003d \u00271.0\u0027"}],"source_content_type":"text/x-python","patch_set":24,"id":"3a50d1a3_4c45b26c","line":138,"in_reply_to":"3a50d1a3_2cb2a606","updated":"2015-07-29 11:48:37.000000000","message":"OK coming back to this after reading the spec for a moment.\n\n\n  \"\"\"\n  These amount, inventory and resource classes will be nova.objects object classes \n  and will enable Nova to evolve, in a versioned manner, the way that it tracks \n  resources and exposes resource consumption.\n  \"\"\"\n\n\nI still don\u0027t get how this help us - we will only build a parallel versioned hierarchy to the compute_node and instance hierarchy of objects.\n\nWe can\u0027t version code - only data, and all data here (amounts and Inventories as part of Instance and ComputeNode) is already versioned. I really don\u0027t get what this buys us.","commit_id":"8d2979a09ac9ce72bda54fa254aa12b2a0773fae"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c50848e2ec45b333afb3cd28c5b2145de8cc83ab","unresolved":false,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"@base.NovaObjectRegistry.register"},{"line_number":138,"context_line":"class IntegerInventory(base.NovaObject, Inventory):"},{"line_number":139,"context_line":"    \"\"\"Represents the total and used amount of a resource with integers.\"\"\""},{"line_number":140,"context_line":"    # Version 1.0: Initial version"},{"line_number":141,"context_line":"    VERSION \u003d \u00271.0\u0027"}],"source_content_type":"text/x-python","patch_set":24,"id":"3a50d1a3_f4dd5141","line":138,"in_reply_to":"3a50d1a3_4c45b26c","updated":"2015-07-29 16:54:54.000000000","message":"Nikola, the idea is to eventually pull the resource fields out from the compute_nodes and instances tables and place them into a resource_pools table. However, we don\u0027t know whether there will need to be changes to the structure of the resource representations over time, so it was agreed to make these nova.objects so we can evolve the schema over time.\n\nNote that these objects used to not be nova.objects, but were changed to nova.objects after a lengthy back and forth discussion on both the spec and this patch series.","commit_id":"8d2979a09ac9ce72bda54fa254aa12b2a0773fae"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"0ad1aedab17ecb7fc23b5d7e441b0167a6920c62","unresolved":false,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"@base.NovaObjectRegistry.register"},{"line_number":138,"context_line":"class IntegerInventory(base.NovaObject, Inventory):"},{"line_number":139,"context_line":"    \"\"\"Represents the total and used amount of a resource with integers.\"\"\""},{"line_number":140,"context_line":"    # Version 1.0: Initial version"},{"line_number":141,"context_line":"    VERSION \u003d \u00271.0\u0027"}],"source_content_type":"text/x-python","patch_set":24,"id":"3a50d1a3_fa2d3b46","line":138,"in_reply_to":"3a50d1a3_f4dd5141","updated":"2015-07-29 17:19:23.000000000","message":"OK","commit_id":"8d2979a09ac9ce72bda54fa254aa12b2a0773fae"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"05b77174584f90a40ab0f4246ec1f6a050b2a60b","unresolved":false,"context_lines":[{"line_number":289,"context_line":"        requested_topology \u003d amount.topology"},{"line_number":290,"context_line":"        return hw.numa_fit_instance_to_host("},{"line_number":291,"context_line":"                    self.topology, requested_topology,"},{"line_number":292,"context_line":"                    limits\u003dself.limits) is not None"}],"source_content_type":"text/x-python","patch_set":24,"id":"3a50d1a3_112341be","line":292,"updated":"2015-07-29 11:44:05.000000000","message":"So the problem with this (and in a way PCI devices once we get to them) is that can_provide() does not only answer yes or no - but actually gives back the provided topology which needs to be persisted.\n\nIn theory PCI trakcer does the same thing - gives back a list of _actual_ devices provided. Pci device tracking works slightly differently, so this information is recorded with the devices themselves (as oppposed to NUMA claims which are stored with the instance). The result is the same however - we have a sideffect -  a result of this call that needs to be persisted. The can_provide() interface proposed here, does not expose that in any way, and it really should.","commit_id":"8d2979a09ac9ce72bda54fa254aa12b2a0773fae"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"70a9c400c296e9369cd591c0ebec038d1bd421e2","unresolved":false,"context_lines":[{"line_number":289,"context_line":"        requested_topology \u003d amount.topology"},{"line_number":290,"context_line":"        return hw.numa_fit_instance_to_host("},{"line_number":291,"context_line":"                    self.topology, requested_topology,"},{"line_number":292,"context_line":"                    limits\u003dself.limits) is not None"}],"source_content_type":"text/x-python","patch_set":24,"id":"3a50d1a3_e8e551a4","line":292,"in_reply_to":"3a50d1a3_039d22bb","updated":"2015-07-30 10:32:39.000000000","message":"So after talking this out some more with one of the authors on IRC - they seem to believe that the step of fitting a resource request onto a pool of resources is a distinct step from consuming them (I agree), but they also believe that fitting (and persisting the resulting fit) needs not be a part of the resource Inventory interface (I disagree but don\u0027t feel to strongly about it - plus it\u0027s easy to fix later).\n\nSeeing that we have mutual understanding of what is needed here and differ in how it can be accomplished (not important in the long run really) I am OK with this going forward!","commit_id":"8d2979a09ac9ce72bda54fa254aa12b2a0773fae"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"fc22d4656ed6e584f8f5095b48e1bff5ebea4621","unresolved":false,"context_lines":[{"line_number":289,"context_line":"        requested_topology \u003d amount.topology"},{"line_number":290,"context_line":"        return hw.numa_fit_instance_to_host("},{"line_number":291,"context_line":"                    self.topology, requested_topology,"},{"line_number":292,"context_line":"                    limits\u003dself.limits) is not None"}],"source_content_type":"text/x-python","patch_set":24,"id":"3a50d1a3_f1232e56","line":292,"in_reply_to":"3a50d1a3_112341be","updated":"2015-07-29 14:08:00.000000000","message":"You already know this as the NUMAAmount is wrapping an InstanceNUMATopology which specifies exactly which CPUs to pin in which cells (along with some other stuff). The matching is done elsewhere.","commit_id":"8d2979a09ac9ce72bda54fa254aa12b2a0773fae"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"8626bb09d2a8e8f0c6925313233d1bc3b1a8f504","unresolved":false,"context_lines":[{"line_number":289,"context_line":"        requested_topology \u003d amount.topology"},{"line_number":290,"context_line":"        return hw.numa_fit_instance_to_host("},{"line_number":291,"context_line":"                    self.topology, requested_topology,"},{"line_number":292,"context_line":"                    limits\u003dself.limits) is not None"}],"source_content_type":"text/x-python","patch_set":24,"id":"3a50d1a3_039d22bb","line":292,"in_reply_to":"3a50d1a3_9af51784","updated":"2015-07-29 17:56:28.000000000","message":"OK so I get Jay\u0027s take on this (just spoke on IRC) that we should not mix the two and that\u0027s ok.\n\nWe should still have an interface into the fact that some resources need to lock in the specific amoung for which can_provide is true that can do the sideffect.\n\nsomething like provided_amount(amount, *kwargs): which would then go and do the stuff it needs to?","commit_id":"8d2979a09ac9ce72bda54fa254aa12b2a0773fae"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"0ad1aedab17ecb7fc23b5d7e441b0167a6920c62","unresolved":false,"context_lines":[{"line_number":289,"context_line":"        requested_topology \u003d amount.topology"},{"line_number":290,"context_line":"        return hw.numa_fit_instance_to_host("},{"line_number":291,"context_line":"                    self.topology, requested_topology,"},{"line_number":292,"context_line":"                    limits\u003dself.limits) is not None"}],"source_content_type":"text/x-python","patch_set":24,"id":"3a50d1a3_9af51784","line":292,"in_reply_to":"3a50d1a3_b4cb4971","updated":"2015-07-29 17:19:23.000000000","message":"Nope this is not good enough for migrations, and it\u0027s not good enough for claims either acutally. We need to know which EXACT topology we are claiming so that we can persist it on the instance (and potentially migration context objects).\n\nAt first the instance topology on the Instance object contains no specifics of the cells/cpus it is being pinned to - this is only updated after the claim (which calls numa_fit_instance_to_host and uses the topology returned from there). We then update the instance with the topoology that has all the specifics set for the host we are claimed on.\n\nA very similar thing happens with PCI devices.\n\nThis interface gives us no way to consume the sideffect of \"fitting\" these resources.","commit_id":"8d2979a09ac9ce72bda54fa254aa12b2a0773fae"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"15328f14c81e0b1ea2ee51d7a13a783812dd1bd1","unresolved":false,"context_lines":[{"line_number":289,"context_line":"        requested_topology \u003d amount.topology"},{"line_number":290,"context_line":"        return hw.numa_fit_instance_to_host("},{"line_number":291,"context_line":"                    self.topology, requested_topology,"},{"line_number":292,"context_line":"                    limits\u003dself.limits) is not None"}],"source_content_type":"text/x-python","patch_set":24,"id":"3a50d1a3_49979d3a","line":292,"in_reply_to":"3a50d1a3_b5679db6","updated":"2015-07-30 14:49:06.000000000","message":"Sigh, slowly working through this. So it turns out that InstanceNUMATopology can be either fitted or unfitted, Nikola referred to this in his 2nd comment but I missed it. I don\u0027t like this very much, it unifies the types of two very different things imo.\n\nStill, here we are. So I\u0027d suggest that we return the fitted InstanceNUMATopology and pass it back into #consume later. So hopefully resources can be fitted and consumed with code like:\n\n    for resource, amount in request_spec.resources.items():\n        # if amount requires fitting to specific resources, offer is a solution or None\n        # Otherwise it should be True/None\n        offer \u003d compute_node.inventories[resource].can_provide(amount)\n        if offer:\n            # do something... perhaps claim resources on the compute\n            # node, which might eventually call:\n            compute_node.inventories[resource].consume(offer)\n\nWaiting to do this until I can confirm this is the right direction with Jay.","commit_id":"8d2979a09ac9ce72bda54fa254aa12b2a0773fae"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"436c3d63781190f20dd9ddc86a36c0d6194350e2","unresolved":false,"context_lines":[{"line_number":289,"context_line":"        requested_topology \u003d amount.topology"},{"line_number":290,"context_line":"        return hw.numa_fit_instance_to_host("},{"line_number":291,"context_line":"                    self.topology, requested_topology,"},{"line_number":292,"context_line":"                    limits\u003dself.limits) is not None"}],"source_content_type":"text/x-python","patch_set":24,"id":"3a50d1a3_b5679db6","line":292,"in_reply_to":"3a50d1a3_e8e551a4","updated":"2015-07-30 11:06:03.000000000","message":"I finally got Nikola\u0027s point, if we have to pre-fit some resources it destroys the nice uniform resource comparison and consumption code the spec is working towards. So this needs to take unfitted resources. I\u0027ll work on an update after lunch.","commit_id":"8d2979a09ac9ce72bda54fa254aa12b2a0773fae"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c50848e2ec45b333afb3cd28c5b2145de8cc83ab","unresolved":false,"context_lines":[{"line_number":289,"context_line":"        requested_topology \u003d amount.topology"},{"line_number":290,"context_line":"        return hw.numa_fit_instance_to_host("},{"line_number":291,"context_line":"                    self.topology, requested_topology,"},{"line_number":292,"context_line":"                    limits\u003dself.limits) is not None"}],"source_content_type":"text/x-python","patch_set":24,"id":"3a50d1a3_b4cb4971","line":292,"in_reply_to":"3a50d1a3_f1232e56","updated":"2015-07-29 16:54:54.000000000","message":"Right. What lxsli said.","commit_id":"8d2979a09ac9ce72bda54fa254aa12b2a0773fae"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"c38b5820c14baed973fd9977fddd2f0ae20e70fd","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    @property"},{"line_number":37,"context_line":"    def is_negative(self):"},{"line_number":38,"context_line":"        raise NotImplementedError(self.is_negative)"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def __eq__(self, other):"},{"line_number":41,"context_line":"        raise NotImplementedError(self.__eq__)"}],"source_content_type":"text/x-python","patch_set":26,"id":"fa1b9901_7425a8c0","line":38,"updated":"2015-08-17 13:17:17.000000000","message":"this looks like  infinite recursion. self.is_negative is not implement, not implement, not implement....","commit_id":"04d3967a7ab27f82abd53791ee237e8ab4c1a0d3"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"6195f19cb37448bf5d315a5cde6f957c0fab6756","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    @property"},{"line_number":37,"context_line":"    def is_negative(self):"},{"line_number":38,"context_line":"        raise NotImplementedError(self.is_negative)"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def __eq__(self, other):"},{"line_number":41,"context_line":"        raise NotImplementedError(self.__eq__)"}],"source_content_type":"text/x-python","patch_set":26,"id":"fa1b9901_1af9c080","line":38,"in_reply_to":"fa1b9901_7425a8c0","updated":"2015-08-18 12:28:47.000000000","message":"Right, I thought something magic was going on here! I guess this should be:\n\n    raise NotImplementedError(\"Amount.is_negative\")\n\n?","commit_id":"04d3967a7ab27f82abd53791ee237e8ab4c1a0d3"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"c38b5820c14baed973fd9977fddd2f0ae20e70fd","unresolved":false,"context_lines":[{"line_number":287,"context_line":"        if amount.is_negative:"},{"line_number":288,"context_line":"            return False"},{"line_number":289,"context_line":"        requested_topology \u003d amount.topology"},{"line_number":290,"context_line":"        return hw.numa_fit_instance_to_host("},{"line_number":291,"context_line":"                    self.topology, requested_topology,"},{"line_number":292,"context_line":"                    limits\u003dself.limits) is not None"}],"source_content_type":"text/x-python","patch_set":26,"id":"fa1b9901_415541c4","line":290,"updated":"2015-08-17 13:17:17.000000000","message":"emm...currently we depend on the result of this function to know what guest numa topo looks like in the compute node.\n\nJust curiosity after we switch to those resource object, how can we get guest numa topo, re-caculate again after resource claim? But I think you already have plan.","commit_id":"04d3967a7ab27f82abd53791ee237e8ab4c1a0d3"},{"author":{"_account_id":10618,"name":"Pavel Kholkin","email":"p.v.holkin@mail.ru","username":"pkholkin"},"change_message_id":"7927bb6280f9bff50ac7d3c04733c15cb809824f","unresolved":false,"context_lines":[{"line_number":80,"context_line":"        \u0027topology\u0027: fields.ObjectField(\u0027InstanceNUMATopology\u0027),"},{"line_number":81,"context_line":"    }"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    obj_relationships \u003d {"},{"line_number":84,"context_line":"        \u0027topology\u0027: [(\u00271.0\u0027, \u00271.2\u0027)]"},{"line_number":85,"context_line":"    }"},{"line_number":86,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"7a740942_a71b7872","line":83,"updated":"2015-12-07 12:50:28.000000000","message":"obj_relationships were removed by Dan Smith https://review.openstack.org/#/c/231271/9","commit_id":"61940a97c2345a2deeccd43f5a562c98243593f4"},{"author":{"_account_id":10618,"name":"Pavel Kholkin","email":"p.v.holkin@mail.ru","username":"pkholkin"},"change_message_id":"7927bb6280f9bff50ac7d3c04733c15cb809824f","unresolved":false,"context_lines":[{"line_number":235,"context_line":"        \u0027limits\u0027: fields.ObjectField(\u0027NUMATopologyLimits\u0027, nullable\u003dTrue),"},{"line_number":236,"context_line":"    }"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"    obj_relationships \u003d {"},{"line_number":239,"context_line":"        \u0027topology\u0027: [(\u00271.0\u0027, \u00271.2\u0027)],"},{"line_number":240,"context_line":"        \u0027limits\u0027: [(\u00271.0\u0027, \u00271.0\u0027)]"},{"line_number":241,"context_line":"    }"}],"source_content_type":"text/x-python","patch_set":28,"id":"7a740942_6497ceec","line":238,"updated":"2015-12-07 12:50:28.000000000","message":"the same","commit_id":"61940a97c2345a2deeccd43f5a562c98243593f4"}],"nova/resources/amounts.py":[{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"ef92e27a32e12ffc8caa537f9f88eebb5fab249c","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    However, some resources, such as NUMA CPU specifications, are not so"},{"line_number":30,"context_line":"    easy to calculate. Instead, the requested NUMA topology must be compared"},{"line_number":31,"context_line":"    to the used cells, cores, and threads for existing placed guest CPUs."},{"line_number":32,"context_line":"    \"\"\""},{"line_number":33,"context_line":"    pass"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_6d27b5ca","line":32,"updated":"2014-10-16 21:24:56.000000000","message":"The abstract interface has no operators / attributes defined. Things like the prose below \u0027if the amount_spec is negative\u0027 .... - can\u0027t be written against the current base class.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"169bbfa69b21049fc0b78a6bcb9a770b8bee4b2f","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    However, some resources, such as NUMA CPU specifications, are not so"},{"line_number":30,"context_line":"    easy to calculate. Instead, the requested NUMA topology must be compared"},{"line_number":31,"context_line":"    to the used cells, cores, and threads for existing placed guest CPUs."},{"line_number":32,"context_line":"    \"\"\""},{"line_number":33,"context_line":"    pass"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_3d569f2a","line":32,"in_reply_to":"baa201ad_20467d7b","updated":"2014-10-17 22:00:58.000000000","message":"Done","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"7f6a47bb004463da1017ff1a30a329697d053d2a","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    However, some resources, such as NUMA CPU specifications, are not so"},{"line_number":30,"context_line":"    easy to calculate. Instead, the requested NUMA topology must be compared"},{"line_number":31,"context_line":"    to the used cells, cores, and threads for existing placed guest CPUs."},{"line_number":32,"context_line":"    \"\"\""},{"line_number":33,"context_line":"    pass"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_20467d7b","line":32,"in_reply_to":"baa201ad_6d27b5ca","updated":"2014-10-17 16:17:52.000000000","message":"Fair point. I can have the AmountSpec base class contain an is_negative_amount attribute. That would indeed be cleaner.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"ef92e27a32e12ffc8caa537f9f88eebb5fab249c","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    easy to calculate. Instead, the requested NUMA topology must be compared"},{"line_number":31,"context_line":"    to the used cells, cores, and threads for existing placed guest CPUs."},{"line_number":32,"context_line":"    \"\"\""},{"line_number":33,"context_line":"    pass"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"class IntegerAmount(AmountSpec):"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_0da1c9f7","line":33,"updated":"2014-10-16 21:24:56.000000000","message":"This isn\u0027t needed, though flake8 might hate on you. Nit.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"169bbfa69b21049fc0b78a6bcb9a770b8bee4b2f","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    easy to calculate. Instead, the requested NUMA topology must be compared"},{"line_number":31,"context_line":"    to the used cells, cores, and threads for existing placed guest CPUs."},{"line_number":32,"context_line":"    \"\"\""},{"line_number":33,"context_line":"    pass"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"class IntegerAmount(AmountSpec):"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_5d51a32e","line":33,"in_reply_to":"baa201ad_0da1c9f7","updated":"2014-10-17 22:00:58.000000000","message":"Done","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"ef92e27a32e12ffc8caa537f9f88eebb5fab249c","unresolved":false,"context_lines":[{"line_number":34,"context_line":""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"class IntegerAmount(AmountSpec):"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    \"\"\"When an amount is simply an integer.\"\"\""},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def __init__(self, amount):"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_4da7510b","line":37,"updated":"2014-10-16 21:24:56.000000000","message":"The blank line here may interfere with docstring binding though, so please remove it.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"ef92e27a32e12ffc8caa537f9f88eebb5fab249c","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"class IntegerAmount(AmountSpec):"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    \"\"\"When an amount is simply an integer.\"\"\""},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def __init__(self, amount):"},{"line_number":41,"context_line":"        \"\"\"Constructs an amount specifier from a requested integer amount."}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_2df20d06","line":38,"updated":"2014-10-16 21:24:56.000000000","message":"This docstring doesn\u0027t really make sense to me. Imagine reading in API docs:\n\nIntegerAmount: When an amount is simply an integer.\n\nI\u0027d almost rather no docstring.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"ef92e27a32e12ffc8caa537f9f88eebb5fab249c","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    def __init__(self, amount):"},{"line_number":41,"context_line":"        \"\"\"Constructs an amount specifier from a requested integer amount."},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"        :param amount: Simple integer amount requested of the resource."},{"line_number":44,"context_line":"        \"\"\""},{"line_number":45,"context_line":"        self.amount \u003d amount"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_cde0a128","line":43,"updated":"2014-10-16 21:24:56.000000000","message":"I didn\u0027t know python had complex integers :)","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"e9602218f2e642e205453d58d3605557c705719a","unresolved":false,"context_lines":[{"line_number":42,"context_line":""},{"line_number":43,"context_line":"        :param amount: Simple integer amount requested of the resource."},{"line_number":44,"context_line":"        \"\"\""},{"line_number":45,"context_line":"        self.amount \u003d amount"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def __eq__(self, other):"},{"line_number":48,"context_line":"        \"\"\"Returns True if supplied other amount spec is equal to this one.\"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_ecd2cb5a","line":45,"range":{"start_line":45,"start_character":8,"end_line":45,"end_character":12},"updated":"2014-10-16 18:40:08.000000000","message":"Shouldn\u0027t there be some sort of check that the amount is indeed an integer? Passing None works fine, with the result that:\n    IntegerAmount(1) \u003c IntegerAmount(None)","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"169bbfa69b21049fc0b78a6bcb9a770b8bee4b2f","unresolved":false,"context_lines":[{"line_number":42,"context_line":""},{"line_number":43,"context_line":"        :param amount: Simple integer amount requested of the resource."},{"line_number":44,"context_line":"        \"\"\""},{"line_number":45,"context_line":"        self.amount \u003d amount"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def __eq__(self, other):"},{"line_number":48,"context_line":"        \"\"\"Returns True if supplied other amount spec is equal to this one.\"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_bd10cf59","line":45,"in_reply_to":"baa201ad_ecd2cb5a","updated":"2014-10-17 22:00:58.000000000","message":"Done","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"9f5cca81b39d7fc46939111e04176df8c38e0a87","unresolved":false,"context_lines":[{"line_number":42,"context_line":""},{"line_number":43,"context_line":"        :param amount: Simple integer amount requested of the resource."},{"line_number":44,"context_line":"        \"\"\""},{"line_number":45,"context_line":"        self.amount \u003d amount"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def __eq__(self, other):"},{"line_number":48,"context_line":"        \"\"\"Returns True if supplied other amount spec is equal to this one.\"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_97d84a54","line":45,"in_reply_to":"baa201ad_ecd2cb5a","updated":"2014-10-17 18:53:03.000000000","message":"Yep, nice catch. Will do.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"4841251b363f1963d3375ac0d8c2edb4b7649977","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        \"\"\""},{"line_number":45,"context_line":"        self.amount \u003d amount"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def __eq__(self, other):"},{"line_number":48,"context_line":"        \"\"\"Returns True if supplied other amount spec is equal to this one.\"\"\""},{"line_number":49,"context_line":"        if not isinstance(other, IntegerAmount):"},{"line_number":50,"context_line":"            return NotImplemented"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_67741fcf","line":47,"updated":"2014-10-17 14:48:43.000000000","message":"TBH I think these are kinda academic - the reason I don\u0027t have them on NUMA classes is because I actually never needed them except for tests. We really only do claim/free.\n\nAlso this object is just a  placeholder for data and it will always be \"owned\" by a Resource object - do we really need \"Amounts\" that do nothing but indirection?","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"7f6a47bb004463da1017ff1a30a329697d053d2a","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        \"\"\""},{"line_number":45,"context_line":"        self.amount \u003d amount"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def __eq__(self, other):"},{"line_number":48,"context_line":"        \"\"\"Returns True if supplied other amount spec is equal to this one.\"\"\""},{"line_number":49,"context_line":"        if not isinstance(other, IntegerAmount):"},{"line_number":50,"context_line":"            return NotImplemented"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_00d099c7","line":47,"in_reply_to":"baa201ad_67741fcf","updated":"2014-10-17 16:17:52.000000000","message":"It\u0027s true that I only added the __eq__ and __ne__ overloads because it was the only way to test that the objects returned by calls to get_requested_resources() here:\n\nhttps://review.openstack.org/#/c/128758/3/nova/objects/instance.py\n\nmatch an expected set of AmountSpec objects :)","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"3a7df944daab89a3f67d71d2ef426efbccef5911","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        \"\"\""},{"line_number":45,"context_line":"        self.amount \u003d amount"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def __eq__(self, other):"},{"line_number":48,"context_line":"        \"\"\"Returns True if supplied other amount spec is equal to this one.\"\"\""},{"line_number":49,"context_line":"        if not isinstance(other, IntegerAmount):"},{"line_number":50,"context_line":"            return NotImplemented"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_caa312b0","line":47,"in_reply_to":"baa201ad_67741fcf","updated":"2014-10-17 16:20:14.000000000","message":"So after reading the next patch I can see what amount is for.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"ef92e27a32e12ffc8caa537f9f88eebb5fab249c","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        self.amount \u003d amount"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def __eq__(self, other):"},{"line_number":48,"context_line":"        \"\"\"Returns True if supplied other amount spec is equal to this one.\"\"\""},{"line_number":49,"context_line":"        if not isinstance(other, IntegerAmount):"},{"line_number":50,"context_line":"            return NotImplemented"},{"line_number":51,"context_line":"        return self.amount \u003d\u003d other.amount"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_8d48592e","line":48,"updated":"2014-10-16 21:24:56.000000000","message":"I don\u0027t object to this being here, but IMO docstrings on such standard methods are only needed when something unusual is being done. Which this isn\u0027t.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"9f5cca81b39d7fc46939111e04176df8c38e0a87","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        self.amount \u003d amount"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def __eq__(self, other):"},{"line_number":48,"context_line":"        \"\"\"Returns True if supplied other amount spec is equal to this one.\"\"\""},{"line_number":49,"context_line":"        if not isinstance(other, IntegerAmount):"},{"line_number":50,"context_line":"            return NotImplemented"},{"line_number":51,"context_line":"        return self.amount \u003d\u003d other.amount"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_57ea62d8","line":48,"in_reply_to":"baa201ad_8d48592e","updated":"2014-10-17 18:53:03.000000000","message":"OK, fair enough. Will remove in respin.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"ef92e27a32e12ffc8caa537f9f88eebb5fab249c","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        if not isinstance(other, IntegerAmount):"},{"line_number":50,"context_line":"            return NotImplemented"},{"line_number":51,"context_line":"        return self.amount \u003d\u003d other.amount"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def __ne__(self, other):"},{"line_number":54,"context_line":"        return not self \u003d\u003d other"},{"line_number":55,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_cd1341ff","line":52,"updated":"2014-10-16 21:24:56.000000000","message":"You\u0027ll also need to implement __hash__, because if a\u003d\u003db and hash(a) !\u003d hash(b) odd things can happen - e.g.\n\n a \u003d IntegerAmount(1)\n b \u003d IntegerAmount(1)\n c \u003d set([a])\n b in c\n -\u003e False\n\nThis applies to all the objects here - see https://docs.python.org/3/reference/datamodel.html - \"The only required property is that objects which compare equal have the same hash value\"","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"169bbfa69b21049fc0b78a6bcb9a770b8bee4b2f","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        if not isinstance(other, IntegerAmount):"},{"line_number":50,"context_line":"            return NotImplemented"},{"line_number":51,"context_line":"        return self.amount \u003d\u003d other.amount"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def __ne__(self, other):"},{"line_number":54,"context_line":"        return not self \u003d\u003d other"},{"line_number":55,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_6f469125","line":52,"in_reply_to":"baa201ad_cd1341ff","updated":"2014-10-17 22:00:58.000000000","message":"Done","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"4841251b363f1963d3375ac0d8c2edb4b7649977","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        return \"IntegerAmount({0})\".format(self.amount)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"class NUMAAmount(AmountSpec):"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    \"\"\"A specifier that models a NUMA cell topology.\"\"\""},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_874743bb","line":60,"updated":"2014-10-17 14:48:43.000000000","message":"So these (AmountSpec sub types) are meant to model both the host and instance side of a \"resource\", while the host side would also have a usage associated with it. I kinda like it.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c53a4a4ee1fcfd5e9e3be904392d8f0032610998","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        return \"IntegerAmount({0})\".format(self.amount)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"class NUMAAmount(AmountSpec):"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    \"\"\"A specifier that models a NUMA cell topology.\"\"\""},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5a890539_5c94a48a","line":60,"in_reply_to":"5a890539_2b6ec32f","updated":"2014-11-21 21:59:57.000000000","message":"Got it. Thanks, Daniel, will revise.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"a6e908b88162cef58d8aaeb2b6e19257812a29a3","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        return \"IntegerAmount({0})\".format(self.amount)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"class NUMAAmount(AmountSpec):"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    \"\"\"A specifier that models a NUMA cell topology.\"\"\""},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5a890539_2b6ec32f","line":60,"in_reply_to":"baa201ad_1779ba2e","updated":"2014-11-13 13:41:00.000000000","message":"IMHO, this proposed design/approach to dealing over commit is fundamentally broken.\n\nIf I have a scenario\n\n - Over commit ratio of x2\n - Host A with 4 GB of RAM\n - Host B with 8 GB of RAM\n\nNow i want to launch a guest instance with an 6 GB  RAM size.\n\nWith your proposal that over commit ratio be applied to the value before passing it into the AmountSpec/UsageSpec, the scheduler will see host A as having 8 GB and host B as having 16 GB, and be able to pick host A as a target for this VM in preference to host B.\n\nWe should *NEVER* consider the 4 GB host as valid for running this VM, because even if it is the only VM that ever gets scheduled to the host, it is guaranteed to *NEVER* have enough RAM available to run.  IOW you\u0027re allowing the VM to over commit itself.\n\nAs a first pass we should always consider whether the bare physical resource, minus that consumed by the OS services, is large enough to host the VM. Only then, shoud we apply the over commit ratio and consider the usage from other VMs on the host.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"ef36df58d1c5eae3c363d4403a3487f295fe63e1","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        return \"IntegerAmount({0})\".format(self.amount)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"class NUMAAmount(AmountSpec):"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    \"\"\"A specifier that models a NUMA cell topology.\"\"\""},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_84bb8ac3","line":60,"in_reply_to":"baa201ad_1779ba2e","updated":"2014-10-30 18:27:46.000000000","message":"OK - so you don\u0027t want the resource to have anything to do with the overcommit because you want to make sure this is always something that compute nodes set (and now how it is done now through the scheduler.\n\nI agree with that - but I still think resources (in this case Amount and Usage specs) should expose the notion, because if this is completely left up to the compute node to decide - for more complex resources (like NUMA) - it will couple the compute node logic too tightly to the resource as they would need to know implementation details in order to calculate them. Right now this is done through VirtNUMA... classes (which is OK) to reduce the tight coupling, but it still feels like overcommit is something that belongs in this contract.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"9a6b8e42313439a3e8cf7043b647d88be1ae91dc","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        return \"IntegerAmount({0})\".format(self.amount)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"class NUMAAmount(AmountSpec):"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    \"\"\"A specifier that models a NUMA cell topology.\"\"\""},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_7bc2f634","line":60,"in_reply_to":"baa201ad_40dd41df","updated":"2014-10-17 16:37:42.000000000","message":"K - we still are missing oversubscription - but usper easy to add.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"9f5cca81b39d7fc46939111e04176df8c38e0a87","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        return \"IntegerAmount({0})\".format(self.amount)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"class NUMAAmount(AmountSpec):"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    \"\"\"A specifier that models a NUMA cell topology.\"\"\""},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_1779ba2e","line":60,"in_reply_to":"baa201ad_7bc2f634","updated":"2014-10-17 18:53:03.000000000","message":"Actually, we are not missing oversubscription. :) The total capacity should be adjusted when the AmountSpec is constructed.\n\nSo, if the ram_overcommit_ratio is 2.0, and the total amount of physical RAM on the machine is 32GB, and the amount reserved for the dom0 is 2GB, then the UsageSpec for RAM would be constructed with a total amount of (32GB - 2GB) * 2.0 \u003d 60GB. The AmountSpec and UsageSpec objects are specifically designed *not* to have any internal information about overcommit ratios, reserved quantities, or anything like that. :)","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"7f6a47bb004463da1017ff1a30a329697d053d2a","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        return \"IntegerAmount({0})\".format(self.amount)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"class NUMAAmount(AmountSpec):"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    \"\"\"A specifier that models a NUMA cell topology.\"\"\""},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_40dd41df","line":60,"in_reply_to":"baa201ad_874743bb","updated":"2014-10-17 16:17:52.000000000","message":"Partly correct. The AmountSpec and UsageSpecs are interfaces -- though Python doesn\u0027t really have the concept of strict interfaces... they are built this way so that \"amounts\" of similar resource types (regardless of whether the amount itself is simple or complex) can be compared with each other and tested for fit.\n\nThe UsageSpec is an interface representation that stores two things -- one that represents the capacity/limit of the resource type, and one which represents the amount of the resource type that is used.\n\nUsageSpec is intended for ComputeNode objects, which are providers of resources, whereas AmountSpec objects are intended for Instance objects, and are consumers of resources.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"ef92e27a32e12ffc8caa537f9f88eebb5fab249c","unresolved":false,"context_lines":[{"line_number":58,"context_line":""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"class NUMAAmount(AmountSpec):"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    \"\"\"A specifier that models a NUMA cell topology.\"\"\""},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    def __init__(self, topology, is_negative\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_4ddf513a","line":61,"updated":"2014-10-16 21:24:56.000000000","message":"Same issue here with the blank line.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"ef92e27a32e12ffc8caa537f9f88eebb5fab249c","unresolved":false,"context_lines":[{"line_number":104,"context_line":"            self.topology, self.is_negative)"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"class UsageSpec(object):"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"    \"\"\"Represents the used amount of a particular type of resource.\"\"\""},{"line_number":110,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_6d7975c7","line":107,"updated":"2014-10-16 21:24:56.000000000","message":"Same blank line issue.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"ef92e27a32e12ffc8caa537f9f88eebb5fab249c","unresolved":false,"context_lines":[{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        :param amount_spec: `AmountSpec` to modify the usage with."},{"line_number":115,"context_line":"        \"\"\""},{"line_number":116,"context_line":"        raise NotImplementedError"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def has_room_for(self, amount_spec):"},{"line_number":119,"context_line":"        \"\"\"Determine if there is room to fit the supplied amount of resources."}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_0d50e930","line":116,"updated":"2014-10-16 21:24:56.000000000","message":"This is a bug:it should be\n\n raise NotImplementedError()\n\nOr better IMO \n\n raise NotImplementedError(self.update)","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"ef36df58d1c5eae3c363d4403a3487f295fe63e1","unresolved":false,"context_lines":[{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        :param amount_spec: `AmountSpec` to modify the usage with."},{"line_number":115,"context_line":"        \"\"\""},{"line_number":116,"context_line":"        raise NotImplementedError"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def has_room_for(self, amount_spec):"},{"line_number":119,"context_line":"        \"\"\"Determine if there is room to fit the supplied amount of resources."}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_2449fed0","line":116,"in_reply_to":"9aa7fdbe_8f4ad525","updated":"2014-10-30 18:27:46.000000000","message":"FWIW - it\u0027s not a bug - it\u0027s perfectly valid to raise a class instead of instance as the first expression of the raise statement:\n\nhttps://docs.python.org/2/reference/simple_stmts.html#raise","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"169bbfa69b21049fc0b78a6bcb9a770b8bee4b2f","unresolved":false,"context_lines":[{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        :param amount_spec: `AmountSpec` to modify the usage with."},{"line_number":115,"context_line":"        \"\"\""},{"line_number":116,"context_line":"        raise NotImplementedError"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def has_room_for(self, amount_spec):"},{"line_number":119,"context_line":"        \"\"\"Determine if there is room to fit the supplied amount of resources."}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_8f4ad525","line":116,"in_reply_to":"baa201ad_0d50e930","updated":"2014-10-17 22:00:58.000000000","message":"Done","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"9f5cca81b39d7fc46939111e04176df8c38e0a87","unresolved":false,"context_lines":[{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        :param amount_spec: `AmountSpec` to modify the usage with."},{"line_number":115,"context_line":"        \"\"\""},{"line_number":116,"context_line":"        raise NotImplementedError"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def has_room_for(self, amount_spec):"},{"line_number":119,"context_line":"        \"\"\"Determine if there is room to fit the supplied amount of resources."}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_77df660d","line":116,"in_reply_to":"baa201ad_0d50e930","updated":"2014-10-17 18:53:03.000000000","message":"OK. Though the above is done throughout Nova and Neutron in a number of places, with no harmful effect:\n\njaypipes@cranky:~/repos/openstack/nova/nova$ ack-grep \"raise NotImplementedError[^\\(]\" --ignore-dir nova/tests | wc -l\n6\njaypipes@cranky:~/repos/openstack/neutron/neutron$ ack-grep \"raise NotImplementedError[^\\(]\" --ignore-dir tests | wc -l\n7","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"ef92e27a32e12ffc8caa537f9f88eebb5fab249c","unresolved":false,"context_lines":[{"line_number":124,"context_line":""},{"line_number":125,"context_line":"                 If the supplied `amount_spec` is negative, returns False."},{"line_number":126,"context_line":"        \"\"\""},{"line_number":127,"context_line":"        raise NotImplementedError"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"class IntegerUsage(UsageSpec):"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_2d43ed6c","line":127,"updated":"2014-10-16 21:24:56.000000000","message":"Ditto.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"ef92e27a32e12ffc8caa537f9f88eebb5fab249c","unresolved":false,"context_lines":[{"line_number":128,"context_line":""},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"class IntegerUsage(UsageSpec):"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    \"\"\"Represents the total and used amount of a resource with integers.\"\"\""},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"    def __init__(self, total, used):"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_8d2219b7","line":131,"updated":"2014-10-16 21:24:56.000000000","message":"Blank line issue.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"e9602218f2e642e205453d58d3605557c705719a","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        :returns True if the supplied requested amount of resources is able"},{"line_number":163,"context_line":"                 to be consumed on the node, False otherwise."},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"                 If the supplied `amount_spec` is negative, returns False."},{"line_number":166,"context_line":"        \"\"\""},{"line_number":167,"context_line":"        if amount_spec.amount \u003c 1:"},{"line_number":168,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_6c741b68","line":165,"updated":"2014-10-16 18:40:08.000000000","message":"Should read \"is negative or zero, returns False\".","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"169bbfa69b21049fc0b78a6bcb9a770b8bee4b2f","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        :returns True if the supplied requested amount of resources is able"},{"line_number":163,"context_line":"                 to be consumed on the node, False otherwise."},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"                 If the supplied `amount_spec` is negative, returns False."},{"line_number":166,"context_line":"        \"\"\""},{"line_number":167,"context_line":"        if amount_spec.amount \u003c 1:"},{"line_number":168,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_cf54ddcc","line":165,"in_reply_to":"baa201ad_6c741b68","updated":"2014-10-17 22:00:58.000000000","message":"I\u0027ve made it impossible to construct an AmountSpec with a zero amount, so is_negative, returns False is now appropriate here.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"ef92e27a32e12ffc8caa537f9f88eebb5fab249c","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        :returns True if the supplied requested amount of resources is able"},{"line_number":163,"context_line":"                 to be consumed on the node, False otherwise."},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"                 If the supplied `amount_spec` is negative, returns False."},{"line_number":166,"context_line":"        \"\"\""},{"line_number":167,"context_line":"        if amount_spec.amount \u003c 1:"},{"line_number":168,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_8d093933","line":165,"in_reply_to":"baa201ad_6c741b68","updated":"2014-10-16 21:24:56.000000000","message":"Or \u0027is not positive\u0027 ? Would apply to the base class definition too.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"ef92e27a32e12ffc8caa537f9f88eebb5fab249c","unresolved":false,"context_lines":[{"line_number":170,"context_line":""},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"class NUMAUsage(UsageSpec):"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"    \"\"\"Represents the NUMA topology/capacity and the cells that are used.\"\"\""},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"    def __init__(self, capacity, topology):"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_cdffc100","line":173,"updated":"2014-10-16 21:24:56.000000000","message":"And here.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"ef92e27a32e12ffc8caa537f9f88eebb5fab249c","unresolved":false,"context_lines":[{"line_number":219,"context_line":"                    cell.cpuset !\u003d other_cell.cpuset):"},{"line_number":220,"context_line":"                return False"},{"line_number":221,"context_line":"        return True"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"    def __ne__(self, other):"},{"line_number":224,"context_line":"        return not self \u003d\u003d other"},{"line_number":225,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_3f56b006","line":222,"updated":"2014-10-16 21:24:56.000000000","message":"I don\u0027t like that this is a copy of the NumaAmount logic. Surely you can delegate to that - even crudely:\n\n return (NumaAmount(self.capacity) \u003d\u003d NumaAmount(other.capacity)\n     and self.topology \u003d\u003d other.topology)","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"169bbfa69b21049fc0b78a6bcb9a770b8bee4b2f","unresolved":false,"context_lines":[{"line_number":219,"context_line":"                    cell.cpuset !\u003d other_cell.cpuset):"},{"line_number":220,"context_line":"                return False"},{"line_number":221,"context_line":"        return True"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"    def __ne__(self, other):"},{"line_number":224,"context_line":"        return not self \u003d\u003d other"},{"line_number":225,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_affeb936","line":222,"in_reply_to":"baa201ad_3f56b006","updated":"2014-10-17 22:00:58.000000000","message":"I did manage to DRY this up in the respin...","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f5f06ab3d4d31123bdfbda563e342d05bd6d4062","unresolved":false,"context_lines":[{"line_number":219,"context_line":"                    cell.cpuset !\u003d other_cell.cpuset):"},{"line_number":220,"context_line":"                return False"},{"line_number":221,"context_line":"        return True"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"    def __ne__(self, other):"},{"line_number":224,"context_line":"        return not self \u003d\u003d other"},{"line_number":225,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_994fad6a","line":222,"in_reply_to":"baa201ad_3f56b006","updated":"2014-10-17 11:02:27.000000000","message":"Look at it again. It\u0027s not a copy... The capacity object has different comparisons (the cpu_limit and memory_limit comparisons)","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"4841251b363f1963d3375ac0d8c2edb4b7649977","unresolved":false,"context_lines":[{"line_number":223,"context_line":"    def __ne__(self, other):"},{"line_number":224,"context_line":"        return not self \u003d\u003d other"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"    def update(self, amount_spec):"},{"line_number":227,"context_line":"        \"\"\"Update the used amount of resources by the supplied amount."},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"        :param amount_spec: `NUMAAmount` to modify the usage with."}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_073a73e6","line":226,"updated":"2014-10-17 14:48:43.000000000","message":"and this is get_usage_from...","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"ef92e27a32e12ffc8caa537f9f88eebb5fab249c","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        # NOTE(jaypipes): usage_from_instances() is actually a classmethod,"},{"line_number":233,"context_line":"        #                 which is why this looks weird here..."},{"line_number":234,"context_line":"        self.topology \u003d self.topology.usage_from_instances("},{"line_number":235,"context_line":"                self.topology, instance_tops, free\u003damount_spec.is_negative)"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"    def has_room_for(self, amount_spec):"},{"line_number":238,"context_line":"        \"\"\"Determine if there is room to fit the supplied amount of resources."}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_df44c448","line":235,"updated":"2014-10-16 21:24:56.000000000","message":"If its a classmethod, calling it via \n nova.virt.hardware.VirtNUMAHostTopology.usage_from_instances(...)\n\nwould be more appropriate.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"9a6b8e42313439a3e8cf7043b647d88be1ae91dc","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        # NOTE(jaypipes): usage_from_instances() is actually a classmethod,"},{"line_number":233,"context_line":"        #                 which is why this looks weird here..."},{"line_number":234,"context_line":"        self.topology \u003d self.topology.usage_from_instances("},{"line_number":235,"context_line":"                self.topology, instance_tops, free\u003damount_spec.is_negative)"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"    def has_room_for(self, amount_spec):"},{"line_number":238,"context_line":"        \"\"\"Determine if there is room to fit the supplied amount of resources."}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_bb91be48","line":235,"in_reply_to":"baa201ad_3b4dce3e","updated":"2014-10-17 16:37:42.000000000","message":"So this is getting philosophical :) - they do keep data - which is not \u0027state\u0027 as it does not get mutated, since methods don\u0027t mutate them... like I said... call me a functional programming enthusiast :)\n\n\u003e Then objects representing NUMA topologies would be actual objects instead of a kinda-awkward classmethod interface for producing a class that is a collection of Cell objects...\n\nWell the objects would still keep Cell objects that would get mutated...\n\nIn reality we can fix this easily if we want to - and I agree, to 9.5 out of 10 Pythonistas this can seem plain wrong - just saying that in this case - bit mutating them has a benefit.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f78e8466ffa77361d50e9c97242fb743fe5368cb","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        # NOTE(jaypipes): usage_from_instances() is actually a classmethod,"},{"line_number":233,"context_line":"        #                 which is why this looks weird here..."},{"line_number":234,"context_line":"        self.topology \u003d self.topology.usage_from_instances("},{"line_number":235,"context_line":"                self.topology, instance_tops, free\u003damount_spec.is_negative)"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"    def has_room_for(self, amount_spec):"},{"line_number":238,"context_line":"        \"\"\"Determine if there is room to fit the supplied amount of resources."}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_3b4dce3e","line":235,"in_reply_to":"baa201ad_67e41f0d","updated":"2014-10-17 16:29:01.000000000","message":"So, here\u0027s the deal... I don\u0027t mind using classmethods as, say, factories... like we do in nova.objects.Instance.get_by_uuid() and similar.\n\nHowever, the classes in nova/virt/hardware.py *do* actually keep state. All of them do. They all have a cells attribute that contains the cell objects for a topology. So, it\u0027s kinda weird to create an object and then call \"usage_from_instances\" on the object, and have it return a *new* object, instead of simply updating the object itself.\n\nFor instance, instead of the above, if the VirtNUMAHostTopology object just had a method that updated the internal state based on some supplied usage records:\n\n def update_usage(self, instance_usages):\n     # just change the internal usage state of self.cells\n     # instead of creating a brand new object\n     ...\n\nThen objects representing NUMA topologies would be actual objects instead of a kinda-awkward classmethod interface for producing a class that is a collection of Cell objects...","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"4841251b363f1963d3375ac0d8c2edb4b7649977","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        # NOTE(jaypipes): usage_from_instances() is actually a classmethod,"},{"line_number":233,"context_line":"        #                 which is why this looks weird here..."},{"line_number":234,"context_line":"        self.topology \u003d self.topology.usage_from_instances("},{"line_number":235,"context_line":"                self.topology, instance_tops, free\u003damount_spec.is_negative)"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"    def has_room_for(self, amount_spec):"},{"line_number":238,"context_line":"        \"\"\"Determine if there is room to fit the supplied amount of resources."}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_67e41f0d","line":235,"in_reply_to":"baa201ad_b979d194","updated":"2014-10-17 14:48:43.000000000","message":"TBH - I really like that those are class methods - those classes are really not meant to be keeping state, and the fact that they are not expresses it quite nicely. We have NovaOjects for state.\n\nI do agree that it seems a bit unnatural to python programmers - but when I was using the classes in the rest of the code - the fact that the method will not mutate my instance object felt kinda nice.\n\nAlso - no exceptions when things fail - we just return a \u0027Fail\u0027 (None) value that other class methods know how to handle...\n\nCall me a FP enthusiast :)\n\nWhat do you see as the benefit of having them as normal methods that mutate the internal state for the stuff in virt.hardware?","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f5f06ab3d4d31123bdfbda563e342d05bd6d4062","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        # NOTE(jaypipes): usage_from_instances() is actually a classmethod,"},{"line_number":233,"context_line":"        #                 which is why this looks weird here..."},{"line_number":234,"context_line":"        self.topology \u003d self.topology.usage_from_instances("},{"line_number":235,"context_line":"                self.topology, instance_tops, free\u003damount_spec.is_negative)"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"    def has_room_for(self, amount_spec):"},{"line_number":238,"context_line":"        \"\"\"Determine if there is room to fit the supplied amount of resources."}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_b979d194","line":235,"in_reply_to":"baa201ad_df44c448","updated":"2014-10-17 11:02:27.000000000","message":"It really shouldn\u0027t be a classmethod, though. Which was my point.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"4841251b363f1963d3375ac0d8c2edb4b7649977","unresolved":false,"context_lines":[{"line_number":234,"context_line":"        self.topology \u003d self.topology.usage_from_instances("},{"line_number":235,"context_line":"                self.topology, instance_tops, free\u003damount_spec.is_negative)"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"    def has_room_for(self, amount_spec):"},{"line_number":238,"context_line":"        \"\"\"Determine if there is room to fit the supplied amount of resources."},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"        We utilize the `nova.virt.hardware` module\u0027s functions to determine"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_87566328","line":237,"updated":"2014-10-17 14:48:43.000000000","message":"Yeah so this is claim .","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f78e8466ffa77361d50e9c97242fb743fe5368cb","unresolved":false,"context_lines":[{"line_number":234,"context_line":"        self.topology \u003d self.topology.usage_from_instances("},{"line_number":235,"context_line":"                self.topology, instance_tops, free\u003damount_spec.is_negative)"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"    def has_room_for(self, amount_spec):"},{"line_number":238,"context_line":"        \"\"\"Determine if there is room to fit the supplied amount of resources."},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"        We utilize the `nova.virt.hardware` module\u0027s functions to determine"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_db0a02d1","line":237,"in_reply_to":"baa201ad_87566328","updated":"2014-10-17 16:29:01.000000000","message":"Yep. The difference is that unlike the nova.compute.claim.Claim.claim_test() interface, this just returns True or False, and leaves it up to the caller to do things like log a message or construct an error to return to the caller in a retry exception. :)\n\nHave the method do one thing, with a clear binary interface, and do it well.","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"9a6b8e42313439a3e8cf7043b647d88be1ae91dc","unresolved":false,"context_lines":[{"line_number":234,"context_line":"        self.topology \u003d self.topology.usage_from_instances("},{"line_number":235,"context_line":"                self.topology, instance_tops, free\u003damount_spec.is_negative)"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"    def has_room_for(self, amount_spec):"},{"line_number":238,"context_line":"        \"\"\"Determine if there is room to fit the supplied amount of resources."},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"        We utilize the `nova.virt.hardware` module\u0027s functions to determine"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_bbd53e1d","line":237,"in_reply_to":"baa201ad_db0a02d1","updated":"2014-10-17 16:37:42.000000000","message":"Oh heeells yes to that - I hate those strings","commit_id":"900680027854141c2943a7318c064766171a5fa7"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ee55e733f404c258971bb5a8fa804de5b40187e8","unresolved":false,"context_lines":[{"line_number":20,"context_line":"from nova.virt import hardware as hw"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"class AmountSpec(object):"},{"line_number":24,"context_line":"    \"\"\"Abstract base class for a requested amount specifier"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    Interface class that serves to provide a consistent way of comparing"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_ef1172ad","line":23,"updated":"2014-10-31 14:48:47.000000000","message":"if it\u0027s an abstract class, could you please add the decorator @six.add_metaclass(abc.ABCMeta) to the class (for Py3 compatibilility) and provide specifications of the abstract methods by adding @abc.abstractmethod ?\n\nThis way, we could understand what the inherited classes need to implement (assuming __eq__, __ne__, etc.)","commit_id":"2048a1118dc867a3f27d089557906a7289e99bd3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ee533090ea8761ff72e994369295042b8cf972bb","unresolved":false,"context_lines":[{"line_number":20,"context_line":"from nova.virt import hardware as hw"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"class AmountSpec(object):"},{"line_number":24,"context_line":"    \"\"\"Abstract base class for a requested amount specifier"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    Interface class that serves to provide a consistent way of comparing"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_55b75ed8","line":23,"in_reply_to":"7a8c0949_cf73cb4b","updated":"2014-11-03 23:34:25.000000000","message":"Indeed. Yes, please remove that confusing word.","commit_id":"2048a1118dc867a3f27d089557906a7289e99bd3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"de11588b4901276d91f8a8f7fdadb56dc1eb3df8","unresolved":false,"context_lines":[{"line_number":20,"context_line":"from nova.virt import hardware as hw"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"class AmountSpec(object):"},{"line_number":24,"context_line":"    \"\"\"Abstract base class for a requested amount specifier"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    Interface class that serves to provide a consistent way of comparing"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_cf73cb4b","line":23,"in_reply_to":"7a8c0949_ef1172ad","updated":"2014-10-31 23:52:25.000000000","message":"it\u0027s not abstract, it has state... it\u0027s just a base class. I will remove the term \"abstract\" here.","commit_id":"2048a1118dc867a3f27d089557906a7289e99bd3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ee55e733f404c258971bb5a8fa804de5b40187e8","unresolved":false,"context_lines":[{"line_number":37,"context_line":"    \"\"\""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def __init__(self, is_negative\u003dFalse):"},{"line_number":40,"context_line":"        self.is_negative \u003d is_negative"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"class IntegerAmount(AmountSpec):"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_12cee583","line":40,"updated":"2014-10-31 14:48:47.000000000","message":"While I understand the need to set an is_negative attribute for NUMA (and possibly integer), what happens if we compare apples and bananas and consider them if equal or not ?\n\nI mean, could we just not provide the is_negative public and rely on an abstract method is_negative() ?","commit_id":"2048a1118dc867a3f27d089557906a7289e99bd3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"de11588b4901276d91f8a8f7fdadb56dc1eb3df8","unresolved":false,"context_lines":[{"line_number":37,"context_line":"    \"\"\""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def __init__(self, is_negative\u003dFalse):"},{"line_number":40,"context_line":"        self.is_negative \u003d is_negative"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"class IntegerAmount(AmountSpec):"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_6f7c9721","line":40,"in_reply_to":"7a8c0949_12cee583","updated":"2014-10-31 23:52:25.000000000","message":"Sorry, Sylvain, I\u0027m not sure I understand your question :( I\u0027d prefer to keep the shared state in the base class instead of adding abstract methods for really no reason.","commit_id":"2048a1118dc867a3f27d089557906a7289e99bd3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ee533090ea8761ff72e994369295042b8cf972bb","unresolved":false,"context_lines":[{"line_number":37,"context_line":"    \"\"\""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def __init__(self, is_negative\u003dFalse):"},{"line_number":40,"context_line":"        self.is_negative \u003d is_negative"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"class IntegerAmount(AmountSpec):"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_75ba9ac1","line":40,"in_reply_to":"7a8c0949_6f7c9721","updated":"2014-11-03 23:34:25.000000000","message":"Since you convinced me that it is not an abstract class, this comment doesn\u0027t make any sense. Nevermind it.","commit_id":"2048a1118dc867a3f27d089557906a7289e99bd3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ee55e733f404c258971bb5a8fa804de5b40187e8","unresolved":false,"context_lines":[{"line_number":68,"context_line":"    def __repr__(self):"},{"line_number":69,"context_line":"        return \"IntegerAmount({0})\".format(self.amount)"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"class NUMAAmount(AmountSpec):"},{"line_number":73,"context_line":"    \"\"\"A specifier that models a NUMA cell topology.\"\"\""},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_4f060622","line":71,"updated":"2014-10-31 14:48:47.000000000","message":"shouldn\u0027t we compare 2 amounts, one being greater than the other ?","commit_id":"2048a1118dc867a3f27d089557906a7289e99bd3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ee533090ea8761ff72e994369295042b8cf972bb","unresolved":false,"context_lines":[{"line_number":68,"context_line":"    def __repr__(self):"},{"line_number":69,"context_line":"        return \"IntegerAmount({0})\".format(self.amount)"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"class NUMAAmount(AmountSpec):"},{"line_number":73,"context_line":"    \"\"\"A specifier that models a NUMA cell topology.\"\"\""},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_35a9f27b","line":71,"in_reply_to":"7a8c0949_0f8eb32e","updated":"2014-11-03 23:34:25.000000000","message":"My bad, I love writing long sentences so that the meaning is lost meanwhile...\n\nWhy not adding __lt__ and/or __gt__ ?","commit_id":"2048a1118dc867a3f27d089557906a7289e99bd3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"de11588b4901276d91f8a8f7fdadb56dc1eb3df8","unresolved":false,"context_lines":[{"line_number":68,"context_line":"    def __repr__(self):"},{"line_number":69,"context_line":"        return \"IntegerAmount({0})\".format(self.amount)"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"class NUMAAmount(AmountSpec):"},{"line_number":73,"context_line":"    \"\"\"A specifier that models a NUMA cell topology.\"\"\""},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_0f8eb32e","line":71,"in_reply_to":"7a8c0949_4f060622","updated":"2014-10-31 23:52:25.000000000","message":"I don\u0027t understand what you\u0027re asking...","commit_id":"2048a1118dc867a3f27d089557906a7289e99bd3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ee55e733f404c258971bb5a8fa804de5b40187e8","unresolved":false,"context_lines":[{"line_number":87,"context_line":"        \"\"\""},{"line_number":88,"context_line":"        if not isinstance(topology, hw.VirtNUMAInstanceTopology):"},{"line_number":89,"context_line":"            msg \u003d _(\"topology must be a \""},{"line_number":90,"context_line":"                    \"`nova.virt.hardware.VirtNUMAInstanceTopology` object.\")"},{"line_number":91,"context_line":"            raise ValueError(msg)"},{"line_number":92,"context_line":"        self.topology \u003d topology"},{"line_number":93,"context_line":"        self._hash \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_12f985c8","line":90,"updated":"2014-10-31 14:48:47.000000000","message":"I\u0027m not a big fan of classes FQNs written by hand in error messages, just wondering if it\u0027s worth stringify the class name directly.","commit_id":"2048a1118dc867a3f27d089557906a7289e99bd3"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"b7488599019bebda21af79fd295a48c339ecfc12","unresolved":false,"context_lines":[{"line_number":102,"context_line":"            return False"},{"line_number":103,"context_line":"        return hw.topology_is_same(self.topology, other.topology)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def __hash__(self):"},{"line_number":106,"context_line":"        if self._hash is not None:"},{"line_number":107,"context_line":"            return self._hash"},{"line_number":108,"context_line":"        result \u003d hashlib.md5()"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_6b30cb01","line":105,"updated":"2014-11-13 13:46:01.000000000","message":"I saw the comment on the first patch as to why we need this and I think we should drop it - the __eq__ is there only for tests, and there is no need to ever use these as keys in a dictionary or elements of a set. \n\nIf we want to be really pedantic - make __hash__ raise a TypeError or something - this is just useless code.","commit_id":"2048a1118dc867a3f27d089557906a7289e99bd3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ee55e733f404c258971bb5a8fa804de5b40187e8","unresolved":false,"context_lines":[{"line_number":128,"context_line":"    resource that has been used/consumed."},{"line_number":129,"context_line":"    \"\"\""},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    def update(self, amount_spec):"},{"line_number":132,"context_line":"        \"\"\"Update the used amount of resources by the supplied amount."},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"        :param amount_spec: `AmountSpec` to modify the usage with."}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_92dd7517","line":131,"updated":"2014-10-31 14:48:47.000000000","message":"could we consider this class as abstract too, requiring to set update() and has_room_for() ? sounds cleaner to me.","commit_id":"2048a1118dc867a3f27d089557906a7289e99bd3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ee533090ea8761ff72e994369295042b8cf972bb","unresolved":false,"context_lines":[{"line_number":128,"context_line":"    resource that has been used/consumed."},{"line_number":129,"context_line":"    \"\"\""},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    def update(self, amount_spec):"},{"line_number":132,"context_line":"        \"\"\"Update the used amount of resources by the supplied amount."},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"        :param amount_spec: `AmountSpec` to modify the usage with."}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_b5b402d2","line":131,"in_reply_to":"7a8c0949_8fa0e391","updated":"2014-11-03 23:34:25.000000000","message":"Sure, we love charming Pythons, but I\u0027m considering this as a good example of abstract class so that it needs to be implemented accordingly.\n\nI assume it\u0027s maybe bikeshedding, but I really like when patterns are consistent in between all implementations.","commit_id":"2048a1118dc867a3f27d089557906a7289e99bd3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"de11588b4901276d91f8a8f7fdadb56dc1eb3df8","unresolved":false,"context_lines":[{"line_number":128,"context_line":"    resource that has been used/consumed."},{"line_number":129,"context_line":"    \"\"\""},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    def update(self, amount_spec):"},{"line_number":132,"context_line":"        \"\"\"Update the used amount of resources by the supplied amount."},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"        :param amount_spec: `AmountSpec` to modify the usage with."}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_8fa0e391","line":131,"in_reply_to":"7a8c0949_92dd7517","updated":"2014-10-31 23:52:25.000000000","message":"Sure, I guess... it\u0027s Python, though, not C++ :) there\u0027s really nothing about the abc module (or six.abstract_method) that does anything of any use in a duck-typed language...","commit_id":"2048a1118dc867a3f27d089557906a7289e99bd3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ee55e733f404c258971bb5a8fa804de5b40187e8","unresolved":false,"context_lines":[{"line_number":145,"context_line":"                 If the supplied `amount_spec` is negative, returns False."},{"line_number":146,"context_line":"        \"\"\""},{"line_number":147,"context_line":"        raise NotImplementedError(self.has_room_for)"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"class IntegerUsage(UsageSpec):"},{"line_number":151,"context_line":"    \"\"\"Represents the total and used amount of a resource with integers.\"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_adddb216","line":148,"updated":"2014-10-31 14:48:47.000000000","message":"I agree with Nikola IIUC, shouldn\u0027t provide another method called set_overcommitment() ?","commit_id":"2048a1118dc867a3f27d089557906a7289e99bd3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"de11588b4901276d91f8a8f7fdadb56dc1eb3df8","unresolved":false,"context_lines":[{"line_number":145,"context_line":"                 If the supplied `amount_spec` is negative, returns False."},{"line_number":146,"context_line":"        \"\"\""},{"line_number":147,"context_line":"        raise NotImplementedError(self.has_room_for)"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"class IntegerUsage(UsageSpec):"},{"line_number":151,"context_line":"    \"\"\"Represents the total and used amount of a resource with integers.\"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_efb6c7db","line":148,"in_reply_to":"7a8c0949_adddb216","updated":"2014-10-31 23:52:25.000000000","message":"No, overcommitment has nothing to do with a UsageSpec. Overcommitment should be taken into account when constructing the total (or capacity) arguments to the concrete UsageSpec subclasses. In other words, mixing the concept of overcommit ratios into the interface for a usage record is incorrrect.","commit_id":"2048a1118dc867a3f27d089557906a7289e99bd3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ee533090ea8761ff72e994369295042b8cf972bb","unresolved":false,"context_lines":[{"line_number":145,"context_line":"                 If the supplied `amount_spec` is negative, returns False."},{"line_number":146,"context_line":"        \"\"\""},{"line_number":147,"context_line":"        raise NotImplementedError(self.has_room_for)"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"class IntegerUsage(UsageSpec):"},{"line_number":151,"context_line":"    \"\"\"Represents the total and used amount of a resource with integers.\"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_55de7e12","line":148,"in_reply_to":"7a8c0949_efb6c7db","updated":"2014-11-03 23:34:25.000000000","message":"Fair enough.","commit_id":"2048a1118dc867a3f27d089557906a7289e99bd3"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"4433a67ae09bd2d02a23c57d8daa1b3fa54b046b","unresolved":false,"context_lines":[{"line_number":110,"context_line":"        for cell_id, cell in this_id_map.items():"},{"line_number":111,"context_line":"            result.update((cell_id, cell.memory, cell.cpuset))"},{"line_number":112,"context_line":"        self._hash \u003d result.digest()"},{"line_number":113,"context_line":"        return result"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def __ne__(self, other):"},{"line_number":116,"context_line":"        return not self \u003d\u003d other"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_f2cbd91b","line":113,"updated":"2014-12-10 11:06:50.000000000","message":"As said on one of the previous patchest - let\u0027s set __hast__ to raise - there is very limited use for this (we really don\u0027t need to hash these objects in sets or dicts)","commit_id":"260133b5b6c901307bd2c98d2b8d7a80d9aa7db0"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"16d5ba1f97db17136a2e9909970fc89cf1dc8fe5","unresolved":false,"context_lines":[{"line_number":133,"context_line":""},{"line_number":134,"context_line":"        :param amount_spec: `AmountSpec` to modify the usage with."},{"line_number":135,"context_line":"        \"\"\""},{"line_number":136,"context_line":"        raise NotImplementedError(self.update)"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def has_room_for(self, amount_spec):"},{"line_number":139,"context_line":"        \"\"\"Determine if there is room to fit the supplied amount of resources."}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_d3ca864e","line":136,"range":{"start_line":136,"start_character":14,"end_line":136,"end_character":33},"updated":"2014-12-09 13:38:45.000000000","message":"Should NotImplementedError not pass a text as the parameter?","commit_id":"260133b5b6c901307bd2c98d2b8d7a80d9aa7db0"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"16d5ba1f97db17136a2e9909970fc89cf1dc8fe5","unresolved":false,"context_lines":[{"line_number":144,"context_line":""},{"line_number":145,"context_line":"                 If the supplied `amount_spec` is negative, returns False."},{"line_number":146,"context_line":"        \"\"\""},{"line_number":147,"context_line":"        raise NotImplementedError(self.has_room_for)"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"class IntegerUsage(UsageSpec):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_f3c5c23d","line":147,"updated":"2014-12-09 13:38:45.000000000","message":"ditto","commit_id":"260133b5b6c901307bd2c98d2b8d7a80d9aa7db0"},{"author":{"_account_id":9569,"name":"Sergey Nikitin","email":"nikitinserv@gmail.com","username":"snikitin"},"change_message_id":"2e51f278afd2468c605503b31c681ad00898596e","unresolved":false,"context_lines":[{"line_number":183,"context_line":"        if (not isinstance(total, (int, long)) or"},{"line_number":184,"context_line":"                not isinstance(used, (int, long)) or"},{"line_number":185,"context_line":"                (total \u003c 0) or"},{"line_number":186,"context_line":"                (used \u003c 0)):"},{"line_number":187,"context_line":"            msg \u003d _(\"total and used cannot be negative integers\")"},{"line_number":188,"context_line":"            raise ValueError(msg)"},{"line_number":189,"context_line":"        if (overcommit_ratio is not None and ("}],"source_content_type":"text/x-python","patch_set":4,"id":"5a890539_86b1e9e1","line":186,"updated":"2014-11-26 13:17:43.000000000","message":"nit: I think \u0027(\u0027 and \u0027)\u0027 here are useless.","commit_id":"260133b5b6c901307bd2c98d2b8d7a80d9aa7db0"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"4433a67ae09bd2d02a23c57d8daa1b3fa54b046b","unresolved":false,"context_lines":[{"line_number":276,"context_line":"              total RAM (16 \u003c\u003d 16), and the (total * overcommit) minus used is"},{"line_number":277,"context_line":"              \u003e\u003d requested ((16 * 2.0) - 8 \u003e\u003d 16)."},{"line_number":278,"context_line":"        \"\"\""},{"line_number":279,"context_line":"        if not isinstance(topology, hw.VirtNUMAHostTopology):"},{"line_number":280,"context_line":"            msg \u003d _(\"topology must be a `hardware.VirtNUMAHostTopology` \""},{"line_number":281,"context_line":"                    \"object.\")"},{"line_number":282,"context_line":"            raise ValueError(msg)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_f23599f7","line":279,"updated":"2014-12-10 11:06:50.000000000","message":"these too need to go to objects now","commit_id":"260133b5b6c901307bd2c98d2b8d7a80d9aa7db0"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"4433a67ae09bd2d02a23c57d8daa1b3fa54b046b","unresolved":false,"context_lines":[{"line_number":311,"context_line":"            msg \u003d _(\"amount_spec must be an NUMAAmount\")"},{"line_number":312,"context_line":"            raise ValueError(msg)"},{"line_number":313,"context_line":"        instance_tops \u003d [amount_spec.topology]"},{"line_number":314,"context_line":"        self.topology \u003d hw.VirtNUMAHostTopology.usage_from_instances("},{"line_number":315,"context_line":"                self.topology, instance_tops, free\u003damount_spec.is_negative)"},{"line_number":316,"context_line":""},{"line_number":317,"context_line":"    def has_room_for(self, amount_spec):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_1237854f","line":314,"updated":"2014-12-10 11:06:50.000000000","message":"This is gone now right - only objects!","commit_id":"260133b5b6c901307bd2c98d2b8d7a80d9aa7db0"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"4433a67ae09bd2d02a23c57d8daa1b3fa54b046b","unresolved":false,"context_lines":[{"line_number":334,"context_line":"        if amount_spec.is_negative:"},{"line_number":335,"context_line":"            return False"},{"line_number":336,"context_line":"        requested_topology \u003d amount_spec.topology"},{"line_number":337,"context_line":"        return hw.VirtNUMAHostTopology.fit_instance_to_host("},{"line_number":338,"context_line":"                    self.topology, requested_topology,"},{"line_number":339,"context_line":"                    limits_topology\u003dself.limits) is not None"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_52402da5","line":337,"updated":"2014-12-10 11:06:50.000000000","message":"same here","commit_id":"260133b5b6c901307bd2c98d2b8d7a80d9aa7db0"}],"nova/virt/hardware.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"9ccb8545302fdb9c0b92e72910777a49330c6242","unresolved":false,"context_lines":[{"line_number":218,"context_line":"    # Don\u0027t assume cells are ordered by their ID. Make a mapping"},{"line_number":219,"context_line":"    # of cell ID -\u003e cell for a and b, and compare the keys"},{"line_number":220,"context_line":"    # to ensure exact likeness."},{"line_number":221,"context_line":"    def cell_id_map_from_topology(topology):"},{"line_number":222,"context_line":"        return dict((cell.id, cell) for cell in topology.cells)"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"    a_id_map \u003d cell_id_map_from_topology(a)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_9f1f7159","line":221,"updated":"2015-01-18 15:58:20.000000000","message":"any reason that this is an internal method?\n\nif it was global then we could write tests for it","commit_id":"83030092eb719eef00042d944b5b1a3eaf03ed0a"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"1e1551c218df560cfbbf5192f1e31c1843137dbe","unresolved":false,"context_lines":[{"line_number":199,"context_line":"    \"\"\"Returns True if first topology is the same as the second."},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    :param a: First topology object to compare."},{"line_number":202,"context_line":"    :param b: Second topology object to compare."},{"line_number":203,"context_line":"    \"\"\""},{"line_number":204,"context_line":"    if a is None and b is None:"},{"line_number":205,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":6,"id":"3afb71cf_aa9cae5d","line":202,"updated":"2015-06-08 15:03:12.000000000","message":"Can you specify the fully qualified object name ie nova.objects.BLAH - \u0027topology object\u0027 is ambiguous - could be CPU topology or NUMA topology","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"7d56deea13ea72f73b0512cb06cbc41259d18cc8","unresolved":false,"context_lines":[{"line_number":202,"context_line":"    :param b: Second topology object to compare."},{"line_number":203,"context_line":"    \"\"\""},{"line_number":204,"context_line":"    if a is None and b is None:"},{"line_number":205,"context_line":"        return True"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    if len(a.cells) !\u003d len(b.cells):"},{"line_number":208,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":6,"id":"1af86dd1_b4cb3772","line":205,"updated":"2015-06-12 13:55:06.000000000","message":"You also need:\n\n    if a is None or b is None:\n        return False\n\notherwise your next test could error","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3699e856ca64eb5872647fbe20507e276b6fb896","unresolved":false,"context_lines":[{"line_number":202,"context_line":"    :param b: Second topology object to compare."},{"line_number":203,"context_line":"    \"\"\""},{"line_number":204,"context_line":"    if a is None and b is None:"},{"line_number":205,"context_line":"        return True"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    if len(a.cells) !\u003d len(b.cells):"},{"line_number":208,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_58c8da74","line":205,"in_reply_to":"1af86dd1_b4cb3772","updated":"2015-06-13 16:28:18.000000000","message":"Done","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"1e1551c218df560cfbbf5192f1e31c1843137dbe","unresolved":false,"context_lines":[{"line_number":219,"context_line":"        return False"},{"line_number":220,"context_line":"    for a_cell_id, a_cell in a_id_map.items():"},{"line_number":221,"context_line":"        b_cell \u003d b_id_map[a_cell_id]"},{"line_number":222,"context_line":"        # objects.NUMACell implements __eq__"},{"line_number":223,"context_line":"        if a_cell !\u003d b_cell:"},{"line_number":224,"context_line":"            return False"},{"line_number":225,"context_line":"    return True"}],"source_content_type":"text/x-python","patch_set":6,"id":"3afb71cf_2a1c3ec0","line":222,"updated":"2015-06-08 15:03:12.000000000","message":"Could we implement __eq__ on the NUMATopology object too then instead of having this method here ?","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"5da71a821b80bc88ad656201856b5fe73d9359fc","unresolved":false,"context_lines":[{"line_number":219,"context_line":"        return False"},{"line_number":220,"context_line":"    for a_cell_id, a_cell in a_id_map.items():"},{"line_number":221,"context_line":"        b_cell \u003d b_id_map[a_cell_id]"},{"line_number":222,"context_line":"        # objects.NUMACell implements __eq__"},{"line_number":223,"context_line":"        if a_cell !\u003d b_cell:"},{"line_number":224,"context_line":"            return False"},{"line_number":225,"context_line":"    return True"}],"source_content_type":"text/x-python","patch_set":6,"id":"3afb71cf_f76de398","line":222,"in_reply_to":"3afb71cf_2a1c3ec0","updated":"2015-06-08 17:45:00.000000000","message":"Agree - the logic belongs with the NUMATopology object, not outside of it.","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3699e856ca64eb5872647fbe20507e276b6fb896","unresolved":false,"context_lines":[{"line_number":219,"context_line":"        return False"},{"line_number":220,"context_line":"    for a_cell_id, a_cell in a_id_map.items():"},{"line_number":221,"context_line":"        b_cell \u003d b_id_map[a_cell_id]"},{"line_number":222,"context_line":"        # objects.NUMACell implements __eq__"},{"line_number":223,"context_line":"        if a_cell !\u003d b_cell:"},{"line_number":224,"context_line":"            return False"},{"line_number":225,"context_line":"    return True"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_38a636c2","line":222,"in_reply_to":"3afb71cf_f76de398","updated":"2015-06-13 16:28:18.000000000","message":"Done","commit_id":"9a68e038a246427ada4d23c66ac2dbe0d3cae10a"}]}