)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"ed05a4e67304dc81cc5d4fdfdd37d79807849389","unresolved":false,"context_lines":[{"line_number":57,"context_line":"automatically set that provider\u0027s root provider UUID to the root of the"},{"line_number":58,"context_line":"parent provider."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"It\u0027s important to point out that qualitative trait information is only"},{"line_number":61,"context_line":"associated with a resource provider entity, not the resources that"},{"line_number":62,"context_line":"resource provider has in its inventory. This is the reason why nested"},{"line_number":63,"context_line":"resource providers are necessary. In the case of things like NUMA nodes"},{"line_number":64,"context_line":"or SRIOV physical functions, if a compute host had multiple SRIOV"},{"line_number":65,"context_line":"physical functions, each associated with a different network trait,"},{"line_number":66,"context_line":"there would be no way to differentiate between the SRIOV_NET_VF"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"fa6399be_f975b063","line":63,"range":{"start_line":60,"start_character":0,"end_line":63,"end_character":33},"updated":"2016-10-14 15:42:00.000000000","message":"Yeah, so this at least gets to the issue. Is there any reason there couldn\u0027t be traits (or something else like metadata/tags) on the inventory that further qualifies it beyond simply being of a particular resource class? In your example, couldn\u0027t you have two SRIOV_NET_VF inventories, each marked with the specific network they are attached to? Sure, it might complicate some calculations, but not nearly as much as providing multiple layers of rolling up that nested resource providers will require.\n\nAll I\u0027m saying is that it seems that certain design decisions we made look like they might not have been optimal. Instead of treating them as set in stone and working around them with constructions such as this, why not consider revisiting them before the placement code is actually used, and thus much harder to change?","commit_id":"a16e6b0be947f49d85f8d35ea57553e9fba2a210"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"35d38156936e9c18ad84babc3cda6b296e79d8a6","unresolved":false,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":"The root_provider_uuid field allows us to do extremely quick lookups of"},{"line_number":55,"context_line":"an entire tree of providers without needing to perform any recursive"},{"line_number":56,"context_line":"database queries."},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"Logic in this patch ensures that no resource provider can be deleted if"},{"line_number":59,"context_line":"any of its children have any allocations active on them. We also check"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"5a74a57a_bb5594c7","line":56,"updated":"2016-11-23 13:14:32.000000000","message":"How big do we expect these trees to get?","commit_id":"6a345da4d9ee85b280ca0c4d4ee276d5be4fc62a"}],"nova/db/sqlalchemy/api_migrations/migrate_repo/versions/045_nested_resource_providers.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fb32fad296005c4c73bbe20193c98bc41e37a5e3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":40,"id":"5f201791_467b25f4","updated":"2017-06-26 14:32:04.000000000","message":"Forgive any ignorance of this as I walk through this","commit_id":"63577e9677654d1ee22b21e44d456eaec272187a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fb32fad296005c4c73bbe20193c98bc41e37a5e3","unresolved":false,"context_lines":[{"line_number":24,"context_line":"    resource_providers \u003d Table(\u0027resource_providers\u0027, meta, autoload\u003dTrue)"},{"line_number":25,"context_line":"    columns_to_add \u003d ["},{"line_number":26,"context_line":"            (\u0027root_provider_uuid\u0027,"},{"line_number":27,"context_line":"                Column(\u0027root_provider_uuid\u0027, String(length\u003d36))),"},{"line_number":28,"context_line":"            (\u0027parent_provider_uuid\u0027,"},{"line_number":29,"context_line":"                Column(\u0027parent_provider_uuid\u0027, String(length\u003d36))),"},{"line_number":30,"context_line":"    ]"}],"source_content_type":"text/x-python","patch_set":40,"id":"5f201791_664aa1ac","line":27,"range":{"start_line":27,"start_character":16,"end_line":27,"end_character":63},"updated":"2017-06-26 14:32:04.000000000","message":"How come we don\u0027t set the nullable attribute here. Is that not necessary?","commit_id":"63577e9677654d1ee22b21e44d456eaec272187a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fb32fad296005c4c73bbe20193c98bc41e37a5e3","unresolved":false,"context_lines":[{"line_number":32,"context_line":"        if not hasattr(resource_providers.c, col_name):"},{"line_number":33,"context_line":"            resource_providers.create_column(column)"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    indexed_columns \u003d set()"},{"line_number":36,"context_line":"    for idx in resource_providers.indexes:"},{"line_number":37,"context_line":"        for c in idx.columns:"},{"line_number":38,"context_line":"            indexed_columns.add(c.name)"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    if \u0027root_provider_uuid\u0027 not in indexed_columns:"},{"line_number":41,"context_line":"        index \u003d Index(\u0027resource_providers_root_provider_uuid_idx\u0027,"}],"source_content_type":"text/x-python","patch_set":40,"id":"5f201791_261b89d0","line":38,"range":{"start_line":35,"start_character":0,"end_line":38,"end_character":39},"updated":"2017-06-26 14:32:04.000000000","message":"nit: Might be a good application of set comprehension, but this is readable enough as it is","commit_id":"63577e9677654d1ee22b21e44d456eaec272187a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fb32fad296005c4c73bbe20193c98bc41e37a5e3","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    if \u0027root_provider_uuid\u0027 not in indexed_columns:"},{"line_number":41,"context_line":"        index \u003d Index(\u0027resource_providers_root_provider_uuid_idx\u0027,"},{"line_number":42,"context_line":"                resource_providers.c.root_provider_uuid)"},{"line_number":43,"context_line":"        index.create()"},{"line_number":44,"context_line":"    if \u0027parent_provider_uuid\u0027 not in indexed_columns:"},{"line_number":45,"context_line":"        index \u003d Index(\u0027resource_providers_parent_provider_uuid_idx\u0027,"}],"source_content_type":"text/x-python","patch_set":40,"id":"5f201791_4608c590","line":42,"updated":"2017-06-26 14:32:04.000000000","message":"femtonit: missing indentation","commit_id":"63577e9677654d1ee22b21e44d456eaec272187a"}],"nova/db/sqlalchemy/api_migrations/migrate_repo/versions/050_nested_resource_providers.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1af071df985a8b143dbb7f02e04803677dc17780","unresolved":false,"context_lines":[{"line_number":24,"context_line":"    resource_providers \u003d Table(\u0027resource_providers\u0027, meta, autoload\u003dTrue)"},{"line_number":25,"context_line":"    columns_to_add \u003d ["},{"line_number":26,"context_line":"            (\u0027root_provider_uuid\u0027,"},{"line_number":27,"context_line":"                Column(\u0027root_provider_uuid\u0027, String(length\u003d36))),"},{"line_number":28,"context_line":"            (\u0027parent_provider_uuid\u0027,"},{"line_number":29,"context_line":"                Column(\u0027parent_provider_uuid\u0027, String(length\u003d36))),"},{"line_number":30,"context_line":"    ]"}],"source_content_type":"text/x-python","patch_set":44,"id":"7f515b1d_d2a227ae","line":27,"updated":"2017-09-28 17:13:22.000000000","message":"I get the desire to optimize the process of finding the root of a given tree, but I worry that this may limit us in the future when we need to restructure a hierarchy of providers. It shouldn\u0027t happen with compute nodes, but may for something like moving a disk shelf from one NAS device to another.\n\nI\u0027m being told in IRC that the perf benefit is important. So, I leave this here for later when we have to have some explicit restructure code to reparent a tree.","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f14c80a0565ae425bd7fab831882172ea540fd9d","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    columns_to_add \u003d ["},{"line_number":26,"context_line":"            (\u0027root_provider_uuid\u0027,"},{"line_number":27,"context_line":"                Column(\u0027root_provider_uuid\u0027, String(length\u003d36))),"},{"line_number":28,"context_line":"            (\u0027parent_provider_uuid\u0027,"},{"line_number":29,"context_line":"                Column(\u0027parent_provider_uuid\u0027, String(length\u003d36))),"},{"line_number":30,"context_line":"    ]"},{"line_number":31,"context_line":"    for col_name, column in columns_to_add:"}],"source_content_type":"text/x-python","patch_set":44,"id":"7f515b1d_32205b48","line":28,"updated":"2017-09-28 17:08:59.000000000","message":"Why is this not an id link instead of uuid? I thought we were going for high join performance with all this stuff and using ids for links?","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"8fd1d7864a37562122c1331eddb9715b6292dba7","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    columns_to_add \u003d ["},{"line_number":26,"context_line":"            (\u0027root_provider_uuid\u0027,"},{"line_number":27,"context_line":"                Column(\u0027root_provider_uuid\u0027, String(length\u003d36))),"},{"line_number":28,"context_line":"            (\u0027parent_provider_uuid\u0027,"},{"line_number":29,"context_line":"                Column(\u0027parent_provider_uuid\u0027, String(length\u003d36))),"},{"line_number":30,"context_line":"    ]"},{"line_number":31,"context_line":"    for col_name, column in columns_to_add:"}],"source_content_type":"text/x-python","patch_set":44,"id":"7f515b1d_92814ffe","line":28,"in_reply_to":"7f515b1d_32205b48","updated":"2017-09-28 17:15:20.000000000","message":"an excellent point. will change this to be ids, not uuids.","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"484b2fe61ad05e3de091f756c01857b3769d10ca","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    columns_to_add \u003d ["},{"line_number":26,"context_line":"            (\u0027root_provider_uuid\u0027,"},{"line_number":27,"context_line":"                Column(\u0027root_provider_uuid\u0027, String(length\u003d36))),"},{"line_number":28,"context_line":"            (\u0027parent_provider_uuid\u0027,"},{"line_number":29,"context_line":"                Column(\u0027parent_provider_uuid\u0027, String(length\u003d36))),"},{"line_number":30,"context_line":"    ]"},{"line_number":31,"context_line":"    for col_name, column in columns_to_add:"}],"source_content_type":"text/x-python","patch_set":44,"id":"7f515b1d_c78d5532","line":28,"in_reply_to":"7f515b1d_8558c030","updated":"2017-10-04 19:26:09.000000000","message":"Best article describing the issue with using UUIDs as PKs in MySQL/InnoDB is here:\n\nhttps://www.percona.com/blog/2015/04/03/illustrating-primary-key-models-in-innodb-and-their-impact-on-disk-usage/\n\nThat said, the point is moot. We already have an integer PK on the resource_providers table. The secondary indexes being added here should be on the PK, not another secondary index. :)","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ed707f71509b4a75313a6265c677a91587dedb7e","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    columns_to_add \u003d ["},{"line_number":26,"context_line":"            (\u0027root_provider_uuid\u0027,"},{"line_number":27,"context_line":"                Column(\u0027root_provider_uuid\u0027, String(length\u003d36))),"},{"line_number":28,"context_line":"            (\u0027parent_provider_uuid\u0027,"},{"line_number":29,"context_line":"                Column(\u0027parent_provider_uuid\u0027, String(length\u003d36))),"},{"line_number":30,"context_line":"    ]"},{"line_number":31,"context_line":"    for col_name, column in columns_to_add:"}],"source_content_type":"text/x-python","patch_set":44,"id":"7f515b1d_2f25a5c2","line":28,"in_reply_to":"7f515b1d_908c0ed8","updated":"2017-10-09 17:36:04.000000000","message":"Chris, I don\u0027t have the data to answer your questions. I\u0027ll try to put together some time over the next couple weeks to run some comprehensive benchmarks to show the difference in performance of the various queries involved.","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"f597e6b8834b13df7b13eb00cda11455122a5393","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    columns_to_add \u003d ["},{"line_number":26,"context_line":"            (\u0027root_provider_uuid\u0027,"},{"line_number":27,"context_line":"                Column(\u0027root_provider_uuid\u0027, String(length\u003d36))),"},{"line_number":28,"context_line":"            (\u0027parent_provider_uuid\u0027,"},{"line_number":29,"context_line":"                Column(\u0027parent_provider_uuid\u0027, String(length\u003d36))),"},{"line_number":30,"context_line":"    ]"},{"line_number":31,"context_line":"    for col_name, column in columns_to_add:"}],"source_content_type":"text/x-python","patch_set":44,"id":"7f515b1d_8558c030","line":28,"in_reply_to":"7f515b1d_92814ffe","updated":"2017-10-01 14:45:27.000000000","message":"This is in response to both comments in this file.\n\nDo we have any measurements that indicate that join performance or tree searches are appreciably more damaging to the overall performance of satisfying an /allocation_candidates or /resource_providers request than say OVO creation overhead, JSON serialization, HTTP socket handling, or client side handling?\n\nWe got a lot of, and keep introducing more, complexity at the object/data level that makes future maintenance of the resource_providers.py a challenge for everyone, much of it around the fundamental acceptance that we must do our joins on an int. I get that that is the ideal, but how much does it really matter?","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"0d57a5985c55f984d29e18900885e78f2c3f536c","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    columns_to_add \u003d ["},{"line_number":26,"context_line":"            (\u0027root_provider_uuid\u0027,"},{"line_number":27,"context_line":"                Column(\u0027root_provider_uuid\u0027, String(length\u003d36))),"},{"line_number":28,"context_line":"            (\u0027parent_provider_uuid\u0027,"},{"line_number":29,"context_line":"                Column(\u0027parent_provider_uuid\u0027, String(length\u003d36))),"},{"line_number":30,"context_line":"    ]"},{"line_number":31,"context_line":"    for col_name, column in columns_to_add:"}],"source_content_type":"text/x-python","patch_set":44,"id":"7f515b1d_908c0ed8","line":28,"in_reply_to":"7f515b1d_c78d5532","updated":"2017-10-05 19:58:29.000000000","message":"First off, yes, it\u0027s moot in this case, so consider this more of a general exercise on the topic, not something that\u0027s all that relevant for this stack of code, especially since no-orm stack is changing all this eventually. But for the sake of responding and discussion etc:\n\n* I\u0027ve read that article a few times, this isn\u0027t the first time it\u0027s come up\n* That doesn\u0027t answer either of my two questions (what is the impact of db performance relative to other concerns? and how much does it really matter)\n* I\u0027d be surprised if, in our small databases, that file size is a big deal? Or to put it another way: what numbers do we have on innodb file size?\n\nOr to put it more simply, I asked some questions, and it seems to me that you answered something else. If you don\u0027t know the answers (I certainly don\u0027t because I\u0027ve not had the opportunity) that\u0027s cool, just say so and we can move on (we can move on anyway, I\u0027m just making conversation).","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9b5888ab00d1776a29223c16ad919ae62f2773e3","unresolved":false,"context_lines":[{"line_number":28,"context_line":"            (\u0027parent_provider_uuid\u0027,"},{"line_number":29,"context_line":"                Column(\u0027parent_provider_uuid\u0027, String(length\u003d36))),"},{"line_number":30,"context_line":"    ]"},{"line_number":31,"context_line":"    for col_name, column in columns_to_add:"},{"line_number":32,"context_line":"        if not hasattr(resource_providers.c, col_name):"},{"line_number":33,"context_line":"            resource_providers.create_column(column)"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    indexed_columns \u003d set()"},{"line_number":36,"context_line":"    for idx in resource_providers.indexes:"}],"source_content_type":"text/x-python","patch_set":44,"id":"7f515b1d_7368ae20","line":33,"range":{"start_line":31,"start_character":0,"end_line":33,"end_character":52},"updated":"2017-10-04 15:46:40.000000000","message":"Wouldn\u0027t this be the place to populate the root_provider_id of existing RPs with their own UUID?","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"484b2fe61ad05e3de091f756c01857b3769d10ca","unresolved":false,"context_lines":[{"line_number":28,"context_line":"            (\u0027parent_provider_uuid\u0027,"},{"line_number":29,"context_line":"                Column(\u0027parent_provider_uuid\u0027, String(length\u003d36))),"},{"line_number":30,"context_line":"    ]"},{"line_number":31,"context_line":"    for col_name, column in columns_to_add:"},{"line_number":32,"context_line":"        if not hasattr(resource_providers.c, col_name):"},{"line_number":33,"context_line":"            resource_providers.create_column(column)"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    indexed_columns \u003d set()"},{"line_number":36,"context_line":"    for idx in resource_providers.indexes:"}],"source_content_type":"text/x-python","patch_set":44,"id":"7f515b1d_87875d10","line":33,"range":{"start_line":31,"start_character":0,"end_line":33,"end_character":52},"in_reply_to":"7f515b1d_7368ae20","updated":"2017-10-04 19:26:09.000000000","message":"It would, yes. However we don\u0027t do data migrations in schema migrations.","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"}],"nova/db/sqlalchemy/api_models.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a5803e80eab68266e07f1fb68ee8a9a5083fa086","unresolved":false,"context_lines":[{"line_number":304,"context_line":"    uuid \u003d Column(String(36), nullable\u003dFalse)"},{"line_number":305,"context_line":"    name \u003d Column(Unicode(200), nullable\u003dTrue)"},{"line_number":306,"context_line":"    generation \u003d Column(Integer, default\u003d0)"},{"line_number":307,"context_line":"    root_provider_uuid \u003d Column(String(36), nullable\u003dTrue)"},{"line_number":308,"context_line":"    parent_provider_uuid \u003d Column(String(36), nullable\u003dTrue)"},{"line_number":309,"context_line":""},{"line_number":310,"context_line":""}],"source_content_type":"text/x-python","patch_set":38,"id":"df140735_bb3e13d8","line":307,"range":{"start_line":307,"start_character":43,"end_line":307,"end_character":57},"updated":"2017-06-07 13:22:36.000000000","message":"Just one question. This field is \"nullable\u003dTrue\" because you need to keep the backwards compatibility, isn\u0027t it?\n\nBecause in the object, according to https://review.openstack.org/#/c/377138/38/nova/objects/resource_provider.py@335 this field should be always the root ID or the object ID itself (in case there is no parent).\n\nThere should be here a deprecation note to change this parameter to False?","commit_id":"c89c1d1ee2cdc61fd5f2ebc64defda6b937cd229"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ffed97a30a421c8f38adbaade73e4253fcba7872","unresolved":false,"context_lines":[{"line_number":304,"context_line":"    uuid \u003d Column(String(36), nullable\u003dFalse)"},{"line_number":305,"context_line":"    name \u003d Column(Unicode(200), nullable\u003dTrue)"},{"line_number":306,"context_line":"    generation \u003d Column(Integer, default\u003d0)"},{"line_number":307,"context_line":"    root_provider_uuid \u003d Column(String(36), nullable\u003dTrue)"},{"line_number":308,"context_line":"    parent_provider_uuid \u003d Column(String(36), nullable\u003dTrue)"},{"line_number":309,"context_line":""},{"line_number":310,"context_line":""}],"source_content_type":"text/x-python","patch_set":38,"id":"df140735_d7e073cf","line":307,"range":{"start_line":307,"start_character":43,"end_line":307,"end_character":57},"in_reply_to":"df140735_bb3e13d8","updated":"2017-06-07 16:51:48.000000000","message":"I\u0027ll add a note, thanks Rodolfo. I added a note in the object definition (where it does inline data migration) but I\u0027ll add a deprecation note here.","commit_id":"c89c1d1ee2cdc61fd5f2ebc64defda6b937cd229"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2c453b9e5f9707edf1d43ac7c17a7e26b7cb6218","unresolved":false,"context_lines":[{"line_number":311,"context_line":"    # The immediate parent provider of this provider, or NULL if there is no"},{"line_number":312,"context_line":"    # parent. If parent_provider_id \u003d\u003d NULL then root_provider_id \u003d\u003d id"},{"line_number":313,"context_line":"    parent_provider_id \u003d Column(Integer, ForeignKey(\u0027resource_providers.id\u0027),"},{"line_number":314,"context_line":"        nullable\u003dTrue)"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":""},{"line_number":317,"context_line":"class Inventory(API_BASE):"}],"source_content_type":"text/x-python","patch_set":62,"id":"ff82abbf_43208d9b","line":314,"updated":"2017-11-29 15:08:43.000000000","message":"Supposing you want a ON DELETE CASCADE stategy for the FK ?","commit_id":"b10f11d7e8e1afb7a12a470f92c42bf3c23eca95"}],"nova/objects/resource_provider.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"2ea5b35c2723ff247cbc079a17a42bb71c7d6541","unresolved":false,"context_lines":[{"line_number":416,"context_line":"    @staticmethod"},{"line_number":417,"context_line":"    @db_api.api_context_manager.writer"},{"line_number":418,"context_line":"    def _delete(context, _id, root_uuid):"},{"line_number":419,"context_line":"        # TODO(jaypipes): We can remove the OR condition below in favor of just"},{"line_number":420,"context_line":"        # a simple WHERE root_provider_uuid \u003d \u003cUUID\u003e once we can be sure all"},{"line_number":421,"context_line":"        # providers have had their root_provider_uuid value populated."},{"line_number":422,"context_line":"        conn \u003d context.session.connection()"},{"line_number":423,"context_line":"        tree_cond \u003d sa.or_(_RP_TBL.c.id \u003d\u003d _id,"},{"line_number":424,"context_line":"                           _RP_TBL.c.root_provider_uuid \u003d\u003d root_uuid)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a8bc5a2_16e72e41","line":421,"range":{"start_line":419,"start_character":0,"end_line":421,"end_character":70},"updated":"2016-09-28 11:22:36.000000000","message":"A specific timeline on this would help here. Would Pike be a reasonable timeframe?","commit_id":"f7daa324b4b7fdc8380af06c2a64f43256a735d2"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"74705be2b25fa56f5646128b896a4759e81b733a","unresolved":false,"context_lines":[{"line_number":416,"context_line":"    @staticmethod"},{"line_number":417,"context_line":"    @db_api.api_context_manager.writer"},{"line_number":418,"context_line":"    def _delete(context, _id, root_uuid):"},{"line_number":419,"context_line":"        # TODO(jaypipes): We can remove the OR condition below in favor of just"},{"line_number":420,"context_line":"        # a simple WHERE root_provider_uuid \u003d \u003cUUID\u003e once we can be sure all"},{"line_number":421,"context_line":"        # providers have had their root_provider_uuid value populated."},{"line_number":422,"context_line":"        conn \u003d context.session.connection()"},{"line_number":423,"context_line":"        tree_cond \u003d sa.or_(_RP_TBL.c.id \u003d\u003d _id,"},{"line_number":424,"context_line":"                           _RP_TBL.c.root_provider_uuid \u003d\u003d root_uuid)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a95cdbc_fcedeaef","line":421,"range":{"start_line":419,"start_character":0,"end_line":421,"end_character":70},"in_reply_to":"5a8bc5a2_16e72e41","updated":"2016-10-07 15:43:41.000000000","message":"Not sure. @dansmith: how do we handle timelines for things like this?","commit_id":"f7daa324b4b7fdc8380af06c2a64f43256a735d2"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"fca60c1d7a4682fb5abb3399a2546d61f427a0f9","unresolved":false,"context_lines":[{"line_number":268,"context_line":"class ResourceProvider(base.NovaObject):"},{"line_number":269,"context_line":"    # Version 1.0: Initial version"},{"line_number":270,"context_line":"    # Version 1.1: Add destroy()"},{"line_number":271,"context_line":"    # Version 1.2: Add parent_provider_id field"},{"line_number":272,"context_line":"    VERSION \u003d \u00271.2\u0027"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"    SETTABLE_FIELDS \u003d (\u0027name\u0027, \u0027root_provider_uuid\u0027, \u0027parent_provider_uuid\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_79822702","line":271,"updated":"2016-10-11 13:41:13.000000000","message":"For completeness add root_provider_uuid here?\n\nand s/_id/_uuid/","commit_id":"62024153a1ebc6870b89ada84a54c547085f9aad"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6d54fa9ebe1e04d058d8c4a7adb7b452f5ba5ea0","unresolved":false,"context_lines":[{"line_number":268,"context_line":"class ResourceProvider(base.NovaObject):"},{"line_number":269,"context_line":"    # Version 1.0: Initial version"},{"line_number":270,"context_line":"    # Version 1.1: Add destroy()"},{"line_number":271,"context_line":"    # Version 1.2: Add parent_provider_id field"},{"line_number":272,"context_line":"    VERSION \u003d \u00271.2\u0027"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"    SETTABLE_FIELDS \u003d (\u0027name\u0027, \u0027root_provider_uuid\u0027, \u0027parent_provider_uuid\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_58046b16","line":271,"in_reply_to":"fa6399be_79822702","updated":"2016-10-12 17:15:42.000000000","message":"Done","commit_id":"62024153a1ebc6870b89ada84a54c547085f9aad"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"42f4df3679c929d657b811e7b9c5b3e414513a3f","unresolved":false,"context_lines":[{"line_number":278,"context_line":"        \u0027uuid\u0027: fields.UUIDField(nullable\u003dFalse),"},{"line_number":279,"context_line":"        \u0027name\u0027: fields.StringField(nullable\u003dFalse),"},{"line_number":280,"context_line":"        \u0027generation\u0027: fields.IntegerField(nullable\u003dFalse),"},{"line_number":281,"context_line":"        # UUID of the root provider in a hierarchy of providers. Will be equal"},{"line_number":282,"context_line":"        # to the uuid field if this provider is the root provider of a"},{"line_number":283,"context_line":"        # hierarchy."},{"line_number":284,"context_line":"        \u0027root_provider_uuid\u0027: fields.UUIDField(nullable\u003dFalse),"},{"line_number":285,"context_line":"        # UUID of the direct parent provider, or None if this provider is a"},{"line_number":286,"context_line":"        # \"root\" provider."},{"line_number":287,"context_line":"        \u0027parent_provider_uuid\u0027: fields.UUIDField(nullable\u003dTrue),"},{"line_number":288,"context_line":"    }"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"    @base.remotable"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_790cea42","line":287,"range":{"start_line":281,"start_character":8,"end_line":287,"end_character":64},"updated":"2016-10-11 11:53:21.000000000","message":"don\u0027t we need backward compatibility methods for that?","commit_id":"62024153a1ebc6870b89ada84a54c547085f9aad"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6d54fa9ebe1e04d058d8c4a7adb7b452f5ba5ea0","unresolved":false,"context_lines":[{"line_number":278,"context_line":"        \u0027uuid\u0027: fields.UUIDField(nullable\u003dFalse),"},{"line_number":279,"context_line":"        \u0027name\u0027: fields.StringField(nullable\u003dFalse),"},{"line_number":280,"context_line":"        \u0027generation\u0027: fields.IntegerField(nullable\u003dFalse),"},{"line_number":281,"context_line":"        # UUID of the root provider in a hierarchy of providers. Will be equal"},{"line_number":282,"context_line":"        # to the uuid field if this provider is the root provider of a"},{"line_number":283,"context_line":"        # hierarchy."},{"line_number":284,"context_line":"        \u0027root_provider_uuid\u0027: fields.UUIDField(nullable\u003dFalse),"},{"line_number":285,"context_line":"        # UUID of the direct parent provider, or None if this provider is a"},{"line_number":286,"context_line":"        # \"root\" provider."},{"line_number":287,"context_line":"        \u0027parent_provider_uuid\u0027: fields.UUIDField(nullable\u003dTrue),"},{"line_number":288,"context_line":"    }"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"    @base.remotable"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_53328cf2","line":287,"range":{"start_line":281,"start_character":8,"end_line":287,"end_character":64},"in_reply_to":"fa6399be_790cea42","updated":"2016-10-12 17:15:42.000000000","message":"Nope. I perform an online data migration to populate the root_provider_uuid. see code below :)","commit_id":"62024153a1ebc6870b89ada84a54c547085f9aad"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6d54fa9ebe1e04d058d8c4a7adb7b452f5ba5ea0","unresolved":false,"context_lines":[{"line_number":301,"context_line":"        if \u0027root_provider_uuid\u0027 not in self:"},{"line_number":302,"context_line":"            # root provider UUID is optional. If missing, assume the new"},{"line_number":303,"context_line":"            # provider is its own root provider."},{"line_number":304,"context_line":"            self.root_provider_uuid \u003d self.uuid"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"        updates \u003d self.obj_get_changes()"},{"line_number":307,"context_line":"        db_rp \u003d self._create_in_db(self._context, updates)"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_93526412","line":304,"updated":"2016-10-12 17:15:42.000000000","message":"@tdurakov: see here.","commit_id":"62024153a1ebc6870b89ada84a54c547085f9aad"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6d54fa9ebe1e04d058d8c4a7adb7b452f5ba5ea0","unresolved":false,"context_lines":[{"line_number":313,"context_line":"        # TODO(jaypipes): Remove the below conditional once root_provider_uuid"},{"line_number":314,"context_line":"        # is a non-nullable field."},{"line_number":315,"context_line":"        if self.obj_attr_is_set(\u0027root_provider_uuid\u0027):"},{"line_number":316,"context_line":"            root_uuid \u003d self.root_provider_uuid"},{"line_number":317,"context_line":"        self._delete(self._context, self.id, root_uuid)"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"    @base.remotable"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_133f34b5","line":316,"updated":"2016-10-12 17:15:42.000000000","message":"@tdurakov: and see here.","commit_id":"62024153a1ebc6870b89ada84a54c547085f9aad"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"3bc094601166287212a06bbe184fecf9661dad7d","unresolved":false,"context_lines":[{"line_number":325,"context_line":"                action\u003d\u0027save\u0027,"},{"line_number":326,"context_line":"                reason\u003d\u0027Immutable fields changed\u0027)"},{"line_number":327,"context_line":"        self._update_in_db(self._context, self.id, self.uuid, updates)"},{"line_number":328,"context_line":"        # Need to do this because updates may have been modified to set the"},{"line_number":329,"context_line":"        # root provider UUID automatically, but _update_in_db() is a"},{"line_number":330,"context_line":"        # staticmethod and never sets the attributes of the object.  Would be"},{"line_number":331,"context_line":"        # nicer if we didn\u0027t use all the staticmethods and simply modified the"},{"line_number":332,"context_line":"        # object state directly."},{"line_number":333,"context_line":"        for field in self.fields:"},{"line_number":334,"context_line":"            if field in updates:"},{"line_number":335,"context_line":"                setattr(self, field, updates[field])"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_c21a646e","line":332,"range":{"start_line":328,"start_character":0,"end_line":332,"end_character":32},"updated":"2016-10-11 08:42:56.000000000","message":"So, if I get this right, the underlying database is already updated by the ORM Python object is not?","commit_id":"62024153a1ebc6870b89ada84a54c547085f9aad"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6d54fa9ebe1e04d058d8c4a7adb7b452f5ba5ea0","unresolved":false,"context_lines":[{"line_number":325,"context_line":"                action\u003d\u0027save\u0027,"},{"line_number":326,"context_line":"                reason\u003d\u0027Immutable fields changed\u0027)"},{"line_number":327,"context_line":"        self._update_in_db(self._context, self.id, self.uuid, updates)"},{"line_number":328,"context_line":"        # Need to do this because updates may have been modified to set the"},{"line_number":329,"context_line":"        # root provider UUID automatically, but _update_in_db() is a"},{"line_number":330,"context_line":"        # staticmethod and never sets the attributes of the object.  Would be"},{"line_number":331,"context_line":"        # nicer if we didn\u0027t use all the staticmethods and simply modified the"},{"line_number":332,"context_line":"        # object state directly."},{"line_number":333,"context_line":"        for field in self.fields:"},{"line_number":334,"context_line":"            if field in updates:"},{"line_number":335,"context_line":"                setattr(self, field, updates[field])"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_04797ec8","line":332,"range":{"start_line":328,"start_character":0,"end_line":332,"end_character":32},"in_reply_to":"fa6399be_c21a646e","updated":"2016-10-12 17:15:42.000000000","message":"correct.","commit_id":"62024153a1ebc6870b89ada84a54c547085f9aad"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"fca60c1d7a4682fb5abb3399a2546d61f427a0f9","unresolved":false,"context_lines":[{"line_number":381,"context_line":"        self.obj_reset_changes()"},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"    @staticmethod"},{"line_number":384,"context_line":"    def _ensure_root_and_parent_exist(context, uuid, updates):"},{"line_number":385,"context_line":"        \"\"\"Enforces integrity checks against the resource providers table"},{"line_number":386,"context_line":"        itself by checking that the root provider UUID and parent provider UUID"},{"line_number":387,"context_line":"        (if set) actually point to a resource provider that exists."}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_bcc4ad92","line":384,"updated":"2016-10-11 13:41:13.000000000","message":"May be talking crazy here, but isn\u0027t this something that could be done in the sqlA model description?","commit_id":"62024153a1ebc6870b89ada84a54c547085f9aad"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6d54fa9ebe1e04d058d8c4a7adb7b452f5ba5ea0","unresolved":false,"context_lines":[{"line_number":381,"context_line":"        self.obj_reset_changes()"},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"    @staticmethod"},{"line_number":384,"context_line":"    def _ensure_root_and_parent_exist(context, uuid, updates):"},{"line_number":385,"context_line":"        \"\"\"Enforces integrity checks against the resource providers table"},{"line_number":386,"context_line":"        itself by checking that the root provider UUID and parent provider UUID"},{"line_number":387,"context_line":"        (if set) actually point to a resource provider that exists."}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_6455da1b","line":384,"in_reply_to":"fa6399be_bcc4ad92","updated":"2016-10-12 17:15:42.000000000","message":"Sure, but we typically do our validation and compatibility backversioning in the nova.objects area, which is why this is here instead of hidden in the sqlA models.","commit_id":"62024153a1ebc6870b89ada84a54c547085f9aad"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"db2d078905fe2aeddb2253131d1a41324c241573","unresolved":false,"context_lines":[{"line_number":344,"context_line":"        if \u0027name\u0027 not in self:"},{"line_number":345,"context_line":"            raise exception.ObjectActionError(action\u003d\u0027create\u0027,"},{"line_number":346,"context_line":"                                              reason\u003d\u0027name is required\u0027)"},{"line_number":347,"context_line":"        if \u0027root_provider_uuid\u0027 not in self:"},{"line_number":348,"context_line":"            # root provider UUID is optional. If missing, assume the new"},{"line_number":349,"context_line":"            # provider is its own root provider."},{"line_number":350,"context_line":"            self.root_provider_uuid \u003d self.uuid"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":"        updates \u003d self.obj_get_changes()"},{"line_number":353,"context_line":"        db_rp \u003d self._create_in_db(self._context, updates)"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a629dbe_7cae96e8","line":350,"range":{"start_line":347,"start_character":0,"end_line":350,"end_character":47},"updated":"2016-11-14 13:48:26.000000000","message":"you also need to make sure that parent_provider_uuid is not set or is set to None in this case.","commit_id":"435b3fc0653915145e2aea6983979a6ee0660554"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"e3d167f655b0d4230c9cab4c3f60c67a8d36bae3","unresolved":false,"context_lines":[{"line_number":344,"context_line":"        if \u0027name\u0027 not in self:"},{"line_number":345,"context_line":"            raise exception.ObjectActionError(action\u003d\u0027create\u0027,"},{"line_number":346,"context_line":"                                              reason\u003d\u0027name is required\u0027)"},{"line_number":347,"context_line":"        if \u0027root_provider_uuid\u0027 not in self:"},{"line_number":348,"context_line":"            # root provider UUID is optional. If missing, assume the new"},{"line_number":349,"context_line":"            # provider is its own root provider."},{"line_number":350,"context_line":"            self.root_provider_uuid \u003d self.uuid"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":"        updates \u003d self.obj_get_changes()"},{"line_number":353,"context_line":"        db_rp \u003d self._create_in_db(self._context, updates)"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a629dbe_199047ec","line":350,"range":{"start_line":347,"start_character":0,"end_line":350,"end_character":47},"in_reply_to":"9a629dbe_132ca8aa","updated":"2016-11-14 15:10:12.000000000","message":"No, actually, I was thinking that if a user passes in a parent_provider_uuid, we should simply set the root_provider_uuid to that of the parent always, which is correct. No need to raise an error I think?","commit_id":"435b3fc0653915145e2aea6983979a6ee0660554"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"04cb9cf3b485417f7550effe8655cffed981a185","unresolved":false,"context_lines":[{"line_number":344,"context_line":"        if \u0027name\u0027 not in self:"},{"line_number":345,"context_line":"            raise exception.ObjectActionError(action\u003d\u0027create\u0027,"},{"line_number":346,"context_line":"                                              reason\u003d\u0027name is required\u0027)"},{"line_number":347,"context_line":"        if \u0027root_provider_uuid\u0027 not in self:"},{"line_number":348,"context_line":"            # root provider UUID is optional. If missing, assume the new"},{"line_number":349,"context_line":"            # provider is its own root provider."},{"line_number":350,"context_line":"            self.root_provider_uuid \u003d self.uuid"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":"        updates \u003d self.obj_get_changes()"},{"line_number":353,"context_line":"        db_rp \u003d self._create_in_db(self._context, updates)"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a629dbe_275fa60f","line":350,"range":{"start_line":347,"start_character":0,"end_line":350,"end_character":47},"in_reply_to":"9a629dbe_199047ec","updated":"2016-11-14 15:33:04.000000000","message":"so in that case you can raise if you get root_provider_uuid which is not None and set it always internally according to the parent_provider_uuid and remove the db call in lines 435-445.\nSo you have 2 cases:\n1. parent_provider_uuid is set so you set the root_provider_uuid \n2. parent_provider_uuid\u003dNone so you set root_provider_uuid\u003dself.uuid.\n(unless I am missing something)\nI think it will simplify the code.","commit_id":"435b3fc0653915145e2aea6983979a6ee0660554"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d7e43fff283cb4911170966a08cbe49814df5231","unresolved":false,"context_lines":[{"line_number":344,"context_line":"        if \u0027name\u0027 not in self:"},{"line_number":345,"context_line":"            raise exception.ObjectActionError(action\u003d\u0027create\u0027,"},{"line_number":346,"context_line":"                                              reason\u003d\u0027name is required\u0027)"},{"line_number":347,"context_line":"        if \u0027root_provider_uuid\u0027 not in self:"},{"line_number":348,"context_line":"            # root provider UUID is optional. If missing, assume the new"},{"line_number":349,"context_line":"            # provider is its own root provider."},{"line_number":350,"context_line":"            self.root_provider_uuid \u003d self.uuid"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":"        updates \u003d self.obj_get_changes()"},{"line_number":353,"context_line":"        db_rp \u003d self._create_in_db(self._context, updates)"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a629dbe_8234e04a","line":350,"range":{"start_line":347,"start_character":0,"end_line":350,"end_character":47},"in_reply_to":"9a629dbe_275fa60f","updated":"2016-11-14 16:01:14.000000000","message":"Ah, gotcha. Sure, I can make that change. Makes sense, thanks Moshe!","commit_id":"435b3fc0653915145e2aea6983979a6ee0660554"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"85610ce60d195c699af51b94ab35143b06bc1cfc","unresolved":false,"context_lines":[{"line_number":344,"context_line":"        if \u0027name\u0027 not in self:"},{"line_number":345,"context_line":"            raise exception.ObjectActionError(action\u003d\u0027create\u0027,"},{"line_number":346,"context_line":"                                              reason\u003d\u0027name is required\u0027)"},{"line_number":347,"context_line":"        if \u0027root_provider_uuid\u0027 not in self:"},{"line_number":348,"context_line":"            # root provider UUID is optional. If missing, assume the new"},{"line_number":349,"context_line":"            # provider is its own root provider."},{"line_number":350,"context_line":"            self.root_provider_uuid \u003d self.uuid"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":"        updates \u003d self.obj_get_changes()"},{"line_number":353,"context_line":"        db_rp \u003d self._create_in_db(self._context, updates)"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a629dbe_132ca8aa","line":350,"range":{"start_line":347,"start_character":0,"end_line":350,"end_character":47},"in_reply_to":"9a629dbe_33e8cc16","updated":"2016-11-14 14:37:14.000000000","message":"I see so in case the root_provider_uuid is not set and the parent_provider_uuid is set we automatically set the root_provider_uuid. \nso what about conflict like \nRP_A--\u003eRP_B  and RP_C--\u003eRP_D and passing root_provider_uuid\u003dRP_A and parent_provider_uuid\u003dRP_D.\n\ndon\u0027t you need to check in Line 461 that \nif root_provider_uuid and root_provider_uuid !\u003d res[0][0]:\n    raise ERROR","commit_id":"435b3fc0653915145e2aea6983979a6ee0660554"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"dede42ec6dc14eef032674b5c00a6f999eb3dd83","unresolved":false,"context_lines":[{"line_number":344,"context_line":"        if \u0027name\u0027 not in self:"},{"line_number":345,"context_line":"            raise exception.ObjectActionError(action\u003d\u0027create\u0027,"},{"line_number":346,"context_line":"                                              reason\u003d\u0027name is required\u0027)"},{"line_number":347,"context_line":"        if \u0027root_provider_uuid\u0027 not in self:"},{"line_number":348,"context_line":"            # root provider UUID is optional. If missing, assume the new"},{"line_number":349,"context_line":"            # provider is its own root provider."},{"line_number":350,"context_line":"            self.root_provider_uuid \u003d self.uuid"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":"        updates \u003d self.obj_get_changes()"},{"line_number":353,"context_line":"        db_rp \u003d self._create_in_db(self._context, updates)"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a629dbe_33e8cc16","line":350,"range":{"start_line":347,"start_character":0,"end_line":350,"end_character":47},"in_reply_to":"9a629dbe_7cae96e8","updated":"2016-11-14 14:22:06.000000000","message":"Moshe, see line 461. If parent_provider_uuid is not None, we automatically set root_provider_uuid to the value of the parent\u0027s root_provider_uuid.","commit_id":"435b3fc0653915145e2aea6983979a6ee0660554"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"683e757e71199fb00053d4c790892e2a859afb07","unresolved":false,"context_lines":[{"line_number":432,"context_line":""},{"line_number":433,"context_line":"    @staticmethod"},{"line_number":434,"context_line":"    @db_api.api_context_manager.reader"},{"line_number":435,"context_line":"    def _validate_parent_and_set_root(ctx, uuid, updates):"},{"line_number":436,"context_line":"        \"\"\"Enforces integrity checks against the resource providers table"},{"line_number":437,"context_line":"        itself by checking that the parent provider UUID (if set) actually"},{"line_number":438,"context_line":"        point to a resource provider that exists. Also correctly sets the"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a77a97e_21d11afc","line":435,"range":{"start_line":435,"start_character":4,"end_line":435,"end_character":58},"updated":"2016-11-22 17:42:18.000000000","message":"Meta question that you\u0027re just the person to ask: how come we can\u0027t enforce this at the db level through foreign keys, i.e. cascade deletes?","commit_id":"6a345da4d9ee85b280ca0c4d4ee276d5be4fc62a"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"8edf4c88a734f9ef39c6a2a028c0b291de3b13a7","unresolved":false,"context_lines":[{"line_number":538,"context_line":""},{"line_number":539,"context_line":"    @staticmethod"},{"line_number":540,"context_line":"    def _from_db_object(context, resource_provider, db_resource_provider):"},{"line_number":541,"context_line":"        # Online data migration to populate root_provider_uuid"},{"line_number":542,"context_line":"        if db_resource_provider[\u0027root_provider_uuid\u0027] is None:"},{"line_number":543,"context_line":"            uuid \u003d db_resource_provider[\u0027uuid\u0027]"},{"line_number":544,"context_line":"            db_resource_provider[\u0027root_provider_uuid\u0027] \u003d uuid"}],"source_content_type":"text/x-python","patch_set":20,"id":"5a74a57a_984a9026","line":541,"updated":"2016-11-30 11:02:10.000000000","message":"Maybe this has been discussed before, but it occurred to me that maybe we don\u0027t really need to do this as a write. If every time a resource provider is created or updated it gets its parent and root validated, then all we need to do when doing a read is check:\n\n* that root_provider_uuid is set\n* if not, set it to uuid\n\nWe don\u0027t need to write it, do we, because if root \u003d\u003d uuid, then the rp isn\u0027t a child in a tree.\n\nI suppose, however, this could make subsequent calls which rely on root being set in the db (and are not going through the object interface) to be more complex.\n\nThe reason I mention it is because it seems very weird to me to be doing a write in a read, so if we could avoid it that would be nice.","commit_id":"a3276d0c84c0a7dce4d914a9bea419ec70927696"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a8fe0905f82fd70fcb4e09c5b048d890b0501194","unresolved":false,"context_lines":[{"line_number":538,"context_line":""},{"line_number":539,"context_line":"    @staticmethod"},{"line_number":540,"context_line":"    def _from_db_object(context, resource_provider, db_resource_provider):"},{"line_number":541,"context_line":"        # Online data migration to populate root_provider_uuid"},{"line_number":542,"context_line":"        if db_resource_provider[\u0027root_provider_uuid\u0027] is None:"},{"line_number":543,"context_line":"            uuid \u003d db_resource_provider[\u0027uuid\u0027]"},{"line_number":544,"context_line":"            db_resource_provider[\u0027root_provider_uuid\u0027] \u003d uuid"}],"source_content_type":"text/x-python","patch_set":20,"id":"3a71b18c_e0eb96ae","line":541,"in_reply_to":"5a74a57a_984a9026","updated":"2016-12-02 17:19:47.000000000","message":"Hey Chris, sorry didn\u0027t see this review comment until just now. To answer your question, the reason we do this on read is to attempt to get the root_provider_uuid column populated as quickly as possible but without blocking on a database data migration. We want the root_provider_uuid value to be not-None so that we can remove the OR condition in various WHERE clauses, as noted in the code comment above on line 498.","commit_id":"a3276d0c84c0a7dce4d914a9bea419ec70927696"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"97ff6e3bd6718a6860e7590708f7db8e219c1797","unresolved":false,"context_lines":[{"line_number":348,"context_line":"            if \u0027parent_provider_uuid\u0027 in primitive:"},{"line_number":349,"context_line":"                del primitive[\u0027parent_provider_uuid\u0027]"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    @base.remotable"},{"line_number":352,"context_line":"    def create(self):"},{"line_number":353,"context_line":"        if \u0027id\u0027 in self:"},{"line_number":354,"context_line":"            raise exception.ObjectActionError(action\u003d\u0027create\u0027,"}],"source_content_type":"text/x-python","patch_set":25,"id":"ba5201f7_194ba1b1","line":351,"updated":"2017-01-04 12:34:39.000000000","message":"we\u0027ve turned off remotable (see version 1.3 above) so this shouldn\u0027t be here.\n\nI\u0027m not sure, but that might also mean we don\u0027t need to worry about obj_make_compatible?","commit_id":"cfc201976b3f968bef4b8ecad3969aedb365e1bb"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"cb6293eca2d63276694056c321e31ddd788b65a6","unresolved":false,"context_lines":[{"line_number":348,"context_line":"            if \u0027parent_provider_uuid\u0027 in primitive:"},{"line_number":349,"context_line":"                del primitive[\u0027parent_provider_uuid\u0027]"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    @base.remotable"},{"line_number":352,"context_line":"    def create(self):"},{"line_number":353,"context_line":"        if \u0027id\u0027 in self:"},{"line_number":354,"context_line":"            raise exception.ObjectActionError(action\u003d\u0027create\u0027,"}],"source_content_type":"text/x-python","patch_set":27,"id":"ba5201f7_e280484c","line":351,"updated":"2017-01-09 15:23:27.000000000","message":"We\u0027re not remotable any more. See my comment on ps 25.","commit_id":"bb1723f261a618c2661ad0c7175387c1a6b5729a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"89c99a642ba8f6dab883fcb9afbdf4a9f902021d","unresolved":false,"context_lines":[{"line_number":348,"context_line":"            if \u0027parent_provider_uuid\u0027 in primitive:"},{"line_number":349,"context_line":"                del primitive[\u0027parent_provider_uuid\u0027]"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    @base.remotable"},{"line_number":352,"context_line":"    def create(self):"},{"line_number":353,"context_line":"        if \u0027id\u0027 in self:"},{"line_number":354,"context_line":"            raise exception.ObjectActionError(action\u003d\u0027create\u0027,"}],"source_content_type":"text/x-python","patch_set":27,"id":"ba5201f7_31a85f77","line":351,"in_reply_to":"ba5201f7_e280484c","updated":"2017-09-27 18:52:08.000000000","message":"Done","commit_id":"bb1723f261a618c2661ad0c7175387c1a6b5729a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fb32fad296005c4c73bbe20193c98bc41e37a5e3","unresolved":false,"context_lines":[{"line_number":412,"context_line":"        # is an optimization field that allows us to very quickly query for all"},{"line_number":413,"context_line":"        # providers within a particular tree without doing any recursive"},{"line_number":414,"context_line":"        # querying."},{"line_number":415,"context_line":"        \u0027root_provider_uuid\u0027: fields.UUIDField(nullable\u003dFalse),"},{"line_number":416,"context_line":"        # UUID of the direct parent provider, or None if this provider is a"},{"line_number":417,"context_line":"        # \"root\" provider."},{"line_number":418,"context_line":"        \u0027parent_provider_uuid\u0027: fields.UUIDField(nullable\u003dTrue),"}],"source_content_type":"text/x-python","patch_set":40,"id":"5f201791_a68799b5","line":415,"range":{"start_line":415,"start_character":47,"end_line":415,"end_character":61},"updated":"2017-06-26 14:32:04.000000000","message":"Why is this false here, yet true for the database model itself? I\u0027m guessing this only affects creation of new instances or updating existing ones, while the database model has to handle existing records where this isn\u0027t set?","commit_id":"63577e9677654d1ee22b21e44d456eaec272187a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fb32fad296005c4c73bbe20193c98bc41e37a5e3","unresolved":false,"context_lines":[{"line_number":527,"context_line":"        :raises: ObjectActionError if the parent_provider_uuid points to a"},{"line_number":528,"context_line":"                 non-existing provider."},{"line_number":529,"context_line":"        \"\"\""},{"line_number":530,"context_line":"        if \u0027parent_provider_uuid\u0027 not in updates:"},{"line_number":531,"context_line":"            updates[\u0027root_provider_uuid\u0027] \u003d uuid"},{"line_number":532,"context_line":"            return"},{"line_number":533,"context_line":""}],"source_content_type":"text/x-python","patch_set":40,"id":"5f201791_61304be9","line":530,"range":{"start_line":530,"start_character":8,"end_line":530,"end_character":49},"updated":"2017-06-26 14:32:04.000000000","message":"How can this ever be True...","commit_id":"63577e9677654d1ee22b21e44d456eaec272187a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fb32fad296005c4c73bbe20193c98bc41e37a5e3","unresolved":false,"context_lines":[{"line_number":531,"context_line":"            updates[\u0027root_provider_uuid\u0027] \u003d uuid"},{"line_number":532,"context_line":"            return"},{"line_number":533,"context_line":""},{"line_number":534,"context_line":"        parent_uuid \u003d updates[\u0027parent_provider_uuid\u0027]"},{"line_number":535,"context_line":"        if parent_uuid is None:"},{"line_number":536,"context_line":"            updates[\u0027root_provider_uuid\u0027] \u003d uuid"},{"line_number":537,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":40,"id":"5f201791_015517fa","line":534,"range":{"start_line":534,"start_character":8,"end_line":534,"end_character":53},"updated":"2017-06-26 14:32:04.000000000","message":"...if this is never failing? I would have expected\n\n  updates.get(\u0027parent_provider_uuid\u0027)\n\nto prevent a KeyError","commit_id":"63577e9677654d1ee22b21e44d456eaec272187a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c21b899782d825f054161c8bdd306be438de7561","unresolved":false,"context_lines":[{"line_number":517,"context_line":""},{"line_number":518,"context_line":"    @staticmethod"},{"line_number":519,"context_line":"    @db_api.api_context_manager.reader"},{"line_number":520,"context_line":"    def _validate_parent_and_set_root(ctx, uuid, updates):"},{"line_number":521,"context_line":"        \"\"\"Enforces integrity checks against the resource providers table"},{"line_number":522,"context_line":"        itself by checking that the parent provider UUID (if set) actually"},{"line_number":523,"context_line":"        point to a resource provider that exists. Also correctly sets the"},{"line_number":524,"context_line":"        root_provider_uuid value to the root_provider_uuid of the parent (if"},{"line_number":525,"context_line":"        set) or to this provider\u0027s UUID (if parent is not set)."},{"line_number":526,"context_line":""},{"line_number":527,"context_line":"        :raises: ObjectActionError if the parent_provider_uuid points to a"},{"line_number":528,"context_line":"                 non-existing provider."},{"line_number":529,"context_line":"        \"\"\""},{"line_number":530,"context_line":"        if \u0027parent_provider_uuid\u0027 not in updates:"},{"line_number":531,"context_line":"            updates[\u0027root_provider_uuid\u0027] \u003d uuid"},{"line_number":532,"context_line":"            return"},{"line_number":533,"context_line":""},{"line_number":534,"context_line":"        parent_uuid \u003d updates[\u0027parent_provider_uuid\u0027]"},{"line_number":535,"context_line":"        if parent_uuid is None:"},{"line_number":536,"context_line":"            updates[\u0027root_provider_uuid\u0027] \u003d uuid"},{"line_number":537,"context_line":"            return"},{"line_number":538,"context_line":""},{"line_number":539,"context_line":"        # Ensure parent_provider_uuid points to an existing provider"},{"line_number":540,"context_line":"        with db_api.api_context_manager.reader.connection.using(ctx) as conn:"},{"line_number":541,"context_line":"            sel_stmt \u003d sa.select([_RP_TBL.c.root_provider_uuid]).where("},{"line_number":542,"context_line":"                _RP_TBL.c.uuid \u003d\u003d parent_uuid)"},{"line_number":543,"context_line":"            res \u003d conn.execute(sel_stmt).fetchall()"},{"line_number":544,"context_line":"            if not res:"},{"line_number":545,"context_line":"                raise exception.ObjectActionError("},{"line_number":546,"context_line":"                        action\u003d\u0027create\u0027,"},{"line_number":547,"context_line":"                        reason\u003d\u0027parent provider UUID does not exist.\u0027)"},{"line_number":548,"context_line":"            # Ensure that if we update a resource provider\u0027s parent"},{"line_number":549,"context_line":"            # provider UUID, that we automatically set that provider\u0027s root"},{"line_number":550,"context_line":"            # provider UUID to the parent\u0027s root."},{"line_number":551,"context_line":"            updates[\u0027root_provider_uuid\u0027] \u003d res[0][0]"},{"line_number":552,"context_line":""},{"line_number":553,"context_line":"    @staticmethod"},{"line_number":554,"context_line":"    @db_api.api_context_manager.writer"}],"source_content_type":"text/x-python","patch_set":41,"id":"9f436f4f_e422c001","line":551,"range":{"start_line":520,"start_character":3,"end_line":551,"end_character":53},"updated":"2017-08-17 16:57:11.000000000","message":"is there a reason not to make this an insert/update trigger in the db instead of doing it in python.","commit_id":"a0fb3031e8640c563021bcdc931483e68872a282"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"45c2eab39ddfd68f7c6fe1b93aeaabdfd7c60bdc","unresolved":false,"context_lines":[{"line_number":548,"context_line":"            # Ensure that if we update a resource provider\u0027s parent"},{"line_number":549,"context_line":"            # provider UUID, that we automatically set that provider\u0027s root"},{"line_number":550,"context_line":"            # provider UUID to the parent\u0027s root."},{"line_number":551,"context_line":"            updates[\u0027root_provider_uuid\u0027] \u003d res[0][0]"},{"line_number":552,"context_line":""},{"line_number":553,"context_line":"    @staticmethod"},{"line_number":554,"context_line":"    @db_api.api_context_manager.writer"}],"source_content_type":"text/x-python","patch_set":41,"id":"9f436f4f_0475c4cc","line":551,"in_reply_to":"9f436f4f_e422c001","updated":"2017-08-17 17:03:25.000000000","message":"last I recall, we\u0027ve chosen to avoid triggers as much as possible, as a general rule","commit_id":"a0fb3031e8640c563021bcdc931483e68872a282"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"c4893ad2797ffcf66348df7425c25c524bd33f98","unresolved":false,"context_lines":[{"line_number":401,"context_line":"        # to the uuid field if this provider is the root provider of a"},{"line_number":402,"context_line":"        # hierarchy. This field is never manually set by the user. Instead, it"},{"line_number":403,"context_line":"        # is automatically set to either the root provider UUID of the parent"},{"line_number":404,"context_line":"        # or the UUID of the provider itself if there is no parent. This field"},{"line_number":405,"context_line":"        # is an optimization field that allows us to very quickly query for all"},{"line_number":406,"context_line":"        # providers within a particular tree without doing any recursive"},{"line_number":407,"context_line":"        # querying."},{"line_number":408,"context_line":"        \u0027root_provider_uuid\u0027: fields.UUIDField(nullable\u003dFalse),"},{"line_number":409,"context_line":"        # UUID of the direct parent provider, or None if this provider is a"},{"line_number":410,"context_line":"        # \"root\" provider."}],"source_content_type":"text/x-python","patch_set":43,"id":"7f515b1d_51cfceaf","line":407,"range":{"start_line":404,"start_character":68,"end_line":407,"end_character":19},"updated":"2017-09-25 13:12:14.000000000","message":"Is this only expected to be used by the db layer? If yes, can we somehow prevent the user of the ResourceProvider object to reading this field and depending on the value of this field?","commit_id":"387b278875591f57148c1646ef0606ff1813e4c8"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"89c99a642ba8f6dab883fcb9afbdf4a9f902021d","unresolved":false,"context_lines":[{"line_number":401,"context_line":"        # to the uuid field if this provider is the root provider of a"},{"line_number":402,"context_line":"        # hierarchy. This field is never manually set by the user. Instead, it"},{"line_number":403,"context_line":"        # is automatically set to either the root provider UUID of the parent"},{"line_number":404,"context_line":"        # or the UUID of the provider itself if there is no parent. This field"},{"line_number":405,"context_line":"        # is an optimization field that allows us to very quickly query for all"},{"line_number":406,"context_line":"        # providers within a particular tree without doing any recursive"},{"line_number":407,"context_line":"        # querying."},{"line_number":408,"context_line":"        \u0027root_provider_uuid\u0027: fields.UUIDField(nullable\u003dFalse),"},{"line_number":409,"context_line":"        # UUID of the direct parent provider, or None if this provider is a"},{"line_number":410,"context_line":"        # \"root\" provider."}],"source_content_type":"text/x-python","patch_set":43,"id":"7f515b1d_7f1c7ac0","line":407,"range":{"start_line":404,"start_character":68,"end_line":407,"end_character":19},"in_reply_to":"7f515b1d_51cfceaf","updated":"2017-09-27 18:52:08.000000000","message":"Yes, this is a DB-optimization field.\n\nWe do not expose this attribute to end users. The REST API only exposes the parent_provider_uuid attribute:\n\nhttps://review.openstack.org/#/c/384807/44/nova/api/openstack/placement/handlers/resource_provider.py","commit_id":"387b278875591f57148c1646ef0606ff1813e4c8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"c4893ad2797ffcf66348df7425c25c524bd33f98","unresolved":false,"context_lines":[{"line_number":432,"context_line":""},{"line_number":433,"context_line":"    def destroy(self):"},{"line_number":434,"context_line":"        root_uuid \u003d self.uuid"},{"line_number":435,"context_line":"        # TODO(jaypipes): Remove the below conditional once root_provider_uuid"},{"line_number":436,"context_line":"        # is a non-nullable field."},{"line_number":437,"context_line":"        if self.obj_attr_is_set(\u0027root_provider_uuid\u0027):"},{"line_number":438,"context_line":"            root_uuid \u003d self.root_provider_uuid"},{"line_number":439,"context_line":"        self._delete(self._context, self.id, root_uuid)"}],"source_content_type":"text/x-python","patch_set":43,"id":"7f515b1d_ce72fb49","line":436,"range":{"start_line":435,"start_character":8,"end_line":436,"end_character":34},"updated":"2017-09-25 13:12:14.000000000","message":"root_provider_uuid is non nullable based on L408","commit_id":"387b278875591f57148c1646ef0606ff1813e4c8"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"89c99a642ba8f6dab883fcb9afbdf4a9f902021d","unresolved":false,"context_lines":[{"line_number":432,"context_line":""},{"line_number":433,"context_line":"    def destroy(self):"},{"line_number":434,"context_line":"        root_uuid \u003d self.uuid"},{"line_number":435,"context_line":"        # TODO(jaypipes): Remove the below conditional once root_provider_uuid"},{"line_number":436,"context_line":"        # is a non-nullable field."},{"line_number":437,"context_line":"        if self.obj_attr_is_set(\u0027root_provider_uuid\u0027):"},{"line_number":438,"context_line":"            root_uuid \u003d self.root_provider_uuid"},{"line_number":439,"context_line":"        self._delete(self._context, self.id, root_uuid)"}],"source_content_type":"text/x-python","patch_set":43,"id":"7f515b1d_bfd9d2f3","line":436,"range":{"start_line":435,"start_character":8,"end_line":436,"end_character":34},"in_reply_to":"7f515b1d_ce72fb49","updated":"2017-09-27 18:52:08.000000000","message":"We need to deal with existing records in the resource_providers table. The database table column must be added as NULLable because adding a NOT NULL column is a backwards-incompatible schema migration.\n\nThe above comment is referring to the database column being nullable.","commit_id":"387b278875591f57148c1646ef0606ff1813e4c8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"c4893ad2797ffcf66348df7425c25c524bd33f98","unresolved":false,"context_lines":[{"line_number":520,"context_line":"        :raises: ObjectActionError if the parent_provider_uuid points to a"},{"line_number":521,"context_line":"                 non-existing provider."},{"line_number":522,"context_line":"        \"\"\""},{"line_number":523,"context_line":"        if \u0027parent_provider_uuid\u0027 not in updates:"},{"line_number":524,"context_line":"            updates[\u0027root_provider_uuid\u0027] \u003d uuid"},{"line_number":525,"context_line":"            return"},{"line_number":526,"context_line":""}],"source_content_type":"text/x-python","patch_set":43,"id":"7f515b1d_6e00a768","line":523,"range":{"start_line":523,"start_character":11,"end_line":523,"end_character":48},"updated":"2017-09-25 13:12:14.000000000","message":"What if the updates doesn\u0027t contain the parent_provider_uuid as that is not changed in this update of the object  but the object itself has a parent_provider_uuid set?\nI assume obj_get_changes() only returns the fields that are changed.","commit_id":"387b278875591f57148c1646ef0606ff1813e4c8"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"89c99a642ba8f6dab883fcb9afbdf4a9f902021d","unresolved":false,"context_lines":[{"line_number":520,"context_line":"        :raises: ObjectActionError if the parent_provider_uuid points to a"},{"line_number":521,"context_line":"                 non-existing provider."},{"line_number":522,"context_line":"        \"\"\""},{"line_number":523,"context_line":"        if \u0027parent_provider_uuid\u0027 not in updates:"},{"line_number":524,"context_line":"            updates[\u0027root_provider_uuid\u0027] \u003d uuid"},{"line_number":525,"context_line":"            return"},{"line_number":526,"context_line":""}],"source_content_type":"text/x-python","patch_set":43,"id":"7f515b1d_7f7b5abd","line":523,"range":{"start_line":523,"start_character":11,"end_line":523,"end_character":48},"in_reply_to":"7f515b1d_6e00a768","updated":"2017-09-27 18:52:08.000000000","message":"Good catch. I\u0027ll add code to handle that situation.","commit_id":"387b278875591f57148c1646ef0606ff1813e4c8"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9b5888ab00d1776a29223c16ad919ae62f2773e3","unresolved":false,"context_lines":[{"line_number":389,"context_line":""},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"@db_api.api_context_manager.reader"},{"line_number":392,"context_line":"def _root_provider_uuid_from_parent(context, parent_uuid):"},{"line_number":393,"context_line":"    \"\"\"Given the UUID of a parent resource provider, returns the root provider"},{"line_number":394,"context_line":"    UUID of that parent."},{"line_number":395,"context_line":""}],"source_content_type":"text/x-python","patch_set":44,"id":"7f515b1d_b02bbc2e","line":392,"range":{"start_line":392,"start_character":29,"end_line":392,"end_character":35},"updated":"2017-10-04 15:46:40.000000000","message":"References to \u0027parent\u0027 throughout this method seem unnecessary and confusing.  It looks to me like this method\u0027s purpose is to get the root provider UUID from *any* RP - be it a parent, child, leaf, root, whatever.  Would be better as e.g.\n\n def _get_root_provider_uuid(context, rp_uuid):\n\n[Later] Ah, it looks like it\u0027s named thus because of how it\u0027s being used.  Which isn\u0027t a good reason.","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9b5888ab00d1776a29223c16ad919ae62f2773e3","unresolved":false,"context_lines":[{"line_number":398,"context_line":"    :param parent_uuid: The UUID of the parent provider to look up."},{"line_number":399,"context_line":"    \"\"\""},{"line_number":400,"context_line":"    conn \u003d context.session.connection()"},{"line_number":401,"context_line":"    sel_stmt \u003d sa.select([_RP_TBL.c.root_provider_uuid]).where("},{"line_number":402,"context_line":"        _RP_TBL.c.uuid \u003d\u003d parent_uuid)"},{"line_number":403,"context_line":"    res \u003d conn.execute(sel_stmt).fetchall()"},{"line_number":404,"context_line":"    if not res:"},{"line_number":405,"context_line":"        return None"}],"source_content_type":"text/x-python","patch_set":44,"id":"7f515b1d_f0d6d427","line":402,"range":{"start_line":401,"start_character":0,"end_line":402,"end_character":38},"updated":"2017-10-04 15:46:40.000000000","message":"Are we enforcing (including populating fields on upgrade) that root RPs\u0027 root_provider_uuid fields are populated with their own UUID?  If so, this is fine; if not, this is busted.","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"8f9be0999670e36c2194cd594652d55251f6c0f6","unresolved":false,"context_lines":[{"line_number":398,"context_line":"    :param parent_uuid: The UUID of the parent provider to look up."},{"line_number":399,"context_line":"    \"\"\""},{"line_number":400,"context_line":"    conn \u003d context.session.connection()"},{"line_number":401,"context_line":"    sel_stmt \u003d sa.select([_RP_TBL.c.root_provider_uuid]).where("},{"line_number":402,"context_line":"        _RP_TBL.c.uuid \u003d\u003d parent_uuid)"},{"line_number":403,"context_line":"    res \u003d conn.execute(sel_stmt).fetchall()"},{"line_number":404,"context_line":"    if not res:"},{"line_number":405,"context_line":"        return None"}],"source_content_type":"text/x-python","patch_set":44,"id":"5f4e5783_286c0d35","line":402,"range":{"start_line":401,"start_character":0,"end_line":402,"end_character":38},"in_reply_to":"7f515b1d_f0d6d427","updated":"2017-10-10 15:23:55.000000000","message":"This has all been redone in the latest revision. I still need to add a nova-manage migration patch, though.","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9b5888ab00d1776a29223c16ad919ae62f2773e3","unresolved":false,"context_lines":[{"line_number":454,"context_line":"        # is a non-nullable field."},{"line_number":455,"context_line":"        if self.obj_attr_is_set(\u0027root_provider_uuid\u0027):"},{"line_number":456,"context_line":"            root_uuid \u003d self.root_provider_uuid"},{"line_number":457,"context_line":"        self._delete(self._context, self.id, root_uuid)"},{"line_number":458,"context_line":""},{"line_number":459,"context_line":"    def save(self):"},{"line_number":460,"context_line":"        updates \u003d self.obj_get_changes()"}],"source_content_type":"text/x-python","patch_set":44,"id":"7f515b1d_d0b6388b","line":457,"range":{"start_line":457,"start_character":45,"end_line":457,"end_character":54},"updated":"2017-10-04 15:46:40.000000000","message":"/me surprised - see below","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9b5888ab00d1776a29223c16ad919ae62f2773e3","unresolved":false,"context_lines":[{"line_number":533,"context_line":"        if \u0027parent_provider_uuid\u0027 in updates:"},{"line_number":534,"context_line":"            # If user supplied a parent, make sure it exists"},{"line_number":535,"context_line":"            parent_uuid \u003d updates[\u0027parent_provider_uuid\u0027]"},{"line_number":536,"context_line":"            root_uuid \u003d _root_provider_uuid_from_parent(context, parent_uuid)"},{"line_number":537,"context_line":"            if root_uuid is None:"},{"line_number":538,"context_line":"                raise exception.ObjectActionError("},{"line_number":539,"context_line":"                        action\u003d\u0027create\u0027,"}],"source_content_type":"text/x-python","patch_set":44,"id":"7f515b1d_d37c0274","line":536,"range":{"start_line":536,"start_character":24,"end_line":536,"end_character":77},"updated":"2017-10-04 15:46:40.000000000","message":"This doesn\u0027t seem like the right call to ensure the parent RP exists.  It\u0027s checking that some RP exists whose root_provider_uuid is actually set.  If we\u0027re guaranteed that all RPs will have that field set, then this is okay (if a bit confusing); but I gather from various comments and extra conditionals throughout that this isn\u0027t necessarily guaranteed.\n\nWouldn\u0027t it be clearer and more correct to use get_by_uuid?\n\n[Later] Oh, because you use the root_uuid below.  Which makes sense.  In that case, _root_provider_uuid_from_parent needs to guarantee a result if parent_uuid exists - which I\u0027m not sure it does right now.  See comment L401-2.","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"8f9be0999670e36c2194cd594652d55251f6c0f6","unresolved":false,"context_lines":[{"line_number":533,"context_line":"        if \u0027parent_provider_uuid\u0027 in updates:"},{"line_number":534,"context_line":"            # If user supplied a parent, make sure it exists"},{"line_number":535,"context_line":"            parent_uuid \u003d updates[\u0027parent_provider_uuid\u0027]"},{"line_number":536,"context_line":"            root_uuid \u003d _root_provider_uuid_from_parent(context, parent_uuid)"},{"line_number":537,"context_line":"            if root_uuid is None:"},{"line_number":538,"context_line":"                raise exception.ObjectActionError("},{"line_number":539,"context_line":"                        action\u003d\u0027create\u0027,"}],"source_content_type":"text/x-python","patch_set":44,"id":"5f4e5783_08695124","line":536,"range":{"start_line":536,"start_character":24,"end_line":536,"end_character":77},"in_reply_to":"7f515b1d_d37c0274","updated":"2017-10-10 15:23:55.000000000","message":"All changed in latest rev","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9b5888ab00d1776a29223c16ad919ae62f2773e3","unresolved":false,"context_lines":[{"line_number":549,"context_line":"    @db_api.api_context_manager.writer"},{"line_number":550,"context_line":"    def _delete(context, _id, root_uuid):"},{"line_number":551,"context_line":"        # TODO(jaypipes): We can remove the OR condition below in favor of just"},{"line_number":552,"context_line":"        # a simple WHERE root_provider_uuid \u003d \u003cUUID\u003e once we can be sure all"},{"line_number":553,"context_line":"        # providers have had their root_provider_uuid value populated."},{"line_number":554,"context_line":"        conn \u003d context.session.connection()"},{"line_number":555,"context_line":"        tree_cond \u003d sa.or_(_RP_TBL.c.id \u003d\u003d _id,"},{"line_number":556,"context_line":"                           _RP_TBL.c.root_provider_uuid \u003d\u003d root_uuid)"}],"source_content_type":"text/x-python","patch_set":44,"id":"7f515b1d_301d8cbd","line":553,"range":{"start_line":552,"start_character":53,"end_line":553,"end_character":70},"updated":"2017-10-04 15:46:40.000000000","message":"Why can\u0027t we be sure of that?","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9b5888ab00d1776a29223c16ad919ae62f2773e3","unresolved":false,"context_lines":[{"line_number":552,"context_line":"        # a simple WHERE root_provider_uuid \u003d \u003cUUID\u003e once we can be sure all"},{"line_number":553,"context_line":"        # providers have had their root_provider_uuid value populated."},{"line_number":554,"context_line":"        conn \u003d context.session.connection()"},{"line_number":555,"context_line":"        tree_cond \u003d sa.or_(_RP_TBL.c.id \u003d\u003d _id,"},{"line_number":556,"context_line":"                           _RP_TBL.c.root_provider_uuid \u003d\u003d root_uuid)"},{"line_number":557,"context_line":"        tree_sel \u003d sa.select([_RP_TBL.c.id]).where(tree_cond)"},{"line_number":558,"context_line":"        tree_res \u003d conn.execute(tree_sel)"},{"line_number":559,"context_line":"        tree_ids \u003d [row[0] for row in tree_res]"}],"source_content_type":"text/x-python","patch_set":44,"id":"7f515b1d_13273ab9","line":556,"range":{"start_line":555,"start_character":27,"end_line":556,"end_character":68},"updated":"2017-10-04 15:46:40.000000000","message":"Whoah, do we want to delete the whole tree, even if I asked to delete a leaf or a middle node?  I wouldn\u0027t have thought so.  I would have thought we want to delete the requested node, and possibly its descendants, but leave its ancestors alone.  E.g. given:\n\n ROOT\n  |\n  +-\u003e MID1\n  |    |\n  |    +-\u003e LEAF1\n  |    |\n  |    +-\u003e LEAF2\n  |\n  +-\u003e MID2\n       |\n       +-\u003e LEAF3\n       |\n       +-\u003e LEAF4\n\n...If I ask to delete MID2, I should end up with either:\n\n ROOT\n  |\n  +-\u003e MID1\n       |\n       +-\u003e LEAF1\n       |\n       +-\u003e LEAF2\n\n...or:\n\n ROOT\n  |\n  +-\u003e MID1\n  |    |\n  |    +-\u003e LEAF1\n  |    |\n  |    +-\u003e LEAF2\n  |\n  +-\u003e LEAF3\n  |\n  +-\u003e LEAF4\n\n(Based on comment in your test case, I assume you intended the former.)","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"8f9be0999670e36c2194cd594652d55251f6c0f6","unresolved":false,"context_lines":[{"line_number":552,"context_line":"        # a simple WHERE root_provider_uuid \u003d \u003cUUID\u003e once we can be sure all"},{"line_number":553,"context_line":"        # providers have had their root_provider_uuid value populated."},{"line_number":554,"context_line":"        conn \u003d context.session.connection()"},{"line_number":555,"context_line":"        tree_cond \u003d sa.or_(_RP_TBL.c.id \u003d\u003d _id,"},{"line_number":556,"context_line":"                           _RP_TBL.c.root_provider_uuid \u003d\u003d root_uuid)"},{"line_number":557,"context_line":"        tree_sel \u003d sa.select([_RP_TBL.c.id]).where(tree_cond)"},{"line_number":558,"context_line":"        tree_res \u003d conn.execute(tree_sel)"},{"line_number":559,"context_line":"        tree_ids \u003d [row[0] for row in tree_res]"}],"source_content_type":"text/x-python","patch_set":44,"id":"5f4e5783_28552d51","line":556,"range":{"start_line":555,"start_character":27,"end_line":556,"end_character":68},"in_reply_to":"7f515b1d_13273ab9","updated":"2017-10-10 15:23:55.000000000","message":"fixed in latest rev","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"949d54bf507b2a587b86876f5fb860f340fb0226","unresolved":false,"context_lines":[{"line_number":589,"context_line":"    def _update_in_db(context, id, uuid, updates):"},{"line_number":590,"context_line":"        if \u0027parent_provider_uuid\u0027 in updates:"},{"line_number":591,"context_line":"            if updates[\u0027parent_provider_uuid\u0027] is not None:"},{"line_number":592,"context_line":"                # If user supplied a parent, make sure it exists"},{"line_number":593,"context_line":"                parent_uuid \u003d updates[\u0027parent_provider_uuid\u0027]"},{"line_number":594,"context_line":"                root_uuid \u003d _root_provider_uuid_from_parent("},{"line_number":595,"context_line":"                    context, parent_uuid)"}],"source_content_type":"text/x-python","patch_set":44,"id":"7f515b1d_12e2ffa0","line":592,"updated":"2017-09-28 17:31:16.000000000","message":"Further, if they provided a parent that is outside their root\u0027s tree, we need to explode as well. Or we need to update all our children. Suggest we do the former for the sake of complexity, but if we change it later we\u0027ll need to handle it in the API as a microversion. So, we could try to do the limiting of that there now.","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"f597e6b8834b13df7b13eb00cda11455122a5393","unresolved":false,"context_lines":[{"line_number":589,"context_line":"    def _update_in_db(context, id, uuid, updates):"},{"line_number":590,"context_line":"        if \u0027parent_provider_uuid\u0027 in updates:"},{"line_number":591,"context_line":"            if updates[\u0027parent_provider_uuid\u0027] is not None:"},{"line_number":592,"context_line":"                # If user supplied a parent, make sure it exists"},{"line_number":593,"context_line":"                parent_uuid \u003d updates[\u0027parent_provider_uuid\u0027]"},{"line_number":594,"context_line":"                root_uuid \u003d _root_provider_uuid_from_parent("},{"line_number":595,"context_line":"                    context, parent_uuid)"}],"source_content_type":"text/x-python","patch_set":44,"id":"7f515b1d_25b7cc3c","line":592,"in_reply_to":"7f515b1d_12e2ffa0","updated":"2017-10-01 14:45:27.000000000","message":"Can you expand on this a bit more? thinking out loud: I\u0027m updating an existing RP, so it has a root already, if that root is different, then I also have a previous parent, which may or may not be that root. So you\u0027re saying, \"the new parent needs to be somewhere under the root or me and everyone under me needs to be updated\"\n\nHmmm. Yeah.","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"8f9be0999670e36c2194cd594652d55251f6c0f6","unresolved":false,"context_lines":[{"line_number":589,"context_line":"    def _update_in_db(context, id, uuid, updates):"},{"line_number":590,"context_line":"        if \u0027parent_provider_uuid\u0027 in updates:"},{"line_number":591,"context_line":"            if updates[\u0027parent_provider_uuid\u0027] is not None:"},{"line_number":592,"context_line":"                # If user supplied a parent, make sure it exists"},{"line_number":593,"context_line":"                parent_uuid \u003d updates[\u0027parent_provider_uuid\u0027]"},{"line_number":594,"context_line":"                root_uuid \u003d _root_provider_uuid_from_parent("},{"line_number":595,"context_line":"                    context, parent_uuid)"}],"source_content_type":"text/x-python","patch_set":44,"id":"5f4e5783_a8efdd85","line":592,"in_reply_to":"7f515b1d_12e2ffa0","updated":"2017-10-10 15:23:55.000000000","message":"In the latest rev, you will note that I am always setting the root provider ID to the parent\u0027s root provider ID if the parent_provider_uuid field is set.","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"949d54bf507b2a587b86876f5fb860f340fb0226","unresolved":false,"context_lines":[{"line_number":608,"context_line":""},{"line_number":609,"context_line":"    @staticmethod"},{"line_number":610,"context_line":"    def _from_db_object(context, resource_provider, db_resource_provider):"},{"line_number":611,"context_line":"        # Online data migration to populate root_provider_uuid"},{"line_number":612,"context_line":"        if db_resource_provider[\u0027root_provider_uuid\u0027] is None:"},{"line_number":613,"context_line":"            uuid \u003d db_resource_provider[\u0027uuid\u0027]"},{"line_number":614,"context_line":"            db_resource_provider[\u0027root_provider_uuid\u0027] \u003d uuid"}],"source_content_type":"text/x-python","patch_set":44,"id":"7f515b1d_f249a3b6","line":611,"updated":"2017-09-28 17:31:16.000000000","message":"We need a way to push these manually too","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f590586ec4d47a2d6cc8278d0ba4e53b0257bfd3","unresolved":false,"context_lines":[{"line_number":608,"context_line":""},{"line_number":609,"context_line":"    @staticmethod"},{"line_number":610,"context_line":"    def _from_db_object(context, resource_provider, db_resource_provider):"},{"line_number":611,"context_line":"        # Online data migration to populate root_provider_uuid"},{"line_number":612,"context_line":"        if db_resource_provider[\u0027root_provider_uuid\u0027] is None:"},{"line_number":613,"context_line":"            uuid \u003d db_resource_provider[\u0027uuid\u0027]"},{"line_number":614,"context_line":"            db_resource_provider[\u0027root_provider_uuid\u0027] \u003d uuid"}],"source_content_type":"text/x-python","patch_set":44,"id":"7f515b1d_7ea21cfe","line":611,"in_reply_to":"7f515b1d_f249a3b6","updated":"2017-09-28 20:26:11.000000000","message":"ya, I need to add a stopper patch with a migration.","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ebb214d8462d4aa6726c4e22aecf7abfeb5768a5","unresolved":false,"context_lines":[{"line_number":396,"context_line":"    :param uuid: The UUID to look up"},{"line_number":397,"context_line":"    \"\"\""},{"line_number":398,"context_line":"    # SELECT"},{"line_number":399,"context_line":"    #   rp.id, rp.uuid,"},{"line_number":400,"context_line":"    #   parent.id AS parent_id, parent.uuid AS parent_uuid,"},{"line_number":401,"context_line":"    #   root.id AS root_id, root.uuid AS root_uuid"},{"line_number":402,"context_line":"    # FROM resource_providers AS rp"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_c99e9506","line":399,"updated":"2017-10-11 12:51:01.000000000","message":"rp.name, rp.generation,","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ebb214d8462d4aa6726c4e22aecf7abfeb5768a5","unresolved":false,"context_lines":[{"line_number":397,"context_line":"    \"\"\""},{"line_number":398,"context_line":"    # SELECT"},{"line_number":399,"context_line":"    #   rp.id, rp.uuid,"},{"line_number":400,"context_line":"    #   parent.id AS parent_id, parent.uuid AS parent_uuid,"},{"line_number":401,"context_line":"    #   root.id AS root_id, root.uuid AS root_uuid"},{"line_number":402,"context_line":"    # FROM resource_providers AS rp"},{"line_number":403,"context_line":"    # LEFT JOIN resource_providers AS parent"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_698561ad","line":400,"range":{"start_line":400,"start_character":8,"end_line":400,"end_character":30},"updated":"2017-10-11 12:51:01.000000000","message":"no?","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ebb214d8462d4aa6726c4e22aecf7abfeb5768a5","unresolved":false,"context_lines":[{"line_number":397,"context_line":"    \"\"\""},{"line_number":398,"context_line":"    # SELECT"},{"line_number":399,"context_line":"    #   rp.id, rp.uuid,"},{"line_number":400,"context_line":"    #   parent.id AS parent_id, parent.uuid AS parent_uuid,"},{"line_number":401,"context_line":"    #   root.id AS root_id, root.uuid AS root_uuid"},{"line_number":402,"context_line":"    # FROM resource_providers AS rp"},{"line_number":403,"context_line":"    # LEFT JOIN resource_providers AS parent"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_c9b7b586","line":400,"range":{"start_line":400,"start_character":47,"end_line":400,"end_character":59},"updated":"2017-10-11 12:51:01.000000000","message":"parent_provider_uuid","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ebb214d8462d4aa6726c4e22aecf7abfeb5768a5","unresolved":false,"context_lines":[{"line_number":398,"context_line":"    # SELECT"},{"line_number":399,"context_line":"    #   rp.id, rp.uuid,"},{"line_number":400,"context_line":"    #   parent.id AS parent_id, parent.uuid AS parent_uuid,"},{"line_number":401,"context_line":"    #   root.id AS root_id, root.uuid AS root_uuid"},{"line_number":402,"context_line":"    # FROM resource_providers AS rp"},{"line_number":403,"context_line":"    # LEFT JOIN resource_providers AS parent"},{"line_number":404,"context_line":"    #   ON rp.parent_provider_id \u003d parent.id"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_09beed64","line":401,"range":{"start_line":401,"start_character":8,"end_line":401,"end_character":27},"updated":"2017-10-11 12:51:01.000000000","message":"no?","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ebb214d8462d4aa6726c4e22aecf7abfeb5768a5","unresolved":false,"context_lines":[{"line_number":398,"context_line":"    # SELECT"},{"line_number":399,"context_line":"    #   rp.id, rp.uuid,"},{"line_number":400,"context_line":"    #   parent.id AS parent_id, parent.uuid AS parent_uuid,"},{"line_number":401,"context_line":"    #   root.id AS root_id, root.uuid AS root_uuid"},{"line_number":402,"context_line":"    # FROM resource_providers AS rp"},{"line_number":403,"context_line":"    # LEFT JOIN resource_providers AS parent"},{"line_number":404,"context_line":"    #   ON rp.parent_provider_id \u003d parent.id"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_89b1bd92","line":401,"range":{"start_line":401,"start_character":41,"end_line":401,"end_character":50},"updated":"2017-10-11 12:51:01.000000000","message":"root_provider_uuid","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2e12b3b05873b15ce465d96be33b183ed8169070","unresolved":false,"context_lines":[{"line_number":398,"context_line":"    # SELECT"},{"line_number":399,"context_line":"    #   rp.id, rp.uuid,"},{"line_number":400,"context_line":"    #   parent.id AS parent_id, parent.uuid AS parent_uuid,"},{"line_number":401,"context_line":"    #   root.id AS root_id, root.uuid AS root_uuid"},{"line_number":402,"context_line":"    # FROM resource_providers AS rp"},{"line_number":403,"context_line":"    # LEFT JOIN resource_providers AS parent"},{"line_number":404,"context_line":"    #   ON rp.parent_provider_id \u003d parent.id"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_90666229","line":401,"range":{"start_line":401,"start_character":41,"end_line":401,"end_character":50},"in_reply_to":"5f4e5783_89b1bd92","updated":"2017-10-11 16:26:55.000000000","message":"Sorry, apparently I pasted this SQL code comment in the wrong place. This is the SQL for the _provider_ids_from_uuid() function.","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a4ac0b479b6e40847c80750dd394d6de36e1a114","unresolved":false,"context_lines":[{"line_number":398,"context_line":"    # SELECT"},{"line_number":399,"context_line":"    #   rp.id, rp.uuid,"},{"line_number":400,"context_line":"    #   parent.id AS parent_id, parent.uuid AS parent_uuid,"},{"line_number":401,"context_line":"    #   root.id AS root_id, root.uuid AS root_uuid"},{"line_number":402,"context_line":"    # FROM resource_providers AS rp"},{"line_number":403,"context_line":"    # LEFT JOIN resource_providers AS parent"},{"line_number":404,"context_line":"    #   ON rp.parent_provider_id \u003d parent.id"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_93630cb2","line":401,"range":{"start_line":401,"start_character":41,"end_line":401,"end_character":50},"in_reply_to":"5f4e5783_90666229","updated":"2017-10-11 16:31:53.000000000","message":"Done","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ebb214d8462d4aa6726c4e22aecf7abfeb5768a5","unresolved":false,"context_lines":[{"line_number":466,"context_line":"    # aggregate uuid has never been seen before. Code further"},{"line_number":467,"context_line":"    # below will update the associations."},{"line_number":468,"context_line":"    for agg_uuid in to_add:"},{"line_number":469,"context_line":"        found_agg \u003d context.session.query(models.PlacementAggregate.uuid).\\"},{"line_number":470,"context_line":"            filter_by(uuid\u003dagg_uuid).first()"},{"line_number":471,"context_line":"        if not found_agg:"},{"line_number":472,"context_line":"            new_aggregate \u003d models.PlacementAggregate(uuid\u003dagg_uuid)"},{"line_number":473,"context_line":"            try:"},{"line_number":474,"context_line":"                context.session.add(new_aggregate)"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_5d4a5f55","line":471,"range":{"start_line":469,"start_character":0,"end_line":471,"end_character":25},"updated":"2017-10-11 12:51:01.000000000","message":"Is it really worth doing this, versus just attempting the add and bouncing on DBDuplicateEntry?\n\n[Sorry, realize this isn\u0027t part of this patch, but noticed it.]","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ebb214d8462d4aa6726c4e22aecf7abfeb5768a5","unresolved":false,"context_lines":[{"line_number":517,"context_line":"    \u0027ProviderIds\u0027, \u0027id uuid parent_id parent_uuid root_id root_uuid\u0027)"},{"line_number":518,"context_line":""},{"line_number":519,"context_line":""},{"line_number":520,"context_line":"def _provider_ids_from_uuid(context, uuid):"},{"line_number":521,"context_line":"    \"\"\"Given the UUID of a parent resource provider, returns a namedtuple"},{"line_number":522,"context_line":"    (ProviderIds) with the internal ID, the UUID, the parent provider\u0027s"},{"line_number":523,"context_line":"    internal ID, parent provider\u0027s UUID, the root provider\u0027s internal ID and"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_bdaf1393","line":520,"range":{"start_line":520,"start_character":4,"end_line":520,"end_character":27},"updated":"2017-10-11 12:51:01.000000000","message":"Again I\u0027m not seeing the need for both this and _get_provider_by_uuid (L391).  Am I missing some subtle and important distinction?","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6e1dd07df28d51d45ac4b1d7d726c5ef9d7c05d8","unresolved":false,"context_lines":[{"line_number":517,"context_line":"    \u0027ProviderIds\u0027, \u0027id uuid parent_id parent_uuid root_id root_uuid\u0027)"},{"line_number":518,"context_line":""},{"line_number":519,"context_line":""},{"line_number":520,"context_line":"def _provider_ids_from_uuid(context, uuid):"},{"line_number":521,"context_line":"    \"\"\"Given the UUID of a parent resource provider, returns a namedtuple"},{"line_number":522,"context_line":"    (ProviderIds) with the internal ID, the UUID, the parent provider\u0027s"},{"line_number":523,"context_line":"    internal ID, parent provider\u0027s UUID, the root provider\u0027s internal ID and"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_aa556c52","line":520,"range":{"start_line":520,"start_character":4,"end_line":520,"end_character":27},"in_reply_to":"5f4e5783_bdaf1393","updated":"2017-10-11 14:49:49.000000000","message":"Okay, I see this was done in the next patch in the series.  Thanks!","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ebb214d8462d4aa6726c4e22aecf7abfeb5768a5","unresolved":false,"context_lines":[{"line_number":518,"context_line":""},{"line_number":519,"context_line":""},{"line_number":520,"context_line":"def _provider_ids_from_uuid(context, uuid):"},{"line_number":521,"context_line":"    \"\"\"Given the UUID of a parent resource provider, returns a namedtuple"},{"line_number":522,"context_line":"    (ProviderIds) with the internal ID, the UUID, the parent provider\u0027s"},{"line_number":523,"context_line":"    internal ID, parent provider\u0027s UUID, the root provider\u0027s internal ID and"},{"line_number":524,"context_line":"    the root provider UUID."}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_1dd3c73a","line":521,"range":{"start_line":521,"start_character":27,"end_line":521,"end_character":33},"updated":"2017-10-11 12:51:01.000000000","message":"strike","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a4ac0b479b6e40847c80750dd394d6de36e1a114","unresolved":false,"context_lines":[{"line_number":518,"context_line":""},{"line_number":519,"context_line":""},{"line_number":520,"context_line":"def _provider_ids_from_uuid(context, uuid):"},{"line_number":521,"context_line":"    \"\"\"Given the UUID of a parent resource provider, returns a namedtuple"},{"line_number":522,"context_line":"    (ProviderIds) with the internal ID, the UUID, the parent provider\u0027s"},{"line_number":523,"context_line":"    internal ID, parent provider\u0027s UUID, the root provider\u0027s internal ID and"},{"line_number":524,"context_line":"    the root provider UUID."}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_535d94e7","line":521,"range":{"start_line":521,"start_character":27,"end_line":521,"end_character":33},"in_reply_to":"5f4e5783_1dd3c73a","updated":"2017-10-11 16:31:53.000000000","message":"Done","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ebb214d8462d4aa6726c4e22aecf7abfeb5768a5","unresolved":false,"context_lines":[{"line_number":684,"context_line":""},{"line_number":685,"context_line":"        db_rp \u003d models.ResourceProvider()"},{"line_number":686,"context_line":"        db_rp.update(updates)"},{"line_number":687,"context_line":"        context.session.add(db_rp)"},{"line_number":688,"context_line":"        context.session.flush()"},{"line_number":689,"context_line":""},{"line_number":690,"context_line":"        self.id \u003d db_rp.id"},{"line_number":691,"context_line":"        self.generation \u003d db_rp.generation"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_9df9f781","line":688,"range":{"start_line":687,"start_character":1,"end_line":688,"end_character":31},"updated":"2017-10-11 12:51:01.000000000","message":"Shouldn\u0027t this be removed...","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2e12b3b05873b15ce465d96be33b183ed8169070","unresolved":false,"context_lines":[{"line_number":684,"context_line":""},{"line_number":685,"context_line":"        db_rp \u003d models.ResourceProvider()"},{"line_number":686,"context_line":"        db_rp.update(updates)"},{"line_number":687,"context_line":"        context.session.add(db_rp)"},{"line_number":688,"context_line":"        context.session.flush()"},{"line_number":689,"context_line":""},{"line_number":690,"context_line":"        self.id \u003d db_rp.id"},{"line_number":691,"context_line":"        self.generation \u003d db_rp.generation"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_532594aa","line":688,"range":{"start_line":687,"start_character":1,"end_line":688,"end_character":31},"in_reply_to":"5f4e5783_9df9f781","updated":"2017-10-11 16:26:55.000000000","message":"Nope, needed otherwise db_rp.id isn\u0027t set for line 690.","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ebb214d8462d4aa6726c4e22aecf7abfeb5768a5","unresolved":false,"context_lines":[{"line_number":695,"context_line":"            # root_provider_id needs to be set to this provider\u0027s newly-created"},{"line_number":696,"context_line":"            # internal ID"},{"line_number":697,"context_line":"            db_rp.root_provider_id \u003d db_rp.id"},{"line_number":698,"context_line":"            context.session.add(db_rp)"},{"line_number":699,"context_line":"            context.session.flush()"},{"line_number":700,"context_line":"            self.root_provider_uuid \u003d self.uuid"},{"line_number":701,"context_line":""},{"line_number":702,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_5df37f9f","line":699,"range":{"start_line":698,"start_character":0,"end_line":699,"end_character":35},"updated":"2017-10-11 12:51:01.000000000","message":"...and this pulled out of the conditional?\n\nOtherwise it looks like we\u0027re adding the same record twice, the second time with root_provider_id filled in, which should bounce because the RP UUID is unique-restricted?\n\n[Later] Yeah, I don\u0027t think I saw a unit test that exercises this path.","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"8564cd24e2f331a69d0f90b05cfa47df10e670a6","unresolved":false,"context_lines":[{"line_number":695,"context_line":"            # root_provider_id needs to be set to this provider\u0027s newly-created"},{"line_number":696,"context_line":"            # internal ID"},{"line_number":697,"context_line":"            db_rp.root_provider_id \u003d db_rp.id"},{"line_number":698,"context_line":"            context.session.add(db_rp)"},{"line_number":699,"context_line":"            context.session.flush()"},{"line_number":700,"context_line":"            self.root_provider_uuid \u003d self.uuid"},{"line_number":701,"context_line":""},{"line_number":702,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_76b87e75","line":699,"range":{"start_line":698,"start_character":0,"end_line":699,"end_character":35},"in_reply_to":"5f4e5783_56d462bd","updated":"2017-10-11 17:13:49.000000000","message":"No worries. :)\n\nYeah, so session.add(\u003csome_orm_object\u003e) doesn\u0027t do an INSERT. The Session remembers if the record has already been inserted. It has set the \u003csome_orm_object\u003e.id field to the last insert ID from the DB table and the next time we call context.session.add(\u003csome_orm_object\u003e) it\u0027s going to issue the UPDATE statement.","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2e12b3b05873b15ce465d96be33b183ed8169070","unresolved":false,"context_lines":[{"line_number":695,"context_line":"            # root_provider_id needs to be set to this provider\u0027s newly-created"},{"line_number":696,"context_line":"            # internal ID"},{"line_number":697,"context_line":"            db_rp.root_provider_id \u003d db_rp.id"},{"line_number":698,"context_line":"            context.session.add(db_rp)"},{"line_number":699,"context_line":"            context.session.flush()"},{"line_number":700,"context_line":"            self.root_provider_uuid \u003d self.uuid"},{"line_number":701,"context_line":""},{"line_number":702,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_73577004","line":699,"range":{"start_line":698,"start_character":0,"end_line":699,"end_character":35},"in_reply_to":"5f4e5783_5df37f9f","updated":"2017-10-11 16:26:55.000000000","message":"Nope, we need this second save in the conditional only. This is the path (common) for when we have no parent and need to set the root_provider_id to the ID that was just generated for us by the DB for this provider.","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"04dbd78293603584de06cd17eb82f2347e152098","unresolved":false,"context_lines":[{"line_number":695,"context_line":"            # root_provider_id needs to be set to this provider\u0027s newly-created"},{"line_number":696,"context_line":"            # internal ID"},{"line_number":697,"context_line":"            db_rp.root_provider_id \u003d db_rp.id"},{"line_number":698,"context_line":"            context.session.add(db_rp)"},{"line_number":699,"context_line":"            context.session.flush()"},{"line_number":700,"context_line":"            self.root_provider_uuid \u003d self.uuid"},{"line_number":701,"context_line":""},{"line_number":702,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_73e770e6","line":699,"range":{"start_line":698,"start_character":0,"end_line":699,"end_character":35},"in_reply_to":"5f4e5783_73577004","updated":"2017-10-11 16:40:04.000000000","message":"Oh, so context.session.add(db_rp) also updates the db_rp object; and `updates` doesn\u0027t already contain an `id` field; and this second `add` is smart enough to do an UPDATE instead of INSERT for some reason?  Or should this second one be a .save?","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9ee5f03251e0d253e699e8f3da469b6ac61cbcd1","unresolved":false,"context_lines":[{"line_number":695,"context_line":"            # root_provider_id needs to be set to this provider\u0027s newly-created"},{"line_number":696,"context_line":"            # internal ID"},{"line_number":697,"context_line":"            db_rp.root_provider_id \u003d db_rp.id"},{"line_number":698,"context_line":"            context.session.add(db_rp)"},{"line_number":699,"context_line":"            context.session.flush()"},{"line_number":700,"context_line":"            self.root_provider_uuid \u003d self.uuid"},{"line_number":701,"context_line":""},{"line_number":702,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_56d462bd","line":699,"range":{"start_line":698,"start_character":0,"end_line":699,"end_character":35},"in_reply_to":"5f4e5783_73d1b0cd","updated":"2017-10-11 17:09:33.000000000","message":"Thanks for being patient with my ignorance of how these python constructs map to actual SQL.\n\nWhat I meant was, I sort of assumed that `context.session.add(db_rp)` corresponds to `INSERT INTO db_rp.__table__ VALUES (val for val in db_rp)`\n\nIf that were the case, then the one on L687 makes sense.  But then the one on L698 would INSERT a *second* record, identical to the first except for the root_provider_id being set to the id of the first one.\n\nAnd I further assumed that db_rp.save() would correspond to `UPDATE db_rp.__table__ SET (...) WHERE id \u003d db_rp.id`, which is why I thought the second one should do that instead.\n\nBut digging into oslo_db.sqlalchemy.models.ModelBase a bit, it looks like context.session.add actually *does* have the smarts to do an INSERT or UPDATE as appropriate.  And it\u0027s also functionally identical to context.session.add+flush, except that the latter doesn\u0027t handle transaction semantics.\n\nSo TIL, and you can ignore me :)","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a0e79ae4ca43f65a097334f42bcbe90130f0eab9","unresolved":false,"context_lines":[{"line_number":695,"context_line":"            # root_provider_id needs to be set to this provider\u0027s newly-created"},{"line_number":696,"context_line":"            # internal ID"},{"line_number":697,"context_line":"            db_rp.root_provider_id \u003d db_rp.id"},{"line_number":698,"context_line":"            context.session.add(db_rp)"},{"line_number":699,"context_line":"            context.session.flush()"},{"line_number":700,"context_line":"            self.root_provider_uuid \u003d self.uuid"},{"line_number":701,"context_line":""},{"line_number":702,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_73d1b0cd","line":699,"range":{"start_line":698,"start_character":0,"end_line":699,"end_character":35},"in_reply_to":"5f4e5783_73e770e6","updated":"2017-10-11 16:44:21.000000000","message":"the line:\n\n db_rp.update(updates)\n\nDoes nothing but set db_rp\u0027s attributes that match the keys in the updates dict to the value in that dict.\n\nThere\u0027s no need to call db_rp.update() here since I\u0027m just setting the db_rp.root_provider_id value manually.\n\nAlso, not sure what you mean by \"should this second one be a .save\"?","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ebb214d8462d4aa6726c4e22aecf7abfeb5768a5","unresolved":false,"context_lines":[{"line_number":704,"context_line":"    def _delete(context, _id):"},{"line_number":705,"context_line":"        conn \u003d context.session.connection()"},{"line_number":706,"context_line":""},{"line_number":707,"context_line":"        # Don\u0027t allow deleting a provider if it has any child providers at all"},{"line_number":708,"context_line":"        child_sel \u003d sa.select([_RP_TBL.c.id])"},{"line_number":709,"context_line":"        child_sel \u003d child_sel.where(_RP_TBL.c.parent_provider_id \u003d\u003d _id)"},{"line_number":710,"context_line":"        child_res \u003d conn.execute(child_sel.limit(1)).fetchone()"},{"line_number":711,"context_line":"        if child_res:"},{"line_number":712,"context_line":"            raise exception.CannotDeleteParentResourceProvider()"},{"line_number":713,"context_line":""},{"line_number":714,"context_line":"        # Don\u0027t delete the resource provider if it has allocations."},{"line_number":715,"context_line":"        rp_allocations \u003d context.session.query(models.Allocation).\\"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_23308f56","line":712,"range":{"start_line":707,"start_character":0,"end_line":712,"end_character":64},"updated":"2017-10-11 12:51:01.000000000","message":"Maybe I\u0027m being overly paranoid, but does anything about this method prevent a child provider being created between this block and the stuff below?\n\nIf this is an issue, perhaps we should consider deleting the RP record first, and doing it conditionally, like:\n\nDELETE FROM ResourceProvider WHERE ID \u003d $_id AND NOT EXISTS (SELECT id FROM ResourceProvider WHERE parent_provider_id \u003d $_id)\n\nThen raise CannotDeleteParentResourceProvider if that guy returns a zero deleted row count.\n\nThat way any subsequent child creations would bounce because the parent\u0027s RP UUID no longer exists.","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2e12b3b05873b15ce465d96be33b183ed8169070","unresolved":false,"context_lines":[{"line_number":704,"context_line":"    def _delete(context, _id):"},{"line_number":705,"context_line":"        conn \u003d context.session.connection()"},{"line_number":706,"context_line":""},{"line_number":707,"context_line":"        # Don\u0027t allow deleting a provider if it has any child providers at all"},{"line_number":708,"context_line":"        child_sel \u003d sa.select([_RP_TBL.c.id])"},{"line_number":709,"context_line":"        child_sel \u003d child_sel.where(_RP_TBL.c.parent_provider_id \u003d\u003d _id)"},{"line_number":710,"context_line":"        child_res \u003d conn.execute(child_sel.limit(1)).fetchone()"},{"line_number":711,"context_line":"        if child_res:"},{"line_number":712,"context_line":"            raise exception.CannotDeleteParentResourceProvider()"},{"line_number":713,"context_line":""},{"line_number":714,"context_line":"        # Don\u0027t delete the resource provider if it has allocations."},{"line_number":715,"context_line":"        rp_allocations \u003d context.session.query(models.Allocation).\\"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_d35e2411","line":712,"range":{"start_line":707,"start_character":0,"end_line":712,"end_character":64},"in_reply_to":"5f4e5783_23308f56","updated":"2017-10-11 16:26:55.000000000","message":"No, you\u0027re right, there\u0027s a possible race. I should wrap the entire thing in a transaction. I was trying to change as little code as possible here but failed.","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ebb214d8462d4aa6726c4e22aecf7abfeb5768a5","unresolved":false,"context_lines":[{"line_number":726,"context_line":"        RPA_model \u003d models.ResourceProviderAggregate"},{"line_number":727,"context_line":"        context.session.query(RPA_model).\\"},{"line_number":728,"context_line":"                filter(RPA_model.resource_provider_id \u003d\u003d _id).delete()"},{"line_number":729,"context_line":"        # Now delete the RP records"},{"line_number":730,"context_line":"        result \u003d context.session.query(models.ResourceProvider).\\"},{"line_number":731,"context_line":"                 filter(models.ResourceProvider.id \u003d\u003d _id).\\"},{"line_number":732,"context_line":"                 delete(synchronize_session\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_438a6b1b","line":729,"range":{"start_line":729,"start_character":28,"end_line":729,"end_character":35},"updated":"2017-10-11 12:51:01.000000000","message":"record","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"8564cd24e2f331a69d0f90b05cfa47df10e670a6","unresolved":false,"context_lines":[{"line_number":726,"context_line":"        RPA_model \u003d models.ResourceProviderAggregate"},{"line_number":727,"context_line":"        context.session.query(RPA_model).\\"},{"line_number":728,"context_line":"                filter(RPA_model.resource_provider_id \u003d\u003d _id).delete()"},{"line_number":729,"context_line":"        # Now delete the RP records"},{"line_number":730,"context_line":"        result \u003d context.session.query(models.ResourceProvider).\\"},{"line_number":731,"context_line":"                 filter(models.ResourceProvider.id \u003d\u003d _id).\\"},{"line_number":732,"context_line":"                 delete(synchronize_session\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_f6c58e3d","line":729,"range":{"start_line":729,"start_character":28,"end_line":729,"end_character":35},"in_reply_to":"5f4e5783_438a6b1b","updated":"2017-10-11 17:13:49.000000000","message":"Done","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ebb214d8462d4aa6726c4e22aecf7abfeb5768a5","unresolved":false,"context_lines":[{"line_number":738,"context_line":"        if \u0027parent_provider_uuid\u0027 in updates:"},{"line_number":739,"context_line":"            parent_uuid \u003d updates.pop(\u0027parent_provider_uuid\u0027)"},{"line_number":740,"context_line":"            if parent_uuid is not None:"},{"line_number":741,"context_line":"                # User supplied a parent, let\u0027s make sure it exists"},{"line_number":742,"context_line":"                parent_ids \u003d _provider_ids_from_uuid(context, parent_uuid)"},{"line_number":743,"context_line":"                if parent_ids is None:"},{"line_number":744,"context_line":"                    raise exception.ObjectActionError("},{"line_number":745,"context_line":"                            action\u003d\u0027create\u0027,"},{"line_number":746,"context_line":"                            reason\u003d\u0027parent provider UUID does not exist.\u0027)"},{"line_number":747,"context_line":"                updates[\u0027root_provider_id\u0027] \u003d parent_ids.root_id"},{"line_number":748,"context_line":"                updates[\u0027parent_provider_id\u0027] \u003d parent_ids.id"},{"line_number":749,"context_line":"                self.root_provider_uuid \u003d parent_ids.root_uuid"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_63a8c71e","line":746,"range":{"start_line":741,"start_character":0,"end_line":746,"end_character":74},"updated":"2017-10-11 12:51:01.000000000","message":"Similar concern about the parent being deleted between here and below.  UPDATE ... WHERE EXISTS (SELECT id from ResourceProvider WHERE id \u003d\u003d $parent_provider_uuid) ?","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2e12b3b05873b15ce465d96be33b183ed8169070","unresolved":false,"context_lines":[{"line_number":738,"context_line":"        if \u0027parent_provider_uuid\u0027 in updates:"},{"line_number":739,"context_line":"            parent_uuid \u003d updates.pop(\u0027parent_provider_uuid\u0027)"},{"line_number":740,"context_line":"            if parent_uuid is not None:"},{"line_number":741,"context_line":"                # User supplied a parent, let\u0027s make sure it exists"},{"line_number":742,"context_line":"                parent_ids \u003d _provider_ids_from_uuid(context, parent_uuid)"},{"line_number":743,"context_line":"                if parent_ids is None:"},{"line_number":744,"context_line":"                    raise exception.ObjectActionError("},{"line_number":745,"context_line":"                            action\u003d\u0027create\u0027,"},{"line_number":746,"context_line":"                            reason\u003d\u0027parent provider UUID does not exist.\u0027)"},{"line_number":747,"context_line":"                updates[\u0027root_provider_id\u0027] \u003d parent_ids.root_id"},{"line_number":748,"context_line":"                updates[\u0027parent_provider_id\u0027] \u003d parent_ids.id"},{"line_number":749,"context_line":"                self.root_provider_uuid \u003d parent_ids.root_uuid"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_93ec2c2a","line":746,"range":{"start_line":741,"start_character":0,"end_line":746,"end_character":74},"in_reply_to":"5f4e5783_63a8c71e","updated":"2017-10-11 16:26:55.000000000","message":"The foreign key constraint on parent_provider_id will raise an integrity error if that happens. I\u0027ll add an except block.","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a4ac0b479b6e40847c80750dd394d6de36e1a114","unresolved":false,"context_lines":[{"line_number":738,"context_line":"        if \u0027parent_provider_uuid\u0027 in updates:"},{"line_number":739,"context_line":"            parent_uuid \u003d updates.pop(\u0027parent_provider_uuid\u0027)"},{"line_number":740,"context_line":"            if parent_uuid is not None:"},{"line_number":741,"context_line":"                # User supplied a parent, let\u0027s make sure it exists"},{"line_number":742,"context_line":"                parent_ids \u003d _provider_ids_from_uuid(context, parent_uuid)"},{"line_number":743,"context_line":"                if parent_ids is None:"},{"line_number":744,"context_line":"                    raise exception.ObjectActionError("},{"line_number":745,"context_line":"                            action\u003d\u0027create\u0027,"},{"line_number":746,"context_line":"                            reason\u003d\u0027parent provider UUID does not exist.\u0027)"},{"line_number":747,"context_line":"                updates[\u0027root_provider_id\u0027] \u003d parent_ids.root_id"},{"line_number":748,"context_line":"                updates[\u0027parent_provider_id\u0027] \u003d parent_ids.id"},{"line_number":749,"context_line":"                self.root_provider_uuid \u003d parent_ids.root_uuid"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_f31b6018","line":746,"range":{"start_line":741,"start_character":0,"end_line":746,"end_character":74},"in_reply_to":"5f4e5783_93ec2c2a","updated":"2017-10-11 16:31:53.000000000","message":"LOL, only I forgot to include the FK definitions in the damn schema migration... will add in next rev.","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ebb214d8462d4aa6726c4e22aecf7abfeb5768a5","unresolved":false,"context_lines":[{"line_number":1326,"context_line":"        #          \u0027MEMORY_MB\u0027: 1024"},{"line_number":1327,"context_line":"        #      }"},{"line_number":1328,"context_line":"        #  }"},{"line_number":1329,"context_line":"        if not filters:"},{"line_number":1330,"context_line":"            filters \u003d {}"},{"line_number":1331,"context_line":"        else:"},{"line_number":1332,"context_line":"            # Since we modify the filters, copy them so that we don\u0027t modify"},{"line_number":1333,"context_line":"            # them in the calling program."},{"line_number":1334,"context_line":"            filters \u003d copy.deepcopy(filters)"},{"line_number":1335,"context_line":"        name \u003d filters.pop(\u0027name\u0027, None)"},{"line_number":1336,"context_line":"        uuid \u003d filters.pop(\u0027uuid\u0027, None)"},{"line_number":1337,"context_line":"        member_of \u003d filters.pop(\u0027member_of\u0027, [])"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_764ac755","line":1334,"range":{"start_line":1329,"start_character":0,"end_line":1334,"end_character":44},"updated":"2017-10-11 12:51:01.000000000","message":"More tersely:\n\n filters \u003d copy.deepcopy(filters) or {}","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ebb214d8462d4aa6726c4e22aecf7abfeb5768a5","unresolved":false,"context_lines":[{"line_number":1341,"context_line":"        # and we want to make sure those class names aren\u0027t incorrect."},{"line_number":1342,"context_line":"        resources \u003d {_RC_CACHE.id_from_string(r_name): amount"},{"line_number":1343,"context_line":"                     for r_name, amount in resources.items()}"},{"line_number":1344,"context_line":"        rp \u003d sa.alias(_RP_TBL, name\u003d\"rp\")"},{"line_number":1345,"context_line":"        root_rp \u003d sa.alias(_RP_TBL, name\u003d\"root_rp\")"},{"line_number":1346,"context_line":"        parent_rp \u003d sa.alias(_RP_TBL, name\u003d\"parent_rp\")"},{"line_number":1347,"context_line":""},{"line_number":1348,"context_line":"        cols \u003d ["},{"line_number":1349,"context_line":"            rp.c.id,"},{"line_number":1350,"context_line":"            rp.c.uuid,"},{"line_number":1351,"context_line":"            rp.c.name,"},{"line_number":1352,"context_line":"            rp.c.generation,"},{"line_number":1353,"context_line":"            root_rp.c.uuid.label(\"root_provider_uuid\"),"},{"line_number":1354,"context_line":"            parent_rp.c.uuid.label(\"parent_provider_uuid\"),"},{"line_number":1355,"context_line":"        ]"},{"line_number":1356,"context_line":""},{"line_number":1357,"context_line":"        # TODO(jaypipes): Convert this to an inner join once all"},{"line_number":1358,"context_line":"        # root_provider_id values are NOT NULL"},{"line_number":1359,"context_line":"        rp_to_root \u003d sa.outerjoin(rp, root_rp,"},{"line_number":1360,"context_line":"            rp.c.root_provider_id \u003d\u003d root_rp.c.id)"},{"line_number":1361,"context_line":"        rp_to_parent \u003d sa.outerjoin(rp_to_root, parent_rp,"},{"line_number":1362,"context_line":"            rp.c.parent_provider_id \u003d\u003d parent_rp.c.id)"},{"line_number":1363,"context_line":""},{"line_number":1364,"context_line":"        query \u003d sa.select(cols).select_from(rp_to_parent)"},{"line_number":1365,"context_line":""},{"line_number":1366,"context_line":"        if name:"},{"line_number":1367,"context_line":"            query \u003d query.where(rp.c.name \u003d\u003d name)"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_f67d976e","line":1364,"range":{"start_line":1344,"start_character":0,"end_line":1364,"end_character":57},"updated":"2017-10-11 12:51:01.000000000","message":"This chunk seems to be duplicated in a few places; should be factored out (in a future change).","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ebb214d8462d4aa6726c4e22aecf7abfeb5768a5","unresolved":false,"context_lines":[{"line_number":1363,"context_line":""},{"line_number":1364,"context_line":"        query \u003d sa.select(cols).select_from(rp_to_parent)"},{"line_number":1365,"context_line":""},{"line_number":1366,"context_line":"        if name:"},{"line_number":1367,"context_line":"            query \u003d query.where(rp.c.name \u003d\u003d name)"},{"line_number":1368,"context_line":"        if uuid:"},{"line_number":1369,"context_line":"            query \u003d query.where(rp.c.uuid \u003d\u003d uuid)"},{"line_number":1370,"context_line":""},{"line_number":1371,"context_line":"        # If \u0027member_of\u0027 has values join with the PlacementAggregates to"},{"line_number":1372,"context_line":"        # get those resource providers that are associated with any of the"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_16eef321","line":1369,"range":{"start_line":1366,"start_character":0,"end_line":1369,"end_character":50},"updated":"2017-10-11 12:51:01.000000000","message":"Assume this implicitly ANDs","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2e12b3b05873b15ce465d96be33b183ed8169070","unresolved":false,"context_lines":[{"line_number":1363,"context_line":""},{"line_number":1364,"context_line":"        query \u003d sa.select(cols).select_from(rp_to_parent)"},{"line_number":1365,"context_line":""},{"line_number":1366,"context_line":"        if name:"},{"line_number":1367,"context_line":"            query \u003d query.where(rp.c.name \u003d\u003d name)"},{"line_number":1368,"context_line":"        if uuid:"},{"line_number":1369,"context_line":"            query \u003d query.where(rp.c.uuid \u003d\u003d uuid)"},{"line_number":1370,"context_line":""},{"line_number":1371,"context_line":"        # If \u0027member_of\u0027 has values join with the PlacementAggregates to"},{"line_number":1372,"context_line":"        # get those resource providers that are associated with any of the"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_f3e6c044","line":1369,"range":{"start_line":1366,"start_character":0,"end_line":1369,"end_character":50},"in_reply_to":"5f4e5783_16eef321","updated":"2017-10-11 16:26:55.000000000","message":"correct.","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ebb214d8462d4aa6726c4e22aecf7abfeb5768a5","unresolved":false,"context_lines":[{"line_number":1380,"context_line":"                [resource_provider_id]).select_from(join_statement)"},{"line_number":1381,"context_line":"            query \u003d query.where(rp.c.id.in_(rps_in_aggregates))"},{"line_number":1382,"context_line":""},{"line_number":1383,"context_line":"        if not resources:"},{"line_number":1384,"context_line":"            # Returns quickly the list in case we don\u0027t need to check the"},{"line_number":1385,"context_line":"            # resource usage"},{"line_number":1386,"context_line":"            return [dict(r) for r in context.session.execute(query).fetchall()]"},{"line_number":1387,"context_line":""},{"line_number":1388,"context_line":"        # NOTE(sbauza): In case we want to look at the resource criteria, then"},{"line_number":1389,"context_line":"        # the SQL generated from this case looks something like:"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_1694b336","line":1386,"range":{"start_line":1383,"start_character":0,"end_line":1386,"end_character":79},"updated":"2017-10-11 12:51:01.000000000","message":"This is slightly ew; might read better as e.g.:\n\n if resources:\n     query \u003d query.where(_resource_usage_filter(resources))\n\n return [dict(r)...]\n\nAgain, a refactor for a future change.","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"8564cd24e2f331a69d0f90b05cfa47df10e670a6","unresolved":false,"context_lines":[{"line_number":1380,"context_line":"                [resource_provider_id]).select_from(join_statement)"},{"line_number":1381,"context_line":"            query \u003d query.where(rp.c.id.in_(rps_in_aggregates))"},{"line_number":1382,"context_line":""},{"line_number":1383,"context_line":"        if not resources:"},{"line_number":1384,"context_line":"            # Returns quickly the list in case we don\u0027t need to check the"},{"line_number":1385,"context_line":"            # resource usage"},{"line_number":1386,"context_line":"            return [dict(r) for r in context.session.execute(query).fetchall()]"},{"line_number":1387,"context_line":""},{"line_number":1388,"context_line":"        # NOTE(sbauza): In case we want to look at the resource criteria, then"},{"line_number":1389,"context_line":"        # the SQL generated from this case looks something like:"}],"source_content_type":"text/x-python","patch_set":45,"id":"5f4e5783_36d1e6fc","line":1386,"range":{"start_line":1383,"start_character":0,"end_line":1386,"end_character":79},"in_reply_to":"5f4e5783_1694b336","updated":"2017-10-11 17:13:49.000000000","message":"Done","commit_id":"bcd1505925ec266a6dee2aec5ea7f99a676df925"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6ff2d7abe294b2e1961a719c2038a5187bda162b","unresolved":false,"context_lines":[{"line_number":491,"context_line":"        conn.execute(insert_aggregates)"},{"line_number":492,"context_line":""},{"line_number":493,"context_line":""},{"line_number":494,"context_line":"@db_api.api_context_manager.writer"},{"line_number":495,"context_line":"def _has_child_providers(context, rp_id):"},{"line_number":496,"context_line":"    \"\"\"Returns True if the supplied resource provider has any child providers,"},{"line_number":497,"context_line":"    Falso otherwise"}],"source_content_type":"text/x-python","patch_set":48,"id":"5f4e5783_618676de","line":494,"range":{"start_line":494,"start_character":0,"end_line":494,"end_character":34},"updated":"2017-10-11 17:51:55.000000000","message":"Is this a writer?","commit_id":"26e2c9169ed043990a291e8111fb473e93bf2daf"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cb1b81544408f88bfe6a1fef7a13a16c5b6681ec","unresolved":false,"context_lines":[{"line_number":491,"context_line":"        conn.execute(insert_aggregates)"},{"line_number":492,"context_line":""},{"line_number":493,"context_line":""},{"line_number":494,"context_line":"@db_api.api_context_manager.writer"},{"line_number":495,"context_line":"def _has_child_providers(context, rp_id):"},{"line_number":496,"context_line":"    \"\"\"Returns True if the supplied resource provider has any child providers,"},{"line_number":497,"context_line":"    Falso otherwise"}],"source_content_type":"text/x-python","patch_set":48,"id":"5f4e5783_5c0c813a","line":494,"range":{"start_line":494,"start_character":0,"end_line":494,"end_character":34},"in_reply_to":"5f4e5783_618676de","updated":"2017-10-11 17:55:23.000000000","message":"oink, nice catch. copy pasta.","commit_id":"26e2c9169ed043990a291e8111fb473e93bf2daf"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6ff2d7abe294b2e1961a719c2038a5187bda162b","unresolved":false,"context_lines":[{"line_number":776,"context_line":"        db_rp \u003d context.session.query(models.ResourceProvider).filter_by("},{"line_number":777,"context_line":"            id\u003did).first()"},{"line_number":778,"context_line":"        db_rp.update(updates)"},{"line_number":779,"context_line":"        db_rp.save(context.session)"},{"line_number":780,"context_line":""},{"line_number":781,"context_line":"    @staticmethod"},{"line_number":782,"context_line":"    @db_api.api_context_manager.writer  # Needed for online data migration"}],"source_content_type":"text/x-python","patch_set":48,"id":"5f4e5783_01ac6232","line":779,"range":{"start_line":779,"start_character":0,"end_line":779,"end_character":35},"updated":"2017-10-11 17:51:55.000000000","message":"Did you want to convert IntegrityError to the same ObjectActionError from L765 (race condition between pre-transaction check and here)?","commit_id":"26e2c9169ed043990a291e8111fb473e93bf2daf"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cb1b81544408f88bfe6a1fef7a13a16c5b6681ec","unresolved":false,"context_lines":[{"line_number":776,"context_line":"        db_rp \u003d context.session.query(models.ResourceProvider).filter_by("},{"line_number":777,"context_line":"            id\u003did).first()"},{"line_number":778,"context_line":"        db_rp.update(updates)"},{"line_number":779,"context_line":"        db_rp.save(context.session)"},{"line_number":780,"context_line":""},{"line_number":781,"context_line":"    @staticmethod"},{"line_number":782,"context_line":"    @db_api.api_context_manager.writer  # Needed for online data migration"}],"source_content_type":"text/x-python","patch_set":48,"id":"5f4e5783_5c25a1aa","line":779,"range":{"start_line":779,"start_character":0,"end_line":779,"end_character":35},"in_reply_to":"5f4e5783_01ac6232","updated":"2017-10-11 17:55:23.000000000","message":"yeah, good point. will do.","commit_id":"26e2c9169ed043990a291e8111fb473e93bf2daf"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6ff2d7abe294b2e1961a719c2038a5187bda162b","unresolved":false,"context_lines":[{"line_number":1404,"context_line":"        if not resources:"},{"line_number":1405,"context_line":"            # Returns quickly the list in case we don\u0027t need to check the"},{"line_number":1406,"context_line":"            # resource usage"},{"line_number":1407,"context_line":"            res \u003d context.session.execute(query).fetchall()"},{"line_number":1408,"context_line":"            return [dict(r) for r in res]"},{"line_number":1409,"context_line":""},{"line_number":1410,"context_line":"        # NOTE(sbauza): In case we want to look at the resource criteria, then"}],"source_content_type":"text/x-python","patch_set":48,"id":"5f4e5783_6149d689","line":1407,"updated":"2017-10-11 17:51:55.000000000","message":"I don\u0027t object to this, but it\u0027s not at all what I was suggesting in PS45.  Again, it\u0027s something for a followon change set, but my point was that it would read better if the resources subquery were factored out into a separate method.","commit_id":"26e2c9169ed043990a291e8111fb473e93bf2daf"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cb1b81544408f88bfe6a1fef7a13a16c5b6681ec","unresolved":false,"context_lines":[{"line_number":1404,"context_line":"        if not resources:"},{"line_number":1405,"context_line":"            # Returns quickly the list in case we don\u0027t need to check the"},{"line_number":1406,"context_line":"            # resource usage"},{"line_number":1407,"context_line":"            res \u003d context.session.execute(query).fetchall()"},{"line_number":1408,"context_line":"            return [dict(r) for r in res]"},{"line_number":1409,"context_line":""},{"line_number":1410,"context_line":"        # NOTE(sbauza): In case we want to look at the resource criteria, then"}],"source_content_type":"text/x-python","patch_set":48,"id":"5f4e5783_bc4db573","line":1407,"in_reply_to":"5f4e5783_6149d689","updated":"2017-10-11 17:55:23.000000000","message":"oh! sorry, yeah, I can work on that in a later refactoring.","commit_id":"26e2c9169ed043990a291e8111fb473e93bf2daf"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"1cad2d7f560896aebb8cd74ea6d8903b2d8399a4","unresolved":false,"context_lines":[{"line_number":494,"context_line":"@db_api.api_context_manager.reader"},{"line_number":495,"context_line":"def _has_child_providers(context, rp_id):"},{"line_number":496,"context_line":"    \"\"\"Returns True if the supplied resource provider has any child providers,"},{"line_number":497,"context_line":"    Falso otherwise"},{"line_number":498,"context_line":"    \"\"\""},{"line_number":499,"context_line":"    # Don\u0027t allow deleting a provider if it has any child providers at all"},{"line_number":500,"context_line":"    child_sel \u003d sa.select([_RP_TBL.c.id])"}],"source_content_type":"text/x-python","patch_set":49,"id":"5f4e5783_de8144fd","line":497,"range":{"start_line":497,"start_character":4,"end_line":497,"end_character":9},"updated":"2017-10-13 17:28:54.000000000","message":"False","commit_id":"7f1093bf35fbcdabb582aa2e8aed35ed8b339ed1"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"1cad2d7f560896aebb8cd74ea6d8903b2d8399a4","unresolved":false,"context_lines":[{"line_number":696,"context_line":"            updates[\u0027parent_provider_id\u0027] \u003d parent_id"},{"line_number":697,"context_line":"            self.root_provider_uuid \u003d parent_ids.root_uuid"},{"line_number":698,"context_line":""},{"line_number":699,"context_line":"        db_rp \u003d models.ResourceProvider()"},{"line_number":700,"context_line":"        db_rp.update(updates)"},{"line_number":701,"context_line":"        context.session.add(db_rp)"},{"line_number":702,"context_line":"        context.session.flush()"}],"source_content_type":"text/x-python","patch_set":49,"id":"5f4e5783_3ec0582a","line":699,"updated":"2017-10-13 17:28:54.000000000","message":"Is there a reason this orm-ism wasn\u0027t changed in the the de-orm series? just curious?","commit_id":"7f1093bf35fbcdabb582aa2e8aed35ed8b339ed1"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"06f8ee5e5fb68f71f7713fae0a115b3320d6392d","unresolved":false,"context_lines":[{"line_number":696,"context_line":"            updates[\u0027parent_provider_id\u0027] \u003d parent_id"},{"line_number":697,"context_line":"            self.root_provider_uuid \u003d parent_ids.root_uuid"},{"line_number":698,"context_line":""},{"line_number":699,"context_line":"        db_rp \u003d models.ResourceProvider()"},{"line_number":700,"context_line":"        db_rp.update(updates)"},{"line_number":701,"context_line":"        context.session.add(db_rp)"},{"line_number":702,"context_line":"        context.session.flush()"}],"source_content_type":"text/x-python","patch_set":49,"id":"3f4b6375_20b64cfc","line":699,"in_reply_to":"3f4b6375_e021544f","updated":"2017-10-19 16:39:24.000000000","message":"Thanks, good to know.\n\nTurns out it is fairly useful to keep it this way anyway: without the orm, the defaults for updated_at and created_at, brought in from the oslo_db TimestampMixin wouldn\u0027t work. They are done orm side, not in db, for updated_at.","commit_id":"7f1093bf35fbcdabb582aa2e8aed35ed8b339ed1"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a36fa7b4e11e61207df5a8dd24b7d9505c7067ae","unresolved":false,"context_lines":[{"line_number":696,"context_line":"            updates[\u0027parent_provider_id\u0027] \u003d parent_id"},{"line_number":697,"context_line":"            self.root_provider_uuid \u003d parent_ids.root_uuid"},{"line_number":698,"context_line":""},{"line_number":699,"context_line":"        db_rp \u003d models.ResourceProvider()"},{"line_number":700,"context_line":"        db_rp.update(updates)"},{"line_number":701,"context_line":"        context.session.add(db_rp)"},{"line_number":702,"context_line":"        context.session.flush()"}],"source_content_type":"text/x-python","patch_set":49,"id":"3f4b6375_e021544f","line":699,"in_reply_to":"5f4e5783_3ec0582a","updated":"2017-10-19 16:36:33.000000000","message":"The de-orm series doesn\u0027t change the single-object update/insert calls. It only changed the read queries, which were the things I needed to change in order to do the parent_provider_id/root_provider_id self-referential joins without producing a crazy-inefficient query plan for every ORM-using query that touched the ResourceProvider model.","commit_id":"7f1093bf35fbcdabb582aa2e8aed35ed8b339ed1"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"86d473817c4e512fdc93beb496cc8dc95c77137c","unresolved":false,"context_lines":[{"line_number":497,"context_line":"    \"\"\"Returns True if the supplied resource provider has any child providers,"},{"line_number":498,"context_line":"    Falso otherwise"},{"line_number":499,"context_line":"    \"\"\""},{"line_number":500,"context_line":"    # Don\u0027t allow deleting a provider if it has any child providers at all"},{"line_number":501,"context_line":"    child_sel \u003d sa.select([_RP_TBL.c.id])"},{"line_number":502,"context_line":"    child_sel \u003d child_sel.where(_RP_TBL.c.parent_provider_id \u003d\u003d rp_id)"},{"line_number":503,"context_line":"    child_res \u003d context.session.execute(child_sel.limit(1)).fetchone()"}],"source_content_type":"text/x-python","patch_set":52,"id":"3f4b6375_a1658c47","line":500,"updated":"2017-10-20 12:06:31.000000000","message":"I think this comment is unnecessary and a bit misleading here. The _delete() already has a nice comment stating this.","commit_id":"be733dbad357e1a095b7c2892c86a673db6d05d7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6c0085e728c9d57740ae86dbde7ca249f4656fcb","unresolved":false,"context_lines":[{"line_number":497,"context_line":"    \"\"\"Returns True if the supplied resource provider has any child providers,"},{"line_number":498,"context_line":"    Falso otherwise"},{"line_number":499,"context_line":"    \"\"\""},{"line_number":500,"context_line":"    # Don\u0027t allow deleting a provider if it has any child providers at all"},{"line_number":501,"context_line":"    child_sel \u003d sa.select([_RP_TBL.c.id])"},{"line_number":502,"context_line":"    child_sel \u003d child_sel.where(_RP_TBL.c.parent_provider_id \u003d\u003d rp_id)"},{"line_number":503,"context_line":"    child_res \u003d context.session.execute(child_sel.limit(1)).fetchone()"}],"source_content_type":"text/x-python","patch_set":52,"id":"3f4b6375_78ea9eac","line":500,"in_reply_to":"3f4b6375_225ab74e","updated":"2017-10-20 16:21:08.000000000","message":"Done","commit_id":"be733dbad357e1a095b7c2892c86a673db6d05d7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0dfd467663fe71fa6afbe6b71a094a2af777c965","unresolved":false,"context_lines":[{"line_number":497,"context_line":"    \"\"\"Returns True if the supplied resource provider has any child providers,"},{"line_number":498,"context_line":"    Falso otherwise"},{"line_number":499,"context_line":"    \"\"\""},{"line_number":500,"context_line":"    # Don\u0027t allow deleting a provider if it has any child providers at all"},{"line_number":501,"context_line":"    child_sel \u003d sa.select([_RP_TBL.c.id])"},{"line_number":502,"context_line":"    child_sel \u003d child_sel.where(_RP_TBL.c.parent_provider_id \u003d\u003d rp_id)"},{"line_number":503,"context_line":"    child_res \u003d context.session.execute(child_sel.limit(1)).fetchone()"}],"source_content_type":"text/x-python","patch_set":52,"id":"3f4b6375_225ab74e","line":500,"in_reply_to":"3f4b6375_a1658c47","updated":"2017-10-20 15:04:40.000000000","message":"Sorry, copy pasta :)","commit_id":"be733dbad357e1a095b7c2892c86a673db6d05d7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"86d473817c4e512fdc93beb496cc8dc95c77137c","unresolved":false,"context_lines":[{"line_number":687,"context_line":"        parent_uuid \u003d updates.pop(\u0027parent_provider_uuid\u0027)"},{"line_number":688,"context_line":"        if parent_uuid is not None:"},{"line_number":689,"context_line":"            parent_ids \u003d _provider_ids_from_uuid(context, parent_uuid)"},{"line_number":690,"context_line":"            if parent_ids is None:"},{"line_number":691,"context_line":"                raise exception.ObjectActionError("},{"line_number":692,"context_line":"                        action\u003d\u0027create\u0027,"},{"line_number":693,"context_line":"                        reason\u003d\u0027parent provider UUID does not exist.\u0027)"},{"line_number":694,"context_line":"            parent_id \u003d parent_ids.id"},{"line_number":695,"context_line":"            root_id \u003d parent_ids.root_id"},{"line_number":696,"context_line":"            updates[\u0027root_provider_id\u0027] \u003d root_id"},{"line_number":697,"context_line":"            updates[\u0027parent_provider_id\u0027] \u003d parent_id"}],"source_content_type":"text/x-python","patch_set":52,"id":"3f4b6375_418938c8","line":694,"range":{"start_line":690,"start_character":1,"end_line":694,"end_character":1},"updated":"2017-10-20 12:06:31.000000000","message":"We might need to check if the provided parent_uuid is not the same as our own uuid. I guess that would cause some strange behavior in later queries.\n\nAlso we might need to consider to disallow setting the parent to an rp that is a descendant rp for this rp as that would create a loop in our graph.","commit_id":"be733dbad357e1a095b7c2892c86a673db6d05d7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"176e95fe087cf22a6c1705d66285ad5348c5246d","unresolved":false,"context_lines":[{"line_number":687,"context_line":"        parent_uuid \u003d updates.pop(\u0027parent_provider_uuid\u0027)"},{"line_number":688,"context_line":"        if parent_uuid is not None:"},{"line_number":689,"context_line":"            parent_ids \u003d _provider_ids_from_uuid(context, parent_uuid)"},{"line_number":690,"context_line":"            if parent_ids is None:"},{"line_number":691,"context_line":"                raise exception.ObjectActionError("},{"line_number":692,"context_line":"                        action\u003d\u0027create\u0027,"},{"line_number":693,"context_line":"                        reason\u003d\u0027parent provider UUID does not exist.\u0027)"},{"line_number":694,"context_line":"            parent_id \u003d parent_ids.id"},{"line_number":695,"context_line":"            root_id \u003d parent_ids.root_id"},{"line_number":696,"context_line":"            updates[\u0027root_provider_id\u0027] \u003d root_id"},{"line_number":697,"context_line":"            updates[\u0027parent_provider_id\u0027] \u003d parent_id"}],"source_content_type":"text/x-python","patch_set":52,"id":"3f4b6375_b0968a3d","line":694,"range":{"start_line":690,"start_character":1,"end_line":694,"end_character":1},"in_reply_to":"3f4b6375_1ef6f3fa","updated":"2017-10-25 10:50:14.000000000","message":"Good point, referencing a child not possible during create.","commit_id":"be733dbad357e1a095b7c2892c86a673db6d05d7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6c0085e728c9d57740ae86dbde7ca249f4656fcb","unresolved":false,"context_lines":[{"line_number":687,"context_line":"        parent_uuid \u003d updates.pop(\u0027parent_provider_uuid\u0027)"},{"line_number":688,"context_line":"        if parent_uuid is not None:"},{"line_number":689,"context_line":"            parent_ids \u003d _provider_ids_from_uuid(context, parent_uuid)"},{"line_number":690,"context_line":"            if parent_ids is None:"},{"line_number":691,"context_line":"                raise exception.ObjectActionError("},{"line_number":692,"context_line":"                        action\u003d\u0027create\u0027,"},{"line_number":693,"context_line":"                        reason\u003d\u0027parent provider UUID does not exist.\u0027)"},{"line_number":694,"context_line":"            parent_id \u003d parent_ids.id"},{"line_number":695,"context_line":"            root_id \u003d parent_ids.root_id"},{"line_number":696,"context_line":"            updates[\u0027root_provider_id\u0027] \u003d root_id"},{"line_number":697,"context_line":"            updates[\u0027parent_provider_id\u0027] \u003d parent_id"}],"source_content_type":"text/x-python","patch_set":52,"id":"3f4b6375_1ef6f3fa","line":694,"range":{"start_line":690,"start_character":1,"end_line":694,"end_character":1},"in_reply_to":"3f4b6375_418938c8","updated":"2017-10-20 16:21:08.000000000","message":"BTW, I just realized that there is not a possibility of creating a provider with a parent that is a descendant of this resource provider. This is possible for *re-parenting* when doing updates, but not when creating a new provider (since clearly a new provider doesn\u0027t have any descendants yet).","commit_id":"be733dbad357e1a095b7c2892c86a673db6d05d7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0dfd467663fe71fa6afbe6b71a094a2af777c965","unresolved":false,"context_lines":[{"line_number":687,"context_line":"        parent_uuid \u003d updates.pop(\u0027parent_provider_uuid\u0027)"},{"line_number":688,"context_line":"        if parent_uuid is not None:"},{"line_number":689,"context_line":"            parent_ids \u003d _provider_ids_from_uuid(context, parent_uuid)"},{"line_number":690,"context_line":"            if parent_ids is None:"},{"line_number":691,"context_line":"                raise exception.ObjectActionError("},{"line_number":692,"context_line":"                        action\u003d\u0027create\u0027,"},{"line_number":693,"context_line":"                        reason\u003d\u0027parent provider UUID does not exist.\u0027)"},{"line_number":694,"context_line":"            parent_id \u003d parent_ids.id"},{"line_number":695,"context_line":"            root_id \u003d parent_ids.root_id"},{"line_number":696,"context_line":"            updates[\u0027root_provider_id\u0027] \u003d root_id"},{"line_number":697,"context_line":"            updates[\u0027parent_provider_id\u0027] \u003d parent_id"}],"source_content_type":"text/x-python","patch_set":52,"id":"3f4b6375_62254fc2","line":694,"range":{"start_line":690,"start_character":1,"end_line":694,"end_character":1},"in_reply_to":"3f4b6375_418938c8","updated":"2017-10-20 15:04:40.000000000","message":"Both good points. Will address.","commit_id":"be733dbad357e1a095b7c2892c86a673db6d05d7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"86d473817c4e512fdc93beb496cc8dc95c77137c","unresolved":false,"context_lines":[{"line_number":717,"context_line":"    @staticmethod"},{"line_number":718,"context_line":"    @db_api.api_context_manager.writer"},{"line_number":719,"context_line":"    def _delete(context, _id):"},{"line_number":720,"context_line":"        # Do a quick check to see if the provider is a parent. If it is, Don\u0027t"},{"line_number":721,"context_line":"        # allow deleting the provider. Note that the foreign key constraint on"},{"line_number":722,"context_line":"        # resource_providers.parent_provider_id will prevent deletion of the"},{"line_number":723,"context_line":"        # parent within the transaction below. This is just a quick"}],"source_content_type":"text/x-python","patch_set":52,"id":"3f4b6375_21099c30","line":720,"range":{"start_line":720,"start_character":73,"end_line":720,"end_character":74},"updated":"2017-10-20 12:06:31.000000000","message":"nit: s/D/d/","commit_id":"be733dbad357e1a095b7c2892c86a673db6d05d7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0dfd467663fe71fa6afbe6b71a094a2af777c965","unresolved":false,"context_lines":[{"line_number":717,"context_line":"    @staticmethod"},{"line_number":718,"context_line":"    @db_api.api_context_manager.writer"},{"line_number":719,"context_line":"    def _delete(context, _id):"},{"line_number":720,"context_line":"        # Do a quick check to see if the provider is a parent. If it is, Don\u0027t"},{"line_number":721,"context_line":"        # allow deleting the provider. Note that the foreign key constraint on"},{"line_number":722,"context_line":"        # resource_providers.parent_provider_id will prevent deletion of the"},{"line_number":723,"context_line":"        # parent within the transaction below. This is just a quick"}],"source_content_type":"text/x-python","patch_set":52,"id":"3f4b6375_422013b1","line":720,"range":{"start_line":720,"start_character":73,"end_line":720,"end_character":74},"in_reply_to":"3f4b6375_21099c30","updated":"2017-10-20 15:04:40.000000000","message":"Done","commit_id":"be733dbad357e1a095b7c2892c86a673db6d05d7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"86d473817c4e512fdc93beb496cc8dc95c77137c","unresolved":false,"context_lines":[{"line_number":761,"context_line":"            parent_uuid \u003d updates.pop(\u0027parent_provider_uuid\u0027)"},{"line_number":762,"context_line":"            if parent_uuid is not None:"},{"line_number":763,"context_line":"                # User supplied a parent, let\u0027s make sure it exists"},{"line_number":764,"context_line":"                parent_ids \u003d _provider_ids_from_uuid(context, parent_uuid)"},{"line_number":765,"context_line":"                if parent_ids is None:"},{"line_number":766,"context_line":"                    raise exception.ObjectActionError("},{"line_number":767,"context_line":"                            action\u003d\u0027create\u0027,"},{"line_number":768,"context_line":"                            reason\u003d\u0027parent provider UUID does not exist.\u0027)"},{"line_number":769,"context_line":"                updates[\u0027root_provider_id\u0027] \u003d parent_ids.root_id"},{"line_number":770,"context_line":"                updates[\u0027parent_provider_id\u0027] \u003d parent_ids.id"},{"line_number":771,"context_line":"                self.root_provider_uuid \u003d parent_ids.root_uuid"},{"line_number":772,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":52,"id":"3f4b6375_53fa2b75","line":769,"range":{"start_line":764,"start_character":1,"end_line":769,"end_character":2},"updated":"2017-10-20 12:06:31.000000000","message":"Same comment as in _create_in_db","commit_id":"be733dbad357e1a095b7c2892c86a673db6d05d7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"86d473817c4e512fdc93beb496cc8dc95c77137c","unresolved":false,"context_lines":[{"line_number":767,"context_line":"                            action\u003d\u0027create\u0027,"},{"line_number":768,"context_line":"                            reason\u003d\u0027parent provider UUID does not exist.\u0027)"},{"line_number":769,"context_line":"                updates[\u0027root_provider_id\u0027] \u003d parent_ids.root_id"},{"line_number":770,"context_line":"                updates[\u0027parent_provider_id\u0027] \u003d parent_ids.id"},{"line_number":771,"context_line":"                self.root_provider_uuid \u003d parent_ids.root_uuid"},{"line_number":772,"context_line":"            else:"},{"line_number":773,"context_line":"                # No parent... root provider ID is equal to ourselves"}],"source_content_type":"text/x-python","patch_set":52,"id":"3f4b6375_d66049a8","line":770,"updated":"2017-10-20 12:06:31.000000000","message":"If this RP has allocations then re-parenting can break such allocations. Consider that there is allocation on this RP (e.g. some VCPU) and the original allocation request also asked for some traits (e.g. some CPU instruction set) that was fulfilled by the original parent. Now we re-parent this RP to a parent that doesn\u0027t have that trait. This causes that our allocation does not compatible with the original allocation request any more.\n\nI think this can be solved if we restrict the re-parenting use case to only those RPs where the RP and every descendant RP has empty allocations.","commit_id":"be733dbad357e1a095b7c2892c86a673db6d05d7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0dfd467663fe71fa6afbe6b71a094a2af777c965","unresolved":false,"context_lines":[{"line_number":767,"context_line":"                            action\u003d\u0027create\u0027,"},{"line_number":768,"context_line":"                            reason\u003d\u0027parent provider UUID does not exist.\u0027)"},{"line_number":769,"context_line":"                updates[\u0027root_provider_id\u0027] \u003d parent_ids.root_id"},{"line_number":770,"context_line":"                updates[\u0027parent_provider_id\u0027] \u003d parent_ids.id"},{"line_number":771,"context_line":"                self.root_provider_uuid \u003d parent_ids.root_uuid"},{"line_number":772,"context_line":"            else:"},{"line_number":773,"context_line":"                # No parent... root provider ID is equal to ourselves"}],"source_content_type":"text/x-python","patch_set":52,"id":"3f4b6375_7d0cd03a","line":770,"in_reply_to":"3f4b6375_d66049a8","updated":"2017-10-20 15:04:40.000000000","message":"Another good point. Will address.","commit_id":"be733dbad357e1a095b7c2892c86a673db6d05d7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"86d473817c4e512fdc93beb496cc8dc95c77137c","unresolved":false,"context_lines":[{"line_number":770,"context_line":"                updates[\u0027parent_provider_id\u0027] \u003d parent_ids.id"},{"line_number":771,"context_line":"                self.root_provider_uuid \u003d parent_ids.root_uuid"},{"line_number":772,"context_line":"            else:"},{"line_number":773,"context_line":"                # No parent... root provider ID is equal to ourselves"},{"line_number":774,"context_line":"                updates[\u0027root_provider_id\u0027] \u003d id"},{"line_number":775,"context_line":"                updates[\u0027parent_provider_id\u0027] \u003d None"},{"line_number":776,"context_line":"                self.root_provider_uuid \u003d self.uuid"}],"source_content_type":"text/x-python","patch_set":52,"id":"3f4b6375_b6ecad29","line":773,"range":{"start_line":773,"start_character":0,"end_line":773,"end_character":69},"updated":"2017-10-20 12:06:31.000000000","message":"If our RP wasn\u0027t a root RP before this update then here we actually promoting a child RP to a new root RP. I don\u0027t see why we need to support this but also I don\u0027t see how this can break something in placement today so this could be OK.\n\nDoes the ProviderTree cache on the SchedulerReportClient side handles this change transparently?","commit_id":"be733dbad357e1a095b7c2892c86a673db6d05d7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0dfd467663fe71fa6afbe6b71a094a2af777c965","unresolved":false,"context_lines":[{"line_number":770,"context_line":"                updates[\u0027parent_provider_id\u0027] \u003d parent_ids.id"},{"line_number":771,"context_line":"                self.root_provider_uuid \u003d parent_ids.root_uuid"},{"line_number":772,"context_line":"            else:"},{"line_number":773,"context_line":"                # No parent... root provider ID is equal to ourselves"},{"line_number":774,"context_line":"                updates[\u0027root_provider_id\u0027] \u003d id"},{"line_number":775,"context_line":"                updates[\u0027parent_provider_id\u0027] \u003d None"},{"line_number":776,"context_line":"                self.root_provider_uuid \u003d self.uuid"}],"source_content_type":"text/x-python","patch_set":52,"id":"3f4b6375_dd9bc4c6","line":773,"range":{"start_line":773,"start_character":0,"end_line":773,"end_character":69},"in_reply_to":"3f4b6375_b6ecad29","updated":"2017-10-20 15:04:40.000000000","message":"Well, the whole re-parenting thing is a bit of a very odd edge case that really doesn\u0027t have much of a use case at the moment. So much of this specific code is mostly theoretical at this point. :(\n\nThat said, you made me remember that I am forgetting to update the resource provider\u0027s generation when the parent or root changes. That generation change will allow the ProviderTree cache to refresh its inventory information when it notices an updated generation.","commit_id":"be733dbad357e1a095b7c2892c86a673db6d05d7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"176e95fe087cf22a6c1705d66285ad5348c5246d","unresolved":false,"context_lines":[{"line_number":766,"context_line":"    @db_api.api_context_manager.writer"},{"line_number":767,"context_line":"    def _update_in_db(self, context, id, updates):"},{"line_number":768,"context_line":"        if \u0027parent_provider_uuid\u0027 in updates:"},{"line_number":769,"context_line":"            # TODO(jaypipes): For now, \"re-parenting\" and \"un-parenting\" are"},{"line_number":770,"context_line":"            # not possible. If the provider already had a parent, we don\u0027t"},{"line_number":771,"context_line":"            # allow changing that parent due to various issues, including:"},{"line_number":772,"context_line":"            #"},{"line_number":773,"context_line":"            # * if there are allocations made against a provider that has one"}],"source_content_type":"text/x-python","patch_set":53,"id":"3f4b6375_105e7e11","line":770,"range":{"start_line":769,"start_character":30,"end_line":770,"end_character":27},"updated":"2017-10-25 10:50:14.000000000","message":"I agree that this is the easiest way to avoid the problems with re-parenting. Let\u0027s solve the re-parenting later / separately.","commit_id":"2911b1ac15d5863b964f983f5fd19d0201b2b70c"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"5f7b3b880813534f472e6d1fa9d16004e691db6f","unresolved":false,"context_lines":[{"line_number":770,"context_line":"            # not possible. If the provider already had a parent, we don\u0027t"},{"line_number":771,"context_line":"            # allow changing that parent due to various issues, including:"},{"line_number":772,"context_line":"            #"},{"line_number":773,"context_line":"            # * if there are allocations made against a provider that has one"},{"line_number":774,"context_line":"            #   set of traits and we re-parent to a provider that has a"},{"line_number":775,"context_line":"            #   different set of traits, the original placement constraints for"},{"line_number":776,"context_line":"            #   the consumer may be violated"},{"line_number":777,"context_line":"            # * if the new parent is a descendent of this resource provider, we"},{"line_number":778,"context_line":"            #   introduce the possibility of a loop in the graph, which would"},{"line_number":779,"context_line":"            #   be very bad"}],"source_content_type":"text/x-python","patch_set":53,"id":"3f4b6375_38eadf74","line":776,"range":{"start_line":773,"start_character":0,"end_line":776,"end_character":44},"updated":"2017-10-20 21:51:40.000000000","message":"Why is it a problem to violate the original constraints after the deploy has already happened?\n\nDiscussion started: http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2017-10-20.log.html#t2017-10-20T21:34:44","commit_id":"2911b1ac15d5863b964f983f5fd19d0201b2b70c"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"2f34870880dd9430bb9998ea67ad6a62f2f28c60","unresolved":false,"context_lines":[{"line_number":822,"context_line":"            # for this resource provider in between the above check for a valid"},{"line_number":823,"context_line":"            # parent provider and here..."},{"line_number":824,"context_line":"            raise exception.ObjectActionError("},{"line_number":825,"context_line":"                    action\u003d\u0027create\u0027,"},{"line_number":826,"context_line":"                    reason\u003d\u0027parent provider UUID does not exist.\u0027)"},{"line_number":827,"context_line":""},{"line_number":828,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":53,"id":"3f4b6375_f6655a46","line":825,"updated":"2017-10-23 12:34:16.000000000","message":"update?","commit_id":"2911b1ac15d5863b964f983f5fd19d0201b2b70c"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2901535b0d8581fa7e3e28889913f99190f1203e","unresolved":false,"context_lines":[{"line_number":822,"context_line":"            # for this resource provider in between the above check for a valid"},{"line_number":823,"context_line":"            # parent provider and here..."},{"line_number":824,"context_line":"            raise exception.ObjectActionError("},{"line_number":825,"context_line":"                    action\u003d\u0027create\u0027,"},{"line_number":826,"context_line":"                    reason\u003d\u0027parent provider UUID does not exist.\u0027)"},{"line_number":827,"context_line":""},{"line_number":828,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":53,"id":"3f4b6375_11fb1d47","line":825,"in_reply_to":"3f4b6375_d278ffc4","updated":"2017-10-25 12:14:57.000000000","message":"Done","commit_id":"2911b1ac15d5863b964f983f5fd19d0201b2b70c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b7f7a69592192ee9a1e22adbf2ea7bed462ca7c0","unresolved":false,"context_lines":[{"line_number":822,"context_line":"            # for this resource provider in between the above check for a valid"},{"line_number":823,"context_line":"            # parent provider and here..."},{"line_number":824,"context_line":"            raise exception.ObjectActionError("},{"line_number":825,"context_line":"                    action\u003d\u0027create\u0027,"},{"line_number":826,"context_line":"                    reason\u003d\u0027parent provider UUID does not exist.\u0027)"},{"line_number":827,"context_line":""},{"line_number":828,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":53,"id":"3f4b6375_d278ffc4","line":825,"in_reply_to":"3f4b6375_f6655a46","updated":"2017-10-24 17:12:54.000000000","message":"Yeah. Could fix this in a follow-on, but should be fixed one way or the other.","commit_id":"2911b1ac15d5863b964f983f5fd19d0201b2b70c"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"f8e077b2a67a75d8495637d1b7702c69b413fda1","unresolved":false,"context_lines":[{"line_number":601,"context_line":"        if \u0027root_provider_uuid\u0027 in self:"},{"line_number":602,"context_line":"            raise exception.ObjectActionError("},{"line_number":603,"context_line":"                action\u003d\u0027create\u0027,"},{"line_number":604,"context_line":"                reason\u003d\u0027root provider UUID cannot be manually set.\u0027)"},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"        self.obj_set_defaults()"},{"line_number":607,"context_line":"        updates \u003d self.obj_get_changes()"}],"source_content_type":"text/x-python","patch_set":54,"id":"3f4b6375_3d1b7173","line":604,"range":{"start_line":604,"start_character":23,"end_line":604,"end_character":67},"updated":"2017-10-26 07:57:56.000000000","message":"nit: Add marker function _().","commit_id":"511b7be9c6a67456e528c71241a388c99a083e70"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1360eaacce4841545682db451572fa9df1a8008a","unresolved":false,"context_lines":[{"line_number":601,"context_line":"        if \u0027root_provider_uuid\u0027 in self:"},{"line_number":602,"context_line":"            raise exception.ObjectActionError("},{"line_number":603,"context_line":"                action\u003d\u0027create\u0027,"},{"line_number":604,"context_line":"                reason\u003d\u0027root provider UUID cannot be manually set.\u0027)"},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"        self.obj_set_defaults()"},{"line_number":607,"context_line":"        updates \u003d self.obj_get_changes()"}],"source_content_type":"text/x-python","patch_set":54,"id":"3f4b6375_23447246","line":604,"range":{"start_line":604,"start_character":23,"end_line":604,"end_character":67},"in_reply_to":"3f4b6375_3d1b7173","updated":"2017-10-26 09:13:02.000000000","message":"Done","commit_id":"511b7be9c6a67456e528c71241a388c99a083e70"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"f8e077b2a67a75d8495637d1b7702c69b413fda1","unresolved":false,"context_lines":[{"line_number":689,"context_line":"            if parent_uuid \u003d\u003d self.uuid:"},{"line_number":690,"context_line":"                raise exception.ObjectActionError("},{"line_number":691,"context_line":"                        action\u003d\u0027create\u0027,"},{"line_number":692,"context_line":"                        reason\u003d\u0027parent provider UUID cannot be same as UUID. \u0027"},{"line_number":693,"context_line":"                               \u0027Please set parent provider UUID to None if \u0027"},{"line_number":694,"context_line":"                               \u0027there is no parent.\u0027)"},{"line_number":695,"context_line":""},{"line_number":696,"context_line":"            parent_ids \u003d _provider_ids_from_uuid(context, parent_uuid)"},{"line_number":697,"context_line":"            if parent_ids is None:"}],"source_content_type":"text/x-python","patch_set":54,"id":"3f4b6375_fd2a797c","line":694,"range":{"start_line":692,"start_character":32,"end_line":694,"end_character":52},"updated":"2017-10-26 07:57:56.000000000","message":"ditto","commit_id":"511b7be9c6a67456e528c71241a388c99a083e70"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1360eaacce4841545682db451572fa9df1a8008a","unresolved":false,"context_lines":[{"line_number":689,"context_line":"            if parent_uuid \u003d\u003d self.uuid:"},{"line_number":690,"context_line":"                raise exception.ObjectActionError("},{"line_number":691,"context_line":"                        action\u003d\u0027create\u0027,"},{"line_number":692,"context_line":"                        reason\u003d\u0027parent provider UUID cannot be same as UUID. \u0027"},{"line_number":693,"context_line":"                               \u0027Please set parent provider UUID to None if \u0027"},{"line_number":694,"context_line":"                               \u0027there is no parent.\u0027)"},{"line_number":695,"context_line":""},{"line_number":696,"context_line":"            parent_ids \u003d _provider_ids_from_uuid(context, parent_uuid)"},{"line_number":697,"context_line":"            if parent_ids is None:"}],"source_content_type":"text/x-python","patch_set":54,"id":"3f4b6375_83b1e649","line":694,"range":{"start_line":692,"start_character":32,"end_line":694,"end_character":52},"in_reply_to":"3f4b6375_fd2a797c","updated":"2017-10-26 09:13:02.000000000","message":"Done","commit_id":"511b7be9c6a67456e528c71241a388c99a083e70"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"f8e077b2a67a75d8495637d1b7702c69b413fda1","unresolved":false,"context_lines":[{"line_number":697,"context_line":"            if parent_ids is None:"},{"line_number":698,"context_line":"                raise exception.ObjectActionError("},{"line_number":699,"context_line":"                        action\u003d\u0027create\u0027,"},{"line_number":700,"context_line":"                        reason\u003d\u0027parent provider UUID does not exist.\u0027)"},{"line_number":701,"context_line":""},{"line_number":702,"context_line":"            parent_id \u003d parent_ids.id"},{"line_number":703,"context_line":"            root_id \u003d parent_ids.root_id"}],"source_content_type":"text/x-python","patch_set":54,"id":"3f4b6375_7d432943","line":700,"range":{"start_line":700,"start_character":31,"end_line":700,"end_character":69},"updated":"2017-10-26 07:57:56.000000000","message":"ditto","commit_id":"511b7be9c6a67456e528c71241a388c99a083e70"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1360eaacce4841545682db451572fa9df1a8008a","unresolved":false,"context_lines":[{"line_number":697,"context_line":"            if parent_ids is None:"},{"line_number":698,"context_line":"                raise exception.ObjectActionError("},{"line_number":699,"context_line":"                        action\u003d\u0027create\u0027,"},{"line_number":700,"context_line":"                        reason\u003d\u0027parent provider UUID does not exist.\u0027)"},{"line_number":701,"context_line":""},{"line_number":702,"context_line":"            parent_id \u003d parent_ids.id"},{"line_number":703,"context_line":"            root_id \u003d parent_ids.root_id"}],"source_content_type":"text/x-python","patch_set":54,"id":"3f4b6375_03873664","line":700,"range":{"start_line":700,"start_character":31,"end_line":700,"end_character":69},"in_reply_to":"3f4b6375_7d432943","updated":"2017-10-26 09:13:02.000000000","message":"Done","commit_id":"511b7be9c6a67456e528c71241a388c99a083e70"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"f8e077b2a67a75d8495637d1b7702c69b413fda1","unresolved":false,"context_lines":[{"line_number":795,"context_line":"                        my_ids.parent_id !\u003d parent_ids.id):"},{"line_number":796,"context_line":"                    raise exception.ObjectActionError("},{"line_number":797,"context_line":"                            action\u003d\u0027update\u0027,"},{"line_number":798,"context_line":"                            reason\u003d\u0027re-parenting a provider is not currently \u0027"},{"line_number":799,"context_line":"                                   \u0027allowed.\u0027)"},{"line_number":800,"context_line":""},{"line_number":801,"context_line":"                updates[\u0027root_provider_id\u0027] \u003d parent_ids.root_id"},{"line_number":802,"context_line":"                updates[\u0027parent_provider_id\u0027] \u003d parent_ids.id"}],"source_content_type":"text/x-python","patch_set":54,"id":"3f4b6375_7d3e89c1","line":799,"range":{"start_line":798,"start_character":35,"end_line":799,"end_character":45},"updated":"2017-10-26 07:57:56.000000000","message":"ditto","commit_id":"511b7be9c6a67456e528c71241a388c99a083e70"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1360eaacce4841545682db451572fa9df1a8008a","unresolved":false,"context_lines":[{"line_number":795,"context_line":"                        my_ids.parent_id !\u003d parent_ids.id):"},{"line_number":796,"context_line":"                    raise exception.ObjectActionError("},{"line_number":797,"context_line":"                            action\u003d\u0027update\u0027,"},{"line_number":798,"context_line":"                            reason\u003d\u0027re-parenting a provider is not currently \u0027"},{"line_number":799,"context_line":"                                   \u0027allowed.\u0027)"},{"line_number":800,"context_line":""},{"line_number":801,"context_line":"                updates[\u0027root_provider_id\u0027] \u003d parent_ids.root_id"},{"line_number":802,"context_line":"                updates[\u0027parent_provider_id\u0027] \u003d parent_ids.id"}],"source_content_type":"text/x-python","patch_set":54,"id":"3f4b6375_c37c7e73","line":799,"range":{"start_line":798,"start_character":35,"end_line":799,"end_character":45},"in_reply_to":"3f4b6375_7d3e89c1","updated":"2017-10-26 09:13:02.000000000","message":"Done","commit_id":"511b7be9c6a67456e528c71241a388c99a083e70"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"f8e077b2a67a75d8495637d1b7702c69b413fda1","unresolved":false,"context_lines":[{"line_number":805,"context_line":"                if my_ids.parent_id is not None:"},{"line_number":806,"context_line":"                    raise exception.ObjectActionError("},{"line_number":807,"context_line":"                            action\u003d\u0027update\u0027,"},{"line_number":808,"context_line":"                            reason\u003d\u0027un-parenting a provider is not currently \u0027"},{"line_number":809,"context_line":"                                   \u0027allowed.\u0027)"},{"line_number":810,"context_line":"                # No parent... root provider ID is equal to ourselves"},{"line_number":811,"context_line":"                updates[\u0027root_provider_id\u0027] \u003d id"},{"line_number":812,"context_line":"                updates[\u0027parent_provider_id\u0027] \u003d None"}],"source_content_type":"text/x-python","patch_set":54,"id":"3f4b6375_fd4f1968","line":809,"range":{"start_line":808,"start_character":35,"end_line":809,"end_character":45},"updated":"2017-10-26 07:57:56.000000000","message":"ditto","commit_id":"511b7be9c6a67456e528c71241a388c99a083e70"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1360eaacce4841545682db451572fa9df1a8008a","unresolved":false,"context_lines":[{"line_number":805,"context_line":"                if my_ids.parent_id is not None:"},{"line_number":806,"context_line":"                    raise exception.ObjectActionError("},{"line_number":807,"context_line":"                            action\u003d\u0027update\u0027,"},{"line_number":808,"context_line":"                            reason\u003d\u0027un-parenting a provider is not currently \u0027"},{"line_number":809,"context_line":"                                   \u0027allowed.\u0027)"},{"line_number":810,"context_line":"                # No parent... root provider ID is equal to ourselves"},{"line_number":811,"context_line":"                updates[\u0027root_provider_id\u0027] \u003d id"},{"line_number":812,"context_line":"                updates[\u0027parent_provider_id\u0027] \u003d None"}],"source_content_type":"text/x-python","patch_set":54,"id":"3f4b6375_a3f4e2f1","line":809,"range":{"start_line":808,"start_character":35,"end_line":809,"end_character":45},"in_reply_to":"3f4b6375_fd4f1968","updated":"2017-10-26 09:13:02.000000000","message":"Done","commit_id":"511b7be9c6a67456e528c71241a388c99a083e70"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"f8e077b2a67a75d8495637d1b7702c69b413fda1","unresolved":false,"context_lines":[{"line_number":823,"context_line":"            # parent provider and here..."},{"line_number":824,"context_line":"            raise exception.ObjectActionError("},{"line_number":825,"context_line":"                    action\u003d\u0027update\u0027,"},{"line_number":826,"context_line":"                    reason\u003d\u0027parent provider UUID does not exist.\u0027)"},{"line_number":827,"context_line":""},{"line_number":828,"context_line":"    @staticmethod"},{"line_number":829,"context_line":"    @db_api.api_context_manager.writer  # Needed for online data migration"}],"source_content_type":"text/x-python","patch_set":54,"id":"3f4b6375_3d49315f","line":826,"range":{"start_line":826,"start_character":27,"end_line":826,"end_character":65},"updated":"2017-10-26 07:57:56.000000000","message":"ditto","commit_id":"511b7be9c6a67456e528c71241a388c99a083e70"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1360eaacce4841545682db451572fa9df1a8008a","unresolved":false,"context_lines":[{"line_number":823,"context_line":"            # parent provider and here..."},{"line_number":824,"context_line":"            raise exception.ObjectActionError("},{"line_number":825,"context_line":"                    action\u003d\u0027update\u0027,"},{"line_number":826,"context_line":"                    reason\u003d\u0027parent provider UUID does not exist.\u0027)"},{"line_number":827,"context_line":""},{"line_number":828,"context_line":"    @staticmethod"},{"line_number":829,"context_line":"    @db_api.api_context_manager.writer  # Needed for online data migration"}],"source_content_type":"text/x-python","patch_set":54,"id":"3f4b6375_c3f11e03","line":826,"range":{"start_line":826,"start_character":27,"end_line":826,"end_character":65},"in_reply_to":"3f4b6375_3d49315f","updated":"2017-10-26 09:13:02.000000000","message":"Done","commit_id":"511b7be9c6a67456e528c71241a388c99a083e70"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"92b4e85b965771fa8e9d8b8da0126b72a480473e","unresolved":false,"context_lines":[{"line_number":869,"context_line":"                            action\u003d\u0027update\u0027,"},{"line_number":870,"context_line":"                            reason\u003d_(\u0027un-parenting a provider is not \u0027"},{"line_number":871,"context_line":"                                     \u0027currently allowed.\u0027))"},{"line_number":872,"context_line":"                # No parent... root provider ID is equal to ourselves"},{"line_number":873,"context_line":"                updates[\u0027root_provider_id\u0027] \u003d id"},{"line_number":874,"context_line":"                updates[\u0027parent_provider_id\u0027] \u003d None"},{"line_number":875,"context_line":"                self.root_provider_uuid \u003d self.uuid"}],"source_content_type":"text/x-python","patch_set":57,"id":"3f4b6375_d9c1b8e6","line":872,"updated":"2017-10-30 16:47:11.000000000","message":"As far I can tell this next chunk is never hit by tests, functional or unit.\n\nIn order for us to get here we need an update that has parent_provider_uuid set to None for a resource provider whose parent is already none?\n\nNot entirely sure I\u0027ve got that right...","commit_id":"37ee6beb6116f0bb6f813c7697399b8f0c69bd98"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fe07ce3d0498249286d036eaf30b2ca7aee01e23","unresolved":false,"context_lines":[{"line_number":869,"context_line":"                            action\u003d\u0027update\u0027,"},{"line_number":870,"context_line":"                            reason\u003d_(\u0027un-parenting a provider is not \u0027"},{"line_number":871,"context_line":"                                     \u0027currently allowed.\u0027))"},{"line_number":872,"context_line":"                # No parent... root provider ID is equal to ourselves"},{"line_number":873,"context_line":"                updates[\u0027root_provider_id\u0027] \u003d id"},{"line_number":874,"context_line":"                updates[\u0027parent_provider_id\u0027] \u003d None"},{"line_number":875,"context_line":"                self.root_provider_uuid \u003d self.uuid"}],"source_content_type":"text/x-python","patch_set":57,"id":"3f4b6375_a7f23905","line":872,"in_reply_to":"3f4b6375_a306c61f","updated":"2017-10-31 13:36:36.000000000","message":"Good catch!","commit_id":"37ee6beb6116f0bb6f813c7697399b8f0c69bd98"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b6b1094e835697252ab6631d6d7347fa35714431","unresolved":false,"context_lines":[{"line_number":869,"context_line":"                            action\u003d\u0027update\u0027,"},{"line_number":870,"context_line":"                            reason\u003d_(\u0027un-parenting a provider is not \u0027"},{"line_number":871,"context_line":"                                     \u0027currently allowed.\u0027))"},{"line_number":872,"context_line":"                # No parent... root provider ID is equal to ourselves"},{"line_number":873,"context_line":"                updates[\u0027root_provider_id\u0027] \u003d id"},{"line_number":874,"context_line":"                updates[\u0027parent_provider_id\u0027] \u003d None"},{"line_number":875,"context_line":"                self.root_provider_uuid \u003d self.uuid"}],"source_content_type":"text/x-python","patch_set":57,"id":"3f4b6375_a306c61f","line":872,"in_reply_to":"3f4b6375_d9c1b8e6","updated":"2017-10-30 19:50:59.000000000","message":"Yeah, that\u0027s right. That makes the below useless code, since it\u0027s just going to set the root and parent to what they already are... :) So I\u0027ll remove the below.","commit_id":"37ee6beb6116f0bb6f813c7697399b8f0c69bd98"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"43695361339c9e4e362e5538f239ab59fb851b43","unresolved":false,"context_lines":[{"line_number":410,"context_line":"        rpt.c.uuid,"},{"line_number":411,"context_line":"        rpt.c.name,"},{"line_number":412,"context_line":"        rpt.c.generation,"},{"line_number":413,"context_line":"        root.c.uuid.label(\"root_provider_uuid\"),"},{"line_number":414,"context_line":"        parent.c.uuid.label(\"parent_provider_uuid\"),"},{"line_number":415,"context_line":"    ]"},{"line_number":416,"context_line":"    sel \u003d sa.select(cols).select_from(rp_to_parent).where(rpt.c.uuid \u003d\u003d uuid)"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f4b6375_a04bff28","line":413,"range":{"start_line":413,"start_character":8,"end_line":413,"end_character":47},"updated":"2017-11-02 13:53:48.000000000","message":"If you did\n\n coalesce(root.c.uuid, rpt.c.uuid).label(\"root_provider_uuid\")\n\n...would it save the caller from doing the defaulting?","commit_id":"8ef837d7d9c570d2272dca84cca216597fac42b9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"82494a895b47685097078eaed8b14afd5823e0af","unresolved":false,"context_lines":[{"line_number":410,"context_line":"        rpt.c.uuid,"},{"line_number":411,"context_line":"        rpt.c.name,"},{"line_number":412,"context_line":"        rpt.c.generation,"},{"line_number":413,"context_line":"        root.c.uuid.label(\"root_provider_uuid\"),"},{"line_number":414,"context_line":"        parent.c.uuid.label(\"parent_provider_uuid\"),"},{"line_number":415,"context_line":"    ]"},{"line_number":416,"context_line":"    sel \u003d sa.select(cols).select_from(rp_to_parent).where(rpt.c.uuid \u003d\u003d uuid)"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f4b6375_b541f83b","line":413,"range":{"start_line":413,"start_character":8,"end_line":413,"end_character":47},"in_reply_to":"3f4b6375_a04bff28","updated":"2017-11-02 16:08:53.000000000","message":"I\u0027d prefer not to coalesce anything and let the caller know that the root_provider_uuid is currently un-set.","commit_id":"8ef837d7d9c570d2272dca84cca216597fac42b9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"43695361339c9e4e362e5538f239ab59fb851b43","unresolved":false,"context_lines":[{"line_number":547,"context_line":"@db_api.api_context_manager.reader"},{"line_number":548,"context_line":"def _has_child_providers(context, rp_id):"},{"line_number":549,"context_line":"    \"\"\"Returns True if the supplied resource provider has any child providers,"},{"line_number":550,"context_line":"    Falso otherwise"},{"line_number":551,"context_line":"    \"\"\""},{"line_number":552,"context_line":"    child_sel \u003d sa.select([_RP_TBL.c.id])"},{"line_number":553,"context_line":"    child_sel \u003d child_sel.where(_RP_TBL.c.parent_provider_id \u003d\u003d rp_id)"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f4b6375_60e3c735","line":550,"range":{"start_line":550,"start_character":4,"end_line":550,"end_character":9},"updated":"2017-11-02 13:53:48.000000000","message":"¡Falso!","commit_id":"8ef837d7d9c570d2272dca84cca216597fac42b9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"82494a895b47685097078eaed8b14afd5823e0af","unresolved":false,"context_lines":[{"line_number":547,"context_line":"@db_api.api_context_manager.reader"},{"line_number":548,"context_line":"def _has_child_providers(context, rp_id):"},{"line_number":549,"context_line":"    \"\"\"Returns True if the supplied resource provider has any child providers,"},{"line_number":550,"context_line":"    Falso otherwise"},{"line_number":551,"context_line":"    \"\"\""},{"line_number":552,"context_line":"    child_sel \u003d sa.select([_RP_TBL.c.id])"},{"line_number":553,"context_line":"    child_sel \u003d child_sel.where(_RP_TBL.c.parent_provider_id \u003d\u003d rp_id)"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f4b6375_7547004e","line":550,"range":{"start_line":550,"start_character":4,"end_line":550,"end_character":9},"in_reply_to":"3f4b6375_60e3c735","updated":"2017-11-02 16:08:53.000000000","message":"Done","commit_id":"8ef837d7d9c570d2272dca84cca216597fac42b9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"43695361339c9e4e362e5538f239ab59fb851b43","unresolved":false,"context_lines":[{"line_number":574,"context_line":"    \u0027ProviderIds\u0027, \u0027id uuid parent_id parent_uuid root_id root_uuid\u0027)"},{"line_number":575,"context_line":""},{"line_number":576,"context_line":""},{"line_number":577,"context_line":"def _provider_ids_from_uuid(context, uuid):"},{"line_number":578,"context_line":"    \"\"\"Given the UUID of a resource provider, returns a namedtuple"},{"line_number":579,"context_line":"    (ProviderIds) with the internal ID, the UUID, the parent provider\u0027s"},{"line_number":580,"context_line":"    internal ID, parent provider\u0027s UUID, the root provider\u0027s internal ID and"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f4b6375_20a26faf","line":577,"range":{"start_line":577,"start_character":4,"end_line":577,"end_character":27},"updated":"2017-11-02 13:53:48.000000000","message":"Do we really need both this and _get_provider_by_uuid?  (Did I ask that earlier?)","commit_id":"8ef837d7d9c570d2272dca84cca216597fac42b9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"82494a895b47685097078eaed8b14afd5823e0af","unresolved":false,"context_lines":[{"line_number":574,"context_line":"    \u0027ProviderIds\u0027, \u0027id uuid parent_id parent_uuid root_id root_uuid\u0027)"},{"line_number":575,"context_line":""},{"line_number":576,"context_line":""},{"line_number":577,"context_line":"def _provider_ids_from_uuid(context, uuid):"},{"line_number":578,"context_line":"    \"\"\"Given the UUID of a resource provider, returns a namedtuple"},{"line_number":579,"context_line":"    (ProviderIds) with the internal ID, the UUID, the parent provider\u0027s"},{"line_number":580,"context_line":"    internal ID, parent provider\u0027s UUID, the root provider\u0027s internal ID and"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f4b6375_95da7c75","line":577,"range":{"start_line":577,"start_character":4,"end_line":577,"end_character":27},"in_reply_to":"3f4b6375_20a26faf","updated":"2017-11-02 16:08:53.000000000","message":"Yes, I think so. This is a query that just gets the tree structure via the various IDs. It\u0027s highly optimized and the query will be served entirely from index seeks. I think it\u0027s worthwhile having this function here instead of calling _get_provider_by_uuid() and parsing out the various fields in the return. The ProviderIds namedtuple being returned here just makes things a bit cleaner for callers of this optimized function...","commit_id":"8ef837d7d9c570d2272dca84cca216597fac42b9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"43695361339c9e4e362e5538f239ab59fb851b43","unresolved":false,"context_lines":[{"line_number":601,"context_line":"        me.c.uuid,"},{"line_number":602,"context_line":"        parent.c.id.label(\u0027parent_id\u0027),"},{"line_number":603,"context_line":"        parent.c.uuid.label(\u0027parent_uuid\u0027),"},{"line_number":604,"context_line":"        root.c.id.label(\u0027root_id\u0027),"},{"line_number":605,"context_line":"        root.c.uuid.label(\u0027root_uuid\u0027),"},{"line_number":606,"context_line":"    ]"},{"line_number":607,"context_line":"    # TODO(jaypipes): Change this to an inner join when we are sure all"},{"line_number":608,"context_line":"    # root_provider_id values are NOT NULL"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f4b6375_a0953f42","line":605,"range":{"start_line":604,"start_character":0,"end_line":605,"end_character":39},"updated":"2017-11-02 13:53:48.000000000","message":"Same question about coalesce()","commit_id":"8ef837d7d9c570d2272dca84cca216597fac42b9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"82494a895b47685097078eaed8b14afd5823e0af","unresolved":false,"context_lines":[{"line_number":601,"context_line":"        me.c.uuid,"},{"line_number":602,"context_line":"        parent.c.id.label(\u0027parent_id\u0027),"},{"line_number":603,"context_line":"        parent.c.uuid.label(\u0027parent_uuid\u0027),"},{"line_number":604,"context_line":"        root.c.id.label(\u0027root_id\u0027),"},{"line_number":605,"context_line":"        root.c.uuid.label(\u0027root_uuid\u0027),"},{"line_number":606,"context_line":"    ]"},{"line_number":607,"context_line":"    # TODO(jaypipes): Change this to an inner join when we are sure all"},{"line_number":608,"context_line":"    # root_provider_id values are NOT NULL"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f4b6375_35d3c862","line":605,"range":{"start_line":604,"start_character":0,"end_line":605,"end_character":39},"in_reply_to":"3f4b6375_a0953f42","updated":"2017-11-02 16:08:53.000000000","message":"Same answer :)","commit_id":"8ef837d7d9c570d2272dca84cca216597fac42b9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"43695361339c9e4e362e5538f239ab59fb851b43","unresolved":false,"context_lines":[{"line_number":793,"context_line":"        # parent within the transaction below. This is just a quick"},{"line_number":794,"context_line":"        # short-circuit outside of the transaction boundary."},{"line_number":795,"context_line":"        if _has_child_providers(context, _id):"},{"line_number":796,"context_line":"            raise exception.CannotDeleteParentResourceProvider()"},{"line_number":797,"context_line":""},{"line_number":798,"context_line":"        conn \u003d context.session.connection()"},{"line_number":799,"context_line":"        with conn.begin():"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f4b6375_e07fb7dc","line":796,"range":{"start_line":796,"start_character":28,"end_line":796,"end_character":62},"updated":"2017-11-02 13:53:48.000000000","message":"Why isn\u0027t this an ObjectActionError?\n\nSame question for other exceptions in this method.","commit_id":"8ef837d7d9c570d2272dca84cca216597fac42b9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"82494a895b47685097078eaed8b14afd5823e0af","unresolved":false,"context_lines":[{"line_number":793,"context_line":"        # parent within the transaction below. This is just a quick"},{"line_number":794,"context_line":"        # short-circuit outside of the transaction boundary."},{"line_number":795,"context_line":"        if _has_child_providers(context, _id):"},{"line_number":796,"context_line":"            raise exception.CannotDeleteParentResourceProvider()"},{"line_number":797,"context_line":""},{"line_number":798,"context_line":"        conn \u003d context.session.connection()"},{"line_number":799,"context_line":"        with conn.begin():"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f4b6375_95483c2d","line":796,"range":{"start_line":796,"start_character":28,"end_line":796,"end_character":62},"in_reply_to":"3f4b6375_e07fb7dc","updated":"2017-11-02 16:08:53.000000000","message":"Not sure. We\u0027re pretty inconsistent about this. :( I prefer explicit/distinct exception classes versus a catch-all class like ObjectActionError, I guess.","commit_id":"8ef837d7d9c570d2272dca84cca216597fac42b9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"43695361339c9e4e362e5538f239ab59fb851b43","unresolved":false,"context_lines":[{"line_number":835,"context_line":"            # * if there are allocations made against a provider that has one"},{"line_number":836,"context_line":"            #   set of traits and we re-parent to a provider that has a"},{"line_number":837,"context_line":"            #   different set of traits, the original placement constraints for"},{"line_number":838,"context_line":"            #   the consumer may be violated"},{"line_number":839,"context_line":"            # * if the new parent is a descendent of this resource provider, we"},{"line_number":840,"context_line":"            #   introduce the possibility of a loop in the graph, which would"},{"line_number":841,"context_line":"            #   be very bad"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f4b6375_c06efbfe","line":838,"updated":"2017-11-02 13:53:48.000000000","message":"we decided we don\u0027t care about this","commit_id":"8ef837d7d9c570d2272dca84cca216597fac42b9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"82494a895b47685097078eaed8b14afd5823e0af","unresolved":false,"context_lines":[{"line_number":835,"context_line":"            # * if there are allocations made against a provider that has one"},{"line_number":836,"context_line":"            #   set of traits and we re-parent to a provider that has a"},{"line_number":837,"context_line":"            #   different set of traits, the original placement constraints for"},{"line_number":838,"context_line":"            #   the consumer may be violated"},{"line_number":839,"context_line":"            # * if the new parent is a descendent of this resource provider, we"},{"line_number":840,"context_line":"            #   introduce the possibility of a loop in the graph, which would"},{"line_number":841,"context_line":"            #   be very bad"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f4b6375_755140d9","line":838,"in_reply_to":"3f4b6375_c06efbfe","updated":"2017-11-02 16:08:53.000000000","message":"Done","commit_id":"8ef837d7d9c570d2272dca84cca216597fac42b9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"43695361339c9e4e362e5538f239ab59fb851b43","unresolved":false,"context_lines":[{"line_number":836,"context_line":"            #   set of traits and we re-parent to a provider that has a"},{"line_number":837,"context_line":"            #   different set of traits, the original placement constraints for"},{"line_number":838,"context_line":"            #   the consumer may be violated"},{"line_number":839,"context_line":"            # * if the new parent is a descendent of this resource provider, we"},{"line_number":840,"context_line":"            #   introduce the possibility of a loop in the graph, which would"},{"line_number":841,"context_line":"            #   be very bad"},{"line_number":842,"context_line":"            # * if the new parent is NULL but the old parent was not-NULL, we"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f4b6375_0052d33b","line":839,"range":{"start_line":839,"start_character":39,"end_line":839,"end_character":49},"updated":"2017-11-02 13:53:48.000000000","message":"descendant","commit_id":"8ef837d7d9c570d2272dca84cca216597fac42b9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"82494a895b47685097078eaed8b14afd5823e0af","unresolved":false,"context_lines":[{"line_number":836,"context_line":"            #   set of traits and we re-parent to a provider that has a"},{"line_number":837,"context_line":"            #   different set of traits, the original placement constraints for"},{"line_number":838,"context_line":"            #   the consumer may be violated"},{"line_number":839,"context_line":"            # * if the new parent is a descendent of this resource provider, we"},{"line_number":840,"context_line":"            #   introduce the possibility of a loop in the graph, which would"},{"line_number":841,"context_line":"            #   be very bad"},{"line_number":842,"context_line":"            # * if the new parent is NULL but the old parent was not-NULL, we"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f4b6375_f55650ce","line":839,"range":{"start_line":839,"start_character":39,"end_line":839,"end_character":49},"in_reply_to":"3f4b6375_0052d33b","updated":"2017-11-02 16:08:53.000000000","message":"Done","commit_id":"8ef837d7d9c570d2272dca84cca216597fac42b9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"43695361339c9e4e362e5538f239ab59fb851b43","unresolved":false,"context_lines":[{"line_number":838,"context_line":"            #   the consumer may be violated"},{"line_number":839,"context_line":"            # * if the new parent is a descendent of this resource provider, we"},{"line_number":840,"context_line":"            #   introduce the possibility of a loop in the graph, which would"},{"line_number":841,"context_line":"            #   be very bad"},{"line_number":842,"context_line":"            # * if the new parent is NULL but the old parent was not-NULL, we"},{"line_number":843,"context_line":"            #   could have a possibility where a placement constraint that"},{"line_number":844,"context_line":"            #   relied on traits of the old parent could be violated"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f4b6375_c0471b77","line":841,"updated":"2017-11-02 13:53:48.000000000","message":"like crossing the streams","commit_id":"8ef837d7d9c570d2272dca84cca216597fac42b9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"43695361339c9e4e362e5538f239ab59fb851b43","unresolved":false,"context_lines":[{"line_number":841,"context_line":"            #   be very bad"},{"line_number":842,"context_line":"            # * if the new parent is NULL but the old parent was not-NULL, we"},{"line_number":843,"context_line":"            #   could have a possibility where a placement constraint that"},{"line_number":844,"context_line":"            #   relied on traits of the old parent could be violated"},{"line_number":845,"context_line":"            #"},{"line_number":846,"context_line":"            # So, for now, let\u0027s just prevent re-parenting..."},{"line_number":847,"context_line":"            my_ids \u003d _provider_ids_from_uuid(context, self.uuid)"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f4b6375_260ba8d6","line":844,"updated":"2017-11-02 13:53:48.000000000","message":"Again, don\u0027t care.\n\nBut I imagine we *do* care about orphaning heretofore-descendants.  Which I suppose could be handled by reparenting them to this guy\u0027s former parent.","commit_id":"8ef837d7d9c570d2272dca84cca216597fac42b9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"82494a895b47685097078eaed8b14afd5823e0af","unresolved":false,"context_lines":[{"line_number":841,"context_line":"            #   be very bad"},{"line_number":842,"context_line":"            # * if the new parent is NULL but the old parent was not-NULL, we"},{"line_number":843,"context_line":"            #   could have a possibility where a placement constraint that"},{"line_number":844,"context_line":"            #   relied on traits of the old parent could be violated"},{"line_number":845,"context_line":"            #"},{"line_number":846,"context_line":"            # So, for now, let\u0027s just prevent re-parenting..."},{"line_number":847,"context_line":"            my_ids \u003d _provider_ids_from_uuid(context, self.uuid)"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f4b6375_756a6081","line":844,"in_reply_to":"3f4b6375_260ba8d6","updated":"2017-11-02 16:08:53.000000000","message":"Done","commit_id":"8ef837d7d9c570d2272dca84cca216597fac42b9"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"e181073a851786b1bcf188b4b30c50d06f781226","unresolved":false,"context_lines":[{"line_number":852,"context_line":"                if parent_ids is None:"},{"line_number":853,"context_line":"                    raise exception.ObjectActionError("},{"line_number":854,"context_line":"                            action\u003d\u0027create\u0027,"},{"line_number":855,"context_line":"                            reason\u003d\u0027parent provider UUID does not exist.\u0027)"},{"line_number":856,"context_line":"                if (my_ids.parent_id is not None and"},{"line_number":857,"context_line":"                        my_ids.parent_id !\u003d parent_ids.id):"},{"line_number":858,"context_line":"                    raise exception.ObjectActionError("}],"source_content_type":"text/x-python","patch_set":58,"id":"3f4b6375_ee2d7a46","line":855,"range":{"start_line":855,"start_character":35,"end_line":855,"end_character":73},"updated":"2017-11-02 02:15:14.000000000","message":"nit: missing marker function _()","commit_id":"8ef837d7d9c570d2272dca84cca216597fac42b9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"82494a895b47685097078eaed8b14afd5823e0af","unresolved":false,"context_lines":[{"line_number":852,"context_line":"                if parent_ids is None:"},{"line_number":853,"context_line":"                    raise exception.ObjectActionError("},{"line_number":854,"context_line":"                            action\u003d\u0027create\u0027,"},{"line_number":855,"context_line":"                            reason\u003d\u0027parent provider UUID does not exist.\u0027)"},{"line_number":856,"context_line":"                if (my_ids.parent_id is not None and"},{"line_number":857,"context_line":"                        my_ids.parent_id !\u003d parent_ids.id):"},{"line_number":858,"context_line":"                    raise exception.ObjectActionError("}],"source_content_type":"text/x-python","patch_set":58,"id":"3f4b6375_b5f9f8c8","line":855,"range":{"start_line":855,"start_character":35,"end_line":855,"end_character":73},"in_reply_to":"3f4b6375_ee2d7a46","updated":"2017-11-02 16:08:53.000000000","message":"Done","commit_id":"8ef837d7d9c570d2272dca84cca216597fac42b9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"5a58d62df466bb0f80e3759f135e9df2f3e50047","unresolved":false,"context_lines":[{"line_number":827,"context_line":"        if _has_child_providers(context, _id):"},{"line_number":828,"context_line":"            raise exception.CannotDeleteParentResourceProvider()"},{"line_number":829,"context_line":""},{"line_number":830,"context_line":"        conn \u003d context.session.connection()"},{"line_number":831,"context_line":"        with conn.begin():"},{"line_number":832,"context_line":"            # Don\u0027t delete the resource provider if it has allocations."},{"line_number":833,"context_line":"            rp_allocations \u003d context.session.query(models.Allocation).\\"},{"line_number":834,"context_line":"                 filter(models.Allocation.resource_provider_id \u003d\u003d _id).\\"}],"source_content_type":"text/x-python","patch_set":61,"id":"ff82abbf_fecde1b6","line":831,"range":{"start_line":830,"start_character":0,"end_line":831,"end_character":26},"updated":"2017-11-28 12:48:31.000000000","message":"I guess this needs to be fixed due to https://review.openstack.org/#/c/523192/","commit_id":"99143237559717fed0f8b96e97805a0dc6fcea10"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"769e6b109e04bc8103a9ac0d484cf2957db6db97","unresolved":false,"context_lines":[{"line_number":827,"context_line":"        if _has_child_providers(context, _id):"},{"line_number":828,"context_line":"            raise exception.CannotDeleteParentResourceProvider()"},{"line_number":829,"context_line":""},{"line_number":830,"context_line":"        conn \u003d context.session.connection()"},{"line_number":831,"context_line":"        with conn.begin():"},{"line_number":832,"context_line":"            # Don\u0027t delete the resource provider if it has allocations."},{"line_number":833,"context_line":"            rp_allocations \u003d context.session.query(models.Allocation).\\"},{"line_number":834,"context_line":"                 filter(models.Allocation.resource_provider_id \u003d\u003d _id).\\"}],"source_content_type":"text/x-python","patch_set":61,"id":"ff82abbf_c4860aba","line":831,"range":{"start_line":830,"start_character":0,"end_line":831,"end_character":26},"in_reply_to":"ff82abbf_fecde1b6","updated":"2017-11-28 14:11:12.000000000","message":"yeah, good catch. I\u0027ve updated.","commit_id":"99143237559717fed0f8b96e97805a0dc6fcea10"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6481707bbc0f95ddb856d96de2cb1c7b26abb896","unresolved":false,"context_lines":[{"line_number":580,"context_line":""},{"line_number":581,"context_line":"@db_api.api_context_manager.reader"},{"line_number":582,"context_line":"def _has_child_providers(context, rp_id):"},{"line_number":583,"context_line":"    \"\"\"Returns True if the supplied resource provider has any child providers,"},{"line_number":584,"context_line":"    False otherwise"},{"line_number":585,"context_line":"    \"\"\""},{"line_number":586,"context_line":"    child_sel \u003d sa.select([_RP_TBL.c.id])"}],"source_content_type":"text/x-python","patch_set":62,"id":"ff82abbf_23af892a","line":583,"range":{"start_line":583,"start_character":54,"end_line":583,"end_character":76},"updated":"2017-11-29 15:41:35.000000000","message":"technically, we check it has *at least one* child, but okay.","commit_id":"b10f11d7e8e1afb7a12a470f92c42bf3c23eca95"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6481707bbc0f95ddb856d96de2cb1c7b26abb896","unresolved":false,"context_lines":[{"line_number":692,"context_line":"        self.obj_set_defaults()"},{"line_number":693,"context_line":"        updates \u003d self.obj_get_changes()"},{"line_number":694,"context_line":"        self._create_in_db(self._context, updates)"},{"line_number":695,"context_line":"        self.obj_reset_changes()"},{"line_number":696,"context_line":""},{"line_number":697,"context_line":"    def destroy(self):"},{"line_number":698,"context_line":"        self._delete(self._context, self.id)"}],"source_content_type":"text/x-python","patch_set":62,"id":"ff82abbf_0327e5cb","line":695,"updated":"2017-11-29 15:41:35.000000000","message":"I\u0027d have prefered to fix the problem with the object fields persistence after a save() or create() call in a separate change, but okay.","commit_id":"b10f11d7e8e1afb7a12a470f92c42bf3c23eca95"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6481707bbc0f95ddb856d96de2cb1c7b26abb896","unresolved":false,"context_lines":[{"line_number":695,"context_line":"        self.obj_reset_changes()"},{"line_number":696,"context_line":""},{"line_number":697,"context_line":"    def destroy(self):"},{"line_number":698,"context_line":"        self._delete(self._context, self.id)"},{"line_number":699,"context_line":""},{"line_number":700,"context_line":"    def save(self):"},{"line_number":701,"context_line":"        updates \u003d self.obj_get_changes()"}],"source_content_type":"text/x-python","patch_set":62,"id":"ff82abbf_06437343","line":698,"updated":"2017-11-29 15:41:35.000000000","message":"shouldn\u0027t we reset the changes when we call the deletion ? That makes sense to me given after the deletion, all the fields are now all non-persisted (or all of them needing to be persisted, rather). Anyway, it\u0027s unrelated to that change.","commit_id":"b10f11d7e8e1afb7a12a470f92c42bf3c23eca95"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"69f64308c94459233e715a687f2ed5869f4b5a97","unresolved":false,"context_lines":[{"line_number":825,"context_line":"        # allow deleting the provider. Note that the foreign key constraint on"},{"line_number":826,"context_line":"        # resource_providers.parent_provider_id will prevent deletion of the"},{"line_number":827,"context_line":"        # parent within the transaction below. This is just a quick"},{"line_number":828,"context_line":"        # short-circuit outside of the transaction boundary."},{"line_number":829,"context_line":"        if _has_child_providers(context, _id):"},{"line_number":830,"context_line":"            raise exception.CannotDeleteParentResourceProvider()"},{"line_number":831,"context_line":""}],"source_content_type":"text/x-python","patch_set":62,"id":"ff82abbf_63a6f111","line":828,"updated":"2017-11-29 15:29:28.000000000","message":"All good with me ++","commit_id":"b10f11d7e8e1afb7a12a470f92c42bf3c23eca95"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2c453b9e5f9707edf1d43ac7c17a7e26b7cb6218","unresolved":false,"context_lines":[{"line_number":848,"context_line":"        try:"},{"line_number":849,"context_line":"            result \u003d context.session.query(models.ResourceProvider).\\"},{"line_number":850,"context_line":"                     filter(models.ResourceProvider.id \u003d\u003d _id).\\"},{"line_number":851,"context_line":"                     delete(synchronize_session\u003dFalse)"},{"line_number":852,"context_line":"        except sqla_exc.IntegrityError:"},{"line_number":853,"context_line":"            # NOTE(jaypipes): Another thread snuck in and parented this"},{"line_number":854,"context_line":"            # resource provider in between the above check for"}],"source_content_type":"text/x-python","patch_set":62,"id":"ff82abbf_63583130","line":851,"updated":"2017-11-29 15:08:43.000000000","message":"See my comment on the SQLA model.","commit_id":"b10f11d7e8e1afb7a12a470f92c42bf3c23eca95"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"69f64308c94459233e715a687f2ed5869f4b5a97","unresolved":false,"context_lines":[{"line_number":848,"context_line":"        try:"},{"line_number":849,"context_line":"            result \u003d context.session.query(models.ResourceProvider).\\"},{"line_number":850,"context_line":"                     filter(models.ResourceProvider.id \u003d\u003d _id).\\"},{"line_number":851,"context_line":"                     delete(synchronize_session\u003dFalse)"},{"line_number":852,"context_line":"        except sqla_exc.IntegrityError:"},{"line_number":853,"context_line":"            # NOTE(jaypipes): Another thread snuck in and parented this"},{"line_number":854,"context_line":"            # resource provider in between the above check for"}],"source_content_type":"text/x-python","patch_set":62,"id":"ff82abbf_83d5351e","line":851,"in_reply_to":"ff82abbf_63583130","updated":"2017-11-29 15:29:28.000000000","message":"Nevermind, we don\u0027t allow to delete a parent RP if there are existing children.","commit_id":"b10f11d7e8e1afb7a12a470f92c42bf3c23eca95"}],"nova/tests/functional/db/test_resource_provider.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"2ea5b35c2723ff247cbc079a17a42bb71c7d6541","unresolved":false,"context_lines":[{"line_number":128,"context_line":"        \"\"\"Simulate an old resource provider record in the database that has no"},{"line_number":129,"context_line":"        root_provider_uuid set and ensure that when grabbing the resource"},{"line_number":130,"context_line":"        provider object, the root_provider_uuid field in the table is set to"},{"line_number":131,"context_line":"        the provider\u0027s UUID."},{"line_number":132,"context_line":"        \"\"\""},{"line_number":133,"context_line":"        rp_tbl \u003d resource_provider._RP_TBL"},{"line_number":134,"context_line":"        conn \u003d self.api_db.get_engine().connect()"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a8bc5a2_ad954489","line":131,"updated":"2016-09-28 11:22:36.000000000","message":"Maybe a TODO that this will be removed in a future release?","commit_id":"f7daa324b4b7fdc8380af06c2a64f43256a735d2"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"74705be2b25fa56f5646128b896a4759e81b733a","unresolved":false,"context_lines":[{"line_number":128,"context_line":"        \"\"\"Simulate an old resource provider record in the database that has no"},{"line_number":129,"context_line":"        root_provider_uuid set and ensure that when grabbing the resource"},{"line_number":130,"context_line":"        provider object, the root_provider_uuid field in the table is set to"},{"line_number":131,"context_line":"        the provider\u0027s UUID."},{"line_number":132,"context_line":"        \"\"\""},{"line_number":133,"context_line":"        rp_tbl \u003d resource_provider._RP_TBL"},{"line_number":134,"context_line":"        conn \u003d self.api_db.get_engine().connect()"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a95cdbc_0efb12c2","line":131,"in_reply_to":"5a8bc5a2_ad954489","updated":"2016-10-07 15:43:41.000000000","message":"Done","commit_id":"f7daa324b4b7fdc8380af06c2a64f43256a735d2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"2ea5b35c2723ff247cbc079a17a42bb71c7d6541","unresolved":false,"context_lines":[{"line_number":183,"context_line":"        rp.create()"},{"line_number":184,"context_line":"        rp.parent_provider_uuid \u003d uuidsentinel.noexists"},{"line_number":185,"context_line":"        self.assertRaises(exception.ObjectActionError, rp.save)"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    def test_save_resource_provider(self):"},{"line_number":188,"context_line":"        created_resource_provider \u003d objects.ResourceProvider("},{"line_number":189,"context_line":"            context\u003dself.context,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a8bc5a2_ed1f4cb5","line":186,"updated":"2016-09-28 11:22:36.000000000","message":"Not related to this, but is there a test for errors on changing non-settable fields?","commit_id":"f7daa324b4b7fdc8380af06c2a64f43256a735d2"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"74705be2b25fa56f5646128b896a4759e81b733a","unresolved":false,"context_lines":[{"line_number":183,"context_line":"        rp.create()"},{"line_number":184,"context_line":"        rp.parent_provider_uuid \u003d uuidsentinel.noexists"},{"line_number":185,"context_line":"        self.assertRaises(exception.ObjectActionError, rp.save)"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    def test_save_resource_provider(self):"},{"line_number":188,"context_line":"        created_resource_provider \u003d objects.ResourceProvider("},{"line_number":189,"context_line":"            context\u003dself.context,"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a95cdbc_1c90d642","line":186,"in_reply_to":"5a8bc5a2_ed1f4cb5","updated":"2016-10-07 15:43:41.000000000","message":"Yes, in the unit tests:\n\nhttps://github.com/openstack/nova/blob/master/nova/tests/unit/objects/test_resource_provider.py#L115","commit_id":"f7daa324b4b7fdc8380af06c2a64f43256a735d2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"2ea5b35c2723ff247cbc079a17a42bb71c7d6541","unresolved":false,"context_lines":[{"line_number":199,"context_line":"        )"},{"line_number":200,"context_line":"        self.assertEqual(\u0027new-name\u0027, retrieved_resource_provider.name)"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    def test_nested_providers(self):"},{"line_number":203,"context_line":"        \"\"\"Create a hierarchy of resource providers and run through a series of"},{"line_number":204,"context_line":"        tests that ensure one cannot delete a resource provider that has no"},{"line_number":205,"context_line":"        direct allocations but its child providers do have allocations."}],"source_content_type":"text/x-python","patch_set":2,"id":"5a8bc5a2_ed4d8cc5","line":202,"range":{"start_line":202,"start_character":8,"end_line":202,"end_character":29},"updated":"2016-09-28 11:22:36.000000000","message":"nit: you could probably call this \u0027test_destroy_nested_resource_providers\u0027, seeing as that\u0027s the crux of what you\u0027re trying to test here","commit_id":"f7daa324b4b7fdc8380af06c2a64f43256a735d2"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"74705be2b25fa56f5646128b896a4759e81b733a","unresolved":false,"context_lines":[{"line_number":199,"context_line":"        )"},{"line_number":200,"context_line":"        self.assertEqual(\u0027new-name\u0027, retrieved_resource_provider.name)"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    def test_nested_providers(self):"},{"line_number":203,"context_line":"        \"\"\"Create a hierarchy of resource providers and run through a series of"},{"line_number":204,"context_line":"        tests that ensure one cannot delete a resource provider that has no"},{"line_number":205,"context_line":"        direct allocations but its child providers do have allocations."}],"source_content_type":"text/x-python","patch_set":2,"id":"1a95cdbc_7cb47ad0","line":202,"range":{"start_line":202,"start_character":8,"end_line":202,"end_character":29},"in_reply_to":"5a8bc5a2_ed4d8cc5","updated":"2016-10-07 15:43:41.000000000","message":"Done","commit_id":"f7daa324b4b7fdc8380af06c2a64f43256a735d2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"2ea5b35c2723ff247cbc079a17a42bb71c7d6541","unresolved":false,"context_lines":[{"line_number":265,"context_line":"                          root_rp.destroy)"},{"line_number":266,"context_line":"        self.assertRaises(exception.ResourceProviderInUse,"},{"line_number":267,"context_line":"                          child_rp.destroy)"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    def test_destroy_resource_provider(self):"},{"line_number":270,"context_line":"        created_resource_provider \u003d objects.ResourceProvider("},{"line_number":271,"context_line":"            context\u003dself.context,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a8bc5a2_6de7bcac","line":268,"updated":"2016-09-28 11:22:36.000000000","message":"Any value in asserting that this works if you do it correctly?","commit_id":"f7daa324b4b7fdc8380af06c2a64f43256a735d2"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"74705be2b25fa56f5646128b896a4759e81b733a","unresolved":false,"context_lines":[{"line_number":265,"context_line":"                          root_rp.destroy)"},{"line_number":266,"context_line":"        self.assertRaises(exception.ResourceProviderInUse,"},{"line_number":267,"context_line":"                          child_rp.destroy)"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    def test_destroy_resource_provider(self):"},{"line_number":270,"context_line":"        created_resource_provider \u003d objects.ResourceProvider("},{"line_number":271,"context_line":"            context\u003dself.context,"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a95cdbc_5cb17ee0","line":268,"in_reply_to":"5a8bc5a2_6de7bcac","updated":"2016-10-07 15:43:41.000000000","message":"Done","commit_id":"f7daa324b4b7fdc8380af06c2a64f43256a735d2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"c4893ad2797ffcf66348df7425c25c524bd33f98","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        conn.execute(ins_stmt)"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"        rp_obj.ResourceProvider.get_by_uuid(self.ctx,"},{"line_number":141,"context_line":"                                             uuidsentinel.rp1)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        # The ResourceProvider._from_db_object() method should have performed"},{"line_number":144,"context_line":"        # an online data migration, populating the root_provider_uuid field"}],"source_content_type":"text/x-python","patch_set":43,"id":"7f515b1d_f1b8fa2a","line":141,"range":{"start_line":141,"start_character":0,"end_line":141,"end_character":62},"updated":"2017-09-25 13:12:14.000000000","message":"nit: indentation seems to be off","commit_id":"387b278875591f57148c1646ef0606ff1813e4c8"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"8fd1d7864a37562122c1331eddb9715b6292dba7","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        conn.execute(ins_stmt)"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"        rp_obj.ResourceProvider.get_by_uuid(self.ctx,"},{"line_number":141,"context_line":"                                             uuidsentinel.rp1)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        # The ResourceProvider._from_db_object() method should have performed"},{"line_number":144,"context_line":"        # an online data migration, populating the root_provider_uuid field"}],"source_content_type":"text/x-python","patch_set":43,"id":"7f515b1d_1ae8f479","line":141,"range":{"start_line":141,"start_character":0,"end_line":141,"end_character":62},"in_reply_to":"7f515b1d_f1b8fa2a","updated":"2017-09-28 17:15:20.000000000","message":"Done","commit_id":"387b278875591f57148c1646ef0606ff1813e4c8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"c4893ad2797ffcf66348df7425c25c524bd33f98","unresolved":false,"context_lines":[{"line_number":146,"context_line":"        sel_stmt \u003d sa.select([rp_tbl.c.root_provider_uuid]).where("},{"line_number":147,"context_line":"            rp_tbl.c.id \u003d\u003d 1)"},{"line_number":148,"context_line":"        res \u003d conn.execute(sel_stmt).fetchall()"},{"line_number":149,"context_line":"        self.assertEqual(uuidsentinel.rp1, res[0][0])"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    def test_inherit_root_from_parent(self):"},{"line_number":152,"context_line":"        \"\"\"Tests that if we update an existing provider\u0027s parent provider UUID,"}],"source_content_type":"text/x-python","patch_set":43,"id":"7f515b1d_51122e0b","line":149,"updated":"2017-09-25 13:12:14.000000000","message":"As root_provider_uuid is a field of the ResourceProvider object we might want to assert that the root_provider_uuid value not only set in the db but also set on the obj object itself returned from the get_by_uuid call at L140","commit_id":"387b278875591f57148c1646ef0606ff1813e4c8"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"8fd1d7864a37562122c1331eddb9715b6292dba7","unresolved":false,"context_lines":[{"line_number":146,"context_line":"        sel_stmt \u003d sa.select([rp_tbl.c.root_provider_uuid]).where("},{"line_number":147,"context_line":"            rp_tbl.c.id \u003d\u003d 1)"},{"line_number":148,"context_line":"        res \u003d conn.execute(sel_stmt).fetchall()"},{"line_number":149,"context_line":"        self.assertEqual(uuidsentinel.rp1, res[0][0])"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    def test_inherit_root_from_parent(self):"},{"line_number":152,"context_line":"        \"\"\"Tests that if we update an existing provider\u0027s parent provider UUID,"}],"source_content_type":"text/x-python","patch_set":43,"id":"7f515b1d_f00bf720","line":149,"in_reply_to":"7f515b1d_51122e0b","updated":"2017-09-28 17:15:20.000000000","message":"Done","commit_id":"387b278875591f57148c1646ef0606ff1813e4c8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"c4893ad2797ffcf66348df7425c25c524bd33f98","unresolved":false,"context_lines":[{"line_number":189,"context_line":"            uuid\u003duuidsentinel.rp1,"},{"line_number":190,"context_line":"        )"},{"line_number":191,"context_line":"        rp.create()"},{"line_number":192,"context_line":"        rp.root_provider_uuid \u003d uuidsentinel.noexists"},{"line_number":193,"context_line":"        self.assertRaises(exception.ObjectActionError, rp.save)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def test_save_unknown_parent_provider(self):"}],"source_content_type":"text/x-python","patch_set":43,"id":"7f515b1d_71288a33","line":192,"range":{"start_line":192,"start_character":32,"end_line":192,"end_character":53},"updated":"2017-09-25 13:12:14.000000000","message":"It could even be an existing resource provider uuid and we still except that the save will fail.","commit_id":"387b278875591f57148c1646ef0606ff1813e4c8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"c4893ad2797ffcf66348df7425c25c524bd33f98","unresolved":false,"context_lines":[{"line_number":285,"context_line":"                          root_rp.destroy)"},{"line_number":286,"context_line":"        self.assertRaises(exception.ResourceProviderInUse,"},{"line_number":287,"context_line":"                          child_rp.destroy)"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"    def test_destroy_resource_provider(self):"},{"line_number":290,"context_line":"        created_resource_provider \u003d rp_obj.ResourceProvider("},{"line_number":291,"context_line":"            context\u003dself.ctx,"}],"source_content_type":"text/x-python","patch_set":43,"id":"7f515b1d_31d0d21f","line":288,"updated":"2017-09-25 13:12:14.000000000","message":"I think we need tests where we successfully delete a provider that has parent. Also I think we need a test where we delete a provider that has child providers but no allocations.","commit_id":"387b278875591f57148c1646ef0606ff1813e4c8"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"8fd1d7864a37562122c1331eddb9715b6292dba7","unresolved":false,"context_lines":[{"line_number":285,"context_line":"                          root_rp.destroy)"},{"line_number":286,"context_line":"        self.assertRaises(exception.ResourceProviderInUse,"},{"line_number":287,"context_line":"                          child_rp.destroy)"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"    def test_destroy_resource_provider(self):"},{"line_number":290,"context_line":"        created_resource_provider \u003d rp_obj.ResourceProvider("},{"line_number":291,"context_line":"            context\u003dself.ctx,"}],"source_content_type":"text/x-python","patch_set":43,"id":"7f515b1d_50788b8e","line":288,"in_reply_to":"7f515b1d_31d0d21f","updated":"2017-09-28 17:15:20.000000000","message":"good ideas. added tests.","commit_id":"387b278875591f57148c1646ef0606ff1813e4c8"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9b5888ab00d1776a29223c16ad919ae62f2773e3","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        rp1.parent_provider_uuid \u003d parent_rp.uuid"},{"line_number":179,"context_line":"        rp1.save()"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"        self.assertEqual(uuidsentinel.parent, rp1.root_provider_uuid)"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"    def test_save_root_provider_failed(self):"},{"line_number":184,"context_line":"        \"\"\"Test that if we provide a root_provider_uuid value we get an"}],"source_content_type":"text/x-python","patch_set":44,"id":"7f515b1d_d3a9a290","line":181,"updated":"2017-10-04 15:46:40.000000000","message":"It\u0027s possibly redundant, but wouldn\u0027t mind seeing a grandchild-inherits-root-provider-uuid-also test case.","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"fbf78eb85d6f98715a58c9fbe8964a87b470bf4f","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        rp1.parent_provider_uuid \u003d parent_rp.uuid"},{"line_number":179,"context_line":"        rp1.save()"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"        self.assertEqual(uuidsentinel.parent, rp1.root_provider_uuid)"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"    def test_save_root_provider_failed(self):"},{"line_number":184,"context_line":"        \"\"\"Test that if we provide a root_provider_uuid value we get an"}],"source_content_type":"text/x-python","patch_set":44,"id":"5f4e5783_a8947de9","line":181,"in_reply_to":"7f515b1d_d3a9a290","updated":"2017-10-10 15:33:07.000000000","message":"Done. Added in the test_nested_providers() method below.","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9b5888ab00d1776a29223c16ad919ae62f2773e3","unresolved":false,"context_lines":[{"line_number":294,"context_line":"        alloc_list.delete_all()"},{"line_number":295,"context_line":"        child_rp.destroy()"},{"line_number":296,"context_line":"        # Deleting the parent should have deleted the child..."},{"line_number":297,"context_line":"        self.assertRaises(exception.NotFound,"},{"line_number":298,"context_line":"                          grandchild_rp.destroy)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    def test_destroy_resource_provider(self):"},{"line_number":301,"context_line":"        created_resource_provider \u003d rp_obj.ResourceProvider("}],"source_content_type":"text/x-python","patch_set":44,"id":"7f515b1d_9320ca4b","line":298,"range":{"start_line":297,"start_character":0,"end_line":298,"end_character":48},"updated":"2017-10-04 15:46:40.000000000","message":"Yeah, but I think it also deleted the root.  Add an assertion to make sure that didn\u0027t happen.","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"fbf78eb85d6f98715a58c9fbe8964a87b470bf4f","unresolved":false,"context_lines":[{"line_number":294,"context_line":"        alloc_list.delete_all()"},{"line_number":295,"context_line":"        child_rp.destroy()"},{"line_number":296,"context_line":"        # Deleting the parent should have deleted the child..."},{"line_number":297,"context_line":"        self.assertRaises(exception.NotFound,"},{"line_number":298,"context_line":"                          grandchild_rp.destroy)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    def test_destroy_resource_provider(self):"},{"line_number":301,"context_line":"        created_resource_provider \u003d rp_obj.ResourceProvider("}],"source_content_type":"text/x-python","patch_set":44,"id":"5f4e5783_68456570","line":298,"range":{"start_line":297,"start_character":0,"end_line":298,"end_character":48},"in_reply_to":"7f515b1d_9320ca4b","updated":"2017-10-10 15:33:07.000000000","message":"Done","commit_id":"ffcf399cb4133200cc46b94227adb5c95a98a6b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"86d473817c4e512fdc93beb496cc8dc95c77137c","unresolved":false,"context_lines":[{"line_number":301,"context_line":"        grandchild_rp.destroy()"},{"line_number":302,"context_line":"        child_rp.destroy()"},{"line_number":303,"context_line":"        root_rp.destroy()"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"    def test_destroy_resource_provider(self):"},{"line_number":306,"context_line":"        created_resource_provider \u003d rp_obj.ResourceProvider("},{"line_number":307,"context_line":"            context\u003dself.ctx,"}],"source_content_type":"text/x-python","patch_set":52,"id":"3f4b6375_762bd52f","line":304,"updated":"2017-10-20 12:06:31.000000000","message":"I think we need test cases for the re-parenting case.","commit_id":"be733dbad357e1a095b7c2892c86a673db6d05d7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0dfd467663fe71fa6afbe6b71a094a2af777c965","unresolved":false,"context_lines":[{"line_number":301,"context_line":"        grandchild_rp.destroy()"},{"line_number":302,"context_line":"        child_rp.destroy()"},{"line_number":303,"context_line":"        root_rp.destroy()"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"    def test_destroy_resource_provider(self):"},{"line_number":306,"context_line":"        created_resource_provider \u003d rp_obj.ResourceProvider("},{"line_number":307,"context_line":"            context\u003dself.ctx,"}],"source_content_type":"text/x-python","patch_set":52,"id":"3f4b6375_fdb3204c","line":304,"in_reply_to":"3f4b6375_762bd52f","updated":"2017-10-20 15:04:40.000000000","message":"yep, will add.","commit_id":"be733dbad357e1a095b7c2892c86a673db6d05d7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"73e45486d2a1bff147d7bb11bf1869185fc33c76","unresolved":false,"context_lines":[{"line_number":301,"context_line":"        grandchild_rp.destroy()"},{"line_number":302,"context_line":"        child_rp.destroy()"},{"line_number":303,"context_line":"        root_rp.destroy()"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"    def test_destroy_resource_provider(self):"},{"line_number":306,"context_line":"        created_resource_provider \u003d rp_obj.ResourceProvider("},{"line_number":307,"context_line":"            context\u003dself.ctx,"}],"source_content_type":"text/x-python","patch_set":52,"id":"3f4b6375_185de3e8","line":304,"in_reply_to":"3f4b6375_fdb3204c","updated":"2017-10-20 21:06:23.000000000","message":"Done","commit_id":"be733dbad357e1a095b7c2892c86a673db6d05d7"}],"nova/tests/unit/objects/test_resource_provider.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"f597e6b8834b13df7b13eb00cda11455122a5393","unresolved":false,"context_lines":[{"line_number":103,"context_line":"                          obj.create)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def test_create_no_uuid_fail(self):"},{"line_number":106,"context_line":"        obj \u003d objects.ResourceProvider(context\u003dself.context)"},{"line_number":107,"context_line":"        self.assertRaises(exception.ObjectActionError,"},{"line_number":108,"context_line":"                          obj.create)"},{"line_number":109,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"7a77a97e_311cc969","line":106,"updated":"2017-10-01 14:45:27.000000000","message":"this test doesn\u0027t add the root provider, so is duplicating the test above, not testing what it says it means to test.","commit_id":"435b3fc0653915145e2aea6983979a6ee0660554"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"55b7a74780db243a8ed4d19dd173e12067eabe25","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        self.assertRaises(exception.ObjectActionError,"},{"line_number":134,"context_line":"                          obj.create)"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    def test_create_with_root_provider_uuid_fail(self):"},{"line_number":137,"context_line":"        obj \u003d objects.ResourceProvider(context\u003dself.context)"},{"line_number":138,"context_line":"        self.assertRaises(exception.ObjectActionError,"},{"line_number":139,"context_line":"                          obj.create)"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a77a97e_b1a379ff","line":136,"updated":"2016-11-17 13:06:37.000000000","message":"This test merely duplicates the test above, it doesn\u0027t try to set the root_provider_uuid, so it isn\u0027t testing what it\u0027s name says it is testing.","commit_id":"edf315095f6237dece80efd24255d08ad329b9d9"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"e57af3395aed965ec6e9f50cf4b51dbc99c194d7","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        self.assertRaises(exception.ObjectActionError,"},{"line_number":134,"context_line":"                          obj.create)"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    def test_create_with_root_provider_uuid_fail(self):"},{"line_number":137,"context_line":"        obj \u003d objects.ResourceProvider(context\u003dself.context)"},{"line_number":138,"context_line":"        self.assertRaises(exception.ObjectActionError,"},{"line_number":139,"context_line":"                          obj.create)"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a77a97e_14417b38","line":136,"in_reply_to":"7a77a97e_b1a379ff","updated":"2016-11-17 14:40:51.000000000","message":"yep, I forgot to update the test :)","commit_id":"edf315095f6237dece80efd24255d08ad329b9d9"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"7ea36c6cec850946568f8d6ed3d7e816cf31fea4","unresolved":false,"context_lines":[{"line_number":142,"context_line":"        )"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        exc \u003d self.assertRaises(exception.ObjectActionError, obj.create)"},{"line_number":145,"context_line":"        self.assertIn(\u0027root provider UUID cannot be manually set\u0027, str(exc))"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    def test_v11_contains_no_nested_providers(self):"},{"line_number":148,"context_line":"        \"\"\"Ensure that if we send a resource provider object to a backversioned"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a77a97e_fab8e52c","line":145,"range":{"start_line":145,"start_character":67,"end_line":145,"end_character":70},"updated":"2016-11-21 18:29:29.000000000","message":"nit: should use six.text_type()","commit_id":"6a345da4d9ee85b280ca0c4d4ee276d5be4fc62a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"683e757e71199fb00053d4c790892e2a859afb07","unresolved":false,"context_lines":[{"line_number":188,"context_line":"        self._test_create_in_db()"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    def test_create_in_db_with_parent_provider_uuid(self):"},{"line_number":191,"context_line":"        self._test_create_in_db()"},{"line_number":192,"context_line":"        updates \u003d {\u0027uuid\u0027: _RESOURCE_PROVIDER_UUID2,"},{"line_number":193,"context_line":"                   \u0027name\u0027: _RESOURCE_PROVIDER_NAME2,"},{"line_number":194,"context_line":"                   \u0027parent_provider_uuid\u0027: _RESOURCE_PROVIDER_UUID}"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a77a97e_b62f2f87","line":191,"range":{"start_line":191,"start_character":8,"end_line":191,"end_character":33},"updated":"2016-11-22 17:42:18.000000000","message":"We only care about the first two statements in this function, as there is no value in running the remainder (asserts) twice. If you respin, perhaps you could just duplicate those two lines here, or move the asserts into \u0027test_create_in_db\u0027","commit_id":"6a345da4d9ee85b280ca0c4d4ee276d5be4fc62a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fb32fad296005c4c73bbe20193c98bc41e37a5e3","unresolved":false,"context_lines":[{"line_number":44,"context_line":""},{"line_number":45,"context_line":"_RESOURCE_PROVIDER_ID2 \u003d 2"},{"line_number":46,"context_line":"_RESOURCE_PROVIDER_UUID2 \u003d uuids.resource_provider2"},{"line_number":47,"context_line":"_RESOURCE_PROVIDER_NAME2 \u003d uuids.resource_name2"},{"line_number":48,"context_line":"_RESOURCE_PROVIDER_DB2 \u003d {"},{"line_number":49,"context_line":"    \u0027id\u0027: _RESOURCE_PROVIDER_ID2,"},{"line_number":50,"context_line":"    \u0027uuid\u0027: _RESOURCE_PROVIDER_UUID2,"}],"source_content_type":"text/x-python","patch_set":40,"id":"5f201791_81c36738","line":47,"range":{"start_line":47,"start_character":27,"end_line":47,"end_character":47},"updated":"2017-06-26 14:32:04.000000000","message":"why doesn\u0027t this need \u0027six.text_type\u0027?","commit_id":"63577e9677654d1ee22b21e44d456eaec272187a"}]}