)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"eaaaf31fe6ec49f6cf5ed74f2c266f0997953f8d","unresolved":false,"context_lines":[{"line_number":17,"context_line":"When inventory is deleted if there are existing allocations a"},{"line_number":18,"context_line":"ResourceProviderInUse exception is raised. The reason for this"},{"line_number":19,"context_line":"disjunction is because in the case of deletion we don\u0027t have"},{"line_number":20,"context_line":"insight into which of one or more inventories is being removed."},{"line_number":21,"context_line":"We can do a more complex sql query but it is not clear it is"},{"line_number":22,"context_line":"worth it."},{"line_number":23,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":25,"id":"9ad45d7e_6a65aa1c","line":20,"updated":"2016-08-10 13:57:23.000000000","message":"@cdent: I\u0027m really not following you. We are not making any additional queries other than the one we\u0027re already making to determine if there are any allocations made against the inventory that we are trying to delete.","commit_id":"ecad635ad80367b89a08cc242be0c76d8f5209ae"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cd47d5c772f7c64c6482982245d49b335d9a447f","unresolved":false,"context_lines":[{"line_number":17,"context_line":"When inventory is deleted if there are existing allocations a"},{"line_number":18,"context_line":"ResourceProviderInUse exception is raised. The reason for this"},{"line_number":19,"context_line":"disjunction is because in the case of deletion we don\u0027t have"},{"line_number":20,"context_line":"insight into which of one or more inventories is being removed."},{"line_number":21,"context_line":"We can do a more complex sql query but it is not clear it is"},{"line_number":22,"context_line":"worth it."},{"line_number":23,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":25,"id":"9ad45d7e_9e1b321b","line":20,"updated":"2016-08-09 17:13:18.000000000","message":"Why don\u0027t we? Seems like it\u0027s a simple query not a complex one to determine if any allocations exist for an inventory being deleted.","commit_id":"ecad635ad80367b89a08cc242be0c76d8f5209ae"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"db7af6a90b2c9867c3f1780ebb0f14f7247ed705","unresolved":false,"context_lines":[{"line_number":17,"context_line":"When inventory is deleted if there are existing allocations a"},{"line_number":18,"context_line":"ResourceProviderInUse exception is raised. The reason for this"},{"line_number":19,"context_line":"disjunction is because in the case of deletion we don\u0027t have"},{"line_number":20,"context_line":"insight into which of one or more inventories is being removed."},{"line_number":21,"context_line":"We can do a more complex sql query but it is not clear it is"},{"line_number":22,"context_line":"worth it."},{"line_number":23,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":25,"id":"9ad45d7e_c750fa34","line":20,"in_reply_to":"9ad45d7e_6a65aa1c","updated":"2016-08-10 21:09:37.000000000","message":"I was mostly being obtuse and wrong, but what I was trying to say was \"meh, I don\u0027t want to request multiple rows when I could just get one\".\n\nI\u0027ve changed it.","commit_id":"ecad635ad80367b89a08cc242be0c76d8f5209ae"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"6c8c77fff49ca53900e1f58bc70cea2218eef9ee","unresolved":false,"context_lines":[{"line_number":17,"context_line":"When inventory is deleted if there are existing allocations a"},{"line_number":18,"context_line":"ResourceProviderInUse exception is raised. The reason for this"},{"line_number":19,"context_line":"disjunction is because in the case of deletion we don\u0027t have"},{"line_number":20,"context_line":"insight into which of one or more inventories is being removed."},{"line_number":21,"context_line":"We can do a more complex sql query but it is not clear it is"},{"line_number":22,"context_line":"worth it."},{"line_number":23,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":25,"id":"9ad45d7e_1ca56eb4","line":20,"in_reply_to":"9ad45d7e_9e1b321b","updated":"2016-08-10 08:35:15.000000000","message":"I\u0027ve been assuming throughout most of the process of developing this stuff that we want to minimize the number of queries we\u0027re making. Making an additional query in this case seems wasteful, especially when the three different exceptions do have different meanings.\n\nShould I correct my poor assumption and just make more queries?","commit_id":"ecad635ad80367b89a08cc242be0c76d8f5209ae"}],"nova/exception.py":[{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"a96e0ef074ab207a1a766c7a92e3a63d5a0213d5","unresolved":false,"context_lines":[{"line_number":2103,"context_line":""},{"line_number":2104,"context_line":""},{"line_number":2105,"context_line":"class InventoryCapacityMismatch(NovaException):"},{"line_number":2106,"context_line":"    msg_fmt \u003d _(\"Inventory for \u0027%(resource_class)s\u0027 on \""},{"line_number":2107,"context_line":"                \"\u0027%(resource_provider)s\u0027 invalid.\")"},{"line_number":2108,"context_line":""},{"line_number":2109,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"dada55a8_7ea2ab4c","line":2106,"updated":"2016-07-25 19:28:24.000000000","message":"Just curious, would it be possible to add the text \u0027resource provider\u0027 in front of the RP UUID printouts below? It could make log statements more meaningful.","commit_id":"550f53667d6d6267f7c54070e4ddd48da419be8e"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"5abe27fabbc8a943939332ff0dcae903e143f047","unresolved":false,"context_lines":[{"line_number":2103,"context_line":""},{"line_number":2104,"context_line":""},{"line_number":2105,"context_line":"class InventoryCapacityMismatch(NovaException):"},{"line_number":2106,"context_line":"    msg_fmt \u003d _(\"Inventory for \u0027%(resource_class)s\u0027 on \""},{"line_number":2107,"context_line":"                \"\u0027%(resource_provider)s\u0027 invalid.\")"},{"line_number":2108,"context_line":""},{"line_number":2109,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"bacf61ea_e5ac86ef","line":2106,"in_reply_to":"dada55a8_7ea2ab4c","updated":"2016-07-28 11:19:52.000000000","message":"Done","commit_id":"550f53667d6d6267f7c54070e4ddd48da419be8e"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"f121edc2c47114e1d41fec50fd60e91240225050","unresolved":false,"context_lines":[{"line_number":2103,"context_line":""},{"line_number":2104,"context_line":""},{"line_number":2105,"context_line":"class InventoryCapacityMismatch(NovaException):"},{"line_number":2106,"context_line":"    msg_fmt \u003d _(\"Inventory for \u0027%(resource_class)s\u0027 on \""},{"line_number":2107,"context_line":"                \"\u0027%(resource_provider)s\u0027 invalid.\")"},{"line_number":2108,"context_line":""},{"line_number":2109,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"bacf61ea_64354345","line":2106,"in_reply_to":"dada55a8_7ea2ab4c","updated":"2016-07-28 08:43:11.000000000","message":"That\u0027s a good idea, I\u0027ll add it to the to do list.","commit_id":"550f53667d6d6267f7c54070e4ddd48da419be8e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c3414ecd39d6e0cf15b644cbe02f7e0f48500451","unresolved":false,"context_lines":[{"line_number":2116,"context_line":"    msg_fmt \u003d _(\"Resource provider has allocations.\")"},{"line_number":2117,"context_line":""},{"line_number":2118,"context_line":""},{"line_number":2119,"context_line":"class InventoryMismatch(NovaException):"},{"line_number":2120,"context_line":"    msg_fmt \u003d _(\"Inventory for \u0027%(resource_class)s\u0027 on \""},{"line_number":2121,"context_line":"                \"resource provider \u0027%(resource_provider)s\u0027 invalid.\")"},{"line_number":2122,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"9ad45d7e_b219bd8a","line":2119,"updated":"2016-08-10 23:41:31.000000000","message":"or InvalidInventory to match other exception class names in this file, including, but not limited to:\n\n InvalidReservedMemoryPagesOption\n InvalidConnectionInfo\n InvalidImageFormat\n InvalidToken\n\netc.\n\nAlso, please make InvalidInventory subclass the base Invalid exception to match all the others :)","commit_id":"a00ebcf84c02380f326c817400083b931aa4cc30"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a3b256f06245143919b4a65d09beaafdab6b649f","unresolved":false,"context_lines":[{"line_number":2127,"context_line":"class InvalidInventoryNewCapacityExceeded(InvalidInventory):"},{"line_number":2128,"context_line":"    msg_fmt \u003d _(\"Invalid inventory for \u0027%(resource_class)s\u0027 on \""},{"line_number":2129,"context_line":"                \"resource provider \u0027%(resource_provider)s\u0027. The new total \""},{"line_number":2130,"context_line":"                \"minus reserved amount is less than the existing used amount.\")"},{"line_number":2131,"context_line":""},{"line_number":2132,"context_line":""},{"line_number":2133,"context_line":"class UnsupportedPointerModelRequested(Invalid):"}],"source_content_type":"text/x-python","patch_set":33,"id":"3ac371cc_f9314d5b","line":2130,"updated":"2016-08-17 17:53:35.000000000","message":"++ on all above. really excellent message content. gives the caller all the information they need to determine what exactly is the conflict.","commit_id":"59d4d599e18bf3e1c33d955e38f9ed49cc879496"}],"nova/objects/resource_provider.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cd47d5c772f7c64c6482982245d49b335d9a447f","unresolved":false,"context_lines":[{"line_number":58,"context_line":"            _ALLOC_TBL.c.resource_class_id.in_(to_delete)))"},{"line_number":59,"context_line":"    allocations \u003d conn.execute(allocation_query).first()"},{"line_number":60,"context_line":"    if allocations[\u0027count\u0027]:"},{"line_number":61,"context_line":"        raise exception.ResourceProviderInUse()"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    del_stmt \u003d _INV_TBL.delete().where(sa.and_("},{"line_number":64,"context_line":"            _INV_TBL.c.resource_provider_id \u003d\u003d rp.id,"}],"source_content_type":"text/x-python","patch_set":25,"id":"9ad45d7e_1e274265","line":61,"updated":"2016-08-09 17:13:18.000000000","message":"++","commit_id":"ecad635ad80367b89a08cc242be0c76d8f5209ae"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"eaaaf31fe6ec49f6cf5ed74f2c266f0997953f8d","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    for res_class in to_add:"},{"line_number":80,"context_line":"        inv_record \u003d inv_list.find(res_class)"},{"line_number":81,"context_line":"        if inv_record.capacity \u003c\u003d 0:"},{"line_number":82,"context_line":"            raise exception.InventoryCapacityMismatch("},{"line_number":83,"context_line":"                resource_class\u003dfields.ResourceClass.from_index(res_class),"},{"line_number":84,"context_line":"                resource_provider\u003drp.uuid)"},{"line_number":85,"context_line":"        ins_stmt \u003d _INV_TBL.insert().values("}],"source_content_type":"text/x-python","patch_set":25,"id":"9ad45d7e_0aa6ae52","line":82,"updated":"2016-08-10 13:57:23.000000000","message":"@tdurakov: So, there\u0027s no problem having a base exception class from which both of these slightly different exceptions would inherit. That\u0027s totally cool and would make the REST controller code simpler -- it would just catch on the base exception class and write back the HTTP response code along with a message with more detail about what specifically went wrong. But I think it\u0027s important to convey to the caller as much information about what went wrong as possible, even if the HTTP response code is not so granular.","commit_id":"ecad635ad80367b89a08cc242be0c76d8f5209ae"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cd47d5c772f7c64c6482982245d49b335d9a447f","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    for res_class in to_add:"},{"line_number":80,"context_line":"        inv_record \u003d inv_list.find(res_class)"},{"line_number":81,"context_line":"        if inv_record.capacity \u003c\u003d 0:"},{"line_number":82,"context_line":"            raise exception.InventoryCapacityMismatch("},{"line_number":83,"context_line":"                resource_class\u003dfields.ResourceClass.from_index(res_class),"},{"line_number":84,"context_line":"                resource_provider\u003drp.uuid)"},{"line_number":85,"context_line":"        ins_stmt \u003d _INV_TBL.insert().values("}],"source_content_type":"text/x-python","patch_set":25,"id":"9ad45d7e_fe298e38","line":82,"updated":"2016-08-09 17:13:18.000000000","message":"How about just calling this exception InvalidInventoryNegativeCapacity and making the error message more descriptive, like \"Invalid inventory. The reserved value is greater than total.\"","commit_id":"ecad635ad80367b89a08cc242be0c76d8f5209ae"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"db7af6a90b2c9867c3f1780ebb0f14f7247ed705","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    for res_class in to_add:"},{"line_number":80,"context_line":"        inv_record \u003d inv_list.find(res_class)"},{"line_number":81,"context_line":"        if inv_record.capacity \u003c\u003d 0:"},{"line_number":82,"context_line":"            raise exception.InventoryCapacityMismatch("},{"line_number":83,"context_line":"                resource_class\u003dfields.ResourceClass.from_index(res_class),"},{"line_number":84,"context_line":"                resource_provider\u003drp.uuid)"},{"line_number":85,"context_line":"        ins_stmt \u003d _INV_TBL.insert().values("}],"source_content_type":"text/x-python","patch_set":25,"id":"9ad45d7e_020cc039","line":82,"in_reply_to":"9ad45d7e_0aa6ae52","updated":"2016-08-10 21:09:37.000000000","message":"What I\u0027ve ended up with is three explicit exceptions that subclass a generic one.\n\nI\u0027ve tried to capture the intent of what you\u0027ve suggested without doing exactly what you said (as it didn\u0027t quite fit).\n\nIf it\u0027s not quite right should be easy to fix.","commit_id":"ecad635ad80367b89a08cc242be0c76d8f5209ae"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"50269f90d41442ddc974d8b668f8fa9f243f43cc","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    for res_class in to_add:"},{"line_number":80,"context_line":"        inv_record \u003d inv_list.find(res_class)"},{"line_number":81,"context_line":"        if inv_record.capacity \u003c\u003d 0:"},{"line_number":82,"context_line":"            raise exception.InventoryCapacityMismatch("},{"line_number":83,"context_line":"                resource_class\u003dfields.ResourceClass.from_index(res_class),"},{"line_number":84,"context_line":"                resource_provider\u003drp.uuid)"},{"line_number":85,"context_line":"        ins_stmt \u003d _INV_TBL.insert().values("}],"source_content_type":"text/x-python","patch_set":25,"id":"9ad45d7e_401fef6b","line":82,"in_reply_to":"9ad45d7e_dc4596b3","updated":"2016-08-10 12:23:14.000000000","message":"About exceptions and amount of different types of these exceptions:\nAre we going to handle it different way, in case that Chris described?  If yes - it make sense, If the answer is no I\u0027d prefer use *one* exception type for that, and leave naming as is.","commit_id":"ecad635ad80367b89a08cc242be0c76d8f5209ae"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"6c8c77fff49ca53900e1f58bc70cea2218eef9ee","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    for res_class in to_add:"},{"line_number":80,"context_line":"        inv_record \u003d inv_list.find(res_class)"},{"line_number":81,"context_line":"        if inv_record.capacity \u003c\u003d 0:"},{"line_number":82,"context_line":"            raise exception.InventoryCapacityMismatch("},{"line_number":83,"context_line":"                resource_class\u003dfields.ResourceClass.from_index(res_class),"},{"line_number":84,"context_line":"                resource_provider\u003drp.uuid)"},{"line_number":85,"context_line":"        ins_stmt \u003d _INV_TBL.insert().values("}],"source_content_type":"text/x-python","patch_set":25,"id":"9ad45d7e_dc4596b3","line":82,"in_reply_to":"9ad45d7e_fe298e38","updated":"2016-08-10 08:35:15.000000000","message":"My assumption in this case was that we might like to also use this exception in situations where things other than reserved are out of wack (min_unit \u003e max_unit, things like that).\n\nFrom your comments here and on the other exception I guess you\u0027d prefer multiple more specific exceptions as we do them? That\u0027s weird to my experience, but seems to be the nova norm. I\u0027ll change it, barring someone else saying nooooo.","commit_id":"ecad635ad80367b89a08cc242be0c76d8f5209ae"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cd47d5c772f7c64c6482982245d49b335d9a447f","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        if inv_record.capacity \u003c\u003d 0:"},{"line_number":109,"context_line":"            raise exception.InventoryCapacityMismatch("},{"line_number":110,"context_line":"                resource_class\u003dfields.ResourceClass.from_index(res_class),"},{"line_number":111,"context_line":"                resource_provider\u003drp.uuid)"},{"line_number":112,"context_line":"        allocation_query \u003d sa.select("},{"line_number":113,"context_line":"            [func.coalesce(func.sum(_ALLOC_TBL.c.used), 0)."},{"line_number":114,"context_line":"             label(\u0027usage\u0027)]).where(sa.and_("}],"source_content_type":"text/x-python","patch_set":25,"id":"9ad45d7e_5e21ba4c","line":111,"updated":"2016-08-09 17:13:18.000000000","message":"ditto from above.","commit_id":"ecad635ad80367b89a08cc242be0c76d8f5209ae"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cd47d5c772f7c64c6482982245d49b335d9a447f","unresolved":false,"context_lines":[{"line_number":117,"context_line":"                     _ALLOC_TBL.c.resource_class_id)"},{"line_number":118,"context_line":"        allocations \u003d conn.execute(allocation_query).first()"},{"line_number":119,"context_line":"        if allocations and allocations[\u0027usage\u0027] \u003e inv_record.capacity:"},{"line_number":120,"context_line":"            raise exception.InventoryAllocationMismatch("},{"line_number":121,"context_line":"                resource_class\u003dfields.ResourceClass.from_index(res_class),"},{"line_number":122,"context_line":"                resource_provider\u003drp.uuid)"},{"line_number":123,"context_line":"        upd_stmt \u003d _INV_TBL.update().where(sa.and_("}],"source_content_type":"text/x-python","patch_set":25,"id":"9ad45d7e_3e1cc612","line":120,"updated":"2016-08-09 17:13:18.000000000","message":"How about InvalidInventoryNewCapacityExceeded and making the message text more explicit with \"The new total minus reserved amount is less than the existing used amount of this resource class.\"","commit_id":"ecad635ad80367b89a08cc242be0c76d8f5209ae"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"50269f90d41442ddc974d8b668f8fa9f243f43cc","unresolved":false,"context_lines":[{"line_number":117,"context_line":"                     _ALLOC_TBL.c.resource_class_id)"},{"line_number":118,"context_line":"        allocations \u003d conn.execute(allocation_query).first()"},{"line_number":119,"context_line":"        if allocations and allocations[\u0027usage\u0027] \u003e inv_record.capacity:"},{"line_number":120,"context_line":"            raise exception.InventoryAllocationMismatch("},{"line_number":121,"context_line":"                resource_class\u003dfields.ResourceClass.from_index(res_class),"},{"line_number":122,"context_line":"                resource_provider\u003drp.uuid)"},{"line_number":123,"context_line":"        upd_stmt \u003d _INV_TBL.update().where(sa.and_("}],"source_content_type":"text/x-python","patch_set":25,"id":"9ad45d7e_63eb1d14","line":120,"in_reply_to":"9ad45d7e_3e1cc612","updated":"2016-08-10 12:23:14.000000000","message":"almost the same, it totally depends on situations where this going to be  used.","commit_id":"ecad635ad80367b89a08cc242be0c76d8f5209ae"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"01447a50c4ae2b2c74ee7f03e6cbf8ae7b15d5dc","unresolved":false,"context_lines":[{"line_number":45,"context_line":"    resource class identifiers."},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    If there are allocations for any of the inventories to be deleted raise"},{"line_number":48,"context_line":"    ResourceProviderInUse exception."},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    :param conn: DB connection to use."},{"line_number":51,"context_line":"    :param rp: Resource provider from which to delete inventory."}],"source_content_type":"text/x-python","patch_set":28,"id":"9ad45d7e_48ed16b6","line":48,"updated":"2016-08-11 12:31:48.000000000","message":"s/ResourceProviderInUse/InventoryInUse/","commit_id":"6779961fae7d61869b32a27ddc6629cd2abb8b12"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"d4bce4b5d04b20a14c96795ec9ca0a87b2f92d33","unresolved":false,"context_lines":[{"line_number":45,"context_line":"    resource class identifiers."},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    If there are allocations for any of the inventories to be deleted raise"},{"line_number":48,"context_line":"    ResourceProviderInUse exception."},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    :param conn: DB connection to use."},{"line_number":51,"context_line":"    :param rp: Resource provider from which to delete inventory."}],"source_content_type":"text/x-python","patch_set":28,"id":"9ad45d7e_eb43246a","line":48,"in_reply_to":"9ad45d7e_48ed16b6","updated":"2016-08-11 12:50:19.000000000","message":"Oh the humanity","commit_id":"6779961fae7d61869b32a27ddc6629cd2abb8b12"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a3b256f06245143919b4a65d09beaafdab6b649f","unresolved":false,"context_lines":[{"line_number":59,"context_line":"         ).group_by(_ALLOC_TBL.c.resource_class_id)"},{"line_number":60,"context_line":"    allocations \u003d conn.execute(allocation_query).fetchall()"},{"line_number":61,"context_line":"    if allocations:"},{"line_number":62,"context_line":"        resource_classes \u003d \u0027%s\u0027 % \u0027, \u0027.join([fields.ResourceClass.from_index("},{"line_number":63,"context_line":"            allocation.resource_class) for allocation in allocations])"},{"line_number":64,"context_line":"        raise exception.InventoryInUse(resource_classes\u003dresource_classes,"},{"line_number":65,"context_line":"                                       resource_provider\u003drp.uuid)"}],"source_content_type":"text/x-python","patch_set":33,"id":"3ac371cc_b9ed15b9","line":62,"updated":"2016-08-17 17:53:35.000000000","message":"nit: you don\u0027t need to do:\n\n resource_classes \u003d \u0027%s\u0027 % \u0027,\u0027.join(...)\n\nyou can just do:\n\n resource_classes \u003d \u0027,\u0027.join(...)","commit_id":"59d4d599e18bf3e1c33d955e38f9ed49cc879496"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a3b256f06245143919b4a65d09beaafdab6b649f","unresolved":false,"context_lines":[{"line_number":114,"context_line":"                resource_class\u003dfields.ResourceClass.from_index(res_class),"},{"line_number":115,"context_line":"                resource_provider\u003drp.uuid)"},{"line_number":116,"context_line":"        allocation_query \u003d sa.select("},{"line_number":117,"context_line":"            [func.coalesce(func.sum(_ALLOC_TBL.c.used), 0)."},{"line_number":118,"context_line":"             label(\u0027usage\u0027)]).where(sa.and_("},{"line_number":119,"context_line":"                 _ALLOC_TBL.c.resource_provider_id \u003d\u003d rp.id,"},{"line_number":120,"context_line":"                 _ALLOC_TBL.c.resource_class_id \u003d\u003d res_class)).group_by("}],"source_content_type":"text/x-python","patch_set":33,"id":"3ac371cc_192d49fe","line":117,"updated":"2016-08-17 17:53:35.000000000","message":"Not sure why I didn\u0027t notice this earlier, but above, the func.coalesce() is not needed since you are not left-joining to any table. And the group_by() expression below isn\u0027t correct in this context within the resource class to_update loop (there\u0027s no point grouping on resource class when you\u0027re filtering on one resource class).\n\nFurthermore, we will be executing this query for each resource class in the inventory update request, which is inefficient. You can move this query outside the for loop and use a group_by with an IN(...) expression to do only a single query:\n\n # Check for allocations...\n alloc_query \u003d sa.select([ALLOC_TBL.c.resource_class_id,\n                          func.sum(_ALLOC_TBL.c.used).label(\u0027used\u0027)])\n alloc_query \u003d alloc_query.where(sa.and_(\n     _ALLOC_TBL.c.resource_provider_id \u003d\u003d rp.id,\n     _ALLOC_TBL.c.resource_class_id.in_(to_update)).group_by(\n         _ALLOC_TBL.c.resource_class_id).fetchall()\n if alloc_query:\n     resource_classes \u003d \u0027,\u0027.join([\n         fields.ResourceClass.from_index(record[0])\n         for record in alloc_query\n     ])\n     raise exception.InvalidInventoryNewCapacityExceeded(\n         resource_class\u003dresource_classes,\n         resource_provider\u003drp.uuid)\n\n for res_class in to_update:\n     ...","commit_id":"59d4d599e18bf3e1c33d955e38f9ed49cc879496"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"e30f02d212400d466a7ddf44e307661f1c7868e3","unresolved":false,"context_lines":[{"line_number":114,"context_line":"                resource_class\u003dfields.ResourceClass.from_index(res_class),"},{"line_number":115,"context_line":"                resource_provider\u003drp.uuid)"},{"line_number":116,"context_line":"        allocation_query \u003d sa.select("},{"line_number":117,"context_line":"            [func.coalesce(func.sum(_ALLOC_TBL.c.used), 0)."},{"line_number":118,"context_line":"             label(\u0027usage\u0027)]).where(sa.and_("},{"line_number":119,"context_line":"                 _ALLOC_TBL.c.resource_provider_id \u003d\u003d rp.id,"},{"line_number":120,"context_line":"                 _ALLOC_TBL.c.resource_class_id \u003d\u003d res_class)).group_by("}],"source_content_type":"text/x-python","patch_set":33,"id":"3ac371cc_49506c32","line":117,"in_reply_to":"3ac371cc_192d49fe","updated":"2016-08-18 09:37:59.000000000","message":"Now that I\u0027ve had some sleep, I actually think your suggestions isn\u0027t quite right here. It may be the case that we can do just one query (outside the loop) but the query you\u0027ve given is not correct as it doesn\u0027t check what we need to be checking: that the \u0027used\u0027 is in excess of inventory.capacity (line 123 below).\n\nYour query is checking that there are any allocations. That\u0027s required for deletion (and is done in a way similar to what you describe here in the lines in the 60s above). We need to know the usage here.\n\nYou\u0027re right that the coalesce and group by can go away, that was probably a pasto from some earlier version.\n\nI\u0027ll push a new version with less complexity, but still in the loop and you can look at that one and decide how it can move out of the loop.","commit_id":"59d4d599e18bf3e1c33d955e38f9ed49cc879496"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"0c25bb8be61f65532a20e806b5e8ad83539a996e","unresolved":false,"context_lines":[{"line_number":114,"context_line":"                resource_class\u003dfields.ResourceClass.from_index(res_class),"},{"line_number":115,"context_line":"                resource_provider\u003drp.uuid)"},{"line_number":116,"context_line":"        allocation_query \u003d sa.select("},{"line_number":117,"context_line":"            [func.coalesce(func.sum(_ALLOC_TBL.c.used), 0)."},{"line_number":118,"context_line":"             label(\u0027usage\u0027)]).where(sa.and_("},{"line_number":119,"context_line":"                 _ALLOC_TBL.c.resource_provider_id \u003d\u003d rp.id,"},{"line_number":120,"context_line":"                 _ALLOC_TBL.c.resource_class_id \u003d\u003d res_class)).group_by("}],"source_content_type":"text/x-python","patch_set":33,"id":"3ac371cc_6d5c3419","line":117,"in_reply_to":"3ac371cc_1f4b425a","updated":"2016-08-18 16:02:38.000000000","message":"I\u0027m no SA expert, but maybe something like this:\n\n        allocation_query \u003d sa.select(\n            [func.sum(_ALLOC_TBL.c.used).label(\u0027usage\u0027)]).\n            where(sa.and_(\n                 _ALLOC_TBL.c.resource_provider_id.in_(to_update),\n                 _ALLOC_TBL.c.resource_class_id \u003d\u003d res_class)).\n                 group_by(_ALLOC_TBL.c.resource_class_id)","commit_id":"59d4d599e18bf3e1c33d955e38f9ed49cc879496"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"9d1557812e620136da7595fbfec374b41ed94dd9","unresolved":false,"context_lines":[{"line_number":114,"context_line":"                resource_class\u003dfields.ResourceClass.from_index(res_class),"},{"line_number":115,"context_line":"                resource_provider\u003drp.uuid)"},{"line_number":116,"context_line":"        allocation_query \u003d sa.select("},{"line_number":117,"context_line":"            [func.coalesce(func.sum(_ALLOC_TBL.c.used), 0)."},{"line_number":118,"context_line":"             label(\u0027usage\u0027)]).where(sa.and_("},{"line_number":119,"context_line":"                 _ALLOC_TBL.c.resource_provider_id \u003d\u003d rp.id,"},{"line_number":120,"context_line":"                 _ALLOC_TBL.c.resource_class_id \u003d\u003d res_class)).group_by("}],"source_content_type":"text/x-python","patch_set":33,"id":"3ac371cc_1f4b425a","line":117,"in_reply_to":"3ac371cc_49506c32","updated":"2016-08-18 09:50:27.000000000","message":"I suppose we could do a query that joins with inventory that returns a list of those resource class ids where usage is \u003e total-reserved but I bet Jay can write that faster than I can flail over it.","commit_id":"59d4d599e18bf3e1c33d955e38f9ed49cc879496"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"c74bc1be0515a38d52a9048511f8b5d3132af872","unresolved":false,"context_lines":[{"line_number":53,"context_line":"                      delete."},{"line_number":54,"context_line":"    \"\"\""},{"line_number":55,"context_line":"    allocation_query \u003d sa.select("},{"line_number":56,"context_line":"        [_ALLOC_TBL.c.resource_class_id.label(\u0027resource_class\u0027)]).where("},{"line_number":57,"context_line":"             sa.and_(_ALLOC_TBL.c.resource_provider_id \u003d\u003d rp.id,"},{"line_number":58,"context_line":"                     _ALLOC_TBL.c.resource_class_id.in_(to_delete))"},{"line_number":59,"context_line":"         ).group_by(_ALLOC_TBL.c.resource_class_id)"}],"source_content_type":"text/x-python","patch_set":35,"id":"3ac371cc_01af3d7e","line":56,"range":{"start_line":56,"start_character":40,"end_line":56,"end_character":45},"updated":"2016-08-18 19:57:09.000000000","message":"I had to look this up and it appears to just do \"as resource_class\" in the SQL query. Seems unnecessary but also not a problem.","commit_id":"5d710c4ac12aedaf84c9c314a60101375799f00c"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"c2fee119ea608db64c97e88b48be9bf137e48586","unresolved":false,"context_lines":[{"line_number":53,"context_line":"                      delete."},{"line_number":54,"context_line":"    \"\"\""},{"line_number":55,"context_line":"    allocation_query \u003d sa.select("},{"line_number":56,"context_line":"        [_ALLOC_TBL.c.resource_class_id.label(\u0027resource_class\u0027)]).where("},{"line_number":57,"context_line":"             sa.and_(_ALLOC_TBL.c.resource_provider_id \u003d\u003d rp.id,"},{"line_number":58,"context_line":"                     _ALLOC_TBL.c.resource_class_id.in_(to_delete))"},{"line_number":59,"context_line":"         ).group_by(_ALLOC_TBL.c.resource_class_id)"}],"source_content_type":"text/x-python","patch_set":35,"id":"3ac371cc_2421a7b5","line":56,"in_reply_to":"3ac371cc_01af3d7e","updated":"2016-08-18 20:33:47.000000000","message":"I was for (unnecessary) convenience in the stringification below. If there\u0027s another version of this I\u0027ll get rid of it, or can do so in a followup if that seems like a good idea.","commit_id":"5d710c4ac12aedaf84c9c314a60101375799f00c"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"b7509644265699552714e7f0a4b8b38ac90a10df","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        [_ALLOC_TBL.c.resource_class_id.label(\u0027resource_class\u0027)]).where("},{"line_number":57,"context_line":"             sa.and_(_ALLOC_TBL.c.resource_provider_id \u003d\u003d rp.id,"},{"line_number":58,"context_line":"                     _ALLOC_TBL.c.resource_class_id.in_(to_delete))"},{"line_number":59,"context_line":"         ).group_by(_ALLOC_TBL.c.resource_class_id)"},{"line_number":60,"context_line":"    allocations \u003d conn.execute(allocation_query).fetchall()"},{"line_number":61,"context_line":"    if allocations:"},{"line_number":62,"context_line":"        resource_classes \u003d \u0027, \u0027.join([fields.ResourceClass.from_index("}],"source_content_type":"text/x-python","patch_set":36,"id":"3ac371cc_07e8db15","line":59,"range":{"start_line":59,"start_character":10,"end_line":59,"end_character":51},"updated":"2016-08-19 06:04:57.000000000","message":"The query can be more explicit if:\ns/.group_by(_ALLOC_TBL.c.resource_class_id)/.distinct()\n\nBut just a nit :)","commit_id":"cee4348bd53edb09764397c5889db09f72bdc7a2"}],"nova/tests/functional/db/test_resource_provider.py":[{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"c74bc1be0515a38d52a9048511f8b5d3132af872","unresolved":false,"context_lines":[{"line_number":394,"context_line":"            resource_class\u003dfields.ResourceClass.DISK_GB, total\u003d1)"},{"line_number":395,"context_line":"        disk_inv.obj_set_defaults()"},{"line_number":396,"context_line":"        error \u003d self.assertRaises("},{"line_number":397,"context_line":"            exception.InvalidInventoryNewCapacityExceeded,"},{"line_number":398,"context_line":"            rp.update_inventory, disk_inv)"},{"line_number":399,"context_line":"        self.assertIn(\"Invalid inventory for \u0027%s\u0027\""},{"line_number":400,"context_line":"                      % fields.ResourceClass.DISK_GB, str(error))"}],"source_content_type":"text/x-python","patch_set":35,"id":"3ac371cc_2178792b","line":397,"range":{"start_line":397,"start_character":12,"end_line":397,"end_character":57},"updated":"2016-08-18 19:57:09.000000000","message":"Is this not the test you\u0027re looking for Jay?","commit_id":"5d710c4ac12aedaf84c9c314a60101375799f00c"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"c2fee119ea608db64c97e88b48be9bf137e48586","unresolved":false,"context_lines":[{"line_number":394,"context_line":"            resource_class\u003dfields.ResourceClass.DISK_GB, total\u003d1)"},{"line_number":395,"context_line":"        disk_inv.obj_set_defaults()"},{"line_number":396,"context_line":"        error \u003d self.assertRaises("},{"line_number":397,"context_line":"            exception.InvalidInventoryNewCapacityExceeded,"},{"line_number":398,"context_line":"            rp.update_inventory, disk_inv)"},{"line_number":399,"context_line":"        self.assertIn(\"Invalid inventory for \u0027%s\u0027\""},{"line_number":400,"context_line":"                      % fields.ResourceClass.DISK_GB, str(error))"}],"source_content_type":"text/x-python","patch_set":35,"id":"3ac371cc_84a97b27","line":397,"in_reply_to":"3ac371cc_2178792b","updated":"2016-08-18 20:33:47.000000000","message":"That is supposed to be the test Jay is looking for. So unless it somehow doesn\u0027t do what Jay thinks it needs to do I think we\u0027re okay?","commit_id":"5d710c4ac12aedaf84c9c314a60101375799f00c"}]}