)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fa26e1a6375fddc65df15e147dc86fd7a356c4a1","unresolved":false,"context_lines":[{"line_number":13,"context_line":"authorization from the database in the console_auth_token_get_valid()"},{"line_number":14,"context_line":"method."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"In testing that design with Tempest jobs, we found that noVNC was not"},{"line_number":17,"context_line":"handling the instance UUID in the access URL as expected and would"},{"line_number":18,"context_line":"not work with existing proxies. So, instead we could change to a design"},{"line_number":19,"context_line":"where we run one websocketproxy per cells v2 cell and avoid adding"},{"line_number":20,"context_line":"instance UUID to access URLs. With that, we need a way to get console"},{"line_number":21,"context_line":"token authorizations from the database without the instance UUID."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Part of blueprint convert-consoles-to-objects"},{"line_number":24,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"ff82abbf_1c69ea1f","line":21,"range":{"start_line":16,"start_character":0,"end_line":21,"end_character":65},"updated":"2017-11-24 13:02:37.000000000","message":"Good explanation ++","commit_id":"6a7dcb10e87ec0183730f19d786a416902ea5669"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d63c88031206fcee425a56fc390c9ec52ce55a7f","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Optionalize instance_uuid in console_auth_token_get_valid()"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The original design of the conversion of consoles to objects and"},{"line_number":10,"context_line":"storing the token authorizations in the database backend proposed"},{"line_number":11,"context_line":"adding the instance UUID to the access URL for consoles and did"},{"line_number":12,"context_line":"additional validation of the instance UUID when getting the token"},{"line_number":13,"context_line":"authorization from the database in the console_auth_token_get_valid()"},{"line_number":14,"context_line":"method."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"In testing that design with Tempest jobs, we found that noVNC was not"},{"line_number":17,"context_line":"handling the instance UUID in the access URL as expected and would"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"bf8cb3f7_0b401056","line":14,"range":{"start_line":9,"start_character":0,"end_line":14,"end_character":7},"updated":"2017-12-22 22:03:06.000000000","message":"TODO: the spec needs to be updated","commit_id":"26949cd4cd68aa8e04822ebf7b9b95c97fdd198e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5c5367a34c33f497aa8510a61594199ffe5f764b","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Optionalize instance_uuid in console_auth_token_get_valid()"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The original design of the conversion of consoles to objects and"},{"line_number":10,"context_line":"storing the token authorizations in the database backend proposed"},{"line_number":11,"context_line":"adding the instance UUID to the access URL for consoles and did"},{"line_number":12,"context_line":"additional validation of the instance UUID when getting the token"},{"line_number":13,"context_line":"authorization from the database in the console_auth_token_get_valid()"},{"line_number":14,"context_line":"method."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"In testing that design with Tempest jobs, we found that noVNC was not"},{"line_number":17,"context_line":"handling the instance UUID in the access URL as expected and would"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"9f91af0f_e3870561","line":14,"range":{"start_line":9,"start_character":0,"end_line":14,"end_character":7},"in_reply_to":"bf8cb3f7_0b401056","updated":"2018-01-03 21:44:33.000000000","message":"Ack.","commit_id":"26949cd4cd68aa8e04822ebf7b9b95c97fdd198e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d63c88031206fcee425a56fc390c9ec52ce55a7f","unresolved":false,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"In testing that design with Tempest jobs, we found that noVNC was not"},{"line_number":17,"context_line":"handling the instance UUID in the access URL as expected and would"},{"line_number":18,"context_line":"not work with existing proxies. So, instead we could change to a design"},{"line_number":19,"context_line":"where we run one websocketproxy per cells v2 cell and avoid adding"},{"line_number":20,"context_line":"instance UUID to access URLs. With that, we need a way to get console"},{"line_number":21,"context_line":"token authorizations from the database without the instance UUID."},{"line_number":22,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"bf8cb3f7_ab2484c7","line":19,"range":{"start_line":18,"start_character":32,"end_line":19,"end_character":49},"updated":"2017-12-22 22:03:06.000000000","message":"We are going to need to update the cellsv2 layout docs at some point:\n\nhttps://docs.openstack.org/nova/latest/user/cellsv2-layout.html","commit_id":"26949cd4cd68aa8e04822ebf7b9b95c97fdd198e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5c5367a34c33f497aa8510a61594199ffe5f764b","unresolved":false,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"In testing that design with Tempest jobs, we found that noVNC was not"},{"line_number":17,"context_line":"handling the instance UUID in the access URL as expected and would"},{"line_number":18,"context_line":"not work with existing proxies. So, instead we could change to a design"},{"line_number":19,"context_line":"where we run one websocketproxy per cells v2 cell and avoid adding"},{"line_number":20,"context_line":"instance UUID to access URLs. With that, we need a way to get console"},{"line_number":21,"context_line":"token authorizations from the database without the instance UUID."},{"line_number":22,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"9f91af0f_837e3969","line":19,"range":{"start_line":18,"start_character":32,"end_line":19,"end_character":49},"in_reply_to":"bf8cb3f7_ab2484c7","updated":"2018-01-03 21:44:33.000000000","message":"Ack.","commit_id":"26949cd4cd68aa8e04822ebf7b9b95c97fdd198e"}],"nova/db/sqlalchemy/api.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fa26e1a6375fddc65df15e147dc86fd7a356c4a1","unresolved":false,"context_lines":[{"line_number":7088,"context_line":""},{"line_number":7089,"context_line":""},{"line_number":7090,"context_line":"@pick_context_manager_reader"},{"line_number":7091,"context_line":"def console_auth_token_get(context, token_hash):"},{"line_number":7092,"context_line":"    return context.session.query(models.ConsoleAuthToken).\\"},{"line_number":7093,"context_line":"        filter_by(token_hash\u003dtoken_hash).\\"},{"line_number":7094,"context_line":"        filter(models.ConsoleAuthToken.expires \u003e timeutils.utcnow_ts()).first()"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_dcece290","line":7091,"range":{"start_line":7091,"start_character":4,"end_line":7091,"end_character":26},"updated":"2017-11-24 13:02:37.000000000","message":"The function above has a \u0027_valid\u0027 suffix while this does not. This would suggest to me that, at a cursory glance, the above function validates the token (via it\u0027s expiry) while this one does not. That\u0027s not the case and is misleading.\n\nRather than adding a separate function, could we not simply use the previous one but make the \u0027instance_uuid\u0027 attribute nullable and handle that?","commit_id":"6a7dcb10e87ec0183730f19d786a416902ea5669"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b68ae98f3ddd3ede709399784e8a921d0a127a1d","unresolved":false,"context_lines":[{"line_number":7088,"context_line":""},{"line_number":7089,"context_line":""},{"line_number":7090,"context_line":"@pick_context_manager_reader"},{"line_number":7091,"context_line":"def console_auth_token_get(context, token_hash):"},{"line_number":7092,"context_line":"    return context.session.query(models.ConsoleAuthToken).\\"},{"line_number":7093,"context_line":"        filter_by(token_hash\u003dtoken_hash).\\"},{"line_number":7094,"context_line":"        filter(models.ConsoleAuthToken.expires \u003e timeutils.utcnow_ts()).first()"}],"source_content_type":"text/x-python","patch_set":3,"id":"df87a7cf_4c6c039c","line":7091,"range":{"start_line":7091,"start_character":4,"end_line":7091,"end_character":26},"in_reply_to":"ff82abbf_dcece290","updated":"2017-12-13 22:37:30.000000000","message":"Hm, I see what you mean. I had been thinking \"valid\" was in reference to the check on L7082 about whether the instance exists in the project, but you\u0027re probably right that it\u0027s referring to the expiry.\n\nI can try making the other function\u0027s instance_uuid optional.","commit_id":"6a7dcb10e87ec0183730f19d786a416902ea5669"},{"author":{"_account_id":23630,"name":"Tovin Seven","email":"tovin07@gmail.com","username":"tovin07"},"change_message_id":"14139e0a7799d0e55f2ae4d2b7aa2d7d056d7680","unresolved":false,"context_lines":[{"line_number":6581,"context_line":"        _check_instance_exists_in_project(context, instance_uuid)"},{"line_number":6582,"context_line":"    query \u003d context.session.query(models.ConsoleAuthToken).\\"},{"line_number":6583,"context_line":"        filter_by(token_hash\u003dtoken_hash)"},{"line_number":6584,"context_line":"    if instance_uuid is not None:"},{"line_number":6585,"context_line":"        query \u003d query.filter_by(instance_uuid\u003dinstance_uuid)"},{"line_number":6586,"context_line":"    return query.filter("},{"line_number":6587,"context_line":"        models.ConsoleAuthToken.expires \u003e timeutils.utcnow_ts()).first()"}],"source_content_type":"text/x-python","patch_set":6,"id":"df87a7cf_d5bbb36f","line":6584,"range":{"start_line":6584,"start_character":4,"end_line":6584,"end_character":33},"updated":"2017-12-14 08:00:56.000000000","message":"is this duplicated?","commit_id":"d5b1e01d2febdc67b8fff0121e9ba83f4ec73dd7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"8c1445dfdd8d5eaf11c31710d0ae58055480d5d9","unresolved":false,"context_lines":[{"line_number":6581,"context_line":"        _check_instance_exists_in_project(context, instance_uuid)"},{"line_number":6582,"context_line":"    query \u003d context.session.query(models.ConsoleAuthToken).\\"},{"line_number":6583,"context_line":"        filter_by(token_hash\u003dtoken_hash)"},{"line_number":6584,"context_line":"    if instance_uuid is not None:"},{"line_number":6585,"context_line":"        query \u003d query.filter_by(instance_uuid\u003dinstance_uuid)"},{"line_number":6586,"context_line":"    return query.filter("},{"line_number":6587,"context_line":"        models.ConsoleAuthToken.expires \u003e timeutils.utcnow_ts()).first()"}],"source_content_type":"text/x-python","patch_set":6,"id":"df87a7cf_254ae47f","line":6584,"range":{"start_line":6584,"start_character":4,"end_line":6584,"end_character":33},"in_reply_to":"df87a7cf_d5bbb36f","updated":"2017-12-14 14:33:50.000000000","message":"Nope. Compare this to the new function added in the previous PS [1]. What we\u0027re saying here equates to something like this.\n\n   if we have a instance\u0027s UUID\n       make sure it\u0027s valid for the project\n\n   start building the query we\u0027re going to use\n\n   if we have a instance\u0027s UUID\n       extend the query to filter by that same UUID\n\n    finalize the query and return the first item we get\n\n[1] https://review.openstack.org/#/c/481700/5/nova/db/sqlalchemy/api.py","commit_id":"d5b1e01d2febdc67b8fff0121e9ba83f4ec73dd7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"8c1445dfdd8d5eaf11c31710d0ae58055480d5d9","unresolved":false,"context_lines":[{"line_number":6583,"context_line":"        filter_by(token_hash\u003dtoken_hash)"},{"line_number":6584,"context_line":"    if instance_uuid is not None:"},{"line_number":6585,"context_line":"        query \u003d query.filter_by(instance_uuid\u003dinstance_uuid)"},{"line_number":6586,"context_line":"    return query.filter("},{"line_number":6587,"context_line":"        models.ConsoleAuthToken.expires \u003e timeutils.utcnow_ts()).first()"},{"line_number":6588,"context_line":""},{"line_number":6589,"context_line":""},{"line_number":6590,"context_line":"@pick_context_manager_writer"}],"source_content_type":"text/x-python","patch_set":6,"id":"df87a7cf_053a40e6","line":6587,"range":{"start_line":6586,"start_character":16,"end_line":6587,"end_character":64},"updated":"2017-12-14 14:33:50.000000000","message":"nit: I wonder could we change the order of this and append it on line 6583/6584 above?","commit_id":"d5b1e01d2febdc67b8fff0121e9ba83f4ec73dd7"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ab440b182d1dd8f0d3c72c8baca324954553d415","unresolved":false,"context_lines":[{"line_number":6583,"context_line":"        filter_by(token_hash\u003dtoken_hash)"},{"line_number":6584,"context_line":"    if instance_uuid is not None:"},{"line_number":6585,"context_line":"        query \u003d query.filter_by(instance_uuid\u003dinstance_uuid)"},{"line_number":6586,"context_line":"    return query.filter("},{"line_number":6587,"context_line":"        models.ConsoleAuthToken.expires \u003e timeutils.utcnow_ts()).first()"},{"line_number":6588,"context_line":""},{"line_number":6589,"context_line":""},{"line_number":6590,"context_line":"@pick_context_manager_writer"}],"source_content_type":"text/x-python","patch_set":6,"id":"df87a7cf_8d7c32b6","line":6587,"range":{"start_line":6586,"start_character":16,"end_line":6587,"end_character":64},"in_reply_to":"df87a7cf_053a40e6","updated":"2017-12-14 17:40:07.000000000","message":"I wondered about that but wasn\u0027t 100% sure if the order matters. \n\nI was thinking the order is the order in which it will filter, so you would want more restrictive filters first to cut the result down with fewer scans. The expiry filter is the most generic, so it makes sense for it to be last.\n\nAfter writing the above, I just read that SQL filter predicates are automatically ordered by the optimizer, so probably they could be in any order here after all. I can change it if I have to respin.","commit_id":"d5b1e01d2febdc67b8fff0121e9ba83f4ec73dd7"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"58ec70b0f5094aa8a10f76bd609a47813d254767","unresolved":false,"context_lines":[{"line_number":6583,"context_line":"        filter_by(token_hash\u003dtoken_hash)"},{"line_number":6584,"context_line":"    if instance_uuid is not None:"},{"line_number":6585,"context_line":"        query \u003d query.filter_by(instance_uuid\u003dinstance_uuid)"},{"line_number":6586,"context_line":"    return query.filter("},{"line_number":6587,"context_line":"        models.ConsoleAuthToken.expires \u003e timeutils.utcnow_ts()).first()"},{"line_number":6588,"context_line":""},{"line_number":6589,"context_line":""},{"line_number":6590,"context_line":"@pick_context_manager_writer"}],"source_content_type":"text/x-python","patch_set":6,"id":"df87a7cf_83e2c32e","line":6587,"range":{"start_line":6586,"start_character":16,"end_line":6587,"end_character":64},"in_reply_to":"df87a7cf_8d7c32b6","updated":"2017-12-14 17:52:12.000000000","message":"Actually, moving this line wouldn\u0027t actually make the two \"if instance_uuid\" any better so it seems fine the way it is IMHO.","commit_id":"d5b1e01d2febdc67b8fff0121e9ba83f4ec73dd7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d63c88031206fcee425a56fc390c9ec52ce55a7f","unresolved":false,"context_lines":[{"line_number":1863,"context_line":"    model_query(context, models.Migration).\\"},{"line_number":1864,"context_line":"            filter_by(instance_uuid\u003dinstance_uuid).\\"},{"line_number":1865,"context_line":"            soft_delete()"},{"line_number":1866,"context_line":"    # NOTE(snikitin): We can\u0027t use model_query here, because there is no"},{"line_number":1867,"context_line":"    # column \u0027deleted\u0027 in \u0027tags\u0027 table."},{"line_number":1868,"context_line":"    context.session.query(models.Tag).filter_by("},{"line_number":1869,"context_line":"        resource_id\u003dinstance_uuid).delete()"},{"line_number":1870,"context_line":"    context.session.query(models.ConsoleAuthToken).filter_by("}],"source_content_type":"text/x-python","patch_set":7,"id":"bf8cb3f7_0b19f083","line":1867,"range":{"start_line":1866,"start_character":4,"end_line":1867,"end_character":39},"updated":"2017-12-22 22:03:06.000000000","message":"This could be updated for the consoleauthtoken model too.","commit_id":"26949cd4cd68aa8e04822ebf7b9b95c97fdd198e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5c5367a34c33f497aa8510a61594199ffe5f764b","unresolved":false,"context_lines":[{"line_number":1863,"context_line":"    model_query(context, models.Migration).\\"},{"line_number":1864,"context_line":"            filter_by(instance_uuid\u003dinstance_uuid).\\"},{"line_number":1865,"context_line":"            soft_delete()"},{"line_number":1866,"context_line":"    # NOTE(snikitin): We can\u0027t use model_query here, because there is no"},{"line_number":1867,"context_line":"    # column \u0027deleted\u0027 in \u0027tags\u0027 table."},{"line_number":1868,"context_line":"    context.session.query(models.Tag).filter_by("},{"line_number":1869,"context_line":"        resource_id\u003dinstance_uuid).delete()"},{"line_number":1870,"context_line":"    context.session.query(models.ConsoleAuthToken).filter_by("}],"source_content_type":"text/x-python","patch_set":7,"id":"9f91af0f_434b512a","line":1867,"range":{"start_line":1866,"start_character":4,"end_line":1867,"end_character":39},"in_reply_to":"bf8cb3f7_0b19f083","updated":"2018-01-03 21:44:33.000000000","message":"Ack.","commit_id":"26949cd4cd68aa8e04822ebf7b9b95c97fdd198e"}],"nova/tests/unit/db/test_db_api.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"c99dbf05260875930a8bffbf0033e08ef57fdd33","unresolved":false,"context_lines":[{"line_number":10734,"context_line":"        self._create(hash1, uuid1, 100)"},{"line_number":10735,"context_line":""},{"line_number":10736,"context_line":"        db_obj1 \u003d db.console_auth_token_get(self.context, hash1)"},{"line_number":10737,"context_line":"        self.assertIsNotNone(db_obj1, \"a valid token should be in database\")"},{"line_number":10738,"context_line":""},{"line_number":10739,"context_line":"        db.instance_destroy(self.context, uuid1)"},{"line_number":10740,"context_line":"        db_obj1 \u003d db.console_auth_token_get(self.context, hash1)"}],"source_content_type":"text/x-python","patch_set":2,"id":"df3967d1_0ad2465d","line":10737,"updated":"2017-07-31 14:22:48.000000000","message":"You should have at least an assertion to ensure that the object retrieved is well the one you want. Something like asserting the hash in the object with the hash generated and used to retrieve it.","commit_id":"4c0a242fed4b1c3c65ca414eb2556023473d4b26"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e360928ebfc7cd499fab792e9ad7199335cb562d","unresolved":false,"context_lines":[{"line_number":10734,"context_line":"        self._create(hash1, uuid1, 100)"},{"line_number":10735,"context_line":""},{"line_number":10736,"context_line":"        db_obj1 \u003d db.console_auth_token_get(self.context, hash1)"},{"line_number":10737,"context_line":"        self.assertIsNotNone(db_obj1, \"a valid token should be in database\")"},{"line_number":10738,"context_line":""},{"line_number":10739,"context_line":"        db.instance_destroy(self.context, uuid1)"},{"line_number":10740,"context_line":"        db_obj1 \u003d db.console_auth_token_get(self.context, hash1)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f2577a7_9678cdf8","line":10737,"in_reply_to":"df3967d1_0ad2465d","updated":"2017-09-08 21:14:59.000000000","message":"Will do in the next PS.","commit_id":"4c0a242fed4b1c3c65ca414eb2556023473d4b26"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d63c88031206fcee425a56fc390c9ec52ce55a7f","unresolved":false,"context_lines":[{"line_number":10505,"context_line":"        self.assertIsNotNone(db_obj1, \"a valid token should be in database\")"},{"line_number":10506,"context_line":""},{"line_number":10507,"context_line":"        db.instance_destroy(self.context, uuid1)"},{"line_number":10508,"context_line":"        db_obj1 \u003d db.console_auth_token_get_valid(self.context, hash1)"},{"line_number":10509,"context_line":"        self.assertIsNone(db_obj1, \"the token should have been deleted\")"},{"line_number":10510,"context_line":""},{"line_number":10511,"context_line":"    def test_console_auth_token_get_valid_without_uuid_by_expiry(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf8cb3f7_6b075c1e","line":10508,"updated":"2017-12-22 22:03:06.000000000","message":"OK because https://review.openstack.org/#/c/481700/7/nova/db/sqlalchemy/api.py@1870","commit_id":"26949cd4cd68aa8e04822ebf7b9b95c97fdd198e"}]}
