)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"3b4362362f89f772de3bd093ff1d5d5cf79a3ef7","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Fix update inventory for multiple providers"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"DB call method get_all_by_resource_provider_uuid queries all"},{"line_number":10,"context_line":"Inventories and filter then by provider\u0027s uuid from ResourceProvider,"},{"line_number":11,"context_line":"but query list is not valid (contains inventories for all providers),"},{"line_number":12,"context_line":"the correct way to load ResourceProvider is to use query join. Replace"},{"line_number":13,"context_line":"joinedload with contains_eager to avoid redundant joins."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"bab6814e_83f2bed0","line":10,"updated":"2016-05-19 10:10:41.000000000","message":"typo: them","commit_id":"3388de2ee35743ce37bca4ee03897f8b9991cdd6"}],"nova/objects/resource_provider.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3a1b0c3839d087b3faafe3026c0b6a81a6ed31c1","unresolved":false,"context_lines":[{"line_number":203,"context_line":"    def _get_all_by_resource_provider(context, rp_uuid):"},{"line_number":204,"context_line":"        return context.session.query(models.Inventory).\\"},{"line_number":205,"context_line":"            join(models.Inventory.resource_provider).\\"},{"line_number":206,"context_line":"            options(joinedload(\u0027resource_provider\u0027)).\\"},{"line_number":207,"context_line":"            filter(models.ResourceProvider.uuid \u003d\u003d rp_uuid).all()"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    @base.remotable_classmethod"}],"source_content_type":"text/x-python","patch_set":1,"id":"dab17558_206306df","line":206,"updated":"2016-05-13 13:54:34.000000000","message":"This joinedload() should have already accomplished the join from inventories to resource_providers. Clearly it isn\u0027t, so we need to check that the model backref relationship is properly configured.\n\nFrom looking at the backref configuration:\n\nhttps://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/models.py#L1497-L1498\n\nWe can see that the bug is actually in the definition of the backref:\n\n        primaryjoin\u003d(\u0027and_(Inventory.resource_provider_id \u003d\u003d \u0027\n                     \u0027ResourceProvider.id)\u0027),\n\nThat isn\u0027t a correct join condition. The and_() is not supposed to be there. It should be:\n\n        primaryjoin\u003d(\u0027Inventory.resource_provider_id \u003d\u003d \u0027\n                     \u0027ResourceProvider.id\u0027),\n\nChange that, remove the join() you added above and retry your tests to see if that fixes the issue.","commit_id":"6a8543b752c8da793d4cdda27e43beb50f5f56ee"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"3b60db83b288b223aa3d4c1588bd54d9ff284f02","unresolved":false,"context_lines":[{"line_number":203,"context_line":"    def _get_all_by_resource_provider(context, rp_uuid):"},{"line_number":204,"context_line":"        return context.session.query(models.Inventory).\\"},{"line_number":205,"context_line":"            join(models.Inventory.resource_provider).\\"},{"line_number":206,"context_line":"            options(joinedload(\u0027resource_provider\u0027)).\\"},{"line_number":207,"context_line":"            filter(models.ResourceProvider.uuid \u003d\u003d rp_uuid).all()"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    @base.remotable_classmethod"}],"source_content_type":"text/x-python","patch_set":4,"id":"bab6814e_4299346e","line":206,"updated":"2016-05-18 09:30:38.000000000","message":"I believe after you\u0027ve added an explicit join here you should also change joinedload() to contains_eager() - please see http://docs.sqlalchemy.org/en/latest/orm/loading_relationships.html#contains-eager and http://docs.sqlalchemy.org/en/latest/orm/loading_relationships.html#sqlalchemy.orm.contains_eager for details","commit_id":"1e512c1e72f14d46b27aa4d3a27c68baa3634f5f"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"9834c77e75523632937eb6294d8e7c3fb2ef66b4","unresolved":false,"context_lines":[{"line_number":202,"context_line":"    @db_api.main_context_manager.reader"},{"line_number":203,"context_line":"    def _get_all_by_resource_provider(context, rp_uuid):"},{"line_number":204,"context_line":"        return context.session.query(models.Inventory).\\"},{"line_number":205,"context_line":"            join(models.Inventory.resource_provider).\\"},{"line_number":206,"context_line":"            options(contains_eager(\u0027resource_provider\u0027)).\\"},{"line_number":207,"context_line":"            filter(models.ResourceProvider.uuid \u003d\u003d rp_uuid).all()"},{"line_number":208,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"bab6814e_676fbbc5","line":205,"range":{"start_line":205,"start_character":17,"end_line":205,"end_character":33},"updated":"2016-05-19 21:49:59.000000000","message":"isn\u0027t something that should wait https://review.openstack.org/#/c/315681/ to be merged and then use the API DB model rather ?","commit_id":"3388de2ee35743ce37bca4ee03897f8b9991cdd6"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"08d953267de0c6a1eb2cc874656b550d82863b1f","unresolved":false,"context_lines":[{"line_number":202,"context_line":"    @db_api.main_context_manager.reader"},{"line_number":203,"context_line":"    def _get_all_by_resource_provider(context, rp_uuid):"},{"line_number":204,"context_line":"        return context.session.query(models.Inventory).\\"},{"line_number":205,"context_line":"            join(models.Inventory.resource_provider).\\"},{"line_number":206,"context_line":"            options(contains_eager(\u0027resource_provider\u0027)).\\"},{"line_number":207,"context_line":"            filter(models.ResourceProvider.uuid \u003d\u003d rp_uuid).all()"},{"line_number":208,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"bab6814e_3fda9a4b","line":205,"range":{"start_line":205,"start_character":17,"end_line":205,"end_character":33},"in_reply_to":"bab6814e_676fbbc5","updated":"2016-05-26 19:52:12.000000000","message":"To be clear, now the Inventory table is in the API DB, so please rebase this change to use nova.db.sqlalchemy.api_models.Inventory","commit_id":"3388de2ee35743ce37bca4ee03897f8b9991cdd6"}],"nova/tests/unit/objects/test_resource_provider.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3a1b0c3839d087b3faafe3026c0b6a81a6ed31c1","unresolved":false,"context_lines":[{"line_number":212,"context_line":""},{"line_number":213,"context_line":"        for idx, inv in enumerate(inventories):"},{"line_number":214,"context_line":"            objects.Inventory._update_in_db(self.context,"},{"line_number":215,"context_line":"                                            inv[1].id,"},{"line_number":216,"context_line":"                                            {\u0027total\u0027: total[idx]})"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"        for idx, inv in enumerate(inventories):"}],"source_content_type":"text/x-python","patch_set":1,"id":"dab17558_8062b2d6","line":215,"updated":"2016-05-13 13:54:34.000000000","message":"I don\u0027t understand why you are hard-coding \"1\" as the inv index above? This block is updating only the second inventory\u0027s total value to be equal to 6 (the first two updates will be overwritten by the third and so the total will be set to 3 * 2 \u003d 6). It isn\u0027t updating any of the other inventory records.\n\nI would ask that you rework this unit test in the following ways:\n\n1) Add comments to the unit test that indicate what you are testing, since it\u0027s not very clear\n\n2) Do not iterate over range(3). Instead, create two resource providers and create two inventory records (one for DISK_GB and another for IPV4_ADDRESS resource class) for each of the resource providers, setting the total value for the inventory records to a different value for each resource provider. Then call get_all_by_resource_provider_uuid() on each resource provider UUID and validate that only the inventory records for that resource provider UUID are returned.","commit_id":"6a8543b752c8da793d4cdda27e43beb50f5f56ee"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"355202e2246a278de62232a051ec4c6fbdccf3b2","unresolved":false,"context_lines":[{"line_number":241,"context_line":"        self._create_inventory_in_db(db_rp1.id, id\u003d3,"},{"line_number":242,"context_line":"            resource_provider_id\u003ddb_rp1.id,"},{"line_number":243,"context_line":"            resource_class_id\u003dIPV4_ADDRESS_ID,"},{"line_number":244,"context_line":"            total\u003d\u002710.0.0.1\u0027)"},{"line_number":245,"context_line":"        self._create_inventory_in_db(db_rp2.id, id\u003d4,"},{"line_number":246,"context_line":"            resource_provider_id\u003ddb_rp2.id,"},{"line_number":247,"context_line":"            resource_class_id\u003dIPV4_ADDRESS_ID,"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_6c2729ca","line":244,"updated":"2016-05-17 19:30:31.000000000","message":"This and below is going to fail... total field is an integer, not a string.","commit_id":"962c72019bb468a8679c4f6b760804e183b8f3df"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"bacbf5ac810103d56f82d9499fe829de495b0377","unresolved":false,"context_lines":[{"line_number":238,"context_line":"        IPV4_ADDRESS_ID \u003d objects.fields.ResourceClass.index("},{"line_number":239,"context_line":"            objects.fields.ResourceClass.IPV4_ADDRESS)"},{"line_number":240,"context_line":""},{"line_number":241,"context_line":"        self._create_inventory_in_db(db_rp1.id, id\u003d3,"},{"line_number":242,"context_line":"                                     resource_provider_id\u003ddb_rp1.id,"},{"line_number":243,"context_line":"                                     resource_class_id\u003dIPV4_ADDRESS_ID,"},{"line_number":244,"context_line":"                                     total\u003d2)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bab6814e_a1810192","line":241,"updated":"2016-05-18 09:11:49.000000000","message":"Why do you need to specify id\u003d3? It is not popped from updates in _create_inventory_in_db in this case. Isn\u0027t it better to avoid this?","commit_id":"da9de649097e0a4fd617982643658b3fdfe6aa67"},{"author":{"_account_id":13636,"name":"aarefiev","display_name":"Anton","email":"arefyevanton@gmail.com","username":"aarefiev"},"change_message_id":"5e4033200690c78169d799b2b0a7cbc040fba42e","unresolved":false,"context_lines":[{"line_number":238,"context_line":"        IPV4_ADDRESS_ID \u003d objects.fields.ResourceClass.index("},{"line_number":239,"context_line":"            objects.fields.ResourceClass.IPV4_ADDRESS)"},{"line_number":240,"context_line":""},{"line_number":241,"context_line":"        self._create_inventory_in_db(db_rp1.id, id\u003d3,"},{"line_number":242,"context_line":"                                     resource_provider_id\u003ddb_rp1.id,"},{"line_number":243,"context_line":"                                     resource_class_id\u003dIPV4_ADDRESS_ID,"},{"line_number":244,"context_line":"                                     total\u003d2)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bab6814e_dc2a2ae7","line":241,"in_reply_to":"bab6814e_a1810192","updated":"2016-05-18 09:25:20.000000000","message":"doesn\u0027t matter for test","commit_id":"da9de649097e0a4fd617982643658b3fdfe6aa67"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"bacbf5ac810103d56f82d9499fe829de495b0377","unresolved":false,"context_lines":[{"line_number":242,"context_line":"                                     resource_provider_id\u003ddb_rp1.id,"},{"line_number":243,"context_line":"                                     resource_class_id\u003dIPV4_ADDRESS_ID,"},{"line_number":244,"context_line":"                                     total\u003d2)"},{"line_number":245,"context_line":"        self._create_inventory_in_db(db_rp2.id, id\u003d4,"},{"line_number":246,"context_line":"                                     resource_provider_id\u003ddb_rp2.id,"},{"line_number":247,"context_line":"                                     resource_class_id\u003dIPV4_ADDRESS_ID,"},{"line_number":248,"context_line":"                                     total\u003d4)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bab6814e_41093531","line":245,"updated":"2016-05-18 09:11:49.000000000","message":"ditto","commit_id":"da9de649097e0a4fd617982643658b3fdfe6aa67"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"3b60db83b288b223aa3d4c1588bd54d9ff284f02","unresolved":false,"context_lines":[{"line_number":11,"context_line":"#    under the License."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"import mock"},{"line_number":14,"context_line":"import uuid"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"from nova import exception"},{"line_number":17,"context_line":"from nova import objects"}],"source_content_type":"text/x-python","patch_set":4,"id":"bab6814e_623c3830","line":14,"updated":"2016-05-18 09:30:38.000000000","message":"nit: this module is a part of standard library and should be in the first group of imports, while mock is a 3rd party library (in py2.x :P)","commit_id":"1e512c1e72f14d46b27aa4d3a27c68baa3634f5f"}]}