)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"7bb42e476700db630fe42a321569c048971ead7f","unresolved":false,"context_lines":[{"line_number":14,"context_line":"providers in play."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: I9639d852078c95de506110f24d3f35e7cf5e361e"},{"line_number":17,"context_line":"Related-Bug:#1799892"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3f79a3b5_70581788","line":17,"range":{"start_line":17,"start_character":0,"end_line":17,"end_character":20},"updated":"2018-10-25 19:43:20.000000000","message":"This should be Closes-Bug","commit_id":"7a692756dc3cd77907bfbc3b3c4f5bbab94a8417"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e588a6b73d674a09d79048610704de0234fdc718","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Consider root id is None in the database case"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"There are cases where ``root_provider_id`` of a resource provider is"},{"line_number":10,"context_line":"set to NULL just after it is upgraded to the Rocky release. In such"},{"line_number":11,"context_line":"cases getting allocation candidates raises a Keyerror."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This patch fixes that bug for cases there is no sharing or nested"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f79a3b5_b947577a","line":10,"range":{"start_line":10,"start_character":0,"end_line":10,"end_character":59},"updated":"2018-11-09 21:02:04.000000000","message":"Well, this would happen during the database schema migration, right? I think the issue in the bug is it\u0027s a really old disabled compute node provider which existed before queens when nested resource providers was added, and since it was never listed/shown out of the API the online data migration didn\u0027t happen, but the scheduler still picked it up because of using allocation candidates.","commit_id":"5c1d62682bf062e8cc9d7a0feba19d4ecccd2c3b"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"4c63d67aaeb4f5a0951d0b54b9753394080d8dfb","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Consider root id is None in the database case"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"There are cases where ``root_provider_id`` of a resource provider is"},{"line_number":10,"context_line":"set to NULL just after it is upgraded to the Rocky release. In such"},{"line_number":11,"context_line":"cases getting allocation candidates raises a Keyerror."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This patch fixes that bug for cases there is no sharing or nested"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f79a3b5_cd648351","line":10,"range":{"start_line":10,"start_character":0,"end_line":10,"end_character":59},"in_reply_to":"3f79a3b5_b947577a","updated":"2018-11-15 13:34:19.000000000","message":"Right.","commit_id":"5c1d62682bf062e8cc9d7a0feba19d4ecccd2c3b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e588a6b73d674a09d79048610704de0234fdc718","unresolved":false,"context_lines":[{"line_number":14,"context_line":"providers in play."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: I9639d852078c95de506110f24d3f35e7cf5e361e"},{"line_number":17,"context_line":"Related-Bug:#1799892"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f79a3b5_5956432d","line":17,"range":{"start_line":17,"start_character":0,"end_line":17,"end_character":7},"updated":"2018-11-09 21:02:04.000000000","message":"So should this be Closes-Bug? Or are there other considerations to take care of with sharing/nested providers? How would we know anyway unless parent_provider_uuid is set somewhere?","commit_id":"5c1d62682bf062e8cc9d7a0feba19d4ecccd2c3b"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"4c63d67aaeb4f5a0951d0b54b9753394080d8dfb","unresolved":false,"context_lines":[{"line_number":14,"context_line":"providers in play."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: I9639d852078c95de506110f24d3f35e7cf5e361e"},{"line_number":17,"context_line":"Related-Bug:#1799892"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f79a3b5_4d715313","line":17,"range":{"start_line":17,"start_character":0,"end_line":17,"end_character":7},"in_reply_to":"3f79a3b5_5956432d","updated":"2018-11-15 13:34:19.000000000","message":"This fix only fixes the path where nested/sharing providers are not in play....","commit_id":"5c1d62682bf062e8cc9d7a0feba19d4ecccd2c3b"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"b0c975d46b6d6e8518ccec1624b288d83b3340d5","unresolved":false,"context_lines":[{"line_number":14,"context_line":"providers in play."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: I9639d852078c95de506110f24d3f35e7cf5e361e"},{"line_number":17,"context_line":"Closes-Bug:#1799892"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_d2e16133","line":17,"range":{"start_line":17,"start_character":0,"end_line":17,"end_character":19},"updated":"2018-11-18 08:06:02.000000000","message":"I\u0027d like to backport this to Rocky. But don\u0027t want to port this to placement, instead we\u0027d like to create \"placement-manage db online_data_migrations\" to have the explicit migration script. I will have another launchpad-tracking bug  for the migration. Thoughts?","commit_id":"6c793f1b5dd8269dca1580438d9f7a8e19a00c79"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"148759e5fa1a104c4ad950d2662df3a5adad8b5a","unresolved":false,"context_lines":[{"line_number":14,"context_line":"providers in play."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: I9639d852078c95de506110f24d3f35e7cf5e361e"},{"line_number":17,"context_line":"Closes-Bug:#1799892"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_cefa1d11","line":17,"range":{"start_line":17,"start_character":0,"end_line":17,"end_character":19},"in_reply_to":"3f79a3b5_48f124df","updated":"2018-11-19 04:44:17.000000000","message":"Ah, thanks. I\u0027m good with porting this to placement. My point was we should remember to have some explicit migration script. I\u0027ve opened a new bug while closing the bug 1799892:\nhttps://bugs.launchpad.net/nova/+bug/1803925","commit_id":"6c793f1b5dd8269dca1580438d9f7a8e19a00c79"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"ec30a824b33a1e29d24a9959787d7c00dceaf2fb","unresolved":false,"context_lines":[{"line_number":14,"context_line":"providers in play."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: I9639d852078c95de506110f24d3f35e7cf5e361e"},{"line_number":17,"context_line":"Closes-Bug:#1799892"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_48f124df","line":17,"range":{"start_line":17,"start_character":0,"end_line":17,"end_character":19},"in_reply_to":"3f79a3b5_d2e16133","updated":"2018-11-18 23:34:47.000000000","message":"I\u0027ve already started the process of porting to placement. Once your patch is merged, I will update my patch to be the same.\n\nhttps://review.openstack.org/#/c/613320/","commit_id":"6c793f1b5dd8269dca1580438d9f7a8e19a00c79"}],"nova/api/openstack/placement/objects/resource_provider.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"98a3dea2f7f457b456b25529fee5b67c3380fd2b","unresolved":false,"context_lines":[{"line_number":739,"context_line":"        # Use its id/uuid for the root id/uuid if the root id/uuid is None"},{"line_number":740,"context_line":"        # TODO(tetsuro): Remove this to when we are sure all root_provider_id"},{"line_number":741,"context_line":"        # values are NOT NULL"},{"line_number":742,"context_line":"        d \u003d dict(r.items())"},{"line_number":743,"context_line":"        if d[\u0027root_id\u0027] is None:"},{"line_number":744,"context_line":"            d[\u0027root_id\u0027] \u003d d[\u0027id\u0027]"},{"line_number":745,"context_line":"            d[\u0027root_uuid\u0027] \u003d d[\u0027uuid\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_d5f47d58","line":742,"range":{"start_line":742,"start_character":8,"end_line":742,"end_character":27},"updated":"2018-10-25 19:42:40.000000000","message":"I think .items() is redundant, and might even create an extra copy.\n\n d \u003d dict(r)\n\nought to work.","commit_id":"7a692756dc3cd77907bfbc3b3c4f5bbab94a8417"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"c6da2dbea526c71c3644de96ade25fc85442b276","unresolved":false,"context_lines":[{"line_number":739,"context_line":"        # Use its id/uuid for the root id/uuid if the root id/uuid is None"},{"line_number":740,"context_line":"        # TODO(tetsuro): Remove this to when we are sure all root_provider_id"},{"line_number":741,"context_line":"        # values are NOT NULL"},{"line_number":742,"context_line":"        d \u003d dict(r.items())"},{"line_number":743,"context_line":"        if d[\u0027root_id\u0027] is None:"},{"line_number":744,"context_line":"            d[\u0027root_id\u0027] \u003d d[\u0027id\u0027]"},{"line_number":745,"context_line":"            d[\u0027root_uuid\u0027] \u003d d[\u0027uuid\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_e165341d","line":742,"range":{"start_line":742,"start_character":8,"end_line":742,"end_character":27},"in_reply_to":"3f79a3b5_d5f47d58","updated":"2018-10-26 07:46:47.000000000","message":"Done","commit_id":"7a692756dc3cd77907bfbc3b3c4f5bbab94a8417"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"98a3dea2f7f457b456b25529fee5b67c3380fd2b","unresolved":false,"context_lines":[{"line_number":743,"context_line":"        if d[\u0027root_id\u0027] is None:"},{"line_number":744,"context_line":"            d[\u0027root_id\u0027] \u003d d[\u0027id\u0027]"},{"line_number":745,"context_line":"            d[\u0027root_uuid\u0027] \u003d d[\u0027uuid\u0027]"},{"line_number":746,"context_line":"        ret.update({d[\u0027id\u0027]: ProviderIds(**d)})"},{"line_number":747,"context_line":"    return ret"},{"line_number":748,"context_line":""},{"line_number":749,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_552cedfe","line":746,"range":{"start_line":746,"start_character":8,"end_line":746,"end_character":47},"updated":"2018-10-25 19:42:40.000000000","message":"Why .update? Why not\n\n ret[d[\u0027id\u0027]] \u003d ProviderIds(**d)\n\n?","commit_id":"7a692756dc3cd77907bfbc3b3c4f5bbab94a8417"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"c6da2dbea526c71c3644de96ade25fc85442b276","unresolved":false,"context_lines":[{"line_number":743,"context_line":"        if d[\u0027root_id\u0027] is None:"},{"line_number":744,"context_line":"            d[\u0027root_id\u0027] \u003d d[\u0027id\u0027]"},{"line_number":745,"context_line":"            d[\u0027root_uuid\u0027] \u003d d[\u0027uuid\u0027]"},{"line_number":746,"context_line":"        ret.update({d[\u0027id\u0027]: ProviderIds(**d)})"},{"line_number":747,"context_line":"    return ret"},{"line_number":748,"context_line":""},{"line_number":749,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_016950f5","line":746,"range":{"start_line":746,"start_character":8,"end_line":746,"end_character":47},"in_reply_to":"3f79a3b5_552cedfe","updated":"2018-10-26 07:46:47.000000000","message":"Done","commit_id":"7a692756dc3cd77907bfbc3b3c4f5bbab94a8417"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"98a3dea2f7f457b456b25529fee5b67c3380fd2b","unresolved":false,"context_lines":[{"line_number":2741,"context_line":"    #   FROM allocations"},{"line_number":2742,"context_line":"    #   JOIN resource_providers"},{"line_number":2743,"context_line":"    #     ON allocations.resource_provider_id \u003d resource_providers.id"},{"line_number":2744,"context_line":"    #     AND resource_providers.root_provider_id IN($root_ids)"},{"line_number":2745,"context_line":"    #   GROUP BY resource_provider_id, resource_class_id"},{"line_number":2746,"context_line":"    # )"},{"line_number":2747,"context_line":"    # AS usages"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_b54c619d","line":2744,"range":{"start_line":2744,"start_character":10,"end_line":2744,"end_character":63},"updated":"2018-10-25 19:42:40.000000000","message":"Update this:\n\n AND (rp.root_provider_id IN($root_ids) OR rp.id IN ($root_ids))","commit_id":"7a692756dc3cd77907bfbc3b3c4f5bbab94a8417"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"c6da2dbea526c71c3644de96ade25fc85442b276","unresolved":false,"context_lines":[{"line_number":2741,"context_line":"    #   FROM allocations"},{"line_number":2742,"context_line":"    #   JOIN resource_providers"},{"line_number":2743,"context_line":"    #     ON allocations.resource_provider_id \u003d resource_providers.id"},{"line_number":2744,"context_line":"    #     AND resource_providers.root_provider_id IN($root_ids)"},{"line_number":2745,"context_line":"    #   GROUP BY resource_provider_id, resource_class_id"},{"line_number":2746,"context_line":"    # )"},{"line_number":2747,"context_line":"    # AS usages"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_c16ef8fc","line":2744,"range":{"start_line":2744,"start_character":10,"end_line":2744,"end_character":63},"in_reply_to":"3f79a3b5_b54c619d","updated":"2018-10-26 07:46:47.000000000","message":"Done","commit_id":"7a692756dc3cd77907bfbc3b3c4f5bbab94a8417"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"98a3dea2f7f457b456b25529fee5b67c3380fd2b","unresolved":false,"context_lines":[{"line_number":2747,"context_line":"    # AS usages"},{"line_number":2748,"context_line":"    #   ON inv.resource_provider_id \u003d usage.resource_provider_id"},{"line_number":2749,"context_line":"    #   AND inv.resource_class_id \u003d usage.resource_class_id"},{"line_number":2750,"context_line":"    # WHERE rp.root_provider_id IN ($root_ids)"},{"line_number":2751,"context_line":"    rpt \u003d sa.alias(_RP_TBL, name\u003d\"rp\")"},{"line_number":2752,"context_line":"    inv \u003d sa.alias(_INV_TBL, name\u003d\"inv\")"},{"line_number":2753,"context_line":"    # Build our derived table (subquery in the FROM clause) that sums used"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_7005770e","line":2750,"range":{"start_line":2750,"start_character":6,"end_line":2750,"end_character":46},"updated":"2018-10-25 19:42:40.000000000","message":"likewise","commit_id":"7a692756dc3cd77907bfbc3b3c4f5bbab94a8417"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"c6da2dbea526c71c3644de96ade25fc85442b276","unresolved":false,"context_lines":[{"line_number":2747,"context_line":"    # AS usages"},{"line_number":2748,"context_line":"    #   ON inv.resource_provider_id \u003d usage.resource_provider_id"},{"line_number":2749,"context_line":"    #   AND inv.resource_class_id \u003d usage.resource_class_id"},{"line_number":2750,"context_line":"    # WHERE rp.root_provider_id IN ($root_ids)"},{"line_number":2751,"context_line":"    rpt \u003d sa.alias(_RP_TBL, name\u003d\"rp\")"},{"line_number":2752,"context_line":"    inv \u003d sa.alias(_INV_TBL, name\u003d\"inv\")"},{"line_number":2753,"context_line":"    # Build our derived table (subquery in the FROM clause) that sums used"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_617924c4","line":2750,"range":{"start_line":2750,"start_character":6,"end_line":2750,"end_character":46},"in_reply_to":"3f79a3b5_7005770e","updated":"2018-10-26 07:46:47.000000000","message":"Done","commit_id":"7a692756dc3cd77907bfbc3b3c4f5bbab94a8417"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"98a3dea2f7f457b456b25529fee5b67c3380fd2b","unresolved":false,"context_lines":[{"line_number":2755,"context_line":"    derived_alloc_to_rp \u003d sa.join("},{"line_number":2756,"context_line":"        _ALLOC_TBL, _RP_TBL,"},{"line_number":2757,"context_line":"        sa.and_(_ALLOC_TBL.c.resource_provider_id \u003d\u003d _RP_TBL.c.id,"},{"line_number":2758,"context_line":"                # TODO(tetsuro): Remove this OR condition when all"},{"line_number":2759,"context_line":"                # root_provider_id values are NOT NULL"},{"line_number":2760,"context_line":"                sa.or_(_RP_TBL.c.root_provider_id.in_(root_ids),"},{"line_number":2761,"context_line":"                       _RP_TBL.c.id.in_(root_ids))"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_d0c00b4e","line":2758,"updated":"2018-10-25 19:42:40.000000000","message":"When I revert just this part of the fix, the test case still passes.","commit_id":"7a692756dc3cd77907bfbc3b3c4f5bbab94a8417"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"c6da2dbea526c71c3644de96ade25fc85442b276","unresolved":false,"context_lines":[{"line_number":2755,"context_line":"    derived_alloc_to_rp \u003d sa.join("},{"line_number":2756,"context_line":"        _ALLOC_TBL, _RP_TBL,"},{"line_number":2757,"context_line":"        sa.and_(_ALLOC_TBL.c.resource_provider_id \u003d\u003d _RP_TBL.c.id,"},{"line_number":2758,"context_line":"                # TODO(tetsuro): Remove this OR condition when all"},{"line_number":2759,"context_line":"                # root_provider_id values are NOT NULL"},{"line_number":2760,"context_line":"                sa.or_(_RP_TBL.c.root_provider_id.in_(root_ids),"},{"line_number":2761,"context_line":"                       _RP_TBL.c.id.in_(root_ids))"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_0142b067","line":2758,"in_reply_to":"3f79a3b5_d0c00b4e","updated":"2018-10-26 07:46:47.000000000","message":"Amended the test to have a consumer with an allocation and added a test validating the returned prov_sum. Removing this fails now.","commit_id":"7a692756dc3cd77907bfbc3b3c4f5bbab94a8417"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"98a3dea2f7f457b456b25529fee5b67c3380fd2b","unresolved":false,"context_lines":[{"line_number":2758,"context_line":"                # TODO(tetsuro): Remove this OR condition when all"},{"line_number":2759,"context_line":"                # root_provider_id values are NOT NULL"},{"line_number":2760,"context_line":"                sa.or_(_RP_TBL.c.root_provider_id.in_(root_ids),"},{"line_number":2761,"context_line":"                       _RP_TBL.c.id.in_(root_ids))"},{"line_number":2762,"context_line":"                )"},{"line_number":2763,"context_line":"    )"},{"line_number":2764,"context_line":"    usage \u003d sa.alias("}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_b0378faf","line":2761,"range":{"start_line":2761,"start_character":23,"end_line":2761,"end_character":50},"updated":"2018-10-25 19:42:40.000000000","message":"I was worried that this is now too permissive. But what we\u0027re doing here is getting usages for everything in the tree associated with each ID in root_ids. So even if root_ids contained non-root IDs (which it actually can, based on the way it\u0027s called at [1]), we\u0027re at worst looking over the same results twice, which the db engine will optimize out.\n\n[1] https://github.com/openstack/nova/blob/8ec31bfd4259d78603477f57bf259ab3241219ab/nova/api/openstack/placement/objects/resource_provider.py#L3580-L3583","commit_id":"7a692756dc3cd77907bfbc3b3c4f5bbab94a8417"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"c6da2dbea526c71c3644de96ade25fc85442b276","unresolved":false,"context_lines":[{"line_number":2758,"context_line":"                # TODO(tetsuro): Remove this OR condition when all"},{"line_number":2759,"context_line":"                # root_provider_id values are NOT NULL"},{"line_number":2760,"context_line":"                sa.or_(_RP_TBL.c.root_provider_id.in_(root_ids),"},{"line_number":2761,"context_line":"                       _RP_TBL.c.id.in_(root_ids))"},{"line_number":2762,"context_line":"                )"},{"line_number":2763,"context_line":"    )"},{"line_number":2764,"context_line":"    usage \u003d sa.alias("}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_66793ac4","line":2761,"range":{"start_line":2761,"start_character":23,"end_line":2761,"end_character":50},"in_reply_to":"3f79a3b5_b0378faf","updated":"2018-10-26 07:46:47.000000000","message":"Right. Thank you for reading this deeply.","commit_id":"7a692756dc3cd77907bfbc3b3c4f5bbab94a8417"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"98a3dea2f7f457b456b25529fee5b67c3380fd2b","unresolved":false,"context_lines":[{"line_number":2996,"context_line":"    ret \u003d []"},{"line_number":2997,"context_line":"    for rp_tuple in rp_tuples:"},{"line_number":2998,"context_line":"        rp_id \u003d rp_tuple[0]"},{"line_number":2999,"context_line":"        root_id \u003d rp_tuple[0] if rp_tuple[1] is None else rp_tuple[1]"},{"line_number":3000,"context_line":"        ret.append((rp_id, root_id))"},{"line_number":3001,"context_line":"    return ret"},{"line_number":3002,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_d0a32b9b","line":2999,"range":{"start_line":2999,"start_character":18,"end_line":2999,"end_character":29},"updated":"2018-10-25 19:42:40.000000000","message":"or rp_id. Otherwise you could just do without the rp_id var.","commit_id":"7a692756dc3cd77907bfbc3b3c4f5bbab94a8417"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"c6da2dbea526c71c3644de96ade25fc85442b276","unresolved":false,"context_lines":[{"line_number":2996,"context_line":"    ret \u003d []"},{"line_number":2997,"context_line":"    for rp_tuple in rp_tuples:"},{"line_number":2998,"context_line":"        rp_id \u003d rp_tuple[0]"},{"line_number":2999,"context_line":"        root_id \u003d rp_tuple[0] if rp_tuple[1] is None else rp_tuple[1]"},{"line_number":3000,"context_line":"        ret.append((rp_id, root_id))"},{"line_number":3001,"context_line":"    return ret"},{"line_number":3002,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_a15c1c4c","line":2999,"range":{"start_line":2999,"start_character":18,"end_line":2999,"end_character":29},"in_reply_to":"3f79a3b5_d0a32b9b","updated":"2018-10-26 07:46:47.000000000","message":"Using rp_id is more readable to me. Done.","commit_id":"7a692756dc3cd77907bfbc3b3c4f5bbab94a8417"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e588a6b73d674a09d79048610704de0234fdc718","unresolved":false,"context_lines":[{"line_number":735,"context_line":"    sel \u003d sel.where(me.c.id.in_(rp_ids))"},{"line_number":736,"context_line":""},{"line_number":737,"context_line":"    ret \u003d {}"},{"line_number":738,"context_line":"    for r in context.session.execute(sel):"},{"line_number":739,"context_line":"        # Use its id/uuid for the root id/uuid if the root id/uuid is None"},{"line_number":740,"context_line":"        # TODO(tetsuro): Remove this to when we are sure all root_provider_id"},{"line_number":741,"context_line":"        # values are NOT NULL"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_9935bbf7","line":738,"updated":"2018-11-09 21:02:04.000000000","message":"It might be worth a comment explaining why we need this and reference bug 1799892 - the reason being we could have really old providers with no parent/root provider set and they haven\u0027t undergone a data migration yet.","commit_id":"5c1d62682bf062e8cc9d7a0feba19d4ecccd2c3b"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"4c63d67aaeb4f5a0951d0b54b9753394080d8dfb","unresolved":false,"context_lines":[{"line_number":735,"context_line":"    sel \u003d sel.where(me.c.id.in_(rp_ids))"},{"line_number":736,"context_line":""},{"line_number":737,"context_line":"    ret \u003d {}"},{"line_number":738,"context_line":"    for r in context.session.execute(sel):"},{"line_number":739,"context_line":"        # Use its id/uuid for the root id/uuid if the root id/uuid is None"},{"line_number":740,"context_line":"        # TODO(tetsuro): Remove this to when we are sure all root_provider_id"},{"line_number":741,"context_line":"        # values are NOT NULL"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_ed613f3e","line":738,"in_reply_to":"3f79a3b5_9935bbf7","updated":"2018-11-15 13:34:19.000000000","message":"Ack, will fix.","commit_id":"5c1d62682bf062e8cc9d7a0feba19d4ecccd2c3b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e588a6b73d674a09d79048610704de0234fdc718","unresolved":false,"context_lines":[{"line_number":737,"context_line":"    ret \u003d {}"},{"line_number":738,"context_line":"    for r in context.session.execute(sel):"},{"line_number":739,"context_line":"        # Use its id/uuid for the root id/uuid if the root id/uuid is None"},{"line_number":740,"context_line":"        # TODO(tetsuro): Remove this to when we are sure all root_provider_id"},{"line_number":741,"context_line":"        # values are NOT NULL"},{"line_number":742,"context_line":"        d \u003d dict(r)"},{"line_number":743,"context_line":"        if d[\u0027root_id\u0027] is None:"},{"line_number":744,"context_line":"            d[\u0027root_id\u0027] \u003d d[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_f925cfc0","line":741,"range":{"start_line":740,"start_character":8,"end_line":741,"end_character":29},"updated":"2018-11-09 21:02:04.000000000","message":"We likely need a \"placement-manage db online_data_migrations\" command for stuff like this.","commit_id":"5c1d62682bf062e8cc9d7a0feba19d4ecccd2c3b"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"4c63d67aaeb4f5a0951d0b54b9753394080d8dfb","unresolved":false,"context_lines":[{"line_number":737,"context_line":"    ret \u003d {}"},{"line_number":738,"context_line":"    for r in context.session.execute(sel):"},{"line_number":739,"context_line":"        # Use its id/uuid for the root id/uuid if the root id/uuid is None"},{"line_number":740,"context_line":"        # TODO(tetsuro): Remove this to when we are sure all root_provider_id"},{"line_number":741,"context_line":"        # values are NOT NULL"},{"line_number":742,"context_line":"        d \u003d dict(r)"},{"line_number":743,"context_line":"        if d[\u0027root_id\u0027] is None:"},{"line_number":744,"context_line":"            d[\u0027root_id\u0027] \u003d d[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_2db3b70b","line":741,"range":{"start_line":740,"start_character":8,"end_line":741,"end_character":29},"in_reply_to":"3f79a3b5_f925cfc0","updated":"2018-11-15 13:34:19.000000000","message":"Yes, this kind of TODO is everywhere in the file, so we need a way to say \"okay, we are sure all root_provider_id are not NULL.\"","commit_id":"5c1d62682bf062e8cc9d7a0feba19d4ecccd2c3b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"bee079dcc0c5c023ed00399af48235d4897d29b2","unresolved":false,"context_lines":[{"line_number":2741,"context_line":"    #   FROM allocations"},{"line_number":2742,"context_line":"    #   JOIN resource_providers"},{"line_number":2743,"context_line":"    #     ON allocations.resource_provider_id \u003d resource_providers.id"},{"line_number":2744,"context_line":"    #     AND (resource_providers.root_provider_id IN($root_ids)"},{"line_number":2745,"context_line":"    #          OR resource_providers.id IN($root_ids))"},{"line_number":2746,"context_line":"    #   GROUP BY resource_provider_id, resource_class_id"},{"line_number":2747,"context_line":"    # )"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_73bd6801","line":2744,"range":{"start_line":2744,"start_character":15,"end_line":2744,"end_character":33},"updated":"2018-10-26 20:12:27.000000000","message":"nit, mostly unrelated, this should be `rp` (the alias assigned on L2736) here and below.","commit_id":"5c1d62682bf062e8cc9d7a0feba19d4ecccd2c3b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e588a6b73d674a09d79048610704de0234fdc718","unresolved":false,"context_lines":[{"line_number":2996,"context_line":""},{"line_number":2997,"context_line":"    # TODO(tetsuro): Remove this when all root_provider_id values are NOT NULL"},{"line_number":2998,"context_line":"    ret \u003d []"},{"line_number":2999,"context_line":"    for rp_tuple in rp_tuples:"},{"line_number":3000,"context_line":"        rp_id \u003d rp_tuple[0]"},{"line_number":3001,"context_line":"        root_id \u003d rp_id if rp_tuple[1] is None else rp_tuple[1]"},{"line_number":3002,"context_line":"        ret.append((rp_id, root_id))"},{"line_number":3003,"context_line":"    return ret"},{"line_number":3004,"context_line":""},{"line_number":3005,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_19fa0b11","line":3002,"range":{"start_line":2999,"start_character":4,"end_line":3002,"end_character":36},"updated":"2018-11-09 21:02:04.000000000","message":"Would it be better to do this in _get_providers_with_resource so it\u0027s handled in a single place?","commit_id":"5c1d62682bf062e8cc9d7a0feba19d4ecccd2c3b"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"4c63d67aaeb4f5a0951d0b54b9753394080d8dfb","unresolved":false,"context_lines":[{"line_number":2996,"context_line":""},{"line_number":2997,"context_line":"    # TODO(tetsuro): Remove this when all root_provider_id values are NOT NULL"},{"line_number":2998,"context_line":"    ret \u003d []"},{"line_number":2999,"context_line":"    for rp_tuple in rp_tuples:"},{"line_number":3000,"context_line":"        rp_id \u003d rp_tuple[0]"},{"line_number":3001,"context_line":"        root_id \u003d rp_id if rp_tuple[1] is None else rp_tuple[1]"},{"line_number":3002,"context_line":"        ret.append((rp_id, root_id))"},{"line_number":3003,"context_line":"    return ret"},{"line_number":3004,"context_line":""},{"line_number":3005,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_0d54bb55","line":3002,"range":{"start_line":2999,"start_character":4,"end_line":3002,"end_character":36},"in_reply_to":"3f79a3b5_04de226a","updated":"2018-11-15 13:34:19.000000000","message":"Yes, fair point, and I should see with a new test if moving it to _get_providers_with_resource also fixes the case shared/nested providers are in play. Will do.","commit_id":"5c1d62682bf062e8cc9d7a0feba19d4ecccd2c3b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"65724e5d2b578e647fd632862863656b91c07bab","unresolved":false,"context_lines":[{"line_number":2996,"context_line":""},{"line_number":2997,"context_line":"    # TODO(tetsuro): Remove this when all root_provider_id values are NOT NULL"},{"line_number":2998,"context_line":"    ret \u003d []"},{"line_number":2999,"context_line":"    for rp_tuple in rp_tuples:"},{"line_number":3000,"context_line":"        rp_id \u003d rp_tuple[0]"},{"line_number":3001,"context_line":"        root_id \u003d rp_id if rp_tuple[1] is None else rp_tuple[1]"},{"line_number":3002,"context_line":"        ret.append((rp_id, root_id))"},{"line_number":3003,"context_line":"    return ret"},{"line_number":3004,"context_line":""},{"line_number":3005,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_04de226a","line":3002,"range":{"start_line":2999,"start_character":4,"end_line":3002,"end_character":36},"in_reply_to":"3f79a3b5_19fa0b11","updated":"2018-11-09 21:13:17.000000000","message":"This is a fair question. See below.","commit_id":"5c1d62682bf062e8cc9d7a0feba19d4ecccd2c3b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"65724e5d2b578e647fd632862863656b91c07bab","unresolved":false,"context_lines":[{"line_number":3238,"context_line":"            # If there\u0027s no providers that have one of the resource classes,"},{"line_number":3239,"context_line":"            # then we can short-circuit"},{"line_number":3240,"context_line":"            return []"},{"line_number":3241,"context_line":"        rc_trees \u003d set(p[1] for p in rc_provs_with_inv)"},{"line_number":3242,"context_line":"        provs_with_inv |\u003d set((p[0], p[1], rc_id) for p in rc_provs_with_inv)"},{"line_number":3243,"context_line":""},{"line_number":3244,"context_line":"        sharing_providers \u003d sharing.get(rc_id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_64d35664","line":3241,"range":{"start_line":3241,"start_character":0,"end_line":3241,"end_character":55},"updated":"2018-11-09 21:13:17.000000000","message":"@Matt, so related to your question, could we hit this where p[1] is NULL?","commit_id":"5c1d62682bf062e8cc9d7a0feba19d4ecccd2c3b"}],"nova/tests/functional/api/openstack/placement/db/test_allocation_candidates.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f0542b4051c7437c0dd93828e268f7f9a1ed92ff","unresolved":false,"context_lines":[{"line_number":516,"context_line":"        self._validate_provider_summary_resources(expected, alloc_cands)"},{"line_number":517,"context_line":""},{"line_number":518,"context_line":"        # NOTE(tetsuro): Getting allocation candidates goes through a"},{"line_number":519,"context_line":"        # different path when sharing/nested providers exit, let\u0027s test"},{"line_number":520,"context_line":"        # that case and the path creating a new sharing provider."},{"line_number":521,"context_line":"        ss1 \u003d self._create_provider(\u0027ss1\u0027, uuids.agg1)"},{"line_number":522,"context_line":"        tb.set_traits(ss1, \"MISC_SHARES_VIA_AGGREGATE\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_6b097081","line":519,"range":{"start_line":519,"start_character":55,"end_line":519,"end_character":59},"updated":"2018-11-19 16:09:04.000000000","message":"exist","commit_id":"68e2c205daf38a24ff55b7c29f7690a029422e56"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"817e7edd195e5304f72881e82836d405b4f4d869","unresolved":false,"context_lines":[{"line_number":516,"context_line":"        self._validate_provider_summary_resources(expected, alloc_cands)"},{"line_number":517,"context_line":""},{"line_number":518,"context_line":"        # NOTE(tetsuro): Getting allocation candidates goes through a"},{"line_number":519,"context_line":"        # different path when sharing/nested providers exit, let\u0027s test"},{"line_number":520,"context_line":"        # that case and the path creating a new sharing provider."},{"line_number":521,"context_line":"        ss1 \u003d self._create_provider(\u0027ss1\u0027, uuids.agg1)"},{"line_number":522,"context_line":"        tb.set_traits(ss1, \"MISC_SHARES_VIA_AGGREGATE\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_2f537917","line":519,"range":{"start_line":519,"start_character":55,"end_line":519,"end_character":59},"in_reply_to":"3f79a3b5_6b097081","updated":"2018-11-20 02:27:12.000000000","message":"Done","commit_id":"68e2c205daf38a24ff55b7c29f7690a029422e56"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f0542b4051c7437c0dd93828e268f7f9a1ed92ff","unresolved":false,"context_lines":[{"line_number":518,"context_line":"        # NOTE(tetsuro): Getting allocation candidates goes through a"},{"line_number":519,"context_line":"        # different path when sharing/nested providers exit, let\u0027s test"},{"line_number":520,"context_line":"        # that case and the path creating a new sharing provider."},{"line_number":521,"context_line":"        ss1 \u003d self._create_provider(\u0027ss1\u0027, uuids.agg1)"},{"line_number":522,"context_line":"        tb.set_traits(ss1, \"MISC_SHARES_VIA_AGGREGATE\")"},{"line_number":523,"context_line":"        tb.add_inventory(ss1, fields.ResourceClass.VCPU, 8)"},{"line_number":524,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_6b57305b","line":521,"range":{"start_line":521,"start_character":19,"end_line":521,"end_character":35},"updated":"2018-11-19 16:09:04.000000000","message":"Is this valid to use for the test where the provider doesn\u0027t have a parent set? This will use the object to create the provider record, right? And I thought we needed to do the database insert directly, like above on L460.","commit_id":"68e2c205daf38a24ff55b7c29f7690a029422e56"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"817e7edd195e5304f72881e82836d405b4f4d869","unresolved":false,"context_lines":[{"line_number":518,"context_line":"        # NOTE(tetsuro): Getting allocation candidates goes through a"},{"line_number":519,"context_line":"        # different path when sharing/nested providers exit, let\u0027s test"},{"line_number":520,"context_line":"        # that case and the path creating a new sharing provider."},{"line_number":521,"context_line":"        ss1 \u003d self._create_provider(\u0027ss1\u0027, uuids.agg1)"},{"line_number":522,"context_line":"        tb.set_traits(ss1, \"MISC_SHARES_VIA_AGGREGATE\")"},{"line_number":523,"context_line":"        tb.add_inventory(ss1, fields.ResourceClass.VCPU, 8)"},{"line_number":524,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_8f43ad40","line":521,"range":{"start_line":521,"start_character":19,"end_line":521,"end_character":35},"in_reply_to":"3f79a3b5_6b57305b","updated":"2018-11-20 02:27:12.000000000","message":"\u003e Is this valid to use for the test where the provider doesn\u0027t have a parent set? \n\u003e This will use the object to create the provider record, right?\n\nYes and yes, if parent is not given, it just creates the rp object/record as a root provider.  \n\n\u003e And I thought we needed to do the database insert directly, like above on L460.\n\nWell, we could do that, but the purpose of adding the sharing provider here is just to make \"cn1\" go through the other path than the previous test, so I didn\u0027t want to have more lines unnecessarily.\n\nAdding this new test on PS2, which produced your question [1], fails which means this test works appropriately.\n\nAdded some words in the comment.\n\n[1] https://review.openstack.org/#/c/613305/2/nova/api/openstack/placement/objects/resource_provider.py@3002","commit_id":"68e2c205daf38a24ff55b7c29f7690a029422e56"}]}