)]}'
{"nova/api/openstack/compute/views/servers.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f81f73b1f572bff9ba7556646ec43a3ae217c150","unresolved":false,"context_lines":[{"line_number":221,"context_line":"                                                   servers,"},{"line_number":222,"context_line":"                                                   coll_name)"},{"line_number":223,"context_line":"        servers_dict \u003d dict(servers\u003dserver_list,"},{"line_number":224,"context_line":"                            unavailable_servers\u003dunavailable_servers_list)"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"        if servers_links:"},{"line_number":227,"context_line":"            servers_dict[\"servers_links\"] \u003d servers_links"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_b956fc29","line":224,"range":{"start_line":224,"start_character":28,"end_line":224,"end_character":47},"updated":"2018-06-19 17:47:11.000000000","message":"This is a pretty substantial change in the structure of the result. I don\u0027t see it called out in the spec, but I would expect this would be undesirable for a lot of people. All existing clients wouldn\u0027t be looking for this and thus would have to change how they generate their lists, and have to merge the two lists (in sort order) to keep the same behavior.\n\nI was thinking we\u0027d just include them in the list, as in-order as we can get them. Is that not the plan specifically?","commit_id":"521d2e50200a42a2eb3184fa95f6b456d38118f6"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"88eb1ac6648712f0fd36ecdb64afda6c0bad516b","unresolved":false,"context_lines":[{"line_number":221,"context_line":"                                                   servers,"},{"line_number":222,"context_line":"                                                   coll_name)"},{"line_number":223,"context_line":"        servers_dict \u003d dict(servers\u003dserver_list,"},{"line_number":224,"context_line":"                            unavailable_servers\u003dunavailable_servers_list)"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"        if servers_links:"},{"line_number":227,"context_line":"            servers_dict[\"servers_links\"] \u003d servers_links"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_cffd176f","line":224,"range":{"start_line":224,"start_character":28,"end_line":224,"end_character":47},"in_reply_to":"5f7c97a3_b956fc29","updated":"2018-06-20 09:33:18.000000000","message":"\u003e This is a pretty substantial change in the structure of the result.\n \u003e I don\u0027t see it called out in the spec, but I would expect this\n \u003e would be undesirable for a lot of people. All existing clients\n \u003e wouldn\u0027t be looking for this and thus would have to change how they\n \u003e generate their lists, and have to merge the two lists (in sort\n \u003e order) to keep the same behavior.\n\nright, this was what was proposed in the spec initially and this was also my first POC, but then we changed to go for a new API response and versioned bump.\n\n \u003e \n \u003e I was thinking we\u0027d just include them in the list, as in-order as\n \u003e we can get them. Is that not the plan specifically?\n\nWe are exploring the alternative option in the spec which is to add this new top level key.","commit_id":"521d2e50200a42a2eb3184fa95f6b456d38118f6"}],"nova/compute/api.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f81f73b1f572bff9ba7556646ec43a3ae217c150","unresolved":false,"context_lines":[{"line_number":2530,"context_line":"        if filter_ip:"},{"line_number":2531,"context_line":"            instances \u003d self._ip_filter(instances, filters, orig_limit)"},{"line_number":2532,"context_line":""},{"line_number":2533,"context_line":"        return instances, unavail_insts"},{"line_number":2534,"context_line":""},{"line_number":2535,"context_line":"    def _do_old_style_instance_list_for_poor_cellsv1_users(self,"},{"line_number":2536,"context_line":"                                                           context, filters,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_3938acec","line":2533,"updated":"2018-06-19 17:47:11.000000000","message":"Per my previous comment, I had assumed by this level we would have merged the lists so that the upper layers are just iterating over instance-like things.","commit_id":"521d2e50200a42a2eb3184fa95f6b456d38118f6"}],"nova/compute/instance_list.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f81f73b1f572bff9ba7556646ec43a3ae217c150","unresolved":false,"context_lines":[{"line_number":123,"context_line":"        expected_attrs \u003d copy.copy(expected_attrs)"},{"line_number":124,"context_line":"        expected_attrs.remove(\u0027fault\u0027)"},{"line_number":125,"context_line":"    return (instance_obj._make_instance_list(ctx, objects.InstanceList(),"},{"line_number":126,"context_line":"                instance_generator, expected_attrs), unavailable_instances)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_f921d4b6","line":126,"updated":"2018-06-19 17:47:11.000000000","message":"I think we need to do the merging in this layer so that we can get the instance shells sorted into the list based on the fields that we know or the fields that we\u0027re faking at the moment.","commit_id":"521d2e50200a42a2eb3184fa95f6b456d38118f6"}],"nova/compute/multi_cell_list.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"5e5d2f551fedec8f2a60f0df006acf30194dae05","unresolved":false,"context_lines":[{"line_number":270,"context_line":"                            \u0027power_state\u0027: 0,"},{"line_number":271,"context_line":"                            \u0027image_ref\u0027: req_spec.image.id,"},{"line_number":272,"context_line":"                            \u0027created_at\u0027: (im.created_at).replace(tzinfo\u003dNone),"},{"line_number":273,"context_line":"                            \u0027id\u0027: im.instance_uuid,"},{"line_number":274,"context_line":"                            \u0027availability_zone\u0027: req_spec.availability_zone,"},{"line_number":275,"context_line":"                            \u0027project_id\u0027: im.project_id,"},{"line_number":276,"context_line":"                        })"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_29039e78","line":273,"updated":"2018-06-18 14:44:20.000000000","message":"I would be happy with a single instance uuid but I know others might want to have a skeleton instance with as much info as we can gather.","commit_id":"521d2e50200a42a2eb3184fa95f6b456d38118f6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"df4ace76915432da323ed92b81ebb9362eab7772","unresolved":false,"context_lines":[{"line_number":270,"context_line":"                            \u0027power_state\u0027: 0,"},{"line_number":271,"context_line":"                            \u0027image_ref\u0027: req_spec.image.id,"},{"line_number":272,"context_line":"                            \u0027created_at\u0027: (im.created_at).replace(tzinfo\u003dNone),"},{"line_number":273,"context_line":"                            \u0027id\u0027: im.instance_uuid,"},{"line_number":274,"context_line":"                            \u0027availability_zone\u0027: req_spec.availability_zone,"},{"line_number":275,"context_line":"                            \u0027project_id\u0027: im.project_id,"},{"line_number":276,"context_line":"                        })"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_da374afe","line":273,"in_reply_to":"5f7c97a3_29039e78","updated":"2018-06-18 16:24:31.000000000","message":"I\u0027d be OK with a single instance uuid too, just a list of instance uuids for the ones that we couldn\u0027t get as an indication those aren\u0027t available. If you wanted to get details about a specific instance, you could get it by doing GET /servers/{server_id} and we would try to populate as much of it as possible using the request spec, but not while listing instances.\n\nI also forget what I thought about this when filtering/paging, but I thought we said we wouldn\u0027t return instances in down cells in the case of using filters since we don\u0027t know if they\u0027d be in that list or not. One could argue that returning unavailable instances when filtering is an indication that the results might not be complete, but meh, I\u0027m OK with not returning UUIDs from down cells if we\u0027re filtering. If I\u0027m filtering, it\u0027s because I want to know about things that are actually applicable to those filters so I can do something with them, not a list of stuff on which I can\u0027t do anything.","commit_id":"521d2e50200a42a2eb3184fa95f6b456d38118f6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"29c00b136607e6dd1d8dbf7ec9349fd5bd7ffb99","unresolved":false,"context_lines":[{"line_number":270,"context_line":"                            \u0027power_state\u0027: 0,"},{"line_number":271,"context_line":"                            \u0027image_ref\u0027: req_spec.image.id,"},{"line_number":272,"context_line":"                            \u0027created_at\u0027: (im.created_at).replace(tzinfo\u003dNone),"},{"line_number":273,"context_line":"                            \u0027id\u0027: im.instance_uuid,"},{"line_number":274,"context_line":"                            \u0027availability_zone\u0027: req_spec.availability_zone,"},{"line_number":275,"context_line":"                            \u0027project_id\u0027: im.project_id,"},{"line_number":276,"context_line":"                        })"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_7125eaad","line":273,"in_reply_to":"5f7c97a3_43f2729d","updated":"2018-06-19 08:42:05.000000000","message":"sound good to me.","commit_id":"521d2e50200a42a2eb3184fa95f6b456d38118f6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f81f73b1f572bff9ba7556646ec43a3ae217c150","unresolved":false,"context_lines":[{"line_number":270,"context_line":"                            \u0027power_state\u0027: 0,"},{"line_number":271,"context_line":"                            \u0027image_ref\u0027: req_spec.image.id,"},{"line_number":272,"context_line":"                            \u0027created_at\u0027: (im.created_at).replace(tzinfo\u003dNone),"},{"line_number":273,"context_line":"                            \u0027id\u0027: im.instance_uuid,"},{"line_number":274,"context_line":"                            \u0027availability_zone\u0027: req_spec.availability_zone,"},{"line_number":275,"context_line":"                            \u0027project_id\u0027: im.project_id,"},{"line_number":276,"context_line":"                        })"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_61298a87","line":273,"in_reply_to":"5f7c97a3_7125eaad","updated":"2018-06-19 17:47:11.000000000","message":"Well, what about sorting? I think that at least instance uuid and a state that indicates we don\u0027t know what the state is would be useful.","commit_id":"521d2e50200a42a2eb3184fa95f6b456d38118f6"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"eec38fdfb01d06e9e7aeb3af9d21eb064d22fd0b","unresolved":false,"context_lines":[{"line_number":270,"context_line":"                            \u0027power_state\u0027: 0,"},{"line_number":271,"context_line":"                            \u0027image_ref\u0027: req_spec.image.id,"},{"line_number":272,"context_line":"                            \u0027created_at\u0027: (im.created_at).replace(tzinfo\u003dNone),"},{"line_number":273,"context_line":"                            \u0027id\u0027: im.instance_uuid,"},{"line_number":274,"context_line":"                            \u0027availability_zone\u0027: req_spec.availability_zone,"},{"line_number":275,"context_line":"                            \u0027project_id\u0027: im.project_id,"},{"line_number":276,"context_line":"                        })"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_43f2729d","line":273,"in_reply_to":"5f7c97a3_da374afe","updated":"2018-06-18 19:34:54.000000000","message":"\u003e I\u0027d be OK with a single instance uuid too, just a list of instance\n \u003e uuids for the ones that we couldn\u0027t get as an indication those\n \u003e aren\u0027t available. If you wanted to get details about a specific\n \u003e instance, you could get it by doing GET /servers/{server_id} and we\n \u003e would try to populate as much of it as possible using the request\n \u003e spec, but not while listing instances.\n \u003e \n\numm, I thought we said we were sticking to 503 when doing nova show on an instance in a down cell ? but either ways considering dansmith and mriedem don\u0027t trust the req_spec to be up-to-date, I don\u0027t mind returning just the uuids. Anyways its not like we are returning everything, we might as well just return the uuids. The justification I had was \"let\u0027s return whatever we know..\" but then yea something (uuid) is better than nothing.\n\n \u003e I also forget what I thought about this when filtering/paging, but\n \u003e I thought we said we wouldn\u0027t return instances in down cells in the\n \u003e case of using filters since we don\u0027t know if they\u0027d be in that list\n \u003e or not. One could argue that returning unavailable instances when\n \u003e filtering is an indication that the results might not be complete,\n \u003e but meh, I\u0027m OK with not returning UUIDs from down cells if we\u0027re\n \u003e filtering. If I\u0027m filtering, it\u0027s because I want to know about\n \u003e things that are actually applicable to those filters so I can do\n \u003e something with them, not a list of stuff on which I can\u0027t do\n \u003e anything.\n\nagreed, I will check to skip the whole part if any filters were passed.","commit_id":"521d2e50200a42a2eb3184fa95f6b456d38118f6"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"f7c6e60dbc49834c0f76bd8dc29f9103755498e1","unresolved":false,"context_lines":[{"line_number":275,"context_line":"                            \u0027project_id\u0027: im.project_id,"},{"line_number":276,"context_line":"                        })"},{"line_number":277,"context_line":"                results.pop(k)"},{"line_number":278,"context_line":"        # undoubtedly ugly yielding/ not sure if the above logic of querying"},{"line_number":279,"context_line":"        # for uuids from instance_mapping table should be done here or higher"},{"line_number":280,"context_line":"        # up in the stack. If done higher up, not sure how to connect the"},{"line_number":281,"context_line":"        # \"is the cell down? and if yes, which cell\" part to querying from"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_0cba7ebf","line":278,"updated":"2018-06-17 19:50:11.000000000","message":"open for suggestions, if there was an alternative approach which is what was meant in the spec.","commit_id":"521d2e50200a42a2eb3184fa95f6b456d38118f6"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"eec38fdfb01d06e9e7aeb3af9d21eb064d22fd0b","unresolved":false,"context_lines":[{"line_number":275,"context_line":"                            \u0027project_id\u0027: im.project_id,"},{"line_number":276,"context_line":"                        })"},{"line_number":277,"context_line":"                results.pop(k)"},{"line_number":278,"context_line":"        # undoubtedly ugly yielding/ not sure if the above logic of querying"},{"line_number":279,"context_line":"        # for uuids from instance_mapping table should be done here or higher"},{"line_number":280,"context_line":"        # up in the stack. If done higher up, not sure how to connect the"},{"line_number":281,"context_line":"        # \"is the cell down? and if yes, which cell\" part to querying from"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_43fcb23e","line":278,"in_reply_to":"5f7c97a3_04800946","updated":"2018-06-18 19:34:54.000000000","message":"\u003e so the get_records_sorted() need to be a generator of instance\n \u003e records. I think that breaking the interface by yielding a list\n \u003e below is bad.\n \u003e \n \u003e Another (also bad) option:\n \u003e \n \u003e We cannot just let heapsort try to order our skeleton instance and\n \u003e it will fail due to some missing keys. But we could extend\n \u003e RecordWrapper to handled the missing key in __lt__ and push our\n \u003e skeleton instance records to the end (or to the beginning of the\n \u003e sorted list). \n\nyea,, I just didn\u0027t want to touch the workflow of existing stuff as much as possible.\n\n\u003e Then we yield them one by one as normal instances.\n \u003e Then on the client side of get_records_sorted() we filter the\n \u003e skeletons out by checking for missing fields and move them to the\n \u003e separate API response field.\n \u003e \n \u003e Thoughts?\n\nI guess we can wait to see dansmith\u0027s awesome thoughts and meanwhile I will start with the version bumps and make this look better.","commit_id":"521d2e50200a42a2eb3184fa95f6b456d38118f6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"df4ace76915432da323ed92b81ebb9362eab7772","unresolved":false,"context_lines":[{"line_number":275,"context_line":"                            \u0027project_id\u0027: im.project_id,"},{"line_number":276,"context_line":"                        })"},{"line_number":277,"context_line":"                results.pop(k)"},{"line_number":278,"context_line":"        # undoubtedly ugly yielding/ not sure if the above logic of querying"},{"line_number":279,"context_line":"        # for uuids from instance_mapping table should be done here or higher"},{"line_number":280,"context_line":"        # up in the stack. If done higher up, not sure how to connect the"},{"line_number":281,"context_line":"        # \"is the cell down? and if yes, which cell\" part to querying from"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_fa15664a","line":278,"in_reply_to":"5f7c97a3_04800946","updated":"2018-06-18 16:24:31.000000000","message":"Would need to get input from dansmith on this. There might be other options for putting unavailable things into a different bucket or handled via a different method in the multi-cell lister framework so we don\u0027t have to try and shoe-horn it into this method?","commit_id":"521d2e50200a42a2eb3184fa95f6b456d38118f6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"5e5d2f551fedec8f2a60f0df006acf30194dae05","unresolved":false,"context_lines":[{"line_number":275,"context_line":"                            \u0027project_id\u0027: im.project_id,"},{"line_number":276,"context_line":"                        })"},{"line_number":277,"context_line":"                results.pop(k)"},{"line_number":278,"context_line":"        # undoubtedly ugly yielding/ not sure if the above logic of querying"},{"line_number":279,"context_line":"        # for uuids from instance_mapping table should be done here or higher"},{"line_number":280,"context_line":"        # up in the stack. If done higher up, not sure how to connect the"},{"line_number":281,"context_line":"        # \"is the cell down? and if yes, which cell\" part to querying from"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_04800946","line":278,"in_reply_to":"5f7c97a3_0cba7ebf","updated":"2018-06-18 14:44:20.000000000","message":"so the get_records_sorted() need to be a generator of instance records. I think that breaking the interface by yielding a list below is bad. \n\nAnother (also bad) option:\n\nWe cannot just let heapsort try to order our skeleton instance and it will fail due to some missing keys. But we could extend RecordWrapper to handled the missing key in __lt__ and push our skeleton instance records to the end (or to the beginning of the sorted list). Then we yield them one by one as normal instances. Then on the client side of get_records_sorted() we filter the skeletons out by checking for missing fields and move them to the separate API response field.\n\nThoughts?","commit_id":"521d2e50200a42a2eb3184fa95f6b456d38118f6"}]}