)]}'
{"nova/db/sqlalchemy/api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c1e0185fe0568130220ff883c426360c4cb64aa9","unresolved":false,"context_lines":[{"line_number":2932,"context_line":"def key_pair_get_all_by_user(context, user_id, limit\u003dNone, marker\u003dNone):"},{"line_number":2933,"context_line":"    marker_row \u003d None"},{"line_number":2934,"context_line":"    if marker is not None:"},{"line_number":2935,"context_line":"        marker_row \u003d model_query(context, models.KeyPair, read_deleted\u003d\"no\").\\"},{"line_number":2936,"context_line":"            filter_by(name\u003dmarker).first()"},{"line_number":2937,"context_line":"        if not marker_row:"},{"line_number":2938,"context_line":"            raise exception.MarkerNotFound(marker)"},{"line_number":2939,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_a048ffb3","line":2936,"range":{"start_line":2935,"start_character":8,"end_line":2936,"end_character":42},"updated":"2016-07-07 14:22:45.000000000","message":"Umm, this is also broken. Since names are only unique within the context of the user_id, you could get multiple matches for the name here but across separate user_ids. So this would also need to filter by user_id since we\u0027re filtering on that below when paging.","commit_id":"5e58c8b8032fcd9d3f0b540e79ebd3605a543099"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"078efd764c70b2603c29b976f8694b47265b9b5c","unresolved":false,"context_lines":[{"line_number":2932,"context_line":"def key_pair_get_all_by_user(context, user_id, limit\u003dNone, marker\u003dNone):"},{"line_number":2933,"context_line":"    marker_row \u003d None"},{"line_number":2934,"context_line":"    if marker is not None:"},{"line_number":2935,"context_line":"        marker_row \u003d model_query(context, models.KeyPair, read_deleted\u003d\"no\").\\"},{"line_number":2936,"context_line":"            filter_by(name\u003dmarker).first()"},{"line_number":2937,"context_line":"        if not marker_row:"},{"line_number":2938,"context_line":"            raise exception.MarkerNotFound(marker)"},{"line_number":2939,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_d9c1c2e5","line":2936,"range":{"start_line":2935,"start_character":8,"end_line":2936,"end_character":42},"in_reply_to":"1aa78d24_a048ffb3","updated":"2016-07-07 15:03:47.000000000","message":"Good catch. I thought model_query filtered by context.project_id by default but apparently that is opt in with the project_only argument.","commit_id":"5e58c8b8032fcd9d3f0b540e79ebd3605a543099"},{"author":{"_account_id":10618,"name":"Pavel Kholkin","email":"p.v.holkin@mail.ru","username":"pkholkin"},"change_message_id":"8a59e24fc920eff0edc985390e259462b7dfdce0","unresolved":false,"context_lines":[{"line_number":2932,"context_line":"def key_pair_get_all_by_user(context, user_id, limit\u003dNone, marker\u003dNone):"},{"line_number":2933,"context_line":"    marker_row \u003d None"},{"line_number":2934,"context_line":"    if marker is not None:"},{"line_number":2935,"context_line":"        marker_row \u003d model_query(context, models.KeyPair, read_deleted\u003d\"no\").\\"},{"line_number":2936,"context_line":"            filter_by(name\u003dmarker).first()"},{"line_number":2937,"context_line":"        if not marker_row:"},{"line_number":2938,"context_line":"            raise exception.MarkerNotFound(marker)"},{"line_number":2939,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_f2774c06","line":2936,"range":{"start_line":2935,"start_character":8,"end_line":2936,"end_character":42},"in_reply_to":"1aa78d24_d9c1c2e5","updated":"2016-07-08 09:25:36.000000000","message":"Done","commit_id":"5e58c8b8032fcd9d3f0b540e79ebd3605a543099"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d99267bc898198f980f98c177ba5c9e2336ae339","unresolved":false,"context_lines":[{"line_number":2941,"context_line":"        filter_by(user_id\u003duser_id)"},{"line_number":2942,"context_line":""},{"line_number":2943,"context_line":"    query \u003d sqlalchemyutils.paginate_query("},{"line_number":2944,"context_line":"        query, models.KeyPair, limit, [\u0027name\u0027, \u0027id\u0027], marker\u003dmarker_row)"},{"line_number":2945,"context_line":""},{"line_number":2946,"context_line":"    return query.all()"},{"line_number":2947,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_c0da6b16","line":2944,"range":{"start_line":2944,"start_character":47,"end_line":2944,"end_character":51},"updated":"2016-07-07 14:21:48.000000000","message":"This doesn\u0027t make sense to me. There is a unique constraint in this table on user_id and name, and we\u0027re already filtering on user_id, so the names have to be unique, so there is no point in further ordering by id.","commit_id":"5e58c8b8032fcd9d3f0b540e79ebd3605a543099"},{"author":{"_account_id":10618,"name":"Pavel Kholkin","email":"p.v.holkin@mail.ru","username":"pkholkin"},"change_message_id":"8a59e24fc920eff0edc985390e259462b7dfdce0","unresolved":false,"context_lines":[{"line_number":2941,"context_line":"        filter_by(user_id\u003duser_id)"},{"line_number":2942,"context_line":""},{"line_number":2943,"context_line":"    query \u003d sqlalchemyutils.paginate_query("},{"line_number":2944,"context_line":"        query, models.KeyPair, limit, [\u0027name\u0027, \u0027id\u0027], marker\u003dmarker_row)"},{"line_number":2945,"context_line":""},{"line_number":2946,"context_line":"    return query.all()"},{"line_number":2947,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_d27cd0ed","line":2944,"range":{"start_line":2944,"start_character":47,"end_line":2944,"end_character":51},"in_reply_to":"1aa78d24_c0da6b16","updated":"2016-07-08 09:25:36.000000000","message":"Done","commit_id":"5e58c8b8032fcd9d3f0b540e79ebd3605a543099"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b2c58330da59ceb2e65139ad2cb06f4114ec97a5","unresolved":false,"context_lines":[{"line_number":2935,"context_line":"        marker_row \u003d model_query(context, models.KeyPair, read_deleted\u003d\"no\").\\"},{"line_number":2936,"context_line":"            filter_by(name\u003dmarker).filter_by(user_id\u003duser_id).first()"},{"line_number":2937,"context_line":"        if not marker_row:"},{"line_number":2938,"context_line":"            raise exception.MarkerNotFound(marker)"},{"line_number":2939,"context_line":""},{"line_number":2940,"context_line":"    query \u003d model_query(context, models.KeyPair, read_deleted\u003d\"no\").\\"},{"line_number":2941,"context_line":"        filter_by(user_id\u003duser_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1aa78d24_1b1bb081","line":2938,"range":{"start_line":2938,"start_character":43,"end_line":2938,"end_character":49},"updated":"2016-07-11 15:54:55.000000000","message":"This should actually be marker\u003dmarker, otherwise this overwrites the exception message so you just have something like this:\n\n\u003e\u003e\u003e from nova import exception\n\u003e\u003e\u003e raise exception.MarkerNotFound(\u0027test\u0027)\nTraceback (most recent call last):\n  File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\nnova.exception.MarkerNotFound: test\n\n\nRather than:\n\n\u003e\u003e\u003e raise exception.MarkerNotFound(marker\u003d\u0027test\u0027)\nTraceback (most recent call last):\n  File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\nnova.exception.MarkerNotFound: Marker test could not be found.\n\u003e\u003e\u003e \n\n\n--\n\nBut I see the same thing happening in here elsewhere so we could clean that up in a separate patch.","commit_id":"7a7ff15a5f751a3475fcc103d27782d2360210bc"},{"author":{"_account_id":10618,"name":"Pavel Kholkin","email":"p.v.holkin@mail.ru","username":"pkholkin"},"change_message_id":"e0a2b079eb908c559929b5a1cba6371e3db2d92a","unresolved":false,"context_lines":[{"line_number":2935,"context_line":"        marker_row \u003d model_query(context, models.KeyPair, read_deleted\u003d\"no\").\\"},{"line_number":2936,"context_line":"            filter_by(name\u003dmarker).filter_by(user_id\u003duser_id).first()"},{"line_number":2937,"context_line":"        if not marker_row:"},{"line_number":2938,"context_line":"            raise exception.MarkerNotFound(marker)"},{"line_number":2939,"context_line":""},{"line_number":2940,"context_line":"    query \u003d model_query(context, models.KeyPair, read_deleted\u003d\"no\").\\"},{"line_number":2941,"context_line":"        filter_by(user_id\u003duser_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1aa78d24_8b9c446b","line":2938,"range":{"start_line":2938,"start_character":43,"end_line":2938,"end_character":49},"in_reply_to":"1aa78d24_1b1bb081","updated":"2016-07-12 11:26:06.000000000","message":"understand, decided to do it beautiful here now","commit_id":"7a7ff15a5f751a3475fcc103d27782d2360210bc"}],"nova/objects/keypair.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"68c94b5091b80e1f5d6fb442743aaaab927989ca","unresolved":false,"context_lines":[{"line_number":35,"context_line":"@db_api.api_context_manager.reader"},{"line_number":36,"context_line":"def _get_from_db(context, user_id, name\u003dNone, limit\u003dNone, marker\u003dNone):"},{"line_number":37,"context_line":"    query \u003d context.session.query(api_models.KeyPair).\\"},{"line_number":38,"context_line":"        filter(api_models.KeyPair.user_id \u003d\u003d user_id)"},{"line_number":39,"context_line":"    if name is not None:"},{"line_number":40,"context_line":"        db_keypair \u003d query.filter(api_models.KeyPair.name \u003d\u003d name).\\"},{"line_number":41,"context_line":"                     first()"}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_0815ff47","line":38,"updated":"2016-07-06 13:45:03.000000000","message":"This is just random whitespace damage as far as I can tell. Please fix.","commit_id":"b4ecaff4b282463b0731b4386a85669c8e1fd0e4"},{"author":{"_account_id":10618,"name":"Pavel Kholkin","email":"p.v.holkin@mail.ru","username":"pkholkin"},"change_message_id":"ef76fdbb927cf054f48d07911ad6a3bc14f19902","unresolved":false,"context_lines":[{"line_number":35,"context_line":"@db_api.api_context_manager.reader"},{"line_number":36,"context_line":"def _get_from_db(context, user_id, name\u003dNone, limit\u003dNone, marker\u003dNone):"},{"line_number":37,"context_line":"    query \u003d context.session.query(api_models.KeyPair).\\"},{"line_number":38,"context_line":"        filter(api_models.KeyPair.user_id \u003d\u003d user_id)"},{"line_number":39,"context_line":"    if name is not None:"},{"line_number":40,"context_line":"        db_keypair \u003d query.filter(api_models.KeyPair.name \u003d\u003d name).\\"},{"line_number":41,"context_line":"                     first()"}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_6ba8f877","line":38,"in_reply_to":"1aa78d24_0815ff47","updated":"2016-07-06 15:05:00.000000000","message":"Done","commit_id":"b4ecaff4b282463b0731b4386a85669c8e1fd0e4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"68c94b5091b80e1f5d6fb442743aaaab927989ca","unresolved":false,"context_lines":[{"line_number":51,"context_line":"            raise exception.MarkerNotFound(marker)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    query \u003d sqlalchemyutils.paginate_query("},{"line_number":54,"context_line":"        query, api_models.KeyPair, limit, [\u0027name\u0027, \u0027id\u0027], marker\u003dmarker_row)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    return query.all()"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_333f8c88","line":54,"range":{"start_line":54,"start_character":42,"end_line":54,"end_character":56},"updated":"2016-07-06 13:45:03.000000000","message":"This may have already been discussed, but sorting by name first means that the sort order isn\u0027t as stable as if you sort by id. Paginated queries are unreliable by default, but it\u0027s very easy to affect another user\u0027s paginated request if we sort by a field that is controlled by the user. Was name,id specifically chosen for a reason?","commit_id":"b4ecaff4b282463b0731b4386a85669c8e1fd0e4"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"bfd63180352b82050cfc0b38b0e09b8c5d3add25","unresolved":false,"context_lines":[{"line_number":51,"context_line":"            raise exception.MarkerNotFound(marker)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    query \u003d sqlalchemyutils.paginate_query("},{"line_number":54,"context_line":"        query, api_models.KeyPair, limit, [\u0027name\u0027, \u0027id\u0027], marker\u003dmarker_row)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    return query.all()"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_348132c4","line":54,"range":{"start_line":54,"start_character":42,"end_line":54,"end_character":56},"in_reply_to":"1aa78d24_333f8c88","updated":"2016-07-06 14:39:03.000000000","message":"My guess is that name was chosen in order to account for keypairs being split between the cell and api db where duplicate ids would be possible. I know there was some discussion about this previously but I don\u0027t know where that happened I just remember hearing about decisions being made.\n\nIf keypairs were going to be split into the cell dbs then sorting by name would probably be the better choice. But since they\u0027re going to be in one db using id would be fine. It may lead to some irregularity during the migration though.","commit_id":"b4ecaff4b282463b0731b4386a85669c8e1fd0e4"},{"author":{"_account_id":10618,"name":"Pavel Kholkin","email":"p.v.holkin@mail.ru","username":"pkholkin"},"change_message_id":"ef76fdbb927cf054f48d07911ad6a3bc14f19902","unresolved":false,"context_lines":[{"line_number":51,"context_line":"            raise exception.MarkerNotFound(marker)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    query \u003d sqlalchemyutils.paginate_query("},{"line_number":54,"context_line":"        query, api_models.KeyPair, limit, [\u0027name\u0027, \u0027id\u0027], marker\u003dmarker_row)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    return query.all()"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_6b4478b6","line":54,"range":{"start_line":54,"start_character":42,"end_line":54,"end_character":56},"in_reply_to":"1aa78d24_348132c4","updated":"2016-07-06 15:05:00.000000000","message":"found only this related https://review.openstack.org/#/c/284868/9/nova/db/sqlalchemy/api.py@2918","commit_id":"b4ecaff4b282463b0731b4386a85669c8e1fd0e4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"68c94b5091b80e1f5d6fb442743aaaab927989ca","unresolved":false,"context_lines":[{"line_number":191,"context_line":"    #              KeyPair \u003c\u003d version 1.1"},{"line_number":192,"context_line":"    # Version 1.1: KeyPair \u003c\u003d version 1.2"},{"line_number":193,"context_line":"    # Version 1.2: KeyPair \u003c\u003d version 1.3"},{"line_number":194,"context_line":"    # Version 1.3: KeyPair \u003c\u003d version 1.4"},{"line_number":195,"context_line":"    VERSION \u003d \u00271.3\u0027"},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_a875cbfa","line":194,"updated":"2016-07-06 13:45:03.000000000","message":"You just copied and pasted this from above, but it\u0027s not right. The reason for 1.3 is the new parameters for get_by_user().","commit_id":"b4ecaff4b282463b0731b4386a85669c8e1fd0e4"},{"author":{"_account_id":10618,"name":"Pavel Kholkin","email":"p.v.holkin@mail.ru","username":"pkholkin"},"change_message_id":"ef76fdbb927cf054f48d07911ad6a3bc14f19902","unresolved":false,"context_lines":[{"line_number":191,"context_line":"    #              KeyPair \u003c\u003d version 1.1"},{"line_number":192,"context_line":"    # Version 1.1: KeyPair \u003c\u003d version 1.2"},{"line_number":193,"context_line":"    # Version 1.2: KeyPair \u003c\u003d version 1.3"},{"line_number":194,"context_line":"    # Version 1.3: KeyPair \u003c\u003d version 1.4"},{"line_number":195,"context_line":"    VERSION \u003d \u00271.3\u0027"},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_2ba27099","line":194,"in_reply_to":"1aa78d24_a875cbfa","updated":"2016-07-06 15:05:00.000000000","message":"yes, sorry","commit_id":"b4ecaff4b282463b0731b4386a85669c8e1fd0e4"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"37c16cbe1e2eef58b00287e5e7a8e305019d4e64","unresolved":false,"context_lines":[{"line_number":46,"context_line":"    marker_row \u003d None"},{"line_number":47,"context_line":"    if marker is not None:"},{"line_number":48,"context_line":"        marker_row \u003d context.session.query(api_models.KeyPair).\\"},{"line_number":49,"context_line":"            filter(api_models.KeyPair.name \u003d\u003d marker).first()"},{"line_number":50,"context_line":"        if not marker_row:"},{"line_number":51,"context_line":"            raise exception.MarkerNotFound(marker)"},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1aa78d24_fa8e622c","line":49,"updated":"2016-07-06 19:03:54.000000000","message":"Can you explain why name was chosen for the marker? There was some discussion around this and their is concern that name doesn\u0027t make a good marker, due to the sorting not being stable. And some agreement that id might be better here.","commit_id":"d1bb5815d38a164a09dfa1908a0d40a1fe353e9e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"03d5c2c6613c66ccb574b69d67838dc0cdd1d6a7","unresolved":false,"context_lines":[{"line_number":46,"context_line":"    marker_row \u003d None"},{"line_number":47,"context_line":"    if marker is not None:"},{"line_number":48,"context_line":"        marker_row \u003d context.session.query(api_models.KeyPair).\\"},{"line_number":49,"context_line":"            filter(api_models.KeyPair.name \u003d\u003d marker).first()"},{"line_number":50,"context_line":"        if not marker_row:"},{"line_number":51,"context_line":"            raise exception.MarkerNotFound(marker)"},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1aa78d24_cb99f17a","line":49,"in_reply_to":"1aa78d24_9398c56b","updated":"2016-07-07 13:33:30.000000000","message":"But markers should be opaque to the user, IMHO. I would prefer ID here for the marker and sort key, but I\u0027ll defer to the others if they want to keep name.","commit_id":"d1bb5815d38a164a09dfa1908a0d40a1fe353e9e"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"f90fe5592677a8f3d6b5affe64d63ae767cac446","unresolved":false,"context_lines":[{"line_number":46,"context_line":"    marker_row \u003d None"},{"line_number":47,"context_line":"    if marker is not None:"},{"line_number":48,"context_line":"        marker_row \u003d context.session.query(api_models.KeyPair).\\"},{"line_number":49,"context_line":"            filter(api_models.KeyPair.name \u003d\u003d marker).first()"},{"line_number":50,"context_line":"        if not marker_row:"},{"line_number":51,"context_line":"            raise exception.MarkerNotFound(marker)"},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1aa78d24_e6bd0403","line":49,"in_reply_to":"1aa78d24_9398c56b","updated":"2016-07-07 13:46:54.000000000","message":"I agree that it looks nicer if the marker is a nice name, but the marker should be opaque to the user. The API returns a marker and the user uses it. They really shouldn\u0027t just pick a random keypair name and use it as a marker, because we could change what the marker is at some point.","commit_id":"d1bb5815d38a164a09dfa1908a0d40a1fe353e9e"},{"author":{"_account_id":10618,"name":"Pavel Kholkin","email":"p.v.holkin@mail.ru","username":"pkholkin"},"change_message_id":"e42ebbc2ac1b1b14f96e9077f757fb1ea14e1e35","unresolved":false,"context_lines":[{"line_number":46,"context_line":"    marker_row \u003d None"},{"line_number":47,"context_line":"    if marker is not None:"},{"line_number":48,"context_line":"        marker_row \u003d context.session.query(api_models.KeyPair).\\"},{"line_number":49,"context_line":"            filter(api_models.KeyPair.name \u003d\u003d marker).first()"},{"line_number":50,"context_line":"        if not marker_row:"},{"line_number":51,"context_line":"            raise exception.MarkerNotFound(marker)"},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1aa78d24_9398c56b","line":49,"in_reply_to":"1aa78d24_9843c26b","updated":"2016-07-07 12:12:59.000000000","message":"and also, if I was a user of this api, I would like to use beautiful names (not ids), smth like \"give me 5 keypairs after \u0027my_keypair\u0027 \" :)","commit_id":"d1bb5815d38a164a09dfa1908a0d40a1fe353e9e"},{"author":{"_account_id":10618,"name":"Pavel Kholkin","email":"p.v.holkin@mail.ru","username":"pkholkin"},"change_message_id":"1d1fbe69b4d460ff6faa8c4e17c073e1931881f0","unresolved":false,"context_lines":[{"line_number":46,"context_line":"    marker_row \u003d None"},{"line_number":47,"context_line":"    if marker is not None:"},{"line_number":48,"context_line":"        marker_row \u003d context.session.query(api_models.KeyPair).\\"},{"line_number":49,"context_line":"            filter(api_models.KeyPair.name \u003d\u003d marker).first()"},{"line_number":50,"context_line":"        if not marker_row:"},{"line_number":51,"context_line":"            raise exception.MarkerNotFound(marker)"},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1aa78d24_9843c26b","line":49,"in_reply_to":"1aa78d24_fa8e622c","updated":"2016-07-07 12:01:30.000000000","message":"Andrew, name was chosen for marker, because in current Nova API user is working with kp names (not ids).\n\nhttp://developer.openstack.org/api-ref-compute-v2.1.html#listKeypairs\n\nDeleting or getting keypairs are done using name (keypair ID could be got only after GET request, but names are in other requests too)\n\nAs for paging, marker should be unique and name is ok with it. \nAs for unstable, this is the way of implementation in DB that IDs are auto incrementing in our case (we could have the same problems if it isn\u0027t so). Also if e.g. NOT for keypairs we will use UUIDs (not IDs) somewhere we, will have the same problems with response.","commit_id":"d1bb5815d38a164a09dfa1908a0d40a1fe353e9e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"03d5c2c6613c66ccb574b69d67838dc0cdd1d6a7","unresolved":false,"context_lines":[{"line_number":191,"context_line":"    #              KeyPair \u003c\u003d version 1.1"},{"line_number":192,"context_line":"    # Version 1.1: KeyPair \u003c\u003d version 1.2"},{"line_number":193,"context_line":"    # Version 1.2: KeyPair \u003c\u003d version 1.3"},{"line_number":194,"context_line":"    # Version 1.3: KeyPair \u003c\u003d new parameters \u0027limit\u0027 and \u0027marker\u0027 were"},{"line_number":195,"context_line":"    #              added to \u0027get_by_user()\u0027 method"},{"line_number":196,"context_line":"    VERSION \u003d \u00271.3\u0027"},{"line_number":197,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1aa78d24_7e3274f3","line":194,"range":{"start_line":194,"start_character":19,"end_line":194,"end_character":29},"updated":"2016-07-07 13:33:30.000000000","message":"Eh? This should be removed. The whole comment should be:\n\n Add new parameters \u0027limit\u0027 and \u0027marker\u0027 to get_by_user()","commit_id":"d1bb5815d38a164a09dfa1908a0d40a1fe353e9e"},{"author":{"_account_id":10618,"name":"Pavel Kholkin","email":"p.v.holkin@mail.ru","username":"pkholkin"},"change_message_id":"5b583a90229bb5a0ca7b100adb8d7df64ee8acf3","unresolved":false,"context_lines":[{"line_number":191,"context_line":"    #              KeyPair \u003c\u003d version 1.1"},{"line_number":192,"context_line":"    # Version 1.1: KeyPair \u003c\u003d version 1.2"},{"line_number":193,"context_line":"    # Version 1.2: KeyPair \u003c\u003d version 1.3"},{"line_number":194,"context_line":"    # Version 1.3: KeyPair \u003c\u003d new parameters \u0027limit\u0027 and \u0027marker\u0027 were"},{"line_number":195,"context_line":"    #              added to \u0027get_by_user()\u0027 method"},{"line_number":196,"context_line":"    VERSION \u003d \u00271.3\u0027"},{"line_number":197,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1aa78d24_e02b2780","line":194,"range":{"start_line":194,"start_character":19,"end_line":194,"end_character":29},"in_reply_to":"1aa78d24_7e3274f3","updated":"2016-07-07 14:15:15.000000000","message":"ops) done","commit_id":"d1bb5815d38a164a09dfa1908a0d40a1fe353e9e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dc509b4d37fac810ce3d6785af2f75ec5aaef67c","unresolved":false,"context_lines":[{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    marker_row \u003d None"},{"line_number":47,"context_line":"    if marker is not None:"},{"line_number":48,"context_line":"        marker_row \u003d context.session.query(api_models.KeyPair).\\"},{"line_number":49,"context_line":"            filter(api_models.KeyPair.name \u003d\u003d marker).first()"},{"line_number":50,"context_line":"        if not marker_row:"},{"line_number":51,"context_line":"            raise exception.MarkerNotFound(marker)"},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_c968cff7","line":49,"range":{"start_line":48,"start_character":8,"end_line":49,"end_character":61},"updated":"2016-07-07 16:06:15.000000000","message":"Same comment as in the cell DB API, this has to filter on user_id.","commit_id":"5e58c8b8032fcd9d3f0b540e79ebd3605a543099"},{"author":{"_account_id":10618,"name":"Pavel Kholkin","email":"p.v.holkin@mail.ru","username":"pkholkin"},"change_message_id":"8a59e24fc920eff0edc985390e259462b7dfdce0","unresolved":false,"context_lines":[{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    marker_row \u003d None"},{"line_number":47,"context_line":"    if marker is not None:"},{"line_number":48,"context_line":"        marker_row \u003d context.session.query(api_models.KeyPair).\\"},{"line_number":49,"context_line":"            filter(api_models.KeyPair.name \u003d\u003d marker).first()"},{"line_number":50,"context_line":"        if not marker_row:"},{"line_number":51,"context_line":"            raise exception.MarkerNotFound(marker)"},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_921c08c3","line":49,"range":{"start_line":48,"start_character":8,"end_line":49,"end_character":61},"in_reply_to":"1aa78d24_c968cff7","updated":"2016-07-08 09:25:36.000000000","message":"Done","commit_id":"5e58c8b8032fcd9d3f0b540e79ebd3605a543099"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b2c58330da59ceb2e65139ad2cb06f4114ec97a5","unresolved":false,"context_lines":[{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    marker_row \u003d None"},{"line_number":47,"context_line":"    if marker is not None:"},{"line_number":48,"context_line":"        marker_row \u003d context.session.query(api_models.KeyPair).\\"},{"line_number":49,"context_line":"            filter(api_models.KeyPair.name \u003d\u003d marker).\\"},{"line_number":50,"context_line":"            filter(api_models.KeyPair.user_id \u003d\u003d user_id).first()"},{"line_number":51,"context_line":"        if not marker_row:"},{"line_number":52,"context_line":"            raise exception.MarkerNotFound(marker)"},{"line_number":53,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1aa78d24_36b683dc","line":50,"range":{"start_line":48,"start_character":8,"end_line":50,"end_character":65},"updated":"2016-07-11 15:54:55.000000000","message":"Since I only caught this in review, it would be good to have a test where we have 2 keypairs with the same name but for different user_ids, and make sure that when we ask for the keypairs for the one user_id we don\u0027t get the other keypair back, e.g.:\n\nkeypair1 - userid 1, name\u003dfoo\nkeypair2 - userid 2, name\u003dfoo\n\nQuery for keypair by userid 2 and name foo, assert the kp we get back is for userid 2 and not userid 1.","commit_id":"7a7ff15a5f751a3475fcc103d27782d2360210bc"},{"author":{"_account_id":10618,"name":"Pavel Kholkin","email":"p.v.holkin@mail.ru","username":"pkholkin"},"change_message_id":"e0a2b079eb908c559929b5a1cba6371e3db2d92a","unresolved":false,"context_lines":[{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    marker_row \u003d None"},{"line_number":47,"context_line":"    if marker is not None:"},{"line_number":48,"context_line":"        marker_row \u003d context.session.query(api_models.KeyPair).\\"},{"line_number":49,"context_line":"            filter(api_models.KeyPair.name \u003d\u003d marker).\\"},{"line_number":50,"context_line":"            filter(api_models.KeyPair.user_id \u003d\u003d user_id).first()"},{"line_number":51,"context_line":"        if not marker_row:"},{"line_number":52,"context_line":"            raise exception.MarkerNotFound(marker)"},{"line_number":53,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1aa78d24_9e1da075","line":50,"range":{"start_line":48,"start_character":8,"end_line":50,"end_character":65},"in_reply_to":"1aa78d24_36b683dc","updated":"2016-07-12 11:26:06.000000000","message":"added in \u0027functional.db.test_keypair\u0027","commit_id":"7a7ff15a5f751a3475fcc103d27782d2360210bc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b2c58330da59ceb2e65139ad2cb06f4114ec97a5","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            filter(api_models.KeyPair.name \u003d\u003d marker).\\"},{"line_number":50,"context_line":"            filter(api_models.KeyPair.user_id \u003d\u003d user_id).first()"},{"line_number":51,"context_line":"        if not marker_row:"},{"line_number":52,"context_line":"            raise exception.MarkerNotFound(marker)"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    query \u003d sqlalchemyutils.paginate_query("},{"line_number":55,"context_line":"        query, api_models.KeyPair, limit, [\u0027name\u0027], marker\u003dmarker_row)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1aa78d24_364de372","line":52,"updated":"2016-07-11 15:54:55.000000000","message":"ditto, should be marker\u003dmarker, but we can fix this separately later.","commit_id":"7a7ff15a5f751a3475fcc103d27782d2360210bc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b2c58330da59ceb2e65139ad2cb06f4114ec97a5","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        }"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    @staticmethod"},{"line_number":203,"context_line":"    def _get_from_db(context, user_id, limit, marker):"},{"line_number":204,"context_line":"        return _get_from_db(context, user_id, limit\u003dlimit, marker\u003dmarker)"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":4,"id":"1aa78d24_96aad7d0","line":203,"range":{"start_line":203,"start_character":39,"end_line":203,"end_character":52},"updated":"2016-07-11 15:54:55.000000000","message":"Default these to None?","commit_id":"7a7ff15a5f751a3475fcc103d27782d2360210bc"},{"author":{"_account_id":10618,"name":"Pavel Kholkin","email":"p.v.holkin@mail.ru","username":"pkholkin"},"change_message_id":"e0a2b079eb908c559929b5a1cba6371e3db2d92a","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        }"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    @staticmethod"},{"line_number":203,"context_line":"    def _get_from_db(context, user_id, limit, marker):"},{"line_number":204,"context_line":"        return _get_from_db(context, user_id, limit\u003dlimit, marker\u003dmarker)"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":4,"id":"1aa78d24_0b6a14ed","line":203,"range":{"start_line":203,"start_character":39,"end_line":203,"end_character":52},"in_reply_to":"1aa78d24_96aad7d0","updated":"2016-07-12 11:26:06.000000000","message":"Here I decided to do them not optional.\n\n\u0027def _get_from_db(context, user_id, name\u003dNone, limit\u003dNone, marker\u003dNone)\u0027 method is called in two different cases: (line 36)\n\n1) name\u003dsmth, no limit and no marker\n2) limit\u003dsmth, marker\u003dsmth, no name\n\nin the first case it finds kp by name\nin the second case it finds all with paging\n\nAs you can see \u0027def _get_from_db(context, user_id, name)\u0027 method also contains not optional param \u0027name\u0027 (line 128). \n\nSo we can see two _get_from_db methods.","commit_id":"7a7ff15a5f751a3475fcc103d27782d2360210bc"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"c4f0a603f93410c15690fef59ed20730204fe873","unresolved":false,"context_lines":[{"line_number":225,"context_line":"            limit_more \u003d None"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"        if limit_more is None or limit_more \u003e 0:"},{"line_number":228,"context_line":"            main_db_keypairs \u003d db.key_pair_get_all_by_user("},{"line_number":229,"context_line":"                context, user_id, limit\u003dlimit_more, marker\u003dmarker)"},{"line_number":230,"context_line":"        else:"},{"line_number":231,"context_line":"            main_db_keypairs \u003d []"},{"line_number":232,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1aa78d24_0cf87cc2","line":229,"range":{"start_line":228,"start_character":12,"end_line":229,"end_character":66},"updated":"2016-07-12 06:02:45.000000000","message":"After re-read this code, still feel it is wrong. But correct me if i\u0027m unclear my mind....\n\nLet\u0027s think about this case:\n\nlimit is 10. The marker was found in the api_db, api_db return 5 items. Then we still need 5 items from main_db, but the marker isn\u0027t in the main_db, then we get MarkerNotFound at here. So thing is getting wrong.","commit_id":"7a7ff15a5f751a3475fcc103d27782d2360210bc"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"b6a23a7925742ec5c36b3c42e77f7d121aa63d89","unresolved":false,"context_lines":[{"line_number":225,"context_line":"            limit_more \u003d None"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"        if limit_more is None or limit_more \u003e 0:"},{"line_number":228,"context_line":"            main_db_keypairs \u003d db.key_pair_get_all_by_user("},{"line_number":229,"context_line":"                context, user_id, limit\u003dlimit_more, marker\u003dmarker)"},{"line_number":230,"context_line":"        else:"},{"line_number":231,"context_line":"            main_db_keypairs \u003d []"},{"line_number":232,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1aa78d24_4ca8c477","line":229,"range":{"start_line":228,"start_character":12,"end_line":229,"end_character":66},"in_reply_to":"1aa78d24_0cf87cc2","updated":"2016-07-12 06:07:02.000000000","message":"oops, I definitly unclear my mind, there is marker\u003dNone at line 218","commit_id":"7a7ff15a5f751a3475fcc103d27782d2360210bc"},{"author":{"_account_id":10618,"name":"Pavel Kholkin","email":"p.v.holkin@mail.ru","username":"pkholkin"},"change_message_id":"e0a2b079eb908c559929b5a1cba6371e3db2d92a","unresolved":false,"context_lines":[{"line_number":225,"context_line":"            limit_more \u003d None"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"        if limit_more is None or limit_more \u003e 0:"},{"line_number":228,"context_line":"            main_db_keypairs \u003d db.key_pair_get_all_by_user("},{"line_number":229,"context_line":"                context, user_id, limit\u003dlimit_more, marker\u003dmarker)"},{"line_number":230,"context_line":"        else:"},{"line_number":231,"context_line":"            main_db_keypairs \u003d []"},{"line_number":232,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1aa78d24_4b789cc4","line":229,"range":{"start_line":228,"start_character":12,"end_line":229,"end_character":66},"in_reply_to":"1aa78d24_4ca8c477","updated":"2016-07-12 11:26:06.000000000","message":"yes, that\u0027s ok","commit_id":"7a7ff15a5f751a3475fcc103d27782d2360210bc"}],"nova/tests/unit/db/test_db_api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b2c58330da59ceb2e65139ad2cb06f4114ec97a5","unresolved":false,"context_lines":[{"line_number":6960,"context_line":"        self._assertEqualListsOfObjects(key_pairs_user_2, real_keys_2)"},{"line_number":6961,"context_line":""},{"line_number":6962,"context_line":"    def test_key_pair_get_all_by_user_limit_and_marker(self):"},{"line_number":6963,"context_line":"        params \u003d ["},{"line_number":6964,"context_line":"            {\u0027name\u0027: \u0027test_1\u0027, \u0027user_id\u0027: \u0027test_user_id\u0027, \u0027type\u0027: \u0027ssh\u0027},"},{"line_number":6965,"context_line":"            {\u0027name\u0027: \u0027test_2\u0027, \u0027user_id\u0027: \u0027test_user_id\u0027, \u0027type\u0027: \u0027ssh\u0027},"},{"line_number":6966,"context_line":"            {\u0027name\u0027: \u0027test_3\u0027, \u0027user_id\u0027: \u0027test_user_id\u0027, \u0027type\u0027: \u0027ssh\u0027}"},{"line_number":6967,"context_line":"        ]"},{"line_number":6968,"context_line":""},{"line_number":6969,"context_line":"        # check all 3 keypairs"},{"line_number":6970,"context_line":"        keys \u003d [self._create_key_pair(p) for p in params]"}],"source_content_type":"text/x-python","patch_set":4,"id":"1aa78d24_56ea2fad","line":6967,"range":{"start_line":6963,"start_character":8,"end_line":6967,"end_character":9},"updated":"2016-07-11 15:54:55.000000000","message":"Same as in the object code for the API DB:\n\nSince I only caught this in review, it would be good to have a test where we have 2 keypairs with the same name but for different user_ids, and make sure that when we ask for the keypairs for the one user_id we don\u0027t get the other keypair back, e.g.:\n\nkeypair1 - userid 1, name\u003dfoo\nkeypair2 - userid 2, name\u003dfoo\n\nQuery for keypair by userid 2 and name foo, assert the kp we get back is for userid 2 and not userid 1.","commit_id":"7a7ff15a5f751a3475fcc103d27782d2360210bc"},{"author":{"_account_id":10618,"name":"Pavel Kholkin","email":"p.v.holkin@mail.ru","username":"pkholkin"},"change_message_id":"e0a2b079eb908c559929b5a1cba6371e3db2d92a","unresolved":false,"context_lines":[{"line_number":6960,"context_line":"        self._assertEqualListsOfObjects(key_pairs_user_2, real_keys_2)"},{"line_number":6961,"context_line":""},{"line_number":6962,"context_line":"    def test_key_pair_get_all_by_user_limit_and_marker(self):"},{"line_number":6963,"context_line":"        params \u003d ["},{"line_number":6964,"context_line":"            {\u0027name\u0027: \u0027test_1\u0027, \u0027user_id\u0027: \u0027test_user_id\u0027, \u0027type\u0027: \u0027ssh\u0027},"},{"line_number":6965,"context_line":"            {\u0027name\u0027: \u0027test_2\u0027, \u0027user_id\u0027: \u0027test_user_id\u0027, \u0027type\u0027: \u0027ssh\u0027},"},{"line_number":6966,"context_line":"            {\u0027name\u0027: \u0027test_3\u0027, \u0027user_id\u0027: \u0027test_user_id\u0027, \u0027type\u0027: \u0027ssh\u0027}"},{"line_number":6967,"context_line":"        ]"},{"line_number":6968,"context_line":""},{"line_number":6969,"context_line":"        # check all 3 keypairs"},{"line_number":6970,"context_line":"        keys \u003d [self._create_key_pair(p) for p in params]"}],"source_content_type":"text/x-python","patch_set":4,"id":"1aa78d24_de13a863","line":6967,"range":{"start_line":6963,"start_character":8,"end_line":6967,"end_character":9},"in_reply_to":"1aa78d24_56ea2fad","updated":"2016-07-12 11:26:06.000000000","message":"added after this","commit_id":"7a7ff15a5f751a3475fcc103d27782d2360210bc"}]}