)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6bdebb7378b280a183a34a00320776243b954003","unresolved":false,"context_lines":[{"line_number":22,"context_line":"* When set_inventory updates the generation on disk, it seems like"},{"line_number":23,"context_line":"  it should probably be setting rp.generation to the new value and"},{"line_number":24,"context_line":"  resetting the object, otherwise the caller\u0027s picture of the object"},{"line_number":25,"context_line":"  state is no longer accurate."},{"line_number":26,"context_line":"* What should the message associated with ConcurrentUpdateException"},{"line_number":27,"context_line":"  be? Should it subclass to ResourceProviderConcurrent.. and"},{"line_number":28,"context_line":"  Inventory.. and Allocation.. ?"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"7aa08908_709b8f66","line":25,"updated":"2016-06-08 18:40:45.000000000","message":"Yes, good point indeed. ++","commit_id":"6995f8bef5f9830be398c78779e1be0f6b52b43c"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6bdebb7378b280a183a34a00320776243b954003","unresolved":false,"context_lines":[{"line_number":25,"context_line":"  state is no longer accurate."},{"line_number":26,"context_line":"* What should the message associated with ConcurrentUpdateException"},{"line_number":27,"context_line":"  be? Should it subclass to ResourceProviderConcurrent.. and"},{"line_number":28,"context_line":"  Inventory.. and Allocation.. ?"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Partially-Implements: blueprint generic-resource-pools"},{"line_number":31,"context_line":"Change-Id: I2214fa5d176269e65fdcd7a5a7e4958a925235d3"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"7aa08908_f0d5bfe4","line":28,"updated":"2016-06-08 18:40:45.000000000","message":"Meh, not sure there should be a message really. Just means that a concurrent update happened and the caller should retry any logic that it used when initially reading and then updating some value.","commit_id":"6995f8bef5f9830be398c78779e1be0f6b52b43c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b2a46b6c9ebed7e94856b318c60f61c122606101","unresolved":false,"context_lines":[{"line_number":17,"context_line":"A functional test is in place which attempts to exercise all three"},{"line_number":18,"context_line":"of add, delete and update inventory as well as causing a"},{"line_number":19,"context_line":"ConcurrentUpdateException."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Partially-Implements: blueprint generic-resource-pools"},{"line_number":22,"context_line":"Change-Id: I2214fa5d176269e65fdcd7a5a7e4958a925235d3"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"7aa08908_fd0917de","line":20,"updated":"2016-06-14 15:58:35.000000000","message":"Can you add a comment in here about changing the object hash without a version bump because it\u0027s still not used?","commit_id":"716fe675962a51a73551a1fbf49a3569c055e60c"}],"nova/exception.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"710c6f1b6a7924e56e9a50fc7a618085327d52cc","unresolved":false,"context_lines":[{"line_number":2146,"context_line":""},{"line_number":2147,"context_line":""},{"line_number":2148,"context_line":"class ConcurrentUpdateDetected(NovaException):"},{"line_number":2149,"context_line":"    msg_fmt \u003d _(\"Another thread concurrently updated the data. \""},{"line_number":2150,"context_line":"                \"Please retry your update\")"}],"source_content_type":"text/x-python","patch_set":22,"id":"7aa08908_302a2491","line":2149,"updated":"2016-06-16 18:16:43.000000000","message":"Seems we should throw the resource provider name or id or uuid into the error message for context.","commit_id":"3e4497d9cd701e9f4ed8ea6d1ee0a92e24f2598b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"120657de4ea159225acbac91a9add70fc5370a6f","unresolved":false,"context_lines":[{"line_number":2146,"context_line":""},{"line_number":2147,"context_line":""},{"line_number":2148,"context_line":"class ConcurrentUpdateDetected(NovaException):"},{"line_number":2149,"context_line":"    msg_fmt \u003d _(\"Another thread concurrently updated the data. \""},{"line_number":2150,"context_line":"                \"Please retry your update\")"}],"source_content_type":"text/x-python","patch_set":22,"id":"7aa08908_53bedaff","line":2149,"in_reply_to":"7aa08908_302a2491","updated":"2016-06-16 18:20:20.000000000","message":"This exception is returned synchronously to the user making a single update, so I don\u0027t think it\u0027s helpful.","commit_id":"3e4497d9cd701e9f4ed8ea6d1ee0a92e24f2598b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c5650f7507574ec7dabb56bcc84a7457c452e481","unresolved":false,"context_lines":[{"line_number":2146,"context_line":""},{"line_number":2147,"context_line":""},{"line_number":2148,"context_line":"class ConcurrentUpdateDetected(NovaException):"},{"line_number":2149,"context_line":"    msg_fmt \u003d _(\"Another thread concurrently updated the data. \""},{"line_number":2150,"context_line":"                \"Please retry your update\")"}],"source_content_type":"text/x-python","patch_set":22,"id":"7aa08908_d3e64ae7","line":2149,"in_reply_to":"7aa08908_53bedaff","updated":"2016-06-16 18:29:15.000000000","message":"OK, I guess I haven\u0027t looked further up in the series where this is used. But the whole \u0027Please retry your update\u0027 part of it makes it look real user-facing, and for user-facing I\u0027d want some kind of context for the resource that failed.","commit_id":"3e4497d9cd701e9f4ed8ea6d1ee0a92e24f2598b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d12f850cf7103618c2201c388d5c795bfbbe44c2","unresolved":false,"context_lines":[{"line_number":2146,"context_line":""},{"line_number":2147,"context_line":""},{"line_number":2148,"context_line":"class ConcurrentUpdateDetected(NovaException):"},{"line_number":2149,"context_line":"    msg_fmt \u003d _(\"Another thread concurrently updated the data. \""},{"line_number":2150,"context_line":"                \"Please retry your update\")"}],"source_content_type":"text/x-python","patch_set":22,"id":"7aa08908_130c5241","line":2149,"in_reply_to":"7aa08908_d3e64ae7","updated":"2016-06-16 18:33:29.000000000","message":"The user in this case will be a script or client which detects the 409 and refreshes/retries automatically.","commit_id":"3e4497d9cd701e9f4ed8ea6d1ee0a92e24f2598b"}],"nova/objects/resource_provider.py":[{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"b62b81f6545e5ce822736d7b8b98d177ff688c89","unresolved":false,"context_lines":[{"line_number":134,"context_line":"    return res.rowcount"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"@db_api.api_context_manager.writer"},{"line_number":138,"context_line":"def _set_inventory(rp, context, inv_list):"},{"line_number":139,"context_line":"    \"\"\"Given an InventoryList object, replaces the inventory of the"},{"line_number":140,"context_line":"    resource provider in a safe, atomic fashion using the resource"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aa08908_ed1c2fc3","line":137,"updated":"2016-06-07 13:21:25.000000000","message":"The decorator form expects the context to be the very first argument of the function (as it patches it providing new attributes). There\u0027s alaski\u0027s change to make it work for instance and class methods, which have an extra self/cls arguments - https://review.openstack.org/#/c/294283/ , but we haven\u0027t cut a new release yet.\n\nSo you have two options here:\n\n1) change the order of arguments here and pass the context as the first argument\n\n2) use a context manager form  and pass the context instance to it explicitly http://docs.openstack.org/developer/oslo.db/usage.html#session-handling","commit_id":"a1060a335d00f2c646fa6ea1b5ef6cd13f6a9886"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"f7deae442cd2846d9cbed2b7269c920a3ff11653","unresolved":false,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"@db_api.api_context_manager.writer"},{"line_number":138,"context_line":"def _set_inventory(rp, context, inv_list):"},{"line_number":139,"context_line":"    \"\"\"Given an InventoryList object, replaces the inventory of the"},{"line_number":140,"context_line":"    resource provider in a safe, atomic fashion using the resource"},{"line_number":141,"context_line":"    provider\u0027s generation as a consistent view marker."}],"source_content_type":"text/x-python","patch_set":1,"id":"7aa08908_2d7d97e7","line":138,"updated":"2016-06-07 13:16:40.000000000","message":"If I recall correctly, in order for the decorator to be happy, context needs to be the first arg? For more complicated stuff like this I\u0027ve gotten in the habit of using explict context managers:\n\n    with db_api.api_context_manager.writer.using(context):\n\nas it seems easier to trace what\u0027s up.","commit_id":"a1060a335d00f2c646fa6ea1b5ef6cd13f6a9886"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"b62b81f6545e5ce822736d7b8b98d177ff688c89","unresolved":false,"context_lines":[{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    conn \u003d context.session.connection()"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    existing_resources \u003d _get_current_inventory_resources(rp)"},{"line_number":154,"context_line":"    these_resources \u003d set([fields.ResourceClass.index(r.resource_class)"},{"line_number":155,"context_line":"                           for r in inv_list.objects])"},{"line_number":156,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7aa08908_7f5906fe","line":153,"updated":"2016-06-07 13:21:25.000000000","message":"this one expects a connection argument and should probably go into transactional block (so that we select both current generation id and the view of resources)","commit_id":"a1060a335d00f2c646fa6ea1b5ef6cd13f6a9886"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"15dc00ed475fd5cc295ae38737f79ec81c0a0227","unresolved":false,"context_lines":[{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    conn \u003d context.session.connection()"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    existing_resources \u003d _get_current_inventory_resources(rp)"},{"line_number":154,"context_line":"    these_resources \u003d set([fields.ResourceClass.index(r.resource_class)"},{"line_number":155,"context_line":"                           for r in inv_list.objects])"},{"line_number":156,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7aa08908_c2de5d6c","line":153,"in_reply_to":"7aa08908_7f5906fe","updated":"2016-06-07 13:33:42.000000000","message":"On the seconds thought, this can be left outside of the transaction below, but don\u0027t we need to have a consistent view of current generation value + all the resources? Look like right now these are selected separately.","commit_id":"a1060a335d00f2c646fa6ea1b5ef6cd13f6a9886"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"f7deae442cd2846d9cbed2b7269c920a3ff11653","unresolved":false,"context_lines":[{"line_number":159,"context_line":"    # of resource class identifiers."},{"line_number":160,"context_line":"    to_add \u003d these_resources - existing_resources"},{"line_number":161,"context_line":"    to_delete \u003d existing_resources - these_resources"},{"line_number":162,"context_line":"    to_update \u003d these_resources \u0026 existing_resources"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    with conn.begin():"},{"line_number":165,"context_line":"        _delete_inventory_from_provider(conn, rp, to_delete)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aa08908_ad50675f","line":162,"updated":"2016-06-07 13:16:40.000000000","message":"Is this necessary because we want to preserve timestamp metadata? Because otherwise why not just kill and replace wholesale in the transaction?","commit_id":"a1060a335d00f2c646fa6ea1b5ef6cd13f6a9886"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"30ce499075ad3a6873254e8d4fff4e6796e00098","unresolved":false,"context_lines":[{"line_number":155,"context_line":"            in between the time when this object was originally read"},{"line_number":156,"context_line":"            and the call to set the inventory."},{"line_number":157,"context_line":"    \"\"\""},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    conn \u003d context.session.connection()"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"    existing_resources \u003d _get_current_inventory_resources(conn, rp)"}],"source_content_type":"text/x-python","patch_set":14,"id":"7aa08908_018b26c4","line":158,"updated":"2016-06-13 12:38:24.000000000","message":"Super-micro-nit: extra blank line.","commit_id":"9069a5ee7902d6b7385e7582df36863e5d6d1b09"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b477af7a4a81a3d06cbc83629848bcaa0c5d3a5c","unresolved":false,"context_lines":[{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    conn \u003d context.session.connection()"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"    existing_resources \u003d _get_current_inventory_resources(conn, rp)"},{"line_number":162,"context_line":"    these_resources \u003d set([fields.ResourceClass.index(r.resource_class)"},{"line_number":163,"context_line":"                           for r in inv_list.objects])"},{"line_number":164,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7aa08908_0b6fb9fc","line":161,"updated":"2016-06-13 16:00:24.000000000","message":"well, I\u0027m fine with that, but that\u0027s then a bit confusing with db_api.api_context_manager.writer since we read it.\n\nI guess it\u0027s because you want to have all of that behind one single transaction, right?","commit_id":"d8934a918d6a0965d21fc6033b671965d669131d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4526caff1fb8dddfa3b70a6720816519d1993896","unresolved":false,"context_lines":[{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    conn \u003d context.session.connection()"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"    existing_resources \u003d _get_current_inventory_resources(conn, rp)"},{"line_number":162,"context_line":"    these_resources \u003d set([fields.ResourceClass.index(r.resource_class)"},{"line_number":163,"context_line":"                           for r in inv_list.objects])"},{"line_number":164,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7aa08908_78fc2372","line":161,"in_reply_to":"7aa08908_0b6fb9fc","updated":"2016-06-14 13:11:35.000000000","message":"I thought Jay said we\u0027re not doing a merge, we\u0027re setting the inventory to a set. So why not delete all by RP uuid, and then add the ones we\u0027re given here?","commit_id":"d8934a918d6a0965d21fc6033b671965d669131d"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"b6442038578cfe56e1643c1aebf52de62d8c6707","unresolved":false,"context_lines":[{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    conn \u003d context.session.connection()"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"    existing_resources \u003d _get_current_inventory_resources(conn, rp)"},{"line_number":162,"context_line":"    these_resources \u003d set([fields.ResourceClass.index(r.resource_class)"},{"line_number":163,"context_line":"                           for r in inv_list.objects])"},{"line_number":164,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7aa08908_381a9b7d","line":161,"in_reply_to":"7aa08908_78fc2372","updated":"2016-06-14 13:19:34.000000000","message":"Given all the set handling below and the existence of both _add_inventory* and _update_inventory* below that, a merge is the intent.","commit_id":"d8934a918d6a0965d21fc6033b671965d669131d"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b477af7a4a81a3d06cbc83629848bcaa0c5d3a5c","unresolved":false,"context_lines":[{"line_number":167,"context_line":"    # of resource class identifiers."},{"line_number":168,"context_line":"    to_add \u003d these_resources - existing_resources"},{"line_number":169,"context_line":"    to_delete \u003d existing_resources - these_resources"},{"line_number":170,"context_line":"    to_update \u003d these_resources \u0026 existing_resources"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"    with conn.begin():"},{"line_number":173,"context_line":"        if to_delete:"}],"source_content_type":"text/x-python","patch_set":15,"id":"7aa08908_36065c1c","line":170,"updated":"2016-06-13 16:00:24.000000000","message":"kewl logic, thanks Python sets","commit_id":"d8934a918d6a0965d21fc6033b671965d669131d"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b477af7a4a81a3d06cbc83629848bcaa0c5d3a5c","unresolved":false,"context_lines":[{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    # This transaction worked, so this resource provider is in the"},{"line_number":196,"context_line":"    # next generation."},{"line_number":197,"context_line":"    rp.generation \u003d rp.generation + 1"},{"line_number":198,"context_line":"    rp.obj_reset_changes()"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7aa08908_b6b6cc3b","line":197,"updated":"2016-06-13 16:00:24.000000000","message":"why are we not getting the right generation bit from the DB (in the above call), and increase this time in memory ? Couldn\u0027t that be a bit racy ?","commit_id":"d8934a918d6a0965d21fc6033b671965d669131d"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"d538ad69dc5c0cb155afee77b69be037f5f1d0a7","unresolved":false,"context_lines":[{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    # This transaction worked, so this resource provider is in the"},{"line_number":196,"context_line":"    # next generation."},{"line_number":197,"context_line":"    rp.generation \u003d rp.generation + 1"},{"line_number":198,"context_line":"    rp.obj_reset_changes()"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7aa08908_53596c34","line":197,"in_reply_to":"7aa08908_1d57f85a","updated":"2016-06-13 20:57:53.000000000","message":"but we can get the new generation value by returning it wiht _increment_provider_generation() right?","commit_id":"d8934a918d6a0965d21fc6033b671965d669131d"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"eb022bc5272a876db686107176797cd75c9d8948","unresolved":false,"context_lines":[{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    # This transaction worked, so this resource provider is in the"},{"line_number":196,"context_line":"    # next generation."},{"line_number":197,"context_line":"    rp.generation \u003d rp.generation + 1"},{"line_number":198,"context_line":"    rp.obj_reset_changes()"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7aa08908_aa83df01","line":197,"in_reply_to":"7aa08908_53596c34","updated":"2016-06-14 08:48:43.000000000","message":"Why would we want or need to?\n\nThe only field on the resource provider itself that changes in this interaction is the generation (the inventory information is not \"on\" the object) and the only value it can become is what it came in with plus one. There are no other outcomes.\n\nReturning the value from _increment_provider_generation wouldn\u0027t be any different: The generation value it has is \"what it can in with plus one\" not something that was read or returned from the DB.\n\nAlso at the moment there is already a value returned from that method which is being used as a boolean to test the generation increment has worked. If that returns a falsey value, we know what our efforts to do this transaction have failed and an exception is raised.","commit_id":"d8934a918d6a0965d21fc6033b671965d669131d"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"b6442038578cfe56e1643c1aebf52de62d8c6707","unresolved":false,"context_lines":[{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    # This transaction worked, so this resource provider is in the"},{"line_number":196,"context_line":"    # next generation."},{"line_number":197,"context_line":"    rp.generation \u003d rp.generation + 1"},{"line_number":198,"context_line":"    rp.obj_reset_changes()"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7aa08908_13798093","line":197,"in_reply_to":"7aa08908_78aee381","updated":"2016-06-14 13:19:34.000000000","message":"I\u0027m happy to change it, but I want to understand the confusion: as far as I understand thing here there can only be one value for rp.generation on success: the next one. Whether it comes from the db (assuming only the calls that have happened in the transaction) or not.","commit_id":"d8934a918d6a0965d21fc6033b671965d669131d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4526caff1fb8dddfa3b70a6720816519d1993896","unresolved":false,"context_lines":[{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    # This transaction worked, so this resource provider is in the"},{"line_number":196,"context_line":"    # next generation."},{"line_number":197,"context_line":"    rp.generation \u003d rp.generation + 1"},{"line_number":198,"context_line":"    rp.obj_reset_changes()"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7aa08908_78aee381","line":197,"in_reply_to":"7aa08908_aa83df01","updated":"2016-06-14 13:11:35.000000000","message":"As we\u0027re discussing in IRC right now, I\u0027d much prefer if we return the new value of generation from the _increment_provider_generation() method so that it\u0027s clear (and accurate) that it\u0027s what is in the DB.","commit_id":"d8934a918d6a0965d21fc6033b671965d669131d"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"4089df6ad44d4116e7f4203b203b40edbf74b26a","unresolved":false,"context_lines":[{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    # This transaction worked, so this resource provider is in the"},{"line_number":196,"context_line":"    # next generation."},{"line_number":197,"context_line":"    rp.generation \u003d rp.generation + 1"},{"line_number":198,"context_line":"    rp.obj_reset_changes()"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7aa08908_1d57f85a","line":197,"in_reply_to":"7aa08908_b6b6cc3b","updated":"2016-06-13 19:06:59.000000000","message":"Basically because we don\u0027t want to read the database again and we\u0027ve got no existing database object to interrogate here and it doesn\u0027t matter. We don\u0027t need to persist it, because it already has been (if things worked), and we want the caller to have a good view.\n\nIf there have been any problems with the change we\u0027ve exited this code via exception already.","commit_id":"d8934a918d6a0965d21fc6033b671965d669131d"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b477af7a4a81a3d06cbc83629848bcaa0c5d3a5c","unresolved":false,"context_lines":[{"line_number":195,"context_line":"    # This transaction worked, so this resource provider is in the"},{"line_number":196,"context_line":"    # next generation."},{"line_number":197,"context_line":"    rp.generation \u003d rp.generation + 1"},{"line_number":198,"context_line":"    rp.obj_reset_changes()"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"@base.NovaObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":15,"id":"7aa08908_3655bcc1","line":198,"updated":"2016-06-13 16:00:24.000000000","message":"see, I guess it\u0027s because you don\u0027t want to persist the new generation field you just modified previously, right?","commit_id":"d8934a918d6a0965d21fc6033b671965d669131d"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"eb022bc5272a876db686107176797cd75c9d8948","unresolved":false,"context_lines":[{"line_number":195,"context_line":"    # This transaction worked, so this resource provider is in the"},{"line_number":196,"context_line":"    # next generation."},{"line_number":197,"context_line":"    rp.generation \u003d rp.generation + 1"},{"line_number":198,"context_line":"    rp.obj_reset_changes()"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"@base.NovaObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":15,"id":"7aa08908_aab11fe2","line":198,"in_reply_to":"7aa08908_3655bcc1","updated":"2016-06-14 08:48:43.000000000","message":"The new generation, if the incoming generation matched what was in the db, was persisted at line 136.","commit_id":"d8934a918d6a0965d21fc6033b671965d669131d"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b477af7a4a81a3d06cbc83629848bcaa0c5d3a5c","unresolved":false,"context_lines":[{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    @base.remotable"},{"line_number":243,"context_line":"    def set_inventory(self, inv_list):"},{"line_number":244,"context_line":"        _set_inventory(self._context, self, inv_list)"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"    @staticmethod"},{"line_number":247,"context_line":"    def _create_in_db(context, updates):"}],"source_content_type":"text/x-python","patch_set":15,"id":"7aa08908_2838bbed","line":244,"updated":"2016-06-13 16:00:24.000000000","message":"nit: we no longer need to duplicate those methods into private module helper methods since the oslo related bug is now fixed https://bugs.launchpad.net/oslo.db/+bug/1520195","commit_id":"d8934a918d6a0965d21fc6033b671965d669131d"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"4089df6ad44d4116e7f4203b203b40edbf74b26a","unresolved":false,"context_lines":[{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    @base.remotable"},{"line_number":243,"context_line":"    def set_inventory(self, inv_list):"},{"line_number":244,"context_line":"        _set_inventory(self._context, self, inv_list)"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"    @staticmethod"},{"line_number":247,"context_line":"    def _create_in_db(context, updates):"}],"source_content_type":"text/x-python","patch_set":15,"id":"7aa08908_7ddf1cd6","line":244,"in_reply_to":"7aa08908_2838bbed","updated":"2016-06-13 19:06:59.000000000","message":"I tend to find the code more readable when it is extracted like that (module methods more comprehensible than class-level staticmethods). Also we don\u0027t want to be giving anyone the impression here that these objects should be subclassed.","commit_id":"d8934a918d6a0965d21fc6033b671965d669131d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4526caff1fb8dddfa3b70a6720816519d1993896","unresolved":false,"context_lines":[{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    @base.remotable"},{"line_number":243,"context_line":"    def set_inventory(self, inv_list):"},{"line_number":244,"context_line":"        _set_inventory(self._context, self, inv_list)"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"    @staticmethod"},{"line_number":247,"context_line":"    def _create_in_db(context, updates):"}],"source_content_type":"text/x-python","patch_set":15,"id":"7aa08908_f89db366","line":244,"in_reply_to":"7aa08908_7ddf1cd6","updated":"2016-06-14 13:11:35.000000000","message":"Duplicating them as module-level methods is less desirable, IMHO, we just had to do it before because of a bug.\n\nAlso, can\u0027t this raise an exception on transaction commit? If we\u0027re racing to set the same generation as someone else, won\u0027t we not know it until the transaction commits? IIUC, that happens when writer() returns.","commit_id":"d8934a918d6a0965d21fc6033b671965d669131d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b2a46b6c9ebed7e94856b318c60f61c122606101","unresolved":false,"context_lines":[{"line_number":191,"context_line":"        # conditions and re-reading the existing inventory information."},{"line_number":192,"context_line":"        rp.generation \u003d _increment_provider_generation(conn, rp)"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"    rp.obj_reset_changes()"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"@base.NovaObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":17,"id":"7aa08908_1d9d03bc","line":194,"updated":"2016-06-14 15:58:35.000000000","message":"This really needs to be in the object method I think. It\u0027s weird to have something else marking an object as clean (even though it\u0027s like an inner function). Of course, I think all of this belongs in the instance method itself, but...","commit_id":"716fe675962a51a73551a1fbf49a3569c055e60c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"710c6f1b6a7924e56e9a50fc7a618085327d52cc","unresolved":false,"context_lines":[{"line_number":72,"context_line":"    :param to_delete: set() containing resource class IDs for records to"},{"line_number":73,"context_line":"                      delete."},{"line_number":74,"context_line":"    \"\"\""},{"line_number":75,"context_line":"    del_stmt \u003d _INV_TBL.delete().where(sa.and_("},{"line_number":76,"context_line":"            _INV_TBL.c.resource_provider_id \u003d\u003d rp.id,"},{"line_number":77,"context_line":"            _INV_TBL.c.resource_class_id.in_(to_delete)))"},{"line_number":78,"context_line":"    conn.execute(del_stmt)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"def _add_inventory_to_provider(conn, rp, inv_list, to_add):"}],"source_content_type":"text/x-python","patch_set":22,"id":"7aa08908_903678b2","line":78,"range":{"start_line":75,"start_character":4,"end_line":78,"end_character":26},"updated":"2016-06-16 18:16:43.000000000","message":"At first I wondered why this is not happening in the Inventory object. I guess this is faster to just do it directly than load up all of the Inventory objects by resource provider and class and then delete them one by one.\n\nSince we\u0027re in a time sensitive block we want this to be as efficient as possible.","commit_id":"3e4497d9cd701e9f4ed8ea6d1ee0a92e24f2598b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"710c6f1b6a7924e56e9a50fc7a618085327d52cc","unresolved":false,"context_lines":[{"line_number":89,"context_line":"    \"\"\""},{"line_number":90,"context_line":"    for res_class in to_add:"},{"line_number":91,"context_line":"        inv_record \u003d inv_list.find(res_class)"},{"line_number":92,"context_line":"        ins_stmt \u003d _INV_TBL.insert().values("},{"line_number":93,"context_line":"                resource_provider_id\u003drp.id,"},{"line_number":94,"context_line":"                resource_class_id\u003dres_class,"},{"line_number":95,"context_line":"                total\u003dinv_record.total,"},{"line_number":96,"context_line":"                reserved\u003dinv_record.reserved,"},{"line_number":97,"context_line":"                min_unit\u003dinv_record.min_unit,"},{"line_number":98,"context_line":"                max_unit\u003dinv_record.max_unit,"},{"line_number":99,"context_line":"                step_size\u003dinv_record.step_size,"},{"line_number":100,"context_line":"                allocation_ratio\u003dinv_record.allocation_ratio)"},{"line_number":101,"context_line":"        conn.execute(ins_stmt)"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"def _update_inventory_for_provider(conn, rp, inv_list, to_update):"}],"source_content_type":"text/x-python","patch_set":22,"id":"7aa08908_d0592090","line":101,"range":{"start_line":92,"start_character":8,"end_line":101,"end_character":30},"updated":"2016-06-16 18:16:43.000000000","message":"Why not create an Inventory object and call create() on it? Because we want to use the same DB session connection I suppose?","commit_id":"3e4497d9cd701e9f4ed8ea6d1ee0a92e24f2598b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"710c6f1b6a7924e56e9a50fc7a618085327d52cc","unresolved":false,"context_lines":[{"line_number":105,"context_line":"    \"\"\"Updates existing inventory records for the supplied resource provider."},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    :param conn: DB connection to use."},{"line_number":108,"context_line":"    :param rp: Resource provider to add inventory to."},{"line_number":109,"context_line":"    :param inv_list: InventoryList object"},{"line_number":110,"context_line":"    :param to_update: set() containing resource class IDs to search inv_list"},{"line_number":111,"context_line":"                      for updating in resource provider."}],"source_content_type":"text/x-python","patch_set":22,"id":"7aa08908_90c67814","line":108,"range":{"start_line":108,"start_character":36,"end_line":108,"end_character":52},"updated":"2016-06-16 18:16:43.000000000","message":"update inventory on, or on which to update inventory, if you care about prepositions. :)","commit_id":"3e4497d9cd701e9f4ed8ea6d1ee0a92e24f2598b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"710c6f1b6a7924e56e9a50fc7a618085327d52cc","unresolved":false,"context_lines":[{"line_number":115,"context_line":"        upd_stmt \u003d _INV_TBL.update().where(sa.and_("},{"line_number":116,"context_line":"                _INV_TBL.c.resource_provider_id \u003d\u003d rp.id,"},{"line_number":117,"context_line":"                _INV_TBL.c.resource_class_id \u003d\u003d res_class)).values("},{"line_number":118,"context_line":"                        total\u003dinv_record.total,"},{"line_number":119,"context_line":"                        reserved\u003dinv_record.reserved,"},{"line_number":120,"context_line":"                        min_unit\u003dinv_record.min_unit,"},{"line_number":121,"context_line":"                        max_unit\u003dinv_record.max_unit,"}],"source_content_type":"text/x-python","patch_set":22,"id":"7aa08908_308ea4d0","line":118,"range":{"start_line":118,"start_character":41,"end_line":118,"end_character":46},"updated":"2016-06-16 18:16:43.000000000","message":"Will this always be set?","commit_id":"3e4497d9cd701e9f4ed8ea6d1ee0a92e24f2598b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"120657de4ea159225acbac91a9add70fc5370a6f","unresolved":false,"context_lines":[{"line_number":115,"context_line":"        upd_stmt \u003d _INV_TBL.update().where(sa.and_("},{"line_number":116,"context_line":"                _INV_TBL.c.resource_provider_id \u003d\u003d rp.id,"},{"line_number":117,"context_line":"                _INV_TBL.c.resource_class_id \u003d\u003d res_class)).values("},{"line_number":118,"context_line":"                        total\u003dinv_record.total,"},{"line_number":119,"context_line":"                        reserved\u003dinv_record.reserved,"},{"line_number":120,"context_line":"                        min_unit\u003dinv_record.min_unit,"},{"line_number":121,"context_line":"                        max_unit\u003dinv_record.max_unit,"}],"source_content_type":"text/x-python","patch_set":22,"id":"7aa08908_932b2233","line":118,"range":{"start_line":118,"start_character":41,"end_line":118,"end_character":46},"in_reply_to":"7aa08908_308ea4d0","updated":"2016-06-16 18:20:20.000000000","message":"Yes.","commit_id":"3e4497d9cd701e9f4ed8ea6d1ee0a92e24f2598b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"710c6f1b6a7924e56e9a50fc7a618085327d52cc","unresolved":false,"context_lines":[{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    :param conn: DB connection to use."},{"line_number":132,"context_line":"    :param rp: `ResourceProvider` whose generation should be updated."},{"line_number":133,"context_line":"    :returns True if the generation was incremented, False otherwise."},{"line_number":134,"context_line":"    \"\"\""},{"line_number":135,"context_line":"    rp_gen \u003d rp.generation"},{"line_number":136,"context_line":"    new_generation \u003d rp_gen + 1"}],"source_content_type":"text/x-python","patch_set":22,"id":"7aa08908_704cac9d","line":133,"updated":"2016-06-16 18:16:43.000000000","message":"Not exactly, it returns the new generation ID, else it raises ConcurrentUpdateDetected.","commit_id":"3e4497d9cd701e9f4ed8ea6d1ee0a92e24f2598b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"120657de4ea159225acbac91a9add70fc5370a6f","unresolved":false,"context_lines":[{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    :param conn: DB connection to use."},{"line_number":132,"context_line":"    :param rp: `ResourceProvider` whose generation should be updated."},{"line_number":133,"context_line":"    :returns True if the generation was incremented, False otherwise."},{"line_number":134,"context_line":"    \"\"\""},{"line_number":135,"context_line":"    rp_gen \u003d rp.generation"},{"line_number":136,"context_line":"    new_generation \u003d rp_gen + 1"}],"source_content_type":"text/x-python","patch_set":22,"id":"7aa08908_f31a6617","line":133,"in_reply_to":"7aa08908_704cac9d","updated":"2016-06-16 18:20:20.000000000","message":"Ah yep, this is leftover from a previous round.","commit_id":"3e4497d9cd701e9f4ed8ea6d1ee0a92e24f2598b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"710c6f1b6a7924e56e9a50fc7a618085327d52cc","unresolved":false,"context_lines":[{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    res \u003d conn.execute(upd_stmt)"},{"line_number":143,"context_line":"    if res.rowcount !\u003d 1:"},{"line_number":144,"context_line":"        raise exception.ConcurrentUpdateDetected"},{"line_number":145,"context_line":"    return new_generation"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"7aa08908_73015604","line":144,"range":{"start_line":144,"start_character":24,"end_line":144,"end_character":48},"updated":"2016-06-16 18:16:43.000000000","message":"This is just the class type isn\u0027t it? Don\u0027t you need to raise ConcurrentUpdateDetected() so it\u0027s an instance of the exception?\n\nWe should also probably put the name or id or uuid of the resource provider in the error message for context.\n\n-1 for this.","commit_id":"3e4497d9cd701e9f4ed8ea6d1ee0a92e24f2598b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"120657de4ea159225acbac91a9add70fc5370a6f","unresolved":false,"context_lines":[{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    res \u003d conn.execute(upd_stmt)"},{"line_number":143,"context_line":"    if res.rowcount !\u003d 1:"},{"line_number":144,"context_line":"        raise exception.ConcurrentUpdateDetected"},{"line_number":145,"context_line":"    return new_generation"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"7aa08908_33150ee5","line":144,"range":{"start_line":144,"start_character":24,"end_line":144,"end_character":48},"in_reply_to":"7aa08908_73015604","updated":"2016-06-16 18:20:20.000000000","message":"It\u0027s legit to do this in py2 and py3. I don\u0027t think the id needs to be in there, but whatever.","commit_id":"3e4497d9cd701e9f4ed8ea6d1ee0a92e24f2598b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"710c6f1b6a7924e56e9a50fc7a618085327d52cc","unresolved":false,"context_lines":[{"line_number":146,"context_line":""},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"@db_api.api_context_manager.writer"},{"line_number":149,"context_line":"def _set_inventory(context, rp, inv_list):"},{"line_number":150,"context_line":"    \"\"\"Given an InventoryList object, replaces the inventory of the"},{"line_number":151,"context_line":"    resource provider in a safe, atomic fashion using the resource"},{"line_number":152,"context_line":"    provider\u0027s generation as a consistent view marker."}],"source_content_type":"text/x-python","patch_set":22,"id":"7aa08908_cddce948","line":149,"range":{"start_line":149,"start_character":19,"end_line":149,"end_character":26},"updated":"2016-06-16 18:16:43.000000000","message":"nit: missing a param docstring for this.","commit_id":"3e4497d9cd701e9f4ed8ea6d1ee0a92e24f2598b"}],"nova/tests/functional/db/test_resource_provider.py":[{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"30ce499075ad3a6873254e8d4fff4e6796e00098","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        # fail when generation wrong"},{"line_number":208,"context_line":"        rp.generation \u003d 0"},{"line_number":209,"context_line":"        self.assertRaises(exception.ConcurrentUpdateDetected,"},{"line_number":210,"context_line":"                          rp.set_inventory, inv_list)"}],"source_content_type":"text/x-python","patch_set":14,"id":"7aa08908_2998c8bf","line":210,"updated":"2016-06-13 12:38:24.000000000","message":"Should we also test that the inventory in the database is unchanged? Or are we just assuming that SA is working as advertised?","commit_id":"9069a5ee7902d6b7385e7582df36863e5d6d1b09"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b2a46b6c9ebed7e94856b318c60f61c122606101","unresolved":false,"context_lines":[{"line_number":205,"context_line":"        self.assertEqual(2048, new_inv_list[0].total)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"        # fail when generation wrong"},{"line_number":208,"context_line":"        rp.generation \u003d 0"},{"line_number":209,"context_line":"        self.assertRaises(exception.ConcurrentUpdateDetected,"},{"line_number":210,"context_line":"                          rp.set_inventory, inv_list)"}],"source_content_type":"text/x-python","patch_set":17,"id":"7aa08908_ddbc1b08","line":208,"updated":"2016-06-14 15:58:35.000000000","message":"I think that we should be checking that the generation is incrementing in the above cases, and then set it back one to test the conflict. Setting it to zero is \"too special\" I think.","commit_id":"716fe675962a51a73551a1fbf49a3569c055e60c"}]}