)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"40e8a2455b7e1e5a14fd750e54b9923282e3a277","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Surya Seetharaman \u003csuryaseetharaman.9@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-10-05 10:44:56 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Return a minimal construct for nova list when a cell is down"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch modifies the existing behavior of nova list which"},{"line_number":10,"context_line":"ignores a down cell to returning a partial construct consisting of"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":20,"id":"3f79a3b5_86cf6c27","line":7,"updated":"2018-10-17 17:10:47.000000000","message":"This is all just plumbing in this patch, right? Nothing changes the behavior of the API until the end of the series when the microversion is requested, which uses the code being added here, right? If so, let\u0027s make that clear in the commit message.","commit_id":"b19a4065956076fef698dd313359e7aa0f58e969"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ea7b0b39890423500419028a4c5dd5c79198b53d","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch augments the existing behavior of nova list which"},{"line_number":10,"context_line":"ignores a down cell to returning a partial construct consisting of"},{"line_number":11,"context_line":"uuid, project_id and created_at fields obtained from the API"},{"line_number":12,"context_line":"instance_mappings table for the instances in the down cell by"},{"line_number":13,"context_line":"introducing a new API microversion. This way when the user"},{"line_number":14,"context_line":"gets partial results for certain records, he/she will know"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":27,"id":"3f79a3b5_c6f935c6","line":11,"range":{"start_line":11,"start_character":0,"end_line":11,"end_character":38},"updated":"2018-10-25 21:58:09.000000000","message":"We\u0027ll also show status as UNKNOWN:\n\nhttps://specs.openstack.org/openstack/nova-specs/specs/stein/approved/handling-down-cell_new.html#proposed-change\n\nBut I expect that happens in the view builder code later in the series.","commit_id":"1e1a552c830236ef23158b40fdac07e4a2cf6f7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b2d95f29e68236b4318257469a4a0b6fa0d298ef","unresolved":false,"context_lines":[{"line_number":18,"context_line":"later in this series."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Related to blueprint handling-down-cell"},{"line_number":21,"context_line":"Closes-Bug: #1726301"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: Ie9a9aed0676d469e5180d4c2d2631cf339335b2d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":27,"id":"3f79a3b5_22a1e7f6","line":21,"range":{"start_line":21,"start_character":0,"end_line":21,"end_character":20},"updated":"2018-10-26 12:42:00.000000000","message":"This seems odd to put in here, that bug is already fixed, why is this in this plumbing change for the feature?","commit_id":"1e1a552c830236ef23158b40fdac07e4a2cf6f7b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c7ac98ec39b5bc8a0d76034b294ae751b1066c9b","unresolved":false,"context_lines":[{"line_number":18,"context_line":"later in this series."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Related to blueprint handling-down-cell"},{"line_number":21,"context_line":"Closes-Bug: #1726301"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: Ie9a9aed0676d469e5180d4c2d2631cf339335b2d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":27,"id":"3f79a3b5_67d92a82","line":21,"range":{"start_line":21,"start_character":0,"end_line":21,"end_character":20},"in_reply_to":"3f79a3b5_22a1e7f6","updated":"2018-10-26 16:43:40.000000000","message":"Unsure, but will remove.","commit_id":"1e1a552c830236ef23158b40fdac07e4a2cf6f7b"}],"nova/api/openstack/compute/config_drive.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e54054085c7a492a24428edeba52f07147e8d800","unresolved":false,"context_lines":[{"line_number":29,"context_line":"            db_server \u003d req.get_db_instance(server[\u0027id\u0027])"},{"line_number":30,"context_line":"            # server[\u0027id\u0027] is guaranteed to be in the cache due to"},{"line_number":31,"context_line":"            # the core API adding it in its \u0027show\u0027/\u0027detail\u0027 methods."},{"line_number":32,"context_line":"            if (api_version_request.is_supported(req, min_version\u003d\u00272.64\u0027) and"},{"line_number":33,"context_line":"                not hasattr(db_server, \u0027created_at\u0027)):"},{"line_number":34,"context_line":"                    continue"},{"line_number":35,"context_line":"            server[ATTRIBUTE_NAME] \u003d db_server[\u0027config_drive\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_33984d74","line":32,"updated":"2018-07-19 23:18:55.000000000","message":"Can we not handle this in the view builder code for the serve response where it\u0027s centralized? Or not yet because these extension controllers are adding stuff to the response body directly?","commit_id":"556689b3dc7a85796a758e668db032ffcf4c3dbb"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"4339d566eac9ee7325b53ee445957f40d9f9bb72","unresolved":false,"context_lines":[{"line_number":29,"context_line":"            db_server \u003d req.get_db_instance(server[\u0027id\u0027])"},{"line_number":30,"context_line":"            # server[\u0027id\u0027] is guaranteed to be in the cache due to"},{"line_number":31,"context_line":"            # the core API adding it in its \u0027show\u0027/\u0027detail\u0027 methods."},{"line_number":32,"context_line":"            if (api_version_request.is_supported(req, min_version\u003d\u00272.64\u0027) and"},{"line_number":33,"context_line":"                not hasattr(db_server, \u0027created_at\u0027)):"},{"line_number":34,"context_line":"                    continue"},{"line_number":35,"context_line":"            server[ATTRIBUTE_NAME] \u003d db_server[\u0027config_drive\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_ba1424a0","line":32,"in_reply_to":"5f7c97a3_33984d74","updated":"2018-07-20 10:15:32.000000000","message":"\u003e Can we not handle this in the view builder code for the serve\n \u003e response where it\u0027s centralized? Or not yet because these extension\n \u003e controllers are adding stuff to the response body directly?\n\nexactly,","commit_id":"556689b3dc7a85796a758e668db032ffcf4c3dbb"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"17f50ab5d445d3b87c2e6a0dfa940492366b853d","unresolved":false,"context_lines":[{"line_number":29,"context_line":"            db_server \u003d req.get_db_instance(server[\u0027id\u0027])"},{"line_number":30,"context_line":"            # server[\u0027id\u0027] is guaranteed to be in the cache due to"},{"line_number":31,"context_line":"            # the core API adding it in its \u0027show\u0027/\u0027detail\u0027 methods."},{"line_number":32,"context_line":"            if (api_version_request.is_supported(req, min_version\u003d\u00272.64\u0027) and"},{"line_number":33,"context_line":"                not hasattr(db_server, \u0027created_at\u0027)):"},{"line_number":34,"context_line":"                    continue"},{"line_number":35,"context_line":"            server[ATTRIBUTE_NAME] \u003d db_server[\u0027config_drive\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_8511c0b1","line":32,"in_reply_to":"5f7c97a3_b3952fe5","updated":"2018-07-20 17:00:18.000000000","message":"cool!, looking into this now.. I will do a rebase then, will make the whole thing much easier if its all merged into the server view.","commit_id":"556689b3dc7a85796a758e668db032ffcf4c3dbb"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d412cba4c2939246df3e7afc1812bb5e49a9ce20","unresolved":false,"context_lines":[{"line_number":29,"context_line":"            db_server \u003d req.get_db_instance(server[\u0027id\u0027])"},{"line_number":30,"context_line":"            # server[\u0027id\u0027] is guaranteed to be in the cache due to"},{"line_number":31,"context_line":"            # the core API adding it in its \u0027show\u0027/\u0027detail\u0027 methods."},{"line_number":32,"context_line":"            if (api_version_request.is_supported(req, min_version\u003d\u00272.64\u0027) and"},{"line_number":33,"context_line":"                not hasattr(db_server, \u0027created_at\u0027)):"},{"line_number":34,"context_line":"                    continue"},{"line_number":35,"context_line":"            server[ATTRIBUTE_NAME] \u003d db_server[\u0027config_drive\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_b3952fe5","line":32,"in_reply_to":"5f7c97a3_ba1424a0","updated":"2018-07-20 12:59:31.000000000","message":"OK, gmann is working on a series of changes to drop these extensions and merge them into the main server controller and view:\n\nhttps://review.openstack.org/#/q/topic:bp/api-extensions-merge-rocky+(status:open+OR+status:merged)\n\nMight be worth rebasing on top of that series to clean this up.","commit_id":"556689b3dc7a85796a758e668db032ffcf4c3dbb"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e54054085c7a492a24428edeba52f07147e8d800","unresolved":false,"context_lines":[{"line_number":30,"context_line":"            # server[\u0027id\u0027] is guaranteed to be in the cache due to"},{"line_number":31,"context_line":"            # the core API adding it in its \u0027show\u0027/\u0027detail\u0027 methods."},{"line_number":32,"context_line":"            if (api_version_request.is_supported(req, min_version\u003d\u00272.64\u0027) and"},{"line_number":33,"context_line":"                not hasattr(db_server, \u0027created_at\u0027)):"},{"line_number":34,"context_line":"                    continue"},{"line_number":35,"context_line":"            server[ATTRIBUTE_NAME] \u003d db_server[\u0027config_drive\u0027]"},{"line_number":36,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_f3873511","line":33,"range":{"start_line":33,"start_character":40,"end_line":33,"end_character":50},"updated":"2018-07-19 23:18:55.000000000","message":"config_drive?","commit_id":"556689b3dc7a85796a758e668db032ffcf4c3dbb"}],"nova/api/openstack/compute/extended_availability_zone.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e54054085c7a492a24428edeba52f07147e8d800","unresolved":false,"context_lines":[{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"class ExtendedAZController(wsgi.Controller):"},{"line_number":27,"context_line":"    def _extend_server(self, context, server, instance):"},{"line_number":28,"context_line":"        # NOTE(mriedem): The OS-EXT-AZ prefix should not be used for new"},{"line_number":29,"context_line":"        # attributes after v2.1. They are only in v2.1 for backward compat"},{"line_number":30,"context_line":"        # with v2.0."}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_536ce969","line":27,"range":{"start_line":27,"start_character":8,"end_line":27,"end_character":22},"updated":"2018-07-19 23:18:55.000000000","message":"You likely need to handle this in here centrally, and check for availability_zone?","commit_id":"556689b3dc7a85796a758e668db032ffcf4c3dbb"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"4339d566eac9ee7325b53ee445957f40d9f9bb72","unresolved":false,"context_lines":[{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"class ExtendedAZController(wsgi.Controller):"},{"line_number":27,"context_line":"    def _extend_server(self, context, server, instance):"},{"line_number":28,"context_line":"        # NOTE(mriedem): The OS-EXT-AZ prefix should not be used for new"},{"line_number":29,"context_line":"        # attributes after v2.1. They are only in v2.1 for backward compat"},{"line_number":30,"context_line":"        # with v2.0."}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_9a1128b2","line":27,"range":{"start_line":27,"start_character":8,"end_line":27,"end_character":22},"in_reply_to":"5f7c97a3_536ce969","updated":"2018-07-20 10:15:32.000000000","message":"ack","commit_id":"556689b3dc7a85796a758e668db032ffcf4c3dbb"}],"nova/api/openstack/compute/extended_server_attributes.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e54054085c7a492a24428edeba52f07147e8d800","unresolved":false,"context_lines":[{"line_number":65,"context_line":"            context.can(servers_policies.SERVERS % \u0027show:host_status\u0027,"},{"line_number":66,"context_line":"                        fatal\u003dFalse)):"},{"line_number":67,"context_line":"            authorize_host_status \u003d True"},{"line_number":68,"context_line":"        server \u003d resp_obj.obj[\u0027server\u0027]"},{"line_number":69,"context_line":"        if (api_version_request.is_supported(req, min_version\u003d\u00272.64\u0027) and"},{"line_number":70,"context_line":"            \u0027created\u0027 not in server):"},{"line_number":71,"context_line":"                authorize_extend \u003d False"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_1390b141","line":68,"updated":"2018-07-19 23:18:55.000000000","message":"Same problem below right?","commit_id":"556689b3dc7a85796a758e668db032ffcf4c3dbb"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"4339d566eac9ee7325b53ee445957f40d9f9bb72","unresolved":false,"context_lines":[{"line_number":65,"context_line":"            context.can(servers_policies.SERVERS % \u0027show:host_status\u0027,"},{"line_number":66,"context_line":"                        fatal\u003dFalse)):"},{"line_number":67,"context_line":"            authorize_host_status \u003d True"},{"line_number":68,"context_line":"        server \u003d resp_obj.obj[\u0027server\u0027]"},{"line_number":69,"context_line":"        if (api_version_request.is_supported(req, min_version\u003d\u00272.64\u0027) and"},{"line_number":70,"context_line":"            \u0027created\u0027 not in server):"},{"line_number":71,"context_line":"                authorize_extend \u003d False"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_5a071071","line":68,"in_reply_to":"5f7c97a3_1390b141","updated":"2018-07-20 10:15:32.000000000","message":"ack","commit_id":"556689b3dc7a85796a758e668db032ffcf4c3dbb"}],"nova/api/openstack/compute/servers.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5b6e515820eef66f6a7874705b69d01623a9711b","unresolved":false,"context_lines":[{"line_number":48,"context_line":""},{"line_number":49,"context_line":"TAG_SEARCH_FILTERS \u003d (\u0027tags\u0027, \u0027tags-any\u0027, \u0027not-tags\u0027, \u0027not-tags-any\u0027)"},{"line_number":50,"context_line":"DEVICE_TAGGING_MIN_COMPUTE_VERSION \u003d 14"},{"line_number":51,"context_line":"PARTIAL_CONSTRUCT_FOR_CELL_DOWN_MIN_VERSION \u003d \u00272.66\u0027"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"CONF \u003d nova.conf.CONF"},{"line_number":54,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_5a6427e5","line":51,"updated":"2018-10-01 16:01:31.000000000","message":"IMHO, Move this and L109-120 to the end patch that adds 2.66 and always pass False from this file until that point. That will address Matt\u0027s concern I think.","commit_id":"1b1e784b94f89bff49ce4a4c1518043e5a1607cd"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"2ee470c9634655770d9601719c13bb8e47d51933","unresolved":false,"context_lines":[{"line_number":48,"context_line":""},{"line_number":49,"context_line":"TAG_SEARCH_FILTERS \u003d (\u0027tags\u0027, \u0027tags-any\u0027, \u0027not-tags\u0027, \u0027not-tags-any\u0027)"},{"line_number":50,"context_line":"DEVICE_TAGGING_MIN_COMPUTE_VERSION \u003d 14"},{"line_number":51,"context_line":"PARTIAL_CONSTRUCT_FOR_CELL_DOWN_MIN_VERSION \u003d \u00272.66\u0027"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"CONF \u003d nova.conf.CONF"},{"line_number":54,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_5d6421e5","line":51,"in_reply_to":"3f79a3b5_5a6427e5","updated":"2018-10-01 16:38:09.000000000","message":"yes this is what I have done, sorry I did not upload the latest patch for this, I was kind of stuck on the best way to do https://review.openstack.org/#/c/591658/7/nova/compute/api.py@2327 ,","commit_id":"1b1e784b94f89bff49ce4a4c1518043e5a1607cd"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"54c685c3dc3467ae16776b7229d04b2f766e2659","unresolved":false,"context_lines":[{"line_number":107,"context_line":"    def _get_servers(self, req, is_detail):"},{"line_number":108,"context_line":"        \"\"\"Returns a list of servers, based on any search options specified.\"\"\""},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"        cell_down_support \u003d api_version_request.is_supported(req,"},{"line_number":111,"context_line":"            min_version\u003dPARTIAL_CONSTRUCT_FOR_CELL_DOWN_MIN_VERSION)"},{"line_number":112,"context_line":"        if cell_down_support:"},{"line_number":113,"context_line":"            # NOTE(tssurya): Minimal constructs would be returned from the down"},{"line_number":114,"context_line":"            # cells if cell_down_support is True, however if filtering, sorting"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_579d9806","line":111,"range":{"start_line":110,"start_character":8,"end_line":111,"end_character":68},"updated":"2018-09-21 14:12:24.000000000","message":"Hmm, I\u0027m a bit concerned that this is enabling functionality for the new microversion before the underlying code can actually support it - is that true or is there something that prevents the user from even being able to request this microversion until later in the series?","commit_id":"1b1e784b94f89bff49ce4a4c1518043e5a1607cd"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"102d0a9f405e913defb2e59b8a53fb65df9662bd","unresolved":false,"context_lines":[{"line_number":107,"context_line":"    def _get_servers(self, req, is_detail):"},{"line_number":108,"context_line":"        \"\"\"Returns a list of servers, based on any search options specified.\"\"\""},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"        cell_down_support \u003d api_version_request.is_supported(req,"},{"line_number":111,"context_line":"            min_version\u003dPARTIAL_CONSTRUCT_FOR_CELL_DOWN_MIN_VERSION)"},{"line_number":112,"context_line":"        if cell_down_support:"},{"line_number":113,"context_line":"            # NOTE(tssurya): Minimal constructs would be returned from the down"},{"line_number":114,"context_line":"            # cells if cell_down_support is True, however if filtering, sorting"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_f75e2443","line":111,"range":{"start_line":110,"start_character":8,"end_line":111,"end_character":68},"in_reply_to":"3f79a3b5_579d9806","updated":"2018-09-21 14:16:15.000000000","message":"My concern is that we could merge this change, but then something else takes the 2.66 microversion, and there is another merged change that actually gets 2.66:\n\nhttps://review.openstack.org/#/c/599276/\n\nMaybe what I need to do is procedurally -2 this change until the rest of the series is approved, or we should move the actual controller handler code (the stuff under nova/api/openstack/compute/) to the end of the series when the microversion is actually supported down the entire stack.","commit_id":"1b1e784b94f89bff49ce4a4c1518043e5a1607cd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"81da63029cf839f6f37b8bf974e3e439893d9dd2","unresolved":false,"context_lines":[{"line_number":271,"context_line":"            instance_list \u003d self.compute_api.get_all(elevated or context,"},{"line_number":272,"context_line":"                    search_opts\u003dsearch_opts, limit\u003dlimit, marker\u003dmarker,"},{"line_number":273,"context_line":"                    expected_attrs\u003dexpected_attrs, sort_keys\u003dsort_keys,"},{"line_number":274,"context_line":"                    sort_dirs\u003dsort_dirs, cell_down_support\u003dFalse)"},{"line_number":275,"context_line":"        except exception.MarkerNotFound:"},{"line_number":276,"context_line":"            msg \u003d _(\u0027marker [%s] not found\u0027) % marker"},{"line_number":277,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_a8eda3b9","line":274,"range":{"start_line":274,"start_character":41,"end_line":274,"end_character":64},"updated":"2018-10-04 18:15:29.000000000","message":"++","commit_id":"10fa28959d2fc8570e5278840235165482a07b11"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"40e8a2455b7e1e5a14fd750e54b9923282e3a277","unresolved":false,"context_lines":[{"line_number":271,"context_line":"            instance_list \u003d self.compute_api.get_all(elevated or context,"},{"line_number":272,"context_line":"                    search_opts\u003dsearch_opts, limit\u003dlimit, marker\u003dmarker,"},{"line_number":273,"context_line":"                    expected_attrs\u003dexpected_attrs, sort_keys\u003dsort_keys,"},{"line_number":274,"context_line":"                    sort_dirs\u003dsort_dirs, cell_down_support\u003dFalse)"},{"line_number":275,"context_line":"        except exception.MarkerNotFound:"},{"line_number":276,"context_line":"            msg \u003d _(\u0027marker [%s] not found\u0027) % marker"},{"line_number":277,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f79a3b5_06db5cea","line":274,"range":{"start_line":274,"start_character":41,"end_line":274,"end_character":64},"updated":"2018-10-17 17:10:47.000000000","message":"How about leaving a TODO here mentioning that this eventually gets set based on the microversion added later.","commit_id":"b19a4065956076fef698dd313359e7aa0f58e969"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7f168b471de1a85c4a5c516d1d968785cb6329a0","unresolved":false,"context_lines":[{"line_number":271,"context_line":"            instance_list \u003d self.compute_api.get_all(elevated or context,"},{"line_number":272,"context_line":"                    search_opts\u003dsearch_opts, limit\u003dlimit, marker\u003dmarker,"},{"line_number":273,"context_line":"                    expected_attrs\u003dexpected_attrs, sort_keys\u003dsort_keys,"},{"line_number":274,"context_line":"                    sort_dirs\u003dsort_dirs, cell_down_support\u003dFalse)"},{"line_number":275,"context_line":"        except exception.MarkerNotFound:"},{"line_number":276,"context_line":"            msg \u003d _(\u0027marker [%s] not found\u0027) % marker"},{"line_number":277,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":30,"id":"3f79a3b5_e7e728ab","line":274,"range":{"start_line":274,"start_character":41,"end_line":274,"end_character":64},"updated":"2018-10-31 18:51:22.000000000","message":"Since this already defaults to False in get_all(), you never needed to change this and could have avoided all of the test_serversV21 creep for needing to update the mocked call assertions. But I guess you\u0027re going to have to do those eventually anyway in this series, so might as well leave it.","commit_id":"76cd5b53bde1f7bfa82042ab6ac8e860b793d15d"}],"nova/api/openstack/compute/views/servers.py":[{"author":{"_account_id":25562,"name":"Chen","email":"dstbtgagt@foxmail.com","username":"chenn2"},"change_message_id":"9cc45be9d0174b5d812d695e8dccc0df72bb0db8","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    _collection_name \u003d \"servers\""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    _progress_statuses \u003d ("},{"line_number":46,"context_line":"        \"ACTIVE\","},{"line_number":47,"context_line":"        \"BUILD\","},{"line_number":48,"context_line":"        \"REBUILD\","},{"line_number":49,"context_line":"        \"RESIZE\","},{"line_number":50,"context_line":"        \"VERIFY_RESIZE\","},{"line_number":51,"context_line":"        \"MIGRATING\","},{"line_number":52,"context_line":"    )"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    _fault_statuses \u003d ("}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_3b511b0f","line":51,"range":{"start_line":46,"start_character":4,"end_line":51,"end_character":20},"updated":"2018-09-06 12:17:32.000000000","message":"Some tests failed  because \"UNKNOWN\" is not added here.","commit_id":"a4ea3fbeaa738b270c2880b1ce565cf16c396a3a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"32f2ae2fc0018733f6a125e81850305d67adab0e","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    _collection_name \u003d \"servers\""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    _progress_statuses \u003d ("},{"line_number":46,"context_line":"        \"ACTIVE\","},{"line_number":47,"context_line":"        \"BUILD\","},{"line_number":48,"context_line":"        \"REBUILD\","},{"line_number":49,"context_line":"        \"RESIZE\","},{"line_number":50,"context_line":"        \"VERIFY_RESIZE\","},{"line_number":51,"context_line":"        \"MIGRATING\","},{"line_number":52,"context_line":"    )"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    _fault_statuses \u003d ("}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_c1aa78d0","line":51,"range":{"start_line":46,"start_character":4,"end_line":51,"end_character":20},"in_reply_to":"3f79a3b5_3b511b0f","updated":"2018-09-06 14:01:52.000000000","message":"This causes us to set the progress field, but I\u0027m not sure why it would matter for existing tests, nor am I sure we really want/need to set progress for UNKNOWN instances.\n\nI tend to wonder if something else in the list plumbing broke which is causing the test failures, as they\u0027re claiming to be missing progress on instances they created. Potentially because the \"if display_name\" sentinel below is not good enough?","commit_id":"a4ea3fbeaa738b270c2880b1ce565cf16c396a3a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5b6e515820eef66f6a7874705b69d01623a9711b","unresolved":false,"context_lines":[{"line_number":147,"context_line":"                    \"server\": {"},{"line_number":148,"context_line":"                        \"id\": instance[\"uuid\"],"},{"line_number":149,"context_line":"                        \"status\": \"UNKNOWN\","},{"line_number":150,"context_line":"                        \"tenant_id\": instance.get(\"project_id\") or \"\","},{"line_number":151,"context_line":"                        \"created\": utils.isotime(instance[\"created_at\"]),"},{"line_number":152,"context_line":"                    },"},{"line_number":153,"context_line":"                }"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_f4d429bd","line":150,"range":{"start_line":150,"start_character":63,"end_line":150,"end_character":69},"updated":"2018-10-01 16:01:31.000000000","message":"When will this happen?","commit_id":"1b1e784b94f89bff49ce4a4c1518043e5a1607cd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c9ea8b0f77ab0fdd089d40c62537ef0d70498c74","unresolved":false,"context_lines":[{"line_number":147,"context_line":"                    \"server\": {"},{"line_number":148,"context_line":"                        \"id\": instance[\"uuid\"],"},{"line_number":149,"context_line":"                        \"status\": \"UNKNOWN\","},{"line_number":150,"context_line":"                        \"tenant_id\": instance.get(\"project_id\") or \"\","},{"line_number":151,"context_line":"                        \"created\": utils.isotime(instance[\"created_at\"]),"},{"line_number":152,"context_line":"                    },"},{"line_number":153,"context_line":"                }"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_c08cc82f","line":150,"range":{"start_line":150,"start_character":63,"end_line":150,"end_character":69},"in_reply_to":"3f79a3b5_1a46afa8","updated":"2018-10-01 17:33:06.000000000","message":"Okay, fair, but if this is a \"minimal\" one, we know where it came from and that it will always have this set. So, better to be explicit I think.","commit_id":"1b1e784b94f89bff49ce4a4c1518043e5a1607cd"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"2ee470c9634655770d9601719c13bb8e47d51933","unresolved":false,"context_lines":[{"line_number":147,"context_line":"                    \"server\": {"},{"line_number":148,"context_line":"                        \"id\": instance[\"uuid\"],"},{"line_number":149,"context_line":"                        \"status\": \"UNKNOWN\","},{"line_number":150,"context_line":"                        \"tenant_id\": instance.get(\"project_id\") or \"\","},{"line_number":151,"context_line":"                        \"created\": utils.isotime(instance[\"created_at\"]),"},{"line_number":152,"context_line":"                    },"},{"line_number":153,"context_line":"                }"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_1a46afa8","line":150,"range":{"start_line":150,"start_character":63,"end_line":150,"end_character":69},"in_reply_to":"3f79a3b5_f4d429bd","updated":"2018-10-01 16:38:09.000000000","message":"umm, I guess I just stuck to the one from L171. I can cut this out.","commit_id":"1b1e784b94f89bff49ce4a4c1518043e5a1607cd"}],"nova/compute/api.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5b6e515820eef66f6a7874705b69d01623a9711b","unresolved":false,"context_lines":[{"line_number":2290,"context_line":"            LOG.warning(\"Cell %s is not responding and hence only \""},{"line_number":2291,"context_line":"                        \"partial results are available from this \""},{"line_number":2292,"context_line":"                        \"cell if any.\", cell_uuid)"},{"line_number":2293,"context_line":"            cell \u003d objects.CellMapping.get_by_uuid(context, cell_uuid)"},{"line_number":2294,"context_line":"            instance_mappings \u003d (objects.InstanceMappingList."},{"line_number":2295,"context_line":"                get_by_cell_and_project(context, cell.id, context.project_id))"},{"line_number":2296,"context_line":"            for im in instance_mappings:"},{"line_number":2297,"context_line":"                if not im.queued_for_delete:"},{"line_number":2298,"context_line":"                    unavailable_servers.objects.append("}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_54763d0e","line":2295,"range":{"start_line":2293,"start_character":0,"end_line":2295,"end_character":78},"updated":"2018-10-01 16:01:31.000000000","message":"We could optimize this later by adding InstanceMappingList.get_by_cell_uuid_and_project()","commit_id":"1b1e784b94f89bff49ce4a4c1518043e5a1607cd"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"2ee470c9634655770d9601719c13bb8e47d51933","unresolved":false,"context_lines":[{"line_number":2290,"context_line":"            LOG.warning(\"Cell %s is not responding and hence only \""},{"line_number":2291,"context_line":"                        \"partial results are available from this \""},{"line_number":2292,"context_line":"                        \"cell if any.\", cell_uuid)"},{"line_number":2293,"context_line":"            cell \u003d objects.CellMapping.get_by_uuid(context, cell_uuid)"},{"line_number":2294,"context_line":"            instance_mappings \u003d (objects.InstanceMappingList."},{"line_number":2295,"context_line":"                get_by_cell_and_project(context, cell.id, context.project_id))"},{"line_number":2296,"context_line":"            for im in instance_mappings:"},{"line_number":2297,"context_line":"                if not im.queued_for_delete:"},{"line_number":2298,"context_line":"                    unavailable_servers.objects.append("}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_3a860bbc","line":2295,"range":{"start_line":2293,"start_character":0,"end_line":2295,"end_character":78},"in_reply_to":"3f79a3b5_54763d0e","updated":"2018-10-01 16:38:09.000000000","message":"ack.","commit_id":"1b1e784b94f89bff49ce4a4c1518043e5a1607cd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5b6e515820eef66f6a7874705b69d01623a9711b","unresolved":false,"context_lines":[{"line_number":2294,"context_line":"            instance_mappings \u003d (objects.InstanceMappingList."},{"line_number":2295,"context_line":"                get_by_cell_and_project(context, cell.id, context.project_id))"},{"line_number":2296,"context_line":"            for im in instance_mappings:"},{"line_number":2297,"context_line":"                if not im.queued_for_delete:"},{"line_number":2298,"context_line":"                    unavailable_servers.objects.append("},{"line_number":2299,"context_line":"                        objects.Instance("},{"line_number":2300,"context_line":"                            context\u003dcontext,"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_74c9f922","line":2297,"updated":"2018-10-01 16:01:31.000000000","message":"Maybe we should filter this out of the query as well, so that we\u0027re not pulling crazy numbers of mappings (since they never get deleted until purge)?","commit_id":"1b1e784b94f89bff49ce4a4c1518043e5a1607cd"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"2ee470c9634655770d9601719c13bb8e47d51933","unresolved":false,"context_lines":[{"line_number":2294,"context_line":"            instance_mappings \u003d (objects.InstanceMappingList."},{"line_number":2295,"context_line":"                get_by_cell_and_project(context, cell.id, context.project_id))"},{"line_number":2296,"context_line":"            for im in instance_mappings:"},{"line_number":2297,"context_line":"                if not im.queued_for_delete:"},{"line_number":2298,"context_line":"                    unavailable_servers.objects.append("},{"line_number":2299,"context_line":"                        objects.Instance("},{"line_number":2300,"context_line":"                            context\u003dcontext,"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_5a1f476f","line":2297,"in_reply_to":"3f79a3b5_74c9f922","updated":"2018-10-01 16:38:09.000000000","message":"yea I can filter this out from the above list of instance_mappings, or would you prefer that I changed the InstanceMappingList.get_by_cell_and_project() to InstanceMappingList.get_not_queued_for_delete_by_cell_and_project() ? - but maybe I would be making this function too specific for this use case then...","commit_id":"1b1e784b94f89bff49ce4a4c1518043e5a1607cd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5b6e515820eef66f6a7874705b69d01623a9711b","unresolved":false,"context_lines":[{"line_number":2571,"context_line":"        if filter_ip:"},{"line_number":2572,"context_line":"            instances \u003d self._ip_filter(instances, filters, orig_limit)"},{"line_number":2573,"context_line":""},{"line_number":2574,"context_line":"        if cell_down_support:"},{"line_number":2575,"context_line":"            return (self._generate_minimal_construct_for_down_cells(context,"},{"line_number":2576,"context_line":"                down_cell_uuids) + instances)"},{"line_number":2577,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_9a55dffb","line":2574,"range":{"start_line":2574,"start_character":8,"end_line":2574,"end_character":28},"updated":"2018-10-01 16:01:31.000000000","message":"Is this tested by existing tests? Meaning, do we assert that we don\u0027t call the minimal generate function?","commit_id":"1b1e784b94f89bff49ce4a4c1518043e5a1607cd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5b6e515820eef66f6a7874705b69d01623a9711b","unresolved":false,"context_lines":[{"line_number":2573,"context_line":""},{"line_number":2574,"context_line":"        if cell_down_support:"},{"line_number":2575,"context_line":"            return (self._generate_minimal_construct_for_down_cells(context,"},{"line_number":2576,"context_line":"                down_cell_uuids) + instances)"},{"line_number":2577,"context_line":""},{"line_number":2578,"context_line":"        return instances"},{"line_number":2579,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_ba253bb0","line":2576,"range":{"start_line":2576,"start_character":33,"end_line":2576,"end_character":34},"updated":"2018-10-01 16:01:31.000000000","message":"I think this is untested","commit_id":"1b1e784b94f89bff49ce4a4c1518043e5a1607cd"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"40e8a2455b7e1e5a14fd750e54b9923282e3a277","unresolved":false,"context_lines":[{"line_number":2284,"context_line":""},{"line_number":2285,"context_line":"        self.compute_rpcapi.trigger_crash_dump(context, instance)"},{"line_number":2286,"context_line":""},{"line_number":2287,"context_line":"    def _generate_minimal_construct_for_down_cells(self, context, down_cells):"},{"line_number":2288,"context_line":"        unavailable_servers \u003d objects.InstanceList()"},{"line_number":2289,"context_line":"        for cell_uuid in down_cells:"},{"line_number":2290,"context_line":"            LOG.warning(\"Cell %s is not responding and hence only \""}],"source_content_type":"text/x-python","patch_set":20,"id":"3f79a3b5_a68e28d5","line":2287,"updated":"2018-10-17 17:10:47.000000000","message":"docs please","commit_id":"b19a4065956076fef698dd313359e7aa0f58e969"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"40e8a2455b7e1e5a14fd750e54b9923282e3a277","unresolved":false,"context_lines":[{"line_number":2284,"context_line":""},{"line_number":2285,"context_line":"        self.compute_rpcapi.trigger_crash_dump(context, instance)"},{"line_number":2286,"context_line":""},{"line_number":2287,"context_line":"    def _generate_minimal_construct_for_down_cells(self, context, down_cells):"},{"line_number":2288,"context_line":"        unavailable_servers \u003d objects.InstanceList()"},{"line_number":2289,"context_line":"        for cell_uuid in down_cells:"},{"line_number":2290,"context_line":"            LOG.warning(\"Cell %s is not responding and hence only \""}],"source_content_type":"text/x-python","patch_set":20,"id":"3f79a3b5_869d2c0b","line":2287,"range":{"start_line":2287,"start_character":66,"end_line":2287,"end_character":76},"updated":"2018-10-17 17:10:47.000000000","message":"down_cell_uuids? Otherwise this variable could be a CellMappingList.","commit_id":"b19a4065956076fef698dd313359e7aa0f58e969"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"40e8a2455b7e1e5a14fd750e54b9923282e3a277","unresolved":false,"context_lines":[{"line_number":2292,"context_line":"                        \"cell if any.\", cell_uuid)"},{"line_number":2293,"context_line":"            instance_mappings \u003d (objects.InstanceMappingList."},{"line_number":2294,"context_line":"                get_not_deleted_by_cell_and_project(context, cell_uuid,"},{"line_number":2295,"context_line":"                context.project_id))"},{"line_number":2296,"context_line":"            for im in instance_mappings:"},{"line_number":2297,"context_line":"                unavailable_servers.objects.append("},{"line_number":2298,"context_line":"                    objects.Instance("}],"source_content_type":"text/x-python","patch_set":20,"id":"3f79a3b5_46a87420","line":2295,"range":{"start_line":2295,"start_character":16,"end_line":2295,"end_character":34},"updated":"2018-10-17 17:10:47.000000000","message":"What if I\u0027m an admin filtering on instances in a given project using the project_id filter? Wouldn\u0027t this be wrong? Or using the all_tenants flag?","commit_id":"b19a4065956076fef698dd313359e7aa0f58e969"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"efe2053b0c6eab32623e1d9e76dfa2cc162d2bed","unresolved":false,"context_lines":[{"line_number":2292,"context_line":"                        \"cell if any.\", cell_uuid)"},{"line_number":2293,"context_line":"            instance_mappings \u003d (objects.InstanceMappingList."},{"line_number":2294,"context_line":"                get_not_deleted_by_cell_and_project(context, cell_uuid,"},{"line_number":2295,"context_line":"                context.project_id))"},{"line_number":2296,"context_line":"            for im in instance_mappings:"},{"line_number":2297,"context_line":"                unavailable_servers.objects.append("},{"line_number":2298,"context_line":"                    objects.Instance("}],"source_content_type":"text/x-python","patch_set":20,"id":"3f79a3b5_2c0c3939","line":2295,"range":{"start_line":2295,"start_character":16,"end_line":2295,"end_character":34},"in_reply_to":"3f79a3b5_0925afbf","updated":"2018-10-17 17:44:03.000000000","message":"For listing across all tenants, the spec doesn\u0027t call that out. Would we just list instance mappings from down cells regardless of project_id in that case? Up to whatever limit.","commit_id":"b19a4065956076fef698dd313359e7aa0f58e969"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6bd63e8c6b12042f7f9e8a88c5e06bfa506f882e","unresolved":false,"context_lines":[{"line_number":2292,"context_line":"                        \"cell if any.\", cell_uuid)"},{"line_number":2293,"context_line":"            instance_mappings \u003d (objects.InstanceMappingList."},{"line_number":2294,"context_line":"                get_not_deleted_by_cell_and_project(context, cell_uuid,"},{"line_number":2295,"context_line":"                context.project_id))"},{"line_number":2296,"context_line":"            for im in instance_mappings:"},{"line_number":2297,"context_line":"                unavailable_servers.objects.append("},{"line_number":2298,"context_line":"                    objects.Instance("}],"source_content_type":"text/x-python","patch_set":20,"id":"3f79a3b5_2ca8b9d8","line":2295,"range":{"start_line":2295,"start_character":16,"end_line":2295,"end_character":34},"in_reply_to":"3f79a3b5_2c0c3939","updated":"2018-10-17 17:53:54.000000000","message":"Also note that the project_id filter only applies if the all_tenants filter is also used (and evaluates to True):\n\nhttps://github.com/openstack/nova/blob/9c5d4eb2008df02837985779d87fedb17b4f70bf/nova/api/openstack/compute/servers.py#L206L248\n\nOtherwise we just filter on the user\u0027s request context (which if that\u0027s an admin then it\u0027s just the admin\u0027s project_id on the context).\n\nDid we talk about all_tenants in the spec review? Maybe we said that we\u0027d just ignore it like all other filters? But it definitely seems like we can honor that since we\u0027re also trying to honor deleted and project_id filtering (note you\u0027re not filtering on deleted properly here either using the queued_for_delete filter, right).","commit_id":"b19a4065956076fef698dd313359e7aa0f58e969"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f79900228c09e1f90d3d8d91371155a4bc711d49","unresolved":false,"context_lines":[{"line_number":2292,"context_line":"                        \"cell if any.\", cell_uuid)"},{"line_number":2293,"context_line":"            instance_mappings \u003d (objects.InstanceMappingList."},{"line_number":2294,"context_line":"                get_not_deleted_by_cell_and_project(context, cell_uuid,"},{"line_number":2295,"context_line":"                context.project_id))"},{"line_number":2296,"context_line":"            for im in instance_mappings:"},{"line_number":2297,"context_line":"                unavailable_servers.objects.append("},{"line_number":2298,"context_line":"                    objects.Instance("}],"source_content_type":"text/x-python","patch_set":20,"id":"3f79a3b5_b05dc855","line":2295,"range":{"start_line":2295,"start_character":16,"end_line":2295,"end_character":34},"in_reply_to":"3f79a3b5_2ca8b9d8","updated":"2018-10-25 13:55:10.000000000","message":"This method is \"get_not_deleted\" which means it\u0027s not listing deleted things. I think we also said that we wouldn\u0027t ever list deleted things minimally, since over time you\u0027d have a crap ton of \"queued for delete\" mappings since they don\u0027t go away until purge.","commit_id":"b19a4065956076fef698dd313359e7aa0f58e969"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8c8d8f9482387038c80bfc6d93277d37d6ae3348","unresolved":false,"context_lines":[{"line_number":2292,"context_line":"                        \"cell if any.\", cell_uuid)"},{"line_number":2293,"context_line":"            instance_mappings \u003d (objects.InstanceMappingList."},{"line_number":2294,"context_line":"                get_not_deleted_by_cell_and_project(context, cell_uuid,"},{"line_number":2295,"context_line":"                context.project_id))"},{"line_number":2296,"context_line":"            for im in instance_mappings:"},{"line_number":2297,"context_line":"                unavailable_servers.objects.append("},{"line_number":2298,"context_line":"                    objects.Instance("}],"source_content_type":"text/x-python","patch_set":20,"id":"3f79a3b5_0925afbf","line":2295,"range":{"start_line":2295,"start_character":16,"end_line":2295,"end_character":34},"in_reply_to":"3f79a3b5_46a87420","updated":"2018-10-17 17:41:12.000000000","message":"This was the section in the spec where we talked about this:\n\nhttps://specs.openstack.org/openstack/nova-specs/specs/stein/approved/handling-down-cell_new.html#edge-cases","commit_id":"b19a4065956076fef698dd313359e7aa0f58e969"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1b269a3233fa088fd8c397145940c402c9ef73c0","unresolved":false,"context_lines":[{"line_number":2292,"context_line":"                        \"cell if any.\", cell_uuid)"},{"line_number":2293,"context_line":"            instance_mappings \u003d (objects.InstanceMappingList."},{"line_number":2294,"context_line":"                get_not_deleted_by_cell_and_project(context, cell_uuid,"},{"line_number":2295,"context_line":"                context.project_id))"},{"line_number":2296,"context_line":"            for im in instance_mappings:"},{"line_number":2297,"context_line":"                unavailable_servers.objects.append("},{"line_number":2298,"context_line":"                    objects.Instance("}],"source_content_type":"text/x-python","patch_set":20,"id":"3f79a3b5_eb743ccf","line":2295,"range":{"start_line":2295,"start_character":16,"end_line":2295,"end_character":34},"in_reply_to":"3f79a3b5_b05dc855","updated":"2018-10-25 20:36:37.000000000","message":"\u003e This method is \"get_not_deleted\" which means it\u0027s not listing\n \u003e deleted things. I think we also said that we wouldn\u0027t ever list\n \u003e deleted things minimally, since over time you\u0027d have a crap ton of\n \u003e \"queued for delete\" mappings since they don\u0027t go away until purge.\n\nI honestly don\u0027t remember all the details of what was discussed in the spec review without going back to look. I know the spec says:\n\nhttps://specs.openstack.org/openstack/nova-specs/specs/stein/approved/handling-down-cell_new.html#edge-cases\n\n\"Filters: If the user is listing servers using filters the results from the down cell will be skipped and no minimalistic construct will be provided since there is no way of validating the filtered results from the down cell if the value of the filter key itself is missing. Note that by default nova list uses the deleted\u003dFalse and project_id\u003dtenant_id filters and since we know both of these values from the instance_mapping table, they will be the only allowed filters. Hence only doing nova list and nova list --minimal will show minimalistic results for the down cell. Other filters like nova list --deleted or nova list --host xx will skip the results for the down cell.\"\n\nWhich to me sounds like since we can know if instances are queue for delete, we can filter on deleted, and that seems important for the all_tenants use case.\n\nHaving said that, I tried skimming the review comments on the spec and didn\u0027t see anything specific about all_tenants. The closest is probably here:\n\nhttps://review.openstack.org/#/c/557369/5/specs/rocky/approved/handling-down-cell.rst@143\n\nAnd that\u0027s probably what resulted in the thing getting documented in the spec that I linked above, so if that\u0027s the case, it\u0027s just a matter of making sure we document the limitations properly with the microversion, caveats, etc etc.","commit_id":"b19a4065956076fef698dd313359e7aa0f58e969"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"40e8a2455b7e1e5a14fd750e54b9923282e3a277","unresolved":false,"context_lines":[{"line_number":2401,"context_line":""},{"line_number":2402,"context_line":"    def get_all(self, context, search_opts\u003dNone, limit\u003dNone, marker\u003dNone,"},{"line_number":2403,"context_line":"                expected_attrs\u003dNone, sort_keys\u003dNone, sort_dirs\u003dNone,"},{"line_number":2404,"context_line":"                cell_down_support\u003dFalse):"},{"line_number":2405,"context_line":"        \"\"\"Get all instances filtered by one of the given parameters."},{"line_number":2406,"context_line":""},{"line_number":2407,"context_line":"        If there is no filter and the context is an admin, it will retrieve"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f79a3b5_86e8cc7b","line":2404,"range":{"start_line":2404,"start_character":16,"end_line":2404,"end_character":33},"updated":"2018-10-17 17:10:47.000000000","message":"Would be good to document this in the docstring.","commit_id":"b19a4065956076fef698dd313359e7aa0f58e969"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"40e8a2455b7e1e5a14fd750e54b9923282e3a277","unresolved":false,"context_lines":[{"line_number":2570,"context_line":"        if filter_ip:"},{"line_number":2571,"context_line":"            instances \u003d self._ip_filter(instances, filters, orig_limit)"},{"line_number":2572,"context_line":""},{"line_number":2573,"context_line":"        if cell_down_support:"},{"line_number":2574,"context_line":"            return (self._generate_minimal_construct_for_down_cells(context,"},{"line_number":2575,"context_line":"                down_cell_uuids) + instances)"},{"line_number":2576,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"3f79a3b5_06f4bc54","line":2573,"updated":"2018-10-17 17:10:47.000000000","message":"Let\u0027s document that this is doing.","commit_id":"b19a4065956076fef698dd313359e7aa0f58e969"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2fb4ce699b47ffa8df34cafaacbdc90fdc61f137","unresolved":false,"context_lines":[{"line_number":2360,"context_line":"            instance_mappings \u003d (objects.InstanceMappingList."},{"line_number":2361,"context_line":"                get_not_deleted_by_cell_and_projects(context, cell_uuid,"},{"line_number":2362,"context_line":"                                                     projects))"},{"line_number":2363,"context_line":"            for im in instance_mappings:"},{"line_number":2364,"context_line":"                unavailable_servers.objects.append("},{"line_number":2365,"context_line":"                    objects.Instance("},{"line_number":2366,"context_line":"                        context\u003dcontext,"}],"source_content_type":"text/x-python","patch_set":27,"id":"3f79a3b5_dd64944f","line":2363,"range":{"start_line":2363,"start_character":12,"end_line":2363,"end_character":40},"updated":"2018-10-26 13:34:26.000000000","message":"Hmm, wouldn\u0027t we want to stop if we\u0027ve reached some limit? The spec doesn\u0027t explicitly called out limits. It says we don\u0027t apply paging with down cells:\n\nhttps://specs.openstack.org/openstack/nova-specs/specs/stein/approved/handling-down-cell_new.html#edge-cases\n\nBut that\u0027s more about finding a marker and such. It seems we could easily apply a limit (not a marker) with down cells though couldn\u0027t we? If I said the limit is 10 and I got 3 from build requests, 2 from cell0, 3 from (up) cell1 and then there are 5 more in (down) cell2, I could still just get back 10 but with this I\u0027d get back 12.","commit_id":"1e1a552c830236ef23158b40fdac07e4a2cf6f7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ea7b0b39890423500419028a4c5dd5c79198b53d","unresolved":false,"context_lines":[{"line_number":2362,"context_line":"                                                     projects))"},{"line_number":2363,"context_line":"            for im in instance_mappings:"},{"line_number":2364,"context_line":"                unavailable_servers.objects.append("},{"line_number":2365,"context_line":"                    objects.Instance("},{"line_number":2366,"context_line":"                        context\u003dcontext,"},{"line_number":2367,"context_line":"                        uuid\u003dim.instance_uuid,"},{"line_number":2368,"context_line":"                        project_id\u003dim.project_id,"},{"line_number":2369,"context_line":"                        created_at\u003dim.created_at"},{"line_number":2370,"context_line":"                    )"},{"line_number":2371,"context_line":"                )"},{"line_number":2372,"context_line":"        return unavailable_servers"},{"line_number":2373,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"3f79a3b5_e6e4710d","line":2370,"range":{"start_line":2365,"start_character":20,"end_line":2370,"end_character":21},"updated":"2018-10-25 21:58:09.000000000","message":"I\u0027m not sure how the view builder is going to know this is a down cell instance to set the status to UNKNOWN, but I guess I\u0027ll find out later (maybe the absence of the vm_state field?).","commit_id":"1e1a552c830236ef23158b40fdac07e4a2cf6f7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ea7b0b39890423500419028a4c5dd5c79198b53d","unresolved":false,"context_lines":[{"line_number":2646,"context_line":""},{"line_number":2647,"context_line":"        if cell_down_support:"},{"line_number":2648,"context_line":"            # API and client want minimal construct instances for any cells"},{"line_number":2649,"context_line":"            # that didn\u0027t return, so generate and prefix those to the actual"},{"line_number":2650,"context_line":"            # results."},{"line_number":2651,"context_line":"            projects \u003d search_opts.get(\u0027project_id\u0027, [context.project_id])"},{"line_number":2652,"context_line":"            if not isinstance(projects, list):"}],"source_content_type":"text/x-python","patch_set":27,"id":"3f79a3b5_869c9d70","line":2649,"range":{"start_line":2649,"start_character":50,"end_line":2649,"end_character":56},"updated":"2018-10-25 21:58:09.000000000","message":"Yeah I reckon prefixing is fine given we can\u0027t maintain any sort order with down-cell instances, as noted in the spec:\n\nhttps://specs.openstack.org/openstack/nova-specs/specs/stein/approved/handling-down-cell_new.html#edge-cases\n\nPlus this way you get to see:\n\nUNKNOWN....\nbuild requests...\ncell0..cellN....","commit_id":"1e1a552c830236ef23158b40fdac07e4a2cf6f7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ea7b0b39890423500419028a4c5dd5c79198b53d","unresolved":false,"context_lines":[{"line_number":2648,"context_line":"            # API and client want minimal construct instances for any cells"},{"line_number":2649,"context_line":"            # that didn\u0027t return, so generate and prefix those to the actual"},{"line_number":2650,"context_line":"            # results."},{"line_number":2651,"context_line":"            projects \u003d search_opts.get(\u0027project_id\u0027, [context.project_id])"},{"line_number":2652,"context_line":"            if not isinstance(projects, list):"},{"line_number":2653,"context_line":"                projects \u003d [projects]"},{"line_number":2654,"context_line":"            return (self._generate_minimal_construct_for_down_cells(context,"}],"source_content_type":"text/x-python","patch_set":27,"id":"3f79a3b5_a61439a0","line":2651,"updated":"2018-10-25 21:58:09.000000000","message":"Just a reminder to myself that project_id is filtered out unless all_tenants is used:\n\nhttps://github.com/openstack/nova/blob/8ec31bfd4259d78603477f57bf259ab3241219ab/nova/api/openstack/compute/servers.py#L206-L248\n\nAnd L2514 above will swap tenant_id for project_id since tenant_id is just an old alias.","commit_id":"1e1a552c830236ef23158b40fdac07e4a2cf6f7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ea7b0b39890423500419028a4c5dd5c79198b53d","unresolved":false,"context_lines":[{"line_number":2649,"context_line":"            # that didn\u0027t return, so generate and prefix those to the actual"},{"line_number":2650,"context_line":"            # results."},{"line_number":2651,"context_line":"            projects \u003d search_opts.get(\u0027project_id\u0027, [context.project_id])"},{"line_number":2652,"context_line":"            if not isinstance(projects, list):"},{"line_number":2653,"context_line":"                projects \u003d [projects]"},{"line_number":2654,"context_line":"            return (self._generate_minimal_construct_for_down_cells(context,"},{"line_number":2655,"context_line":"                down_cell_uuids, projects) + instances)"}],"source_content_type":"text/x-python","patch_set":27,"id":"3f79a3b5_667f01de","line":2652,"range":{"start_line":2652,"start_character":12,"end_line":2652,"end_character":46},"updated":"2018-10-25 21:58:09.000000000","message":"Hmm, if I\u0027m an admin and doing a query like:\n\nGET /servers?all_tenants\u003d1\u0026project_id\u003d1\u0026project_id\u003d2\n\nDoes that get turned into a list or something else?","commit_id":"1e1a552c830236ef23158b40fdac07e4a2cf6f7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"731f00427b25d1857becd011fff0ef849932a5da","unresolved":false,"context_lines":[{"line_number":2649,"context_line":"            # that didn\u0027t return, so generate and prefix those to the actual"},{"line_number":2650,"context_line":"            # results."},{"line_number":2651,"context_line":"            projects \u003d search_opts.get(\u0027project_id\u0027, [context.project_id])"},{"line_number":2652,"context_line":"            if not isinstance(projects, list):"},{"line_number":2653,"context_line":"                projects \u003d [projects]"},{"line_number":2654,"context_line":"            return (self._generate_minimal_construct_for_down_cells(context,"},{"line_number":2655,"context_line":"                down_cell_uuids, projects) + instances)"}],"source_content_type":"text/x-python","patch_set":27,"id":"3f79a3b5_98aceaeb","line":2652,"range":{"start_line":2652,"start_character":12,"end_line":2652,"end_character":46},"in_reply_to":"3f79a3b5_667f01de","updated":"2018-10-26 13:56:06.000000000","message":"https://docs.pylonsproject.org/projects/webob/en/stable/reference.html#query-post-variables\n\nSo apparently we don\u0027t support listing servers for a list of projects, so it\u0027s a non-issue. We don\u0027t use req.GET.getall() for project_id anyway.","commit_id":"1e1a552c830236ef23158b40fdac07e4a2cf6f7b"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"409e10cf1a100d2b673cc481f9c169f6dedc3de7","unresolved":false,"context_lines":[{"line_number":2649,"context_line":"            # that didn\u0027t return, so generate and prefix those to the actual"},{"line_number":2650,"context_line":"            # results."},{"line_number":2651,"context_line":"            projects \u003d search_opts.get(\u0027project_id\u0027, [context.project_id])"},{"line_number":2652,"context_line":"            if not isinstance(projects, list):"},{"line_number":2653,"context_line":"                projects \u003d [projects]"},{"line_number":2654,"context_line":"            return (self._generate_minimal_construct_for_down_cells(context,"},{"line_number":2655,"context_line":"                down_cell_uuids, projects) + instances)"}],"source_content_type":"text/x-python","patch_set":27,"id":"3f79a3b5_7bfd016c","line":2652,"range":{"start_line":2652,"start_character":12,"end_line":2652,"end_character":46},"in_reply_to":"3f79a3b5_98aceaeb","updated":"2018-10-31 12:02:53.000000000","message":"Done","commit_id":"1e1a552c830236ef23158b40fdac07e4a2cf6f7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4ec3dafa8dc2f9d56987b841f517cdd21e7d00dc","unresolved":false,"context_lines":[{"line_number":2341,"context_line":""},{"line_number":2342,"context_line":"    def _generate_minimal_construct_for_down_cells(self, context,"},{"line_number":2343,"context_line":"                                                   down_cell_uuids,"},{"line_number":2344,"context_line":"                                                   projects, limit):"},{"line_number":2345,"context_line":"        \"\"\"Generate a list of minimal instance constructs for a given list of"},{"line_number":2346,"context_line":"        cells that did not respond to a list operation. This will list"},{"line_number":2347,"context_line":"        every instance mapping in the affected cells and return a minimal"}],"source_content_type":"text/x-python","patch_set":28,"id":"3f79a3b5_b80affcc","line":2344,"range":{"start_line":2344,"start_character":61,"end_line":2344,"end_character":66},"updated":"2018-10-26 19:33:58.000000000","message":"nit: forgot this in the docstring","commit_id":"e4b5e39597b943ba6dbc015dcadbb4a85cdf86be"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"409e10cf1a100d2b673cc481f9c169f6dedc3de7","unresolved":false,"context_lines":[{"line_number":2341,"context_line":""},{"line_number":2342,"context_line":"    def _generate_minimal_construct_for_down_cells(self, context,"},{"line_number":2343,"context_line":"                                                   down_cell_uuids,"},{"line_number":2344,"context_line":"                                                   projects, limit):"},{"line_number":2345,"context_line":"        \"\"\"Generate a list of minimal instance constructs for a given list of"},{"line_number":2346,"context_line":"        cells that did not respond to a list operation. This will list"},{"line_number":2347,"context_line":"        every instance mapping in the affected cells and return a minimal"}],"source_content_type":"text/x-python","patch_set":28,"id":"3f79a3b5_80a2ecfa","line":2344,"range":{"start_line":2344,"start_character":61,"end_line":2344,"end_character":66},"in_reply_to":"3f79a3b5_b80affcc","updated":"2018-10-31 12:02:53.000000000","message":"Done","commit_id":"e4b5e39597b943ba6dbc015dcadbb4a85cdf86be"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4ec3dafa8dc2f9d56987b841f517cdd21e7d00dc","unresolved":false,"context_lines":[{"line_number":2359,"context_line":"                        \"cell if any.\", cell_uuid)"},{"line_number":2360,"context_line":"            instance_mappings \u003d (objects.InstanceMappingList."},{"line_number":2361,"context_line":"                get_not_deleted_by_cell_and_projects(context, cell_uuid,"},{"line_number":2362,"context_line":"                                                     projects, limit\u003dlimit))"},{"line_number":2363,"context_line":"            for im in instance_mappings:"},{"line_number":2364,"context_line":"                unavailable_servers.objects.append("},{"line_number":2365,"context_line":"                    objects.Instance("}],"source_content_type":"text/x-python","patch_set":28,"id":"3f79a3b5_38f6efd6","line":2362,"range":{"start_line":2362,"start_character":63,"end_line":2362,"end_character":74},"updated":"2018-10-26 19:33:58.000000000","message":"We would need to decrement our limit based on how many instance mappings we got from each cell until we\u0027ve reached the limit and can stop looping or have no more results.","commit_id":"e4b5e39597b943ba6dbc015dcadbb4a85cdf86be"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5cdcbcef137c1dfc28ac621eb31e4913ed0d9436","unresolved":false,"context_lines":[{"line_number":2349,"context_line":""},{"line_number":2350,"context_line":"        :param context: RequestContext"},{"line_number":2351,"context_line":"        :param down_cell_uuids: A list of cell UUIDs that did not respond"},{"line_number":2352,"context_line":"        :param projects: A list of project IDs to filter mappings"},{"line_number":2353,"context_line":"        :returns: An InstanceList() of partial Instance() objects"},{"line_number":2354,"context_line":"        \"\"\""},{"line_number":2355,"context_line":"        unavailable_servers \u003d objects.InstanceList()"}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_333cd02b","line":2352,"updated":"2018-10-26 21:32:15.000000000","message":"limit param is missing from the docstring here","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"409e10cf1a100d2b673cc481f9c169f6dedc3de7","unresolved":false,"context_lines":[{"line_number":2349,"context_line":""},{"line_number":2350,"context_line":"        :param context: RequestContext"},{"line_number":2351,"context_line":"        :param down_cell_uuids: A list of cell UUIDs that did not respond"},{"line_number":2352,"context_line":"        :param projects: A list of project IDs to filter mappings"},{"line_number":2353,"context_line":"        :returns: An InstanceList() of partial Instance() objects"},{"line_number":2354,"context_line":"        \"\"\""},{"line_number":2355,"context_line":"        unavailable_servers \u003d objects.InstanceList()"}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_6076b095","line":2352,"in_reply_to":"3f79a3b5_333cd02b","updated":"2018-10-31 12:02:53.000000000","message":"Done","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5cdcbcef137c1dfc28ac621eb31e4913ed0d9436","unresolved":false,"context_lines":[{"line_number":2371,"context_line":"                )"},{"line_number":2372,"context_line":"            if limit is not None:"},{"line_number":2373,"context_line":"                limit -\u003d len(instance_mappings)"},{"line_number":2374,"context_line":"                if limit \u003c\u003d 0:"},{"line_number":2375,"context_line":"                    break"},{"line_number":2376,"context_line":"        return unavailable_servers"},{"line_number":2377,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_131af4c5","line":2374,"range":{"start_line":2374,"start_character":25,"end_line":2374,"end_character":27},"updated":"2018-10-26 21:32:15.000000000","message":"When I read this \"\u003c\u003d\", it made me think it implies it\u0027s possible to return a number of servers above limit. But that shouldn\u0027t be possible because limit is passed to each get call and is decremented every iteration.\n\nOr is there a reason this needs to be \u003c\u003d that I\u0027m missing?","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"409e10cf1a100d2b673cc481f9c169f6dedc3de7","unresolved":false,"context_lines":[{"line_number":2371,"context_line":"                )"},{"line_number":2372,"context_line":"            if limit is not None:"},{"line_number":2373,"context_line":"                limit -\u003d len(instance_mappings)"},{"line_number":2374,"context_line":"                if limit \u003c\u003d 0:"},{"line_number":2375,"context_line":"                    break"},{"line_number":2376,"context_line":"        return unavailable_servers"},{"line_number":2377,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_c0e6a44a","line":2374,"range":{"start_line":2374,"start_character":25,"end_line":2374,"end_character":27},"in_reply_to":"3f79a3b5_131af4c5","updated":"2018-10-31 12:02:53.000000000","message":"good point, I think so too that \"\u003d\" should be good enough, but I will wait to see if dansmith had any other reasons for the \"\u003c\u003d\".","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5cdcbcef137c1dfc28ac621eb31e4913ed0d9436","unresolved":false,"context_lines":[{"line_number":2653,"context_line":"            # that didn\u0027t return, so generate and prefix those to the actual"},{"line_number":2654,"context_line":"            # results."},{"line_number":2655,"context_line":"            projects \u003d search_opts.get(\u0027project_id\u0027, [context.project_id])"},{"line_number":2656,"context_line":"            if not isinstance(projects, list):"},{"line_number":2657,"context_line":"                projects \u003d [projects]"},{"line_number":2658,"context_line":"            limit \u003d (limit - len(instances)) if limit else limit"},{"line_number":2659,"context_line":"            return (self._generate_minimal_construct_for_down_cells(context,"},{"line_number":2660,"context_line":"                down_cell_uuids, projects, limit) + instances)"}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_25bc87d9","line":2657,"range":{"start_line":2656,"start_character":12,"end_line":2657,"end_character":37},"updated":"2018-10-26 21:32:15.000000000","message":"I don\u0027t think there\u0027s any test coverage for this case.","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"409e10cf1a100d2b673cc481f9c169f6dedc3de7","unresolved":false,"context_lines":[{"line_number":2653,"context_line":"            # that didn\u0027t return, so generate and prefix those to the actual"},{"line_number":2654,"context_line":"            # results."},{"line_number":2655,"context_line":"            projects \u003d search_opts.get(\u0027project_id\u0027, [context.project_id])"},{"line_number":2656,"context_line":"            if not isinstance(projects, list):"},{"line_number":2657,"context_line":"                projects \u003d [projects]"},{"line_number":2658,"context_line":"            limit \u003d (limit - len(instances)) if limit else limit"},{"line_number":2659,"context_line":"            return (self._generate_minimal_construct_for_down_cells(context,"},{"line_number":2660,"context_line":"                down_cell_uuids, projects, limit) + instances)"}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_a0d5a8ee","line":2657,"range":{"start_line":2656,"start_character":12,"end_line":2657,"end_character":37},"in_reply_to":"3f79a3b5_25bc87d9","updated":"2018-10-31 12:02:53.000000000","message":"Done","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"32d702e4879df4d2303326661eeb691aa6a94fa1","unresolved":false,"context_lines":[{"line_number":2652,"context_line":"            # API and client want minimal construct instances for any cells"},{"line_number":2653,"context_line":"            # that didn\u0027t return, so generate and prefix those to the actual"},{"line_number":2654,"context_line":"            # results."},{"line_number":2655,"context_line":"            projects \u003d search_opts.get(\u0027project_id\u0027, [context.project_id])"},{"line_number":2656,"context_line":"            if not isinstance(projects, list):"},{"line_number":2657,"context_line":"                projects \u003d [projects]"},{"line_number":2658,"context_line":"            limit \u003d (limit - len(instances)) if limit else limit"},{"line_number":2659,"context_line":"            return (self._generate_minimal_construct_for_down_cells(context,"},{"line_number":2660,"context_line":"                down_cell_uuids, projects, limit) + instances)"},{"line_number":2661,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_cecfc752","line":2658,"range":{"start_line":2655,"start_character":12,"end_line":2658,"end_character":64},"updated":"2018-10-30 15:40:00.000000000","message":"sorry probably I am missing something obvious, but why are we allowing filters based on \"projects\" and \"limits\"? In the spec it was specifically agreed that down cell wouldn\u0027t support any kind of filtering/paging/sorting and would just give a 500/skip down cells if something like that was asked (https://review.openstack.org/#/c/591657/9/nova/api/openstack/compute/servers.py@115). My point being we are not allowing any filters right ? Because it seems like we are right now making exceptions like \"--all-tenants\" and \"--limit\".\n\nOkay I see some discussion here https://review.openstack.org/#/c/567785/20/nova/compute/api.py@2295, yea the point was to skip down cells and only return results from up cells, if any of the filters were mentioned. I am not so sure if its a good idea to allow --all-tenants for down cells; its going to be very slow..plus is there any other scenario (other than --all-tenants) where we would have more than one project_id in the query ? and we should surely not allow \"--limit\" filter since it would just make room for more filters - if limit is not specified it would just do it in batches right ? thus returning a list of down cells from InstanceLister?","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"6b0b7dc97655fed5c4a81b03f3ab9b804374f4d9","unresolved":false,"context_lines":[{"line_number":2652,"context_line":"            # API and client want minimal construct instances for any cells"},{"line_number":2653,"context_line":"            # that didn\u0027t return, so generate and prefix those to the actual"},{"line_number":2654,"context_line":"            # results."},{"line_number":2655,"context_line":"            projects \u003d search_opts.get(\u0027project_id\u0027, [context.project_id])"},{"line_number":2656,"context_line":"            if not isinstance(projects, list):"},{"line_number":2657,"context_line":"                projects \u003d [projects]"},{"line_number":2658,"context_line":"            limit \u003d (limit - len(instances)) if limit else limit"},{"line_number":2659,"context_line":"            return (self._generate_minimal_construct_for_down_cells(context,"},{"line_number":2660,"context_line":"                down_cell_uuids, projects, limit) + instances)"},{"line_number":2661,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_80d513ef","line":2658,"range":{"start_line":2655,"start_character":12,"end_line":2658,"end_character":64},"in_reply_to":"3f79a3b5_496ad1ec","updated":"2018-10-30 22:43:41.000000000","message":"also this limit subtraction happening here would be:\n\nlimit - len(build_reqs) on L2609 - len(insts) on L2640 - len(instances) which would be the cleaned up list merge on L2643. Looks like we are double subtracting from the parameter limit. Let me add a test and see if this is true.","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"409e10cf1a100d2b673cc481f9c169f6dedc3de7","unresolved":false,"context_lines":[{"line_number":2652,"context_line":"            # API and client want minimal construct instances for any cells"},{"line_number":2653,"context_line":"            # that didn\u0027t return, so generate and prefix those to the actual"},{"line_number":2654,"context_line":"            # results."},{"line_number":2655,"context_line":"            projects \u003d search_opts.get(\u0027project_id\u0027, [context.project_id])"},{"line_number":2656,"context_line":"            if not isinstance(projects, list):"},{"line_number":2657,"context_line":"                projects \u003d [projects]"},{"line_number":2658,"context_line":"            limit \u003d (limit - len(instances)) if limit else limit"},{"line_number":2659,"context_line":"            return (self._generate_minimal_construct_for_down_cells(context,"},{"line_number":2660,"context_line":"                down_cell_uuids, projects, limit) + instances)"},{"line_number":2661,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_c38d4bdb","line":2658,"range":{"start_line":2655,"start_character":12,"end_line":2658,"end_character":64},"in_reply_to":"3f79a3b5_80d513ef","updated":"2018-10-31 12:02:53.000000000","message":"changed this to be subtracted from orig_limit instead to make it consistent.","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"3efcfc9f2ba8cde35e869606477684993641a14a","unresolved":false,"context_lines":[{"line_number":2652,"context_line":"            # API and client want minimal construct instances for any cells"},{"line_number":2653,"context_line":"            # that didn\u0027t return, so generate and prefix those to the actual"},{"line_number":2654,"context_line":"            # results."},{"line_number":2655,"context_line":"            projects \u003d search_opts.get(\u0027project_id\u0027, [context.project_id])"},{"line_number":2656,"context_line":"            if not isinstance(projects, list):"},{"line_number":2657,"context_line":"                projects \u003d [projects]"},{"line_number":2658,"context_line":"            limit \u003d (limit - len(instances)) if limit else limit"},{"line_number":2659,"context_line":"            return (self._generate_minimal_construct_for_down_cells(context,"},{"line_number":2660,"context_line":"                down_cell_uuids, projects, limit) + instances)"},{"line_number":2661,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_496ad1ec","line":2658,"range":{"start_line":2655,"start_character":12,"end_line":2658,"end_character":64},"in_reply_to":"3f79a3b5_89b549d2","updated":"2018-10-30 16:12:48.000000000","message":"\u003e _generate_minimal_construct_for_down_cells filters instance\n \u003e mappings by project in down cells. The project_id either comes from\n \u003e the context (the caller of the API) or via passing in a request\n \u003e like this, from an admin, for a specific tenant:\n \u003e \n \u003e GET /servers?all_tenants\u003d1\u0026project_id\u003d1234\n \u003e \n \u003e I\u0027m not saying we\u0027re supporting all_tenants, I\u0027m saying we need to\n \u003e support the case that the project_id we have in the search_opts is\n \u003e a specified tenant via the project_id filter parameter, not the\n \u003e request context project_id (the admin in this scenario). Otherwise\n \u003e you could non-admin tenant instances from the up cells and admin\n \u003e instances from the down cells, which would be wrong. We can\u0027t make\n \u003e that distinction by just looking at the search_opts since it\n \u003e doesn\u0027t contain the all_tenants flag:\n \u003e \n \u003e https://github.com/openstack/nova/blob/d1e38f06bb5282dd9bf81d979926a0ebbfd93caa/nova/api/openstack/compute/servers.py#L206-L248\n \u003e \n\nAh okayy, totally missed this pitfall. thanks for clarifying.\n\n \u003e As for the limit, that\u0027s also passed down from the API using a\n \u003e default if the user didn\u0027t specify a limit:\n \u003e \n \u003e https://github.com/openstack/nova/blob/d1e38f06bb5282dd9bf81d979926a0ebbfd93caa/nova/api/openstack/common.py#L238\n \u003e \n \u003e Therefore we can\u0027t just ignore the limit, or blow up if passed in.\n\nokay, right, so we have to do it for the default limit if user doesn\u0027t pass one which makes sense and if are doing that then its pretty trivial to allow the \"--limit\" filter as well; or just ignore it and skip down cells if its explicitly specified - this would be a design choice then.\n\n \u003e Besides, it seems trivial to honor the limit as we\u0027re doing in\n \u003e _generate_minimal_construct_for_down_cells so I\u0027m not sure why we\n \u003e wouldn\u0027t. We\u0027re not honoring paging which to me means it\u0027s more\n \u003e about the marker than the limit. If a marker is provided, we can\u0027t\n \u003e honor that with down cells since the marker could be in a down cell\n \u003e (I mean I guess we\u0027d know if the instance mapping for the marker\n \u003e points to a down cell, but I\u0027m not sure how you\u0027re going to handle\n \u003e that yet).\n\nokay let\u0027s just not allow the marker filter yet at this initial stage and at a later stage we can handle it if everyone feels its needed.","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b15c579af72635a5b0e758dbf3fd855e6d590c0a","unresolved":false,"context_lines":[{"line_number":2652,"context_line":"            # API and client want minimal construct instances for any cells"},{"line_number":2653,"context_line":"            # that didn\u0027t return, so generate and prefix those to the actual"},{"line_number":2654,"context_line":"            # results."},{"line_number":2655,"context_line":"            projects \u003d search_opts.get(\u0027project_id\u0027, [context.project_id])"},{"line_number":2656,"context_line":"            if not isinstance(projects, list):"},{"line_number":2657,"context_line":"                projects \u003d [projects]"},{"line_number":2658,"context_line":"            limit \u003d (limit - len(instances)) if limit else limit"},{"line_number":2659,"context_line":"            return (self._generate_minimal_construct_for_down_cells(context,"},{"line_number":2660,"context_line":"                down_cell_uuids, projects, limit) + instances)"},{"line_number":2661,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_89b549d2","line":2658,"range":{"start_line":2655,"start_character":12,"end_line":2658,"end_character":64},"in_reply_to":"3f79a3b5_cecfc752","updated":"2018-10-30 16:00:35.000000000","message":"_generate_minimal_construct_for_down_cells filters instance mappings by project in down cells. The project_id either comes from the context (the caller of the API) or via passing in a request like this, from an admin, for a specific tenant:\n\nGET /servers?all_tenants\u003d1\u0026project_id\u003d1234\n\nI\u0027m not saying we\u0027re supporting all_tenants, I\u0027m saying we need to support the case that the project_id we have in the search_opts is a specified tenant via the project_id filter parameter, not the request context project_id (the admin in this scenario). Otherwise you could non-admin tenant instances from the up cells and admin instances from the down cells, which would be wrong. We can\u0027t make that distinction by just looking at the search_opts since it doesn\u0027t contain the all_tenants flag:\n\nhttps://github.com/openstack/nova/blob/d1e38f06bb5282dd9bf81d979926a0ebbfd93caa/nova/api/openstack/compute/servers.py#L206-L248\n\nAs for the limit, that\u0027s also passed down from the API using a default if the user didn\u0027t specify a limit:\n\nhttps://github.com/openstack/nova/blob/d1e38f06bb5282dd9bf81d979926a0ebbfd93caa/nova/api/openstack/common.py#L238\n\nTherefore we can\u0027t just ignore the limit, or blow up if passed in. Besides, it seems trivial to honor the limit as we\u0027re doing in _generate_minimal_construct_for_down_cells so I\u0027m not sure why we wouldn\u0027t. We\u0027re not honoring paging which to me means it\u0027s more about the marker than the limit. If a marker is provided, we can\u0027t honor that with down cells since the marker could be in a down cell (I mean I guess we\u0027d know if the instance mapping for the marker points to a down cell, but I\u0027m not sure how you\u0027re going to handle that yet).","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7f168b471de1a85c4a5c516d1d968785cb6329a0","unresolved":false,"context_lines":[{"line_number":2350,"context_line":"        :param context: RequestContext"},{"line_number":2351,"context_line":"        :param down_cell_uuids: A list of cell UUIDs that did not respond"},{"line_number":2352,"context_line":"        :param project: A project ID to filter mappings"},{"line_number":2353,"context_line":"        :param limit: A numeric limit on the number of results, or None"},{"line_number":2354,"context_line":"        :returns: An InstanceList() of partial Instance() objects"},{"line_number":2355,"context_line":"        \"\"\""},{"line_number":2356,"context_line":"        unavailable_servers \u003d objects.InstanceList()"}],"source_content_type":"text/x-python","patch_set":30,"id":"3f79a3b5_a74b70b0","line":2353,"range":{"start_line":2353,"start_character":64,"end_line":2353,"end_character":71},"updated":"2018-10-31 18:51:22.000000000","message":"nit: limit could just be a kwarg: limit\u003dNone","commit_id":"76cd5b53bde1f7bfa82042ab6ac8e860b793d15d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7f168b471de1a85c4a5c516d1d968785cb6329a0","unresolved":false,"context_lines":[{"line_number":2654,"context_line":"            # that didn\u0027t return, so generate and prefix those to the actual"},{"line_number":2655,"context_line":"            # results."},{"line_number":2656,"context_line":"            project \u003d search_opts.get(\u0027project_id\u0027, context.project_id)"},{"line_number":2657,"context_line":"            limit \u003d (orig_limit - len(instances)) if limit else limit"},{"line_number":2658,"context_line":"            return (self._generate_minimal_construct_for_down_cells(context,"},{"line_number":2659,"context_line":"                down_cell_uuids, project, limit) + instances)"},{"line_number":2660,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"3f79a3b5_67e9d8a9","line":2657,"range":{"start_line":2657,"start_character":21,"end_line":2657,"end_character":31},"updated":"2018-10-31 18:51:22.000000000","message":"I see your comment in PS29 but I\u0027m still not sure I understand why orig_limit is being used here.\n\nLet\u0027s say the original limit was 10 and we got 2 build requests and 5 instances from \"up\" cells. At that point, $limit would be 3. len(instances) should be 7, right? So then $orig_limit (10) - len(instances) (7) \u003d 3, which is the same as just using the limit variable we already had, and is consistent with the limit subtraction logic above.","commit_id":"76cd5b53bde1f7bfa82042ab6ac8e860b793d15d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4b50b95007c7363019aa0259c3de563361319b99","unresolved":false,"context_lines":[{"line_number":2654,"context_line":"            # that didn\u0027t return, so generate and prefix those to the actual"},{"line_number":2655,"context_line":"            # results."},{"line_number":2656,"context_line":"            project \u003d search_opts.get(\u0027project_id\u0027, context.project_id)"},{"line_number":2657,"context_line":"            limit \u003d (orig_limit - len(instances)) if limit else limit"},{"line_number":2658,"context_line":"            return (self._generate_minimal_construct_for_down_cells(context,"},{"line_number":2659,"context_line":"                down_cell_uuids, project, limit) + instances)"},{"line_number":2660,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"3f79a3b5_125940ff","line":2657,"range":{"start_line":2657,"start_character":21,"end_line":2657,"end_character":31},"in_reply_to":"3f79a3b5_528a5865","updated":"2018-10-31 19:11:31.000000000","message":"Done","commit_id":"76cd5b53bde1f7bfa82042ab6ac8e860b793d15d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ede99bb54836f76fff8b97bdb98c847f0acc8d5b","unresolved":false,"context_lines":[{"line_number":2654,"context_line":"            # that didn\u0027t return, so generate and prefix those to the actual"},{"line_number":2655,"context_line":"            # results."},{"line_number":2656,"context_line":"            project \u003d search_opts.get(\u0027project_id\u0027, context.project_id)"},{"line_number":2657,"context_line":"            limit \u003d (orig_limit - len(instances)) if limit else limit"},{"line_number":2658,"context_line":"            return (self._generate_minimal_construct_for_down_cells(context,"},{"line_number":2659,"context_line":"                down_cell_uuids, project, limit) + instances)"},{"line_number":2660,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"3f79a3b5_528a5865","line":2657,"range":{"start_line":2657,"start_character":21,"end_line":2657,"end_character":31},"in_reply_to":"3f79a3b5_67e9d8a9","updated":"2018-10-31 19:04:20.000000000","message":"Oh nevermind, now I see it, $limit (3) - len(instances) (7) would result in a limit of -4, which is clearly wrong. I didn\u0027t actually see a test wrinkle added for this though?","commit_id":"76cd5b53bde1f7bfa82042ab6ac8e860b793d15d"}],"nova/compute/instance_list.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6e83d4288934f405c5a824751a7a580ffb4d1ef9","unresolved":false,"context_lines":[{"line_number":90,"context_line":"# replicate these for every data type we implement."},{"line_number":91,"context_line":"def get_instances_sorted(ctx, filters, limit, marker, columns_to_join,"},{"line_number":92,"context_line":"                         sort_keys, sort_dirs, cell_mappings\u003dNone,"},{"line_number":93,"context_line":"                         cell_down_support\u003dFalse):"},{"line_number":94,"context_line":"    return InstanceLister(sort_keys, sort_dirs,"},{"line_number":95,"context_line":"                          cells\u003dcell_mappings).get_records_sorted(ctx, filters,"},{"line_number":96,"context_line":"        limit, marker, cell_down_support\u003dcell_down_support,"}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_e0a916dd","line":93,"updated":"2018-08-21 14:46:00.000000000","message":"I don\u0027t really love pushing this down into this layer, especially with the conditional. I would prefer to keep these instance_list and multi_cell_list modules as clean as possible and keep api-specific semantic behavior up closer to the api. I think I would characterize the \"make up some fake instance shells\" as purely api behavior and not anything related to the low-level mechanics of what we\u0027re doing here.","commit_id":"23c08ca84d49686ac990fb0c4d69791ad158edbf"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6e83d4288934f405c5a824751a7a580ffb4d1ef9","unresolved":false,"context_lines":[{"line_number":121,"context_line":"    # unavailable servers from the down cells before calling make_instance_list"},{"line_number":122,"context_line":"    # as it would complain about missing keys."},{"line_number":123,"context_line":"    if cell_down_support:"},{"line_number":124,"context_line":"        unavailable_instance_list \u003d next(instance_generator)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    if \u0027fault\u0027 in expected_attrs:"},{"line_number":127,"context_line":"        # We join fault above, so we need to make sure we don\u0027t ask"}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_e092769f","line":124,"updated":"2018-08-21 14:46:00.000000000","message":"This kind of seems like abuse of the interface and makes it harder to reason about the behavior of things that implement that the CrossCellLister interface.","commit_id":"23c08ca84d49686ac990fb0c4d69791ad158edbf"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"453f5a4cde5b4a8963a58808a4f8d4acb6f29ab8","unresolved":false,"context_lines":[{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    instance_list \u003d instance_obj._make_instance_list(ctx,"},{"line_number":165,"context_line":"        objects.InstanceList(), instance_generator, expected_attrs)"},{"line_number":166,"context_line":"    down_cell_uuids \u003d (instance_lister.cells_failed +"},{"line_number":167,"context_line":"                       instance_lister.cells_timed_out)"},{"line_number":168,"context_line":"    return instance_list, down_cell_uuids"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_1ee85fb1","line":166,"range":{"start_line":166,"start_character":39,"end_line":166,"end_character":51},"updated":"2018-09-25 16:33:32.000000000","message":"this need not be considered; since its ideally not down cells. its just cells which got an exception.","commit_id":"1b1e784b94f89bff49ce4a4c1518043e5a1607cd"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"3e04dac4773fe9300252c93c51c5e3b24857b683","unresolved":false,"context_lines":[{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    instance_list \u003d instance_obj._make_instance_list(ctx,"},{"line_number":165,"context_line":"        objects.InstanceList(), instance_generator, expected_attrs)"},{"line_number":166,"context_line":"    down_cell_uuids \u003d (instance_lister.cells_failed +"},{"line_number":167,"context_line":"                       instance_lister.cells_timed_out)"},{"line_number":168,"context_line":"    return instance_list, down_cell_uuids"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_a42efc89","line":166,"range":{"start_line":166,"start_character":39,"end_line":166,"end_character":51},"in_reply_to":"3f79a3b5_1ee85fb1","updated":"2018-09-25 17:28:41.000000000","message":"okay this should also be handled. for instance the DBConnectionError is handled as raised_sentinel_exception","commit_id":"1b1e784b94f89bff49ce4a4c1518043e5a1607cd"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ea7b0b39890423500419028a4c5dd5c79198b53d","unresolved":false,"context_lines":[{"line_number":133,"context_line":""},{"line_number":134,"context_line":"def get_instance_objects_sorted(ctx, filters, limit, marker, expected_attrs,"},{"line_number":135,"context_line":"                                sort_keys, sort_dirs):"},{"line_number":136,"context_line":"    \"\"\"Same as above, but return an InstanceList.\"\"\""},{"line_number":137,"context_line":"    query_cell_subset \u003d CONF.api.instance_list_per_project_cells"},{"line_number":138,"context_line":"    # NOTE(danms): Replicated in part from instance_get_all_by_sort_filters(),"},{"line_number":139,"context_line":"    # where if we\u0027re not admin we\u0027re restricted to our context\u0027s project"}],"source_content_type":"text/x-python","patch_set":27,"id":"3f79a3b5_6c6123a2","line":136,"range":{"start_line":136,"start_character":3,"end_line":136,"end_character":52},"updated":"2018-10-25 21:58:09.000000000","message":"This should be updated since it\u0027s returning a tuple now. I\u0027ve looked at this change a couple of times on different days and had to remember what the change is below, and how it fits into the overall compute API get_all() method, and it\u0027s to not only get the list of instances from the cell, but also any cells that are down.","commit_id":"1e1a552c830236ef23158b40fdac07e4a2cf6f7b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5cdcbcef137c1dfc28ac621eb31e4913ed0d9436","unresolved":false,"context_lines":[{"line_number":97,"context_line":"                                     batch_size\u003dbatch_size)"},{"line_number":98,"context_line":"    instance_generator \u003d instance_lister.get_records_sorted("},{"line_number":99,"context_line":"        ctx, filters, limit, marker, columns_to_join\u003dcolumns_to_join)"},{"line_number":100,"context_line":"    return instance_lister, instance_generator"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"def get_instance_list_cells_batch_size(limit, cells):"}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_137994c4","line":100,"updated":"2018-10-26 21:32:15.000000000","message":"Note to self: we need to expose the InstanceLister because it tracks a list of cells that raised exceptions and a list of cells that timed out.","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"}],"nova/compute/multi_cell_list.py":[{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"73f38fdf77bdc8ef3edd74c8962ff69a7580d239","unresolved":false,"context_lines":[{"line_number":263,"context_line":"        for cell_uuid in list(results):"},{"line_number":264,"context_line":"            if results[cell_uuid] in (context.did_not_respond_sentinel,"},{"line_number":265,"context_line":"                                      context.raised_exception_sentinel):"},{"line_number":266,"context_line":"                if (len(filters) \u003d\u003d 2 and"},{"line_number":267,"context_line":"                    (\u0027deleted\u0027 and \u0027project_id\u0027 in filters) and"},{"line_number":268,"context_line":"                    not filters[\u0027deleted\u0027] and"},{"line_number":269,"context_line":"                    filters[\u0027project_id\u0027] \u003d\u003d ctx.project_id):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_c4942c8f","line":266,"updated":"2018-07-02 16:23:53.000000000","message":"needs to change if we are going to support --all-tenants, however if we are sticking to \"if filters, then skip concept\" then it should be fine.","commit_id":"45084a93a1707326e660201c0a32ba2ddbb2fbda"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"16718ccdc8e4c9efec9145ecc12cf4a88457d9dd","unresolved":false,"context_lines":[{"line_number":276,"context_line":"                                get_by_cell_id(ctx, cell.id))"},{"line_number":277,"context_line":"                        else:"},{"line_number":278,"context_line":"                            instance_mappings \u003d (objects.InstanceMappingList."},{"line_number":279,"context_line":"                                get_by_cell_and_project(ctx, cell.id,"},{"line_number":280,"context_line":"                                ctx.project_id))"},{"line_number":281,"context_line":"                        for im in instance_mappings:"},{"line_number":282,"context_line":"                            if not im.queued_for_delete:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_a4b7b021","line":279,"updated":"2018-07-02 16:25:49.000000000","message":"This function needs to be added for making it neater.","commit_id":"45084a93a1707326e660201c0a32ba2ddbb2fbda"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"e581247c004e3a8a6ae9f6c1ec314c9d6679949f","unresolved":false,"context_lines":[{"line_number":267,"context_line":"                # or nova list --minimal will return minimalistic results."},{"line_number":268,"context_line":"                # In the rest of the cases, the down cell will be skipped."},{"line_number":269,"context_line":"                condition \u003d (len(filters) \u003d\u003d 2 and"},{"line_number":270,"context_line":"                             (\u0027deleted\u0027 and \u0027project_id\u0027 in filters) and"},{"line_number":271,"context_line":"                             not filters[\u0027deleted\u0027] and"},{"line_number":272,"context_line":"                             filters[\u0027project_id\u0027] \u003d\u003d ctx.project_id and"},{"line_number":273,"context_line":"                             marker is None)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_719e5208","line":270,"updated":"2018-08-20 17:34:14.000000000","message":"eesh: \u0027deleted\u0027 in filters and \u0027project_id\u0027 in filters","commit_id":"23c08ca84d49686ac990fb0c4d69791ad158edbf"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"ded61c2ab7e3bdf6ff3d326beeb9ac62b46032da","unresolved":false,"context_lines":[{"line_number":270,"context_line":"                             (\u0027deleted\u0027 and \u0027project_id\u0027 in filters) and"},{"line_number":271,"context_line":"                             not filters[\u0027deleted\u0027] and"},{"line_number":272,"context_line":"                             filters[\u0027project_id\u0027] \u003d\u003d ctx.project_id and"},{"line_number":273,"context_line":"                             marker is None)"},{"line_number":274,"context_line":"                if condition and cell_down_support:"},{"line_number":275,"context_line":"                    LOG.warning(\"Cell %s is not responding and hence only \""},{"line_number":276,"context_line":"                                \"partial results are available from this \""}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_2afe3205","line":273,"updated":"2018-08-20 08:26:34.000000000","message":"catch the case for the filter \"limit\", and non-default sorting keys/direction if specified.","commit_id":"23c08ca84d49686ac990fb0c4d69791ad158edbf"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"7d36f827adb385ee0e46acd29c01cfe4e3009b24","unresolved":false,"context_lines":[{"line_number":270,"context_line":"                             (\u0027deleted\u0027 and \u0027project_id\u0027 in filters) and"},{"line_number":271,"context_line":"                             not filters[\u0027deleted\u0027] and"},{"line_number":272,"context_line":"                             filters[\u0027project_id\u0027] \u003d\u003d ctx.project_id and"},{"line_number":273,"context_line":"                             marker is None)"},{"line_number":274,"context_line":"                if condition and cell_down_support:"},{"line_number":275,"context_line":"                    LOG.warning(\"Cell %s is not responding and hence only \""},{"line_number":276,"context_line":"                                \"partial results are available from this \""}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_38f3cce9","line":273,"in_reply_to":"3f79a3b5_2afe3205","updated":"2018-08-21 10:00:08.000000000","message":"tested all the 23 filters for nova list: \n\nThe ones that pass this condition and should not pass include:\n\n* --sort\n* --limit\n\nand these need to be dealt with on the nova/api/openstack/compute/server.py level to see if the user provided any non-default values.\n\nThe ones that pass this condition and are okay if they pass include:\n\n* --minimal\n* --fields (basically same as nova list but with limited display columns)\n* --tenant (since this info is legit from API.instance_mappings table for down cells)\n\nThis the only filters that would be support are the above three.","commit_id":"23c08ca84d49686ac990fb0c4d69791ad158edbf"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"83bf64543c6c7083422ed797a18c5bfd7fd72aa1","unresolved":false,"context_lines":[{"line_number":270,"context_line":"                             (\u0027deleted\u0027 and \u0027project_id\u0027 in filters) and"},{"line_number":271,"context_line":"                             not filters[\u0027deleted\u0027] and"},{"line_number":272,"context_line":"                             filters[\u0027project_id\u0027] \u003d\u003d ctx.project_id and"},{"line_number":273,"context_line":"                             marker is None)"},{"line_number":274,"context_line":"                if condition and cell_down_support:"},{"line_number":275,"context_line":"                    LOG.warning(\"Cell %s is not responding and hence only \""},{"line_number":276,"context_line":"                                \"partial results are available from this \""}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_04f8e9c1","line":273,"in_reply_to":"3f79a3b5_38f3cce9","updated":"2018-08-21 12:55:12.000000000","message":"I guess another option is for me to outsource the whole check of no filter/sorter/paginator/marker to nova/api/compute/server.py since it would be more whole and completely eliminates partial results if request.params is not an empty dict.","commit_id":"23c08ca84d49686ac990fb0c4d69791ad158edbf"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"217dad3099d038a15af670c910b0e9efef7f3134","unresolved":false,"context_lines":[{"line_number":270,"context_line":"                             (\u0027deleted\u0027 and \u0027project_id\u0027 in filters) and"},{"line_number":271,"context_line":"                             not filters[\u0027deleted\u0027] and"},{"line_number":272,"context_line":"                             filters[\u0027project_id\u0027] \u003d\u003d ctx.project_id and"},{"line_number":273,"context_line":"                             marker is None)"},{"line_number":274,"context_line":"                if condition and cell_down_support:"},{"line_number":275,"context_line":"                    LOG.warning(\"Cell %s is not responding and hence only \""},{"line_number":276,"context_line":"                                \"partial results are available from this \""}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_c9768c3f","line":273,"in_reply_to":"3f79a3b5_38f3cce9","updated":"2018-08-21 12:18:49.000000000","message":"oh forgot this one: --instance-name in addition to --sort and --limit. let\u0027s add that to the above list.\n\nbtw, what\u0027s the difference between --name and --instance-name ?","commit_id":"23c08ca84d49686ac990fb0c4d69791ad158edbf"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6e83d4288934f405c5a824751a7a580ffb4d1ef9","unresolved":false,"context_lines":[{"line_number":291,"context_line":"                                \"from the results.\", cell_uuid)"},{"line_number":292,"context_line":"                results.pop(cell_uuid)"},{"line_number":293,"context_line":"        if cell_down_support:"},{"line_number":294,"context_line":"            yield unavailable_results"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"        # If a limit was provided, it was passed to the per-cell query"},{"line_number":297,"context_line":"        # routines.  That means we have NUM_CELLS * limit items across"}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_603ae698","line":294,"updated":"2018-08-21 14:46:00.000000000","message":"I really don\u0027t like jamming this into here, but I need to think on what the alternative would be.","commit_id":"23c08ca84d49686ac990fb0c4d69791ad158edbf"}],"nova/tests/functional/compute/test_instance_list.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5cdcbcef137c1dfc28ac621eb31e4913ed0d9436","unresolved":false,"context_lines":[{"line_number":192,"context_line":"                instance_list.get_instances_sorted(self.context, {},"},{"line_number":193,"context_line":"                                                   limit, marker,"},{"line_number":194,"context_line":"                                                   [], [sort_by],"},{"line_number":195,"context_line":"                                                   [sort_dir])[1])"},{"line_number":196,"context_line":"            if not batch:"},{"line_number":197,"context_line":"                # This should only happen when we\u0027ve pulled the last empty"},{"line_number":198,"context_line":"                # page because we used the marker of the last instance. If"}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_731c887a","line":195,"range":{"start_line":195,"start_character":62,"end_line":195,"end_character":65},"updated":"2018-10-26 21:32:15.000000000","message":"Note to self: these are all accessing the second item in the tuple (instance_lister, instance_generator) returned by get_instances_sorted. Previously, only the instance_generator was returned.","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"}],"nova/tests/unit/compute/test_compute.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5cdcbcef137c1dfc28ac621eb31e4913ed0d9436","unresolved":false,"context_lines":[{"line_number":11573,"context_line":"        # Limit is not supplied to the DB when using an IP filter"},{"line_number":11574,"context_line":"        with mock.patch(\u0027nova.compute.instance_list.\u0027"},{"line_number":11575,"context_line":"                        \u0027get_instance_objects_sorted\u0027) as m_get:"},{"line_number":11576,"context_line":"            m_get.return_value \u003d objects.InstanceList(objects\u003d[]), list()"},{"line_number":11577,"context_line":"            self.compute_api.get_all(c, search_opts\u003d{\u0027ip\u0027: \u0027.10\u0027}, limit\u003d1)"},{"line_number":11578,"context_line":"            self.assertEqual(1, m_get.call_count)"},{"line_number":11579,"context_line":"            args \u003d m_get.call_args[0]"}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_73056814","line":11576,"range":{"start_line":11576,"start_character":67,"end_line":11576,"end_character":73},"updated":"2018-10-26 21:32:15.000000000","message":"Note to self: this is the empty down_cell_uuids from get_instance_objects_sorted.","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"}],"nova/tests/unit/compute/test_compute_api.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5b6e515820eef66f6a7874705b69d01623a9711b","unresolved":false,"context_lines":[{"line_number":5541,"context_line":"        self.assertEqual(mock_get_cm.call_count, 3)"},{"line_number":5542,"context_line":""},{"line_number":5543,"context_line":"    @mock.patch.object(objects.BuildRequestList, \u0027get_by_filters\u0027)"},{"line_number":5544,"context_line":"    @mock.patch.object(objects.CellMapping, \u0027get_by_uuid\u0027,"},{"line_number":5545,"context_line":"                       side_effect\u003dexception.CellMappingNotFound(uuid\u003d\u0027fake\u0027))"},{"line_number":5546,"context_line":"    @mock.patch.object(compute_api.API,"},{"line_number":5547,"context_line":"        \u0027_generate_minimal_construct_for_down_cells\u0027)"},{"line_number":5548,"context_line":"    def test_get_all_with_cell_down_support(self, mock_mc,"},{"line_number":5549,"context_line":"                                            mock_cell_mapping_get,"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_d40a4d25","line":5546,"range":{"start_line":5544,"start_character":3,"end_line":5546,"end_character":39},"updated":"2018-10-01 16:01:31.000000000","message":"Why?","commit_id":"1b1e784b94f89bff49ce4a4c1518043e5a1607cd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5b6e515820eef66f6a7874705b69d01623a9711b","unresolved":false,"context_lines":[{"line_number":5559,"context_line":"            mock_inst_get.return_value \u003d objects.InstanceList("},{"line_number":5560,"context_line":"                self.context, objects\u003dcell_instances), list()"},{"line_number":5561,"context_line":"            self.compute_api.get_all(self.context, cell_down_support\u003dTrue)"},{"line_number":5562,"context_line":"            mock_mc.assert_called_once_with(self.context, [])"},{"line_number":5563,"context_line":""},{"line_number":5564,"context_line":"    @mock.patch.object(objects.InstanceMapping, \u0027get_by_instance_uuid\u0027,"},{"line_number":5565,"context_line":"            side_effect\u003dexception.InstanceMappingNotFound(uuid\u003d\u0027fake\u0027))"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_7a02430a","line":5562,"range":{"start_line":5562,"start_character":12,"end_line":5562,"end_character":61},"updated":"2018-10-01 16:01:31.000000000","message":"This is asserting that we call this thing with no missing cells, but not that we merge the results, or that we actually did pass the missing cells right? I think this test should have at least one missing cell.","commit_id":"1b1e784b94f89bff49ce4a4c1518043e5a1607cd"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"2ee470c9634655770d9601719c13bb8e47d51933","unresolved":false,"context_lines":[{"line_number":5559,"context_line":"            mock_inst_get.return_value \u003d objects.InstanceList("},{"line_number":5560,"context_line":"                self.context, objects\u003dcell_instances), list()"},{"line_number":5561,"context_line":"            self.compute_api.get_all(self.context, cell_down_support\u003dTrue)"},{"line_number":5562,"context_line":"            mock_mc.assert_called_once_with(self.context, [])"},{"line_number":5563,"context_line":""},{"line_number":5564,"context_line":"    @mock.patch.object(objects.InstanceMapping, \u0027get_by_instance_uuid\u0027,"},{"line_number":5565,"context_line":"            side_effect\u003dexception.InstanceMappingNotFound(uuid\u003d\u0027fake\u0027))"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_fddf2d64","line":5562,"range":{"start_line":5562,"start_character":12,"end_line":5562,"end_character":61},"in_reply_to":"3f79a3b5_7a02430a","updated":"2018-10-01 16:38:09.000000000","message":"ack, I will test this also with a missing cell, yea should have added it here too,\n\nI will also add the assertion when cell_down_support is passed as False, as in that generate_minimal_construct_for_down_cells is not called.","commit_id":"1b1e784b94f89bff49ce4a4c1518043e5a1607cd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"81da63029cf839f6f37b8bf974e3e439893d9dd2","unresolved":false,"context_lines":[{"line_number":5605,"context_line":"            for i, instance in enumerate(partial_instances + full_instances):"},{"line_number":5606,"context_line":"                # comparing dicts because the same object is not being mocked"},{"line_number":5607,"context_line":"                # for the partial instance from down cell."},{"line_number":5608,"context_line":"                self.assertEqual(instance.__dict__, insts[i].__dict__)"},{"line_number":5609,"context_line":"            mock_get_ims.assert_called_once_with(self.context, uuids.cell1,"},{"line_number":5610,"context_line":"                self.context.project_id)"},{"line_number":5611,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_28e433a8","line":5608,"range":{"start_line":5608,"start_character":16,"end_line":5608,"end_character":70},"updated":"2018-10-04 18:15:29.000000000","message":"I don\u0027t know why, but I don\u0027t love this and it seems like it\u0027s likely to be fragile. Can we just compare a couple of the important bits?\n\nLike maybe this:\n\n def important_bits(inst):\n     return (inst.uuid, inst.status)\n\n for (things):\n     self.assertEqual(important_bits(instance), important_bits(insts[i]))\n\nJust MHO, maybe don\u0027t change this unless someone else agrees.","commit_id":"10fa28959d2fc8570e5278840235165482a07b11"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"1142a342dad7dbae9305218e05045dd9b0c34cd4","unresolved":false,"context_lines":[{"line_number":5605,"context_line":"            for i, instance in enumerate(partial_instances + full_instances):"},{"line_number":5606,"context_line":"                # comparing dicts because the same object is not being mocked"},{"line_number":5607,"context_line":"                # for the partial instance from down cell."},{"line_number":5608,"context_line":"                self.assertEqual(instance.__dict__, insts[i].__dict__)"},{"line_number":5609,"context_line":"            mock_get_ims.assert_called_once_with(self.context, uuids.cell1,"},{"line_number":5610,"context_line":"                self.context.project_id)"},{"line_number":5611,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_49de473d","line":5608,"range":{"start_line":5608,"start_character":16,"end_line":5608,"end_character":70},"in_reply_to":"3f79a3b5_28e433a8","updated":"2018-10-05 08:35:00.000000000","message":"yea I could do that too, I just thought of comparing the whole thing. okay, will wait and see if someones else also feels this is weird.","commit_id":"10fa28959d2fc8570e5278840235165482a07b11"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"40e8a2455b7e1e5a14fd750e54b9923282e3a277","unresolved":false,"context_lines":[{"line_number":5543,"context_line":"    def test_generate_minimal_construct_for_down_cells(self, mock_get_ims):"},{"line_number":5544,"context_line":"        im1 \u003d objects.InstanceMapping(instance_uuid\u003duuids.inst1, cell_id\u003d1,"},{"line_number":5545,"context_line":"            project_id\u003d\u0027fake\u0027, created_at\u003dNone, queued_for_delete\u003dFalse)"},{"line_number":5546,"context_line":"        mock_get_ims.return_value \u003d [im1]"},{"line_number":5547,"context_line":"        down_cell_uuids \u003d [uuids.cell1, uuids.cell2, uuids.cell3]"},{"line_number":5548,"context_line":"        result \u003d self.compute_api._generate_minimal_construct_for_down_cells("},{"line_number":5549,"context_line":"            self.context, down_cell_uuids)"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f79a3b5_06503c2c","line":5546,"range":{"start_line":5546,"start_character":36,"end_line":5546,"end_character":41},"updated":"2018-10-17 17:10:47.000000000","message":"nit: use an InstanceMappingList for the return value","commit_id":"b19a4065956076fef698dd313359e7aa0f58e969"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"40e8a2455b7e1e5a14fd750e54b9923282e3a277","unresolved":false,"context_lines":[{"line_number":5545,"context_line":"            project_id\u003d\u0027fake\u0027, created_at\u003dNone, queued_for_delete\u003dFalse)"},{"line_number":5546,"context_line":"        mock_get_ims.return_value \u003d [im1]"},{"line_number":5547,"context_line":"        down_cell_uuids \u003d [uuids.cell1, uuids.cell2, uuids.cell3]"},{"line_number":5548,"context_line":"        result \u003d self.compute_api._generate_minimal_construct_for_down_cells("},{"line_number":5549,"context_line":"            self.context, down_cell_uuids)"},{"line_number":5550,"context_line":"        for inst in result:"},{"line_number":5551,"context_line":"            self.assertEqual(inst.uuid, im1.instance_uuid)"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f79a3b5_26077821","line":5548,"range":{"start_line":5548,"start_character":8,"end_line":5548,"end_character":14},"updated":"2018-10-17 17:10:47.000000000","message":"Shouldn\u0027t the result size be 1? The test is returning an instance mapping for 3 down cells, but that\u0027s not something that would really happen. Should mock_get_ims use a side_effect to only return a result once out of 3 calls?","commit_id":"b19a4065956076fef698dd313359e7aa0f58e969"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"40e8a2455b7e1e5a14fd750e54b9923282e3a277","unresolved":false,"context_lines":[{"line_number":5552,"context_line":"            self.assertIn(\u0027created_at\u0027, inst)"},{"line_number":5553,"context_line":"            # minimal construct doesn\u0027t contain the usual keys"},{"line_number":5554,"context_line":"            self.assertNotIn(\u0027display_name\u0027, inst)"},{"line_number":5555,"context_line":"        self.assertEqual(mock_get_ims.call_count, 3)"},{"line_number":5556,"context_line":""},{"line_number":5557,"context_line":"    @mock.patch.object(objects.BuildRequestList, \u0027get_by_filters\u0027)"},{"line_number":5558,"context_line":"    @mock.patch.object(objects.InstanceMappingList,"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f79a3b5_e62740c8","line":5555,"range":{"start_line":5555,"start_character":8,"end_line":5555,"end_character":52},"updated":"2018-10-17 17:10:47.000000000","message":"Use mock_get_ims.assert_has_calls so we assert the actual call parameters.","commit_id":"b19a4065956076fef698dd313359e7aa0f58e969"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"40e8a2455b7e1e5a14fd750e54b9923282e3a277","unresolved":false,"context_lines":[{"line_number":5557,"context_line":"    @mock.patch.object(objects.BuildRequestList, \u0027get_by_filters\u0027)"},{"line_number":5558,"context_line":"    @mock.patch.object(objects.InstanceMappingList,"},{"line_number":5559,"context_line":"                       \u0027get_not_deleted_by_cell_and_project\u0027)"},{"line_number":5560,"context_line":"    def test_get_all_without_cell_down_support(self, mock_get_ims,"},{"line_number":5561,"context_line":"                                            mock_buildreq_get):"},{"line_number":5562,"context_line":"        mock_buildreq_get.return_value \u003d objects.BuildRequestList()"},{"line_number":5563,"context_line":"        im1 \u003d objects.InstanceMapping(instance_uuid\u003duuids.inst1, cell_id\u003d1,"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f79a3b5_a9211ba4","line":5560,"range":{"start_line":5560,"start_character":8,"end_line":5560,"end_character":46},"updated":"2018-10-17 17:10:47.000000000","message":"Do we need a new test for this? Couldn\u0027t we just mock out _generate_minimal_construct_for_down_cells on any other existing get_all test and assert it\u0027s not called?","commit_id":"b19a4065956076fef698dd313359e7aa0f58e969"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"40e8a2455b7e1e5a14fd750e54b9923282e3a277","unresolved":false,"context_lines":[{"line_number":5578,"context_line":"            mock_get_ims.assert_not_called()"},{"line_number":5579,"context_line":""},{"line_number":5580,"context_line":"    @mock.patch.object(objects.BuildRequestList, \u0027get_by_filters\u0027)"},{"line_number":5581,"context_line":"    @mock.patch.object(objects.InstanceMappingList,"},{"line_number":5582,"context_line":"                       \u0027get_not_deleted_by_cell_and_project\u0027)"},{"line_number":5583,"context_line":"    def test_get_all_with_cell_down_support(self, mock_get_ims,"},{"line_number":5584,"context_line":"                                            mock_buildreq_get):"},{"line_number":5585,"context_line":"        mock_buildreq_get.return_value \u003d objects.BuildRequestList()"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f79a3b5_691b036f","line":5582,"range":{"start_line":5581,"start_character":3,"end_line":5582,"end_character":61},"updated":"2018-10-17 17:10:47.000000000","message":"Just mock _generate_minimal_construct_for_down_cells - you don\u0027t need to mock the things that happen within _generate_minimal_construct_for_down_cells in this test, otherwise you\u0027re just duplicating work in the test.","commit_id":"b19a4065956076fef698dd313359e7aa0f58e969"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ea7b0b39890423500419028a4c5dd5c79198b53d","unresolved":false,"context_lines":[{"line_number":5637,"context_line":""},{"line_number":5638,"context_line":"    @mock.patch.object(objects.InstanceMappingList,"},{"line_number":5639,"context_line":"                       \u0027get_not_deleted_by_cell_and_projects\u0027)"},{"line_number":5640,"context_line":"    def test_generate_minimal_construct_for_down_cells(self, mock_get_ims):"},{"line_number":5641,"context_line":"        im1 \u003d objects.InstanceMapping(instance_uuid\u003duuids.inst1, cell_id\u003d1,"},{"line_number":5642,"context_line":"            project_id\u003d\u0027fake\u0027, created_at\u003dNone, queued_for_delete\u003dFalse)"},{"line_number":5643,"context_line":"        mock_get_ims.return_value \u003d [im1]"}],"source_content_type":"text/x-python","patch_set":27,"id":"3f79a3b5_4cc22756","line":5640,"range":{"start_line":5640,"start_character":8,"end_line":5640,"end_character":54},"updated":"2018-10-25 21:58:09.000000000","message":"I believe I said this before, but a lot of this is covered by the tests below, except those use a single down cell and this uses multiple. The tests below could just mock out _generate_minimal_construct_for_down_cells but meh.","commit_id":"1e1a552c830236ef23158b40fdac07e4a2cf6f7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ea7b0b39890423500419028a4c5dd5c79198b53d","unresolved":false,"context_lines":[{"line_number":5649,"context_line":"            self.assertIn(\u0027created_at\u0027, inst)"},{"line_number":5650,"context_line":"            # minimal construct doesn\u0027t contain the usual keys"},{"line_number":5651,"context_line":"            self.assertNotIn(\u0027display_name\u0027, inst)"},{"line_number":5652,"context_line":"        self.assertEqual(mock_get_ims.call_count, 3)"},{"line_number":5653,"context_line":""},{"line_number":5654,"context_line":"    @mock.patch.object(objects.BuildRequestList, \u0027get_by_filters\u0027)"},{"line_number":5655,"context_line":"    @mock.patch.object(objects.InstanceMappingList,"}],"source_content_type":"text/x-python","patch_set":27,"id":"3f79a3b5_ccd5d71b","line":5652,"range":{"start_line":5652,"start_character":25,"end_line":5652,"end_character":51},"updated":"2018-10-25 21:58:09.000000000","message":"nit: swap the args (expected, actual)","commit_id":"1e1a552c830236ef23158b40fdac07e4a2cf6f7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ea7b0b39890423500419028a4c5dd5c79198b53d","unresolved":false,"context_lines":[{"line_number":5655,"context_line":"    @mock.patch.object(objects.InstanceMappingList,"},{"line_number":5656,"context_line":"                       \u0027get_not_deleted_by_cell_and_projects\u0027)"},{"line_number":5657,"context_line":"    def test_get_all_without_cell_down_support(self, mock_get_ims,"},{"line_number":5658,"context_line":"                                            mock_buildreq_get):"},{"line_number":5659,"context_line":"        mock_buildreq_get.return_value \u003d objects.BuildRequestList()"},{"line_number":5660,"context_line":"        im1 \u003d objects.InstanceMapping(instance_uuid\u003duuids.inst1, cell_id\u003d1,"},{"line_number":5661,"context_line":"            project_id\u003d\u0027fake\u0027, created_at\u003dNone, queued_for_delete\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":27,"id":"3f79a3b5_8ce35f06","line":5658,"range":{"start_line":5658,"start_character":44,"end_line":5658,"end_character":61},"updated":"2018-10-25 21:58:09.000000000","message":"nit: alignment","commit_id":"1e1a552c830236ef23158b40fdac07e4a2cf6f7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ea7b0b39890423500419028a4c5dd5c79198b53d","unresolved":false,"context_lines":[{"line_number":5709,"context_line":"    @mock.patch.object(objects.BuildRequestList, \u0027get_by_filters\u0027)"},{"line_number":5710,"context_line":"    @mock.patch.object(objects.InstanceMappingList,"},{"line_number":5711,"context_line":"                       \u0027get_not_deleted_by_cell_and_projects\u0027)"},{"line_number":5712,"context_line":"    def test_get_all_with_cell_down_support_multi(self, mock_get_ims,"},{"line_number":5713,"context_line":"                                                  mock_buildreq_get):"},{"line_number":5714,"context_line":"        mock_buildreq_get.return_value \u003d objects.BuildRequestList()"},{"line_number":5715,"context_line":"        im \u003d objects.InstanceMapping(context\u003dself.context,"}],"source_content_type":"text/x-python","patch_set":27,"id":"3f79a3b5_6ce4c30b","line":5712,"range":{"start_line":5712,"start_character":43,"end_line":5712,"end_character":49},"updated":"2018-10-25 21:58:09.000000000","message":"multi what? would be nice of the tests had comments about what scenarios they were testing and how they are different because it\u0027s straining my eye balls to diff these. I guess multiple projects? 99% of this test could just be put into a common test method and then the two tests could pass in either a single project or a list of projects...","commit_id":"1e1a552c830236ef23158b40fdac07e4a2cf6f7b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4ec3dafa8dc2f9d56987b841f517cdd21e7d00dc","unresolved":false,"context_lines":[{"line_number":5717,"context_line":"                       \u0027get_not_deleted_by_cell_and_projects\u0027)"},{"line_number":5718,"context_line":"    def test_get_all_with_cell_down_support_multi(self, mock_get_ims,"},{"line_number":5719,"context_line":"                                                  mock_buildreq_get):"},{"line_number":5720,"context_line":"        search_opts \u003d {\u0027project_id\u0027: [\u0027proj1\u0027, \u0027proj2\u0027]}"},{"line_number":5721,"context_line":"        self.test_get_all_with_cell_down_support(opts\u003dsearch_opts)"},{"line_number":5722,"context_line":""},{"line_number":5723,"context_line":"    @mock.patch.object(objects.InstanceMapping, \u0027get_by_instance_uuid\u0027,"}],"source_content_type":"text/x-python","patch_set":28,"id":"3f79a3b5_d8b81be9","line":5720,"range":{"start_line":5720,"start_character":8,"end_line":5720,"end_character":56},"updated":"2018-10-26 19:33:58.000000000","message":"Based on my earlier comments and looking at the API code, sine we don\u0027t use req.GET.getall(\u0027project_id\u0027) in the controller code, I don\u0027t think we can actually ever get here for this. IOW, I think we only support filtering on a single project_id, which either comes from the request context or is a filter from an admin if all_tenants\u003dtrue and project_id is specified.","commit_id":"e4b5e39597b943ba6dbc015dcadbb4a85cdf86be"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"df3fce4d1149a2273740cd64ccda3d9d40042162","unresolved":false,"context_lines":[{"line_number":5717,"context_line":"                       \u0027get_not_deleted_by_cell_and_projects\u0027)"},{"line_number":5718,"context_line":"    def test_get_all_with_cell_down_support_multi(self, mock_get_ims,"},{"line_number":5719,"context_line":"                                                  mock_buildreq_get):"},{"line_number":5720,"context_line":"        search_opts \u003d {\u0027project_id\u0027: [\u0027proj1\u0027, \u0027proj2\u0027]}"},{"line_number":5721,"context_line":"        self.test_get_all_with_cell_down_support(opts\u003dsearch_opts)"},{"line_number":5722,"context_line":""},{"line_number":5723,"context_line":"    @mock.patch.object(objects.InstanceMapping, \u0027get_by_instance_uuid\u0027,"}],"source_content_type":"text/x-python","patch_set":28,"id":"3f79a3b5_217677d7","line":5720,"range":{"start_line":5720,"start_character":8,"end_line":5720,"end_character":56},"in_reply_to":"3f79a3b5_d8b81be9","updated":"2018-10-31 13:28:25.000000000","message":"I confirmed in a functional test that with two servers available by different projects, if we filter on both project_ids we still only get one of the servers back, so multiple project_ids for filtering is not supported:\n\ndiff --git a/nova/tests/functional/test_aggregates.py b/nova/tests/functional/test_aggregates.py\nindex 1fd84ba..2cdfc22 100644\n--- a/nova/tests/functional/test_aggregates.py\n+++ b/nova/tests/functional/test_aggregates.py\n@@ -456,3 +456,13 @@ class TestAggregateMultiTenancyIsolationFilter(\n             server \u003d user_api.post_server(server_req)\n         self._wait_for_state_change(user_api, server, \u0027ACTIVE\u0027)\n         self.assertEqual(2, len(self.filtered_hosts))\n+        # As the admin, list servers for all tenants.\n+        servers \u003d self.admin_api.get_servers(search_opts\u003d{\u0027all_tenants\u0027: True})\n+        self.assertEqual(2, len(servers))\n+        # As the admin, try to list servers for both projects specifically. We\n+        # should only get one back since project_id is not repeatable.\n+        self.assertNotEqual(self.admin_api.project_id, user_api.project_id)\n+        servers \u003d self.admin_api.api_get(\n+            \u0027/servers?all_tenants\u003d1\u0026project_id\u003d%s\u0026project_id\u003d%s\u0027 %\n+            (self.admin_api.project_id, user_api.project_id)).body[\u0027servers\u0027]\n+        self.assertEqual(1, len(servers))","commit_id":"e4b5e39597b943ba6dbc015dcadbb4a85cdf86be"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"409e10cf1a100d2b673cc481f9c169f6dedc3de7","unresolved":false,"context_lines":[{"line_number":5717,"context_line":"                       \u0027get_not_deleted_by_cell_and_projects\u0027)"},{"line_number":5718,"context_line":"    def test_get_all_with_cell_down_support_multi(self, mock_get_ims,"},{"line_number":5719,"context_line":"                                                  mock_buildreq_get):"},{"line_number":5720,"context_line":"        search_opts \u003d {\u0027project_id\u0027: [\u0027proj1\u0027, \u0027proj2\u0027]}"},{"line_number":5721,"context_line":"        self.test_get_all_with_cell_down_support(opts\u003dsearch_opts)"},{"line_number":5722,"context_line":""},{"line_number":5723,"context_line":"    @mock.patch.object(objects.InstanceMapping, \u0027get_by_instance_uuid\u0027,"}],"source_content_type":"text/x-python","patch_set":28,"id":"3f79a3b5_80118c5f","line":5720,"range":{"start_line":5720,"start_character":8,"end_line":5720,"end_character":56},"in_reply_to":"3f79a3b5_d8b81be9","updated":"2018-10-31 12:02:53.000000000","message":"ack, I also don\u0027t see a scenario where we could have multiple project id\u0027s coming in from the request.","commit_id":"e4b5e39597b943ba6dbc015dcadbb4a85cdf86be"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5cdcbcef137c1dfc28ac621eb31e4913ed0d9436","unresolved":false,"context_lines":[{"line_number":5643,"context_line":"        mock_get_ims.return_value \u003d [im1]"},{"line_number":5644,"context_line":"        down_cell_uuids \u003d [uuids.cell1, uuids.cell2, uuids.cell3]"},{"line_number":5645,"context_line":"        result \u003d self.compute_api._generate_minimal_construct_for_down_cells("},{"line_number":5646,"context_line":"            self.context, down_cell_uuids, [self.context.project_id], None)"},{"line_number":5647,"context_line":"        for inst in result:"},{"line_number":5648,"context_line":"            self.assertEqual(inst.uuid, im1.instance_uuid)"},{"line_number":5649,"context_line":"            self.assertIn(\u0027created_at\u0027, inst)"}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_850f9b2d","line":5646,"range":{"start_line":5646,"start_character":70,"end_line":5646,"end_character":74},"updated":"2018-10-26 21:32:15.000000000","message":"Note to self: this is limit None.","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5cdcbcef137c1dfc28ac621eb31e4913ed0d9436","unresolved":false,"context_lines":[{"line_number":5691,"context_line":"        with mock.patch(\u0027nova.compute.instance_list.\u0027"},{"line_number":5692,"context_line":"                        \u0027get_instance_objects_sorted\u0027) as mock_inst_get:"},{"line_number":5693,"context_line":"            mock_inst_get.return_value \u003d objects.InstanceList("},{"line_number":5694,"context_line":"                self.context, objects\u003dcell_instances), [uuids.cell1]"},{"line_number":5695,"context_line":"            insts \u003d self.compute_api.get_all(self.context,"},{"line_number":5696,"context_line":"                cell_down_support\u003dFalse)"},{"line_number":5697,"context_line":"            fields \u003d [\u0027metadata\u0027, \u0027info_cache\u0027, \u0027security_groups\u0027]"}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_455c2319","line":5694,"range":{"start_line":5694,"start_character":56,"end_line":5694,"end_character":67},"updated":"2018-10-26 21:32:15.000000000","message":"Note to self: this is a down cell uuid.","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5cdcbcef137c1dfc28ac621eb31e4913ed0d9436","unresolved":false,"context_lines":[{"line_number":5692,"context_line":"                        \u0027get_instance_objects_sorted\u0027) as mock_inst_get:"},{"line_number":5693,"context_line":"            mock_inst_get.return_value \u003d objects.InstanceList("},{"line_number":5694,"context_line":"                self.context, objects\u003dcell_instances), [uuids.cell1]"},{"line_number":5695,"context_line":"            insts \u003d self.compute_api.get_all(self.context,"},{"line_number":5696,"context_line":"                cell_down_support\u003dFalse)"},{"line_number":5697,"context_line":"            fields \u003d [\u0027metadata\u0027, \u0027info_cache\u0027, \u0027security_groups\u0027]"},{"line_number":5698,"context_line":"            mock_inst_get.assert_called_once_with(self.context, {}, None, None,"}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_2576c7a4","line":5695,"range":{"start_line":5695,"start_character":37,"end_line":5695,"end_character":44},"updated":"2018-10-26 21:32:15.000000000","message":"Note to self: the previous two tests _with_ down cell support called _generate_minimal_construct_for_down_cells directly. And bypass using the cell_down_support kwarg to control it.","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5cdcbcef137c1dfc28ac621eb31e4913ed0d9436","unresolved":false,"context_lines":[{"line_number":5699,"context_line":"                fields, None, None)"},{"line_number":5700,"context_line":"            for i, instance in enumerate(cell_instances):"},{"line_number":5701,"context_line":"                self.assertEqual(instance, insts[i])"},{"line_number":5702,"context_line":"            mock_get_ims.assert_not_called()"},{"line_number":5703,"context_line":""},{"line_number":5704,"context_line":"    @mock.patch.object(objects.BuildRequestList, \u0027get_by_filters\u0027)"},{"line_number":5705,"context_line":"    @mock.patch.object(objects.InstanceMappingList,"}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_65acbfea","line":5702,"range":{"start_line":5702,"start_character":12,"end_line":5702,"end_character":44},"updated":"2018-10-26 21:32:15.000000000","message":"I found this assert very confusing, for some reason, because it was more indirect. I think this would make a lot more sense if it were asserting _generate_minimal_construct_for_down_cells was not called.","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"409e10cf1a100d2b673cc481f9c169f6dedc3de7","unresolved":false,"context_lines":[{"line_number":5699,"context_line":"                fields, None, None)"},{"line_number":5700,"context_line":"            for i, instance in enumerate(cell_instances):"},{"line_number":5701,"context_line":"                self.assertEqual(instance, insts[i])"},{"line_number":5702,"context_line":"            mock_get_ims.assert_not_called()"},{"line_number":5703,"context_line":""},{"line_number":5704,"context_line":"    @mock.patch.object(objects.BuildRequestList, \u0027get_by_filters\u0027)"},{"line_number":5705,"context_line":"    @mock.patch.object(objects.InstanceMappingList,"}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_03f6e35f","line":5702,"range":{"start_line":5702,"start_character":12,"end_line":5702,"end_character":44},"in_reply_to":"3f79a3b5_65acbfea","updated":"2018-10-31 12:02:53.000000000","message":"well, the only reason for asserting get_not_deleted_by_cell_and_projects instead of generate_minimal_construct is because we get to go down further into the generate_minimal_construct function instead of having to mock it.","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5cdcbcef137c1dfc28ac621eb31e4913ed0d9436","unresolved":false,"context_lines":[{"line_number":5731,"context_line":"            for i, instance in enumerate(partial_instances + full_instances):"},{"line_number":5732,"context_line":"                # comparing dicts because the same object is not being mocked"},{"line_number":5733,"context_line":"                # for the partial instance from down cell."},{"line_number":5734,"context_line":"                self.assertEqual(instance.__dict__, insts[i].__dict__)"},{"line_number":5735,"context_line":"            if opts:"},{"line_number":5736,"context_line":"                projects \u003d opts[\u0027project_id\u0027]"},{"line_number":5737,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_05d06b2a","line":5734,"range":{"start_line":5734,"start_character":21,"end_line":5734,"end_character":32},"updated":"2018-10-26 21:32:15.000000000","message":"I\u0027m pretty sure we have helper methods for comparing objects. Maybe obj_equal_prims from nova/objects/base.py?","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"409e10cf1a100d2b673cc481f9c169f6dedc3de7","unresolved":false,"context_lines":[{"line_number":5731,"context_line":"            for i, instance in enumerate(partial_instances + full_instances):"},{"line_number":5732,"context_line":"                # comparing dicts because the same object is not being mocked"},{"line_number":5733,"context_line":"                # for the partial instance from down cell."},{"line_number":5734,"context_line":"                self.assertEqual(instance.__dict__, insts[i].__dict__)"},{"line_number":5735,"context_line":"            if opts:"},{"line_number":5736,"context_line":"                projects \u003d opts[\u0027project_id\u0027]"},{"line_number":5737,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_634b17b2","line":5734,"range":{"start_line":5734,"start_character":21,"end_line":5734,"end_character":32},"in_reply_to":"3f79a3b5_05d06b2a","updated":"2018-10-31 12:02:53.000000000","message":"Done","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5cdcbcef137c1dfc28ac621eb31e4913ed0d9436","unresolved":false,"context_lines":[{"line_number":5736,"context_line":"                projects \u003d opts[\u0027project_id\u0027]"},{"line_number":5737,"context_line":"            else:"},{"line_number":5738,"context_line":"                projects \u003d [self.context.project_id]"},{"line_number":5739,"context_line":"            mock_get_ims.assert_called_once_with(self.context, uuids.cell1,"},{"line_number":5740,"context_line":"                                                 projects, limit\u003dNone)"},{"line_number":5741,"context_line":""},{"line_number":5742,"context_line":"    @mock.patch.object(objects.BuildRequestList, \u0027get_by_filters\u0027)"}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_45e983e1","line":5739,"range":{"start_line":5739,"start_character":12,"end_line":5739,"end_character":24},"updated":"2018-10-26 21:32:15.000000000","message":"Similar comment as earlier, seems like this would be easier to parse if it mocked _generate_minimal_construct_for_down_cells instead.","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7f168b471de1a85c4a5c516d1d968785cb6329a0","unresolved":false,"context_lines":[{"line_number":5736,"context_line":"                projects \u003d opts[\u0027project_id\u0027]"},{"line_number":5737,"context_line":"            else:"},{"line_number":5738,"context_line":"                projects \u003d [self.context.project_id]"},{"line_number":5739,"context_line":"            mock_get_ims.assert_called_once_with(self.context, uuids.cell1,"},{"line_number":5740,"context_line":"                                                 projects, limit\u003dNone)"},{"line_number":5741,"context_line":""},{"line_number":5742,"context_line":"    @mock.patch.object(objects.BuildRequestList, \u0027get_by_filters\u0027)"}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_c706ac75","line":5739,"range":{"start_line":5739,"start_character":12,"end_line":5739,"end_character":24},"in_reply_to":"3f79a3b5_45e983e1","updated":"2018-10-31 18:51:22.000000000","message":"Yup, I\u0027ve said the same thing before. We have unit tests for _generate_minimal_construct_for_down_cells specifically and then we have unit tests for get_all which could mock _generate_minimal_construct_for_down_cells but instead they continue to mock the things *within* _generate_minimal_construct_for_down_cells which is redundant.","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7f168b471de1a85c4a5c516d1d968785cb6329a0","unresolved":false,"context_lines":[{"line_number":5706,"context_line":"                       \u0027get_not_deleted_by_cell_and_project\u0027)"},{"line_number":5707,"context_line":"    def test_get_all_with_cell_down_support(self, mock_get_ims,"},{"line_number":5708,"context_line":"                                            mock_buildreq_get,"},{"line_number":5709,"context_line":"                                            opts\u003dNone):"},{"line_number":5710,"context_line":"        mock_buildreq_get.return_value \u003d objects.BuildRequestList()"},{"line_number":5711,"context_line":"        im \u003d objects.InstanceMapping(context\u003dself.context,"},{"line_number":5712,"context_line":"            instance_uuid\u003duuids.inst1, cell_id\u003d1,"}],"source_content_type":"text/x-python","patch_set":30,"id":"3f79a3b5_a7629040","line":5709,"range":{"start_line":5709,"start_character":44,"end_line":5709,"end_character":53},"updated":"2018-10-31 18:51:22.000000000","message":"If we drop the test below, we can drop this kwarg.","commit_id":"76cd5b53bde1f7bfa82042ab6ac8e860b793d15d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4b50b95007c7363019aa0259c3de563361319b99","unresolved":false,"context_lines":[{"line_number":5706,"context_line":"                       \u0027get_not_deleted_by_cell_and_project\u0027)"},{"line_number":5707,"context_line":"    def test_get_all_with_cell_down_support(self, mock_get_ims,"},{"line_number":5708,"context_line":"                                            mock_buildreq_get,"},{"line_number":5709,"context_line":"                                            opts\u003dNone):"},{"line_number":5710,"context_line":"        mock_buildreq_get.return_value \u003d objects.BuildRequestList()"},{"line_number":5711,"context_line":"        im \u003d objects.InstanceMapping(context\u003dself.context,"},{"line_number":5712,"context_line":"            instance_uuid\u003duuids.inst1, cell_id\u003d1,"}],"source_content_type":"text/x-python","patch_set":30,"id":"3f79a3b5_d23988a2","line":5709,"range":{"start_line":5709,"start_character":44,"end_line":5709,"end_character":53},"in_reply_to":"3f79a3b5_a7629040","updated":"2018-10-31 19:11:31.000000000","message":"Done","commit_id":"76cd5b53bde1f7bfa82042ab6ac8e860b793d15d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7f168b471de1a85c4a5c516d1d968785cb6329a0","unresolved":false,"context_lines":[{"line_number":5740,"context_line":"    @mock.patch.object(objects.BuildRequestList, \u0027get_by_filters\u0027)"},{"line_number":5741,"context_line":"    @mock.patch.object(objects.InstanceMappingList,"},{"line_number":5742,"context_line":"                       \u0027get_not_deleted_by_cell_and_project\u0027)"},{"line_number":5743,"context_line":"    def test_get_all_with_cell_down_support_single_pid(self, mock_get_ims,"},{"line_number":5744,"context_line":"                                                       mock_buildreq_get):"},{"line_number":5745,"context_line":"        search_opts \u003d {\u0027project_id\u0027: \u0027proj1\u0027}"},{"line_number":5746,"context_line":"        self.test_get_all_with_cell_down_support(opts\u003dsearch_opts)"}],"source_content_type":"text/x-python","patch_set":30,"id":"3f79a3b5_e78088c1","line":5743,"range":{"start_line":5743,"start_character":8,"end_line":5743,"end_character":54},"updated":"2018-10-31 18:51:22.000000000","message":"We don\u0027t really need this test anymore since it was just originally handling multiple projects, which we no longer have in the code change, so remove this test.","commit_id":"76cd5b53bde1f7bfa82042ab6ac8e860b793d15d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4b50b95007c7363019aa0259c3de563361319b99","unresolved":false,"context_lines":[{"line_number":5740,"context_line":"    @mock.patch.object(objects.BuildRequestList, \u0027get_by_filters\u0027)"},{"line_number":5741,"context_line":"    @mock.patch.object(objects.InstanceMappingList,"},{"line_number":5742,"context_line":"                       \u0027get_not_deleted_by_cell_and_project\u0027)"},{"line_number":5743,"context_line":"    def test_get_all_with_cell_down_support_single_pid(self, mock_get_ims,"},{"line_number":5744,"context_line":"                                                       mock_buildreq_get):"},{"line_number":5745,"context_line":"        search_opts \u003d {\u0027project_id\u0027: \u0027proj1\u0027}"},{"line_number":5746,"context_line":"        self.test_get_all_with_cell_down_support(opts\u003dsearch_opts)"}],"source_content_type":"text/x-python","patch_set":30,"id":"3f79a3b5_f23e0489","line":5743,"range":{"start_line":5743,"start_character":8,"end_line":5743,"end_character":54},"in_reply_to":"3f79a3b5_e78088c1","updated":"2018-10-31 19:11:31.000000000","message":"Done","commit_id":"76cd5b53bde1f7bfa82042ab6ac8e860b793d15d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7f168b471de1a85c4a5c516d1d968785cb6329a0","unresolved":false,"context_lines":[{"line_number":6882,"context_line":"    def test_get_all_without_cell_down_support(self):"},{"line_number":6883,"context_line":"        self.skipTest(\"Cell down handling is not supported for cells_v1.\")"},{"line_number":6884,"context_line":""},{"line_number":6885,"context_line":"    def test_get_all_with_cell_down_support_single_pid(self):"},{"line_number":6886,"context_line":"        self.skipTest(\"Cell down handling is not supported for cells_v1.\")"},{"line_number":6887,"context_line":""},{"line_number":6888,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"3f79a3b5_075b4489","line":6885,"range":{"start_line":6885,"start_character":8,"end_line":6885,"end_character":54},"updated":"2018-10-31 18:51:22.000000000","message":"drop this as well","commit_id":"76cd5b53bde1f7bfa82042ab6ac8e860b793d15d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4b50b95007c7363019aa0259c3de563361319b99","unresolved":false,"context_lines":[{"line_number":6882,"context_line":"    def test_get_all_without_cell_down_support(self):"},{"line_number":6883,"context_line":"        self.skipTest(\"Cell down handling is not supported for cells_v1.\")"},{"line_number":6884,"context_line":""},{"line_number":6885,"context_line":"    def test_get_all_with_cell_down_support_single_pid(self):"},{"line_number":6886,"context_line":"        self.skipTest(\"Cell down handling is not supported for cells_v1.\")"},{"line_number":6887,"context_line":""},{"line_number":6888,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"3f79a3b5_b2408c06","line":6885,"range":{"start_line":6885,"start_character":8,"end_line":6885,"end_character":54},"in_reply_to":"3f79a3b5_075b4489","updated":"2018-10-31 19:11:31.000000000","message":"Done","commit_id":"76cd5b53bde1f7bfa82042ab6ac8e860b793d15d"}],"nova/tests/unit/compute/test_instance_list.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5cdcbcef137c1dfc28ac621eb31e4913ed0d9436","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        insts_by_cell \u003d self.insts.values()"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"        mock_inst.side_effect \u003d insts_by_cell"},{"line_number":72,"context_line":"        obj, insts \u003d instance_list.get_instances_sorted(self.context, {},"},{"line_number":73,"context_line":"                                                        None, None, [],"},{"line_number":74,"context_line":"                                                        [\u0027hostname\u0027], [\u0027asc\u0027])"},{"line_number":75,"context_line":"        insts_one \u003d [inst[\u0027hostname\u0027] for inst in insts]"}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_a5c73765","line":72,"range":{"start_line":72,"start_character":8,"end_line":72,"end_character":11},"updated":"2018-10-26 21:32:15.000000000","message":"Note to self: this is the InstanceLister","commit_id":"b9af166b2650cd748def8e0770f38eb444005a79"}]}