)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"797c153f797f3c5fe86631dd5dcb1a6c10ae0b32","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"5eda73fd_bde015d5","updated":"2022-06-17 07:42:38.000000000","message":"recheck","commit_id":"30aa5a6f96fcc427e1bf3be8c05f6d60aba84dc0"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fec05be1d3e7cb0f22502220be59218b635cfa3e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"8077204c_c520f45a","updated":"2022-08-04 13:40:00.000000000","message":"Couple of suggestions inline. I\u0027m mainly -1 due to missing shadow table and archiving support. ","commit_id":"9a553f252bf640144973312893732fb38cc8eeaa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"aab81123d9d95f32d2b5a4be75fda45b60306c21","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"442dcca0_539066b8","in_reply_to":"53e752a6_396b04b1","updated":"2022-08-08 09:47:03.000000000","message":"Hm, that was a news for me :) But yes we have a spec that deprecated softdelete. So I\u0027m OK not to add it here.","commit_id":"9a553f252bf640144973312893732fb38cc8eeaa"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"fd70e3e607dc70b41332b42e3b6d07aaecc7a2d5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"53e752a6_396b04b1","in_reply_to":"8077204c_c520f45a","updated":"2022-08-05 09:25:44.000000000","message":"I have not put in place a shadow table because there is a test preventing it and explaining soft deletes are deprecated.\n\n https://opendev.org/openstack/nova/src/commit/adeea3d5e7d7337d2817dd5c46334c76c05995ef/nova/tests/unit/db/test_models.py#L21\n \nIf I\u0027m not wrong I also confirmed with Sean that it is not anymore required.","commit_id":"9a553f252bf640144973312893732fb38cc8eeaa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"aab81123d9d95f32d2b5a4be75fda45b60306c21","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"fb2c6105_2491e839","updated":"2022-08-08 09:47:03.000000000","message":"The shadow table issue was resolved, I was wrong. The rest of my comments from PS8 is still valid but those are just nits so I\u0027m OK to handle them in a follow up (but please if you need to respin this patch for any reason then try to include those fixes here).\n+1 as I would like to go through the whole series before approving things.","commit_id":"4b8e00e34faba5f7f0c6fa2ec7ad2febf1f1d138"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4a8c113270f7f4b874ed2ffd41f289b25976bd1a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"b68fe9e3_41f444a4","updated":"2022-08-26 07:31:55.000000000","message":"look good to me","commit_id":"a8a6777cad24ec5742a8ec0a138fb54ec2cc2001"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"f661721273311810334f9c0c8b72dd8e5b8b3ab4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"a0c8b29d_f062e98f","updated":"2022-08-28 06:26:58.000000000","message":"lgtm","commit_id":"e4f3389038d8426e606a4ea36e7d64cb677c3baf"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4c7233de1ea5cfa4126cf0f76d0791e730858065","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"183e3c75_b05de968","updated":"2022-09-01 09:09:20.000000000","message":"recheck nova-multi-cell fixed by https://review.opendev.org/c/openstack/nova/+/855378","commit_id":"89e1986874ef56a7281018fd9ef20dde50a61329"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6f0e00040d0b4b4fa7322a9cb524ece2440e2da7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"47c6a94f_19232ae8","updated":"2022-10-10 17:46:09.000000000","message":"I am really looking forward to this feature. My questions are more to other reviewers than anything else.\n\nI think the index and the back-ref don\u0027t look quite right. I think they should match, and looking at extra specs table, I would drop the deleted bit? I am not 100% sure though, just asking the question.\n\nI wasn\u0027t sure if 255 is long enough for the mount strings. Ceph ones get quite long with the new monitor format I think? But maybe manila doesn\u0027t expose it that way? Does manila have some max length on those?","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"99bff66a4c4f972422e3310a5e4e42cc9712a2b6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"afe02e10_938e9635","updated":"2023-01-24 17:43:05.000000000","message":"I think it worth addressing @John\u0027s comment from PS17. Other than that I think this is still look good. -1 for visibility on John\u0027s comments.","commit_id":"deef700f6de03107d1ad14031ec1853e8996338a"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"1a916c8e772f5afd1d2bafdbf777c2a3c77c2358","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":20,"id":"fe321b37_abba537f","updated":"2023-02-02 13:56:31.000000000","message":"recheck","commit_id":"bf43529f97c6b8c0c3ce2ff16af84e33242387d1"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"eb937a00b65790f5361cd0d0f206e1624640e500","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":22,"id":"0e5e96d0_7d7d7337","updated":"2023-02-08 16:15:15.000000000","message":"I\u0027m quite OK with the change but I have two concerns :\n- I\u0027m not sure about what happens on the DB side when deleting an instance. @Uggla, could you please confirm it keeps the share mapping record in the DB ?\n- my main -1 comes from the connection column length. Merging this patch now would imply a costly DB migration if we were told this length isn\u0027t sufficient in the future, so I tend to be super cautious.","commit_id":"a484fc5434bafdb06887244f7e5e224f3845fd39"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"d6dbe825b4e7fa81bf5085e715c499b30ec6239b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"5cd8127d_82a46a5d","updated":"2023-03-13 09:42:42.000000000","message":"Providing again my -1 because of the question I had for deleting instances + the fact that indeed we could modify the id to be BigInteger.\nSee my main comment on PS23.","commit_id":"33492f6d186724f0587a4ffdad6e4c2cec5e812f"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"35d11a1b23d48af51bd5e932d624190e237e7a13","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":25,"id":"1370b63f_d3a1983b","updated":"2023-04-14 09:48:02.000000000","message":"René, I\u0027m afraid we could be leaking some DB records for share mappings if we don\u0027t delete them when an instance is deleted.","commit_id":"43da83ea74d0da569ad25ae3fb157b562aadf7a7"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"023dd59c6f129753db8f6b0a5408a35700bdceda","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":25,"id":"a95ce0f7_c1a42074","in_reply_to":"1370b63f_d3a1983b","updated":"2023-05-23 09:40:49.000000000","message":"Now, this is fixed by https://review.opendev.org/c/openstack/nova/+/881472","commit_id":"43da83ea74d0da569ad25ae3fb157b562aadf7a7"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"023dd59c6f129753db8f6b0a5408a35700bdceda","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":29,"id":"9abcf3ae_efe5946a","updated":"2023-05-23 09:40:49.000000000","message":"As I said in PS25, now René created a change that removes the DB records https://review.opendev.org/c/openstack/nova/+/881472/\n\nEventually +2.","commit_id":"94fe39d6e7b987e96e2a87907d8894844cb4f97b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"db880686c9e6818befe80811528ba254b3a402a2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":30,"id":"3e12c3cc_87dfa7a8","updated":"2023-06-13 17:32:44.000000000","message":"OK, sounds an easy change to directly call SQLA in the object but I don\u0027t think it\u0027s a blocker for the series. Downgrading to +1 since I\u0027d like others to chime in, but this patch continues to look to me reasonable.","commit_id":"aef34518a3625b4320fda6618039fe5138cf1b13"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"c433f8588a16b2cbdae2bfa6940252d2fabdbec8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"f336867d_9fae36ef","updated":"2023-07-28 10:16:41.000000000","message":"re-adding my +2 because in between PS29 and PS32 it was just a rebase","commit_id":"11e1eb78aa53819038977a55b65a0358e028e481"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f018f172a095990e713f23e707ad75d3c08bce51","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":34,"id":"13445004_097962a3","updated":"2023-11-15 09:22:07.000000000","message":"still look OK to me","commit_id":"0b7b03025676dc201247a730f269f1dc14385a0c"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"0615643abbf89ae80b16d9655bb17b6973ed5924","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":35,"id":"4d3c0ab6_0034b91f","updated":"2023-12-12 10:36:21.000000000","message":"Reapplying my +2 from PS32, no changes between.","commit_id":"d41c7c81698b7525bdaceb5290d7bffb23803785"}],"nova/db/main/api.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6f0e00040d0b4b4fa7322a9cb524ece2440e2da7","unresolved":true,"context_lines":[{"line_number":4852,"context_line":""},{"line_number":4853,"context_line":"@require_context"},{"line_number":4854,"context_line":"@pick_context_manager_reader"},{"line_number":4855,"context_line":"def share_mapping_get_all(context):"},{"line_number":4856,"context_line":"    \"\"\"Get all share_mapping.\"\"\""},{"line_number":4857,"context_line":"    return context.session.query(models.ShareMapping).all()"},{"line_number":4858,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"6cfa2a63_4b585a14","line":4855,"updated":"2022-10-10 17:46:09.000000000","message":"I was actually expecting to see all of this live inside the objects now, following what we did for the API DB? Maybe that is only for the API DB?","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"db880686c9e6818befe80811528ba254b3a402a2","unresolved":true,"context_lines":[{"line_number":4852,"context_line":""},{"line_number":4853,"context_line":"@require_context"},{"line_number":4854,"context_line":"@pick_context_manager_reader"},{"line_number":4855,"context_line":"def share_mapping_get_all(context):"},{"line_number":4856,"context_line":"    \"\"\"Get all share_mapping.\"\"\""},{"line_number":4857,"context_line":"    return context.session.query(models.ShareMapping).all()"},{"line_number":4858,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"f6052587_c49e7532","line":4855,"in_reply_to":"2826f229_f2faf6d3","updated":"2023-06-13 17:32:44.000000000","message":"Fair point.","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"eb937a00b65790f5361cd0d0f206e1624640e500","unresolved":true,"context_lines":[{"line_number":4852,"context_line":""},{"line_number":4853,"context_line":"@require_context"},{"line_number":4854,"context_line":"@pick_context_manager_reader"},{"line_number":4855,"context_line":"def share_mapping_get_all(context):"},{"line_number":4856,"context_line":"    \"\"\"Get all share_mapping.\"\"\""},{"line_number":4857,"context_line":"    return context.session.query(models.ShareMapping).all()"},{"line_number":4858,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"ee217121_8c4aacfe","line":4855,"in_reply_to":"2d0de1f2_c241a6c8","updated":"2023-02-08 16:15:15.000000000","message":"\u003e I\u0027m not against moving this to the OVO but I\u0027m don\u0027t know if we have a rule around it. @bauzas what do you think?\n\nYeah, we had some implicit rule that we were directly calling SQLA for objects that were calling the main API DB, not the cells ones.\n\nI\u0027m quite OK to leave those helper methods here.","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"99bff66a4c4f972422e3310a5e4e42cc9712a2b6","unresolved":true,"context_lines":[{"line_number":4852,"context_line":""},{"line_number":4853,"context_line":"@require_context"},{"line_number":4854,"context_line":"@pick_context_manager_reader"},{"line_number":4855,"context_line":"def share_mapping_get_all(context):"},{"line_number":4856,"context_line":"    \"\"\"Get all share_mapping.\"\"\""},{"line_number":4857,"context_line":"    return context.session.query(models.ShareMapping).all()"},{"line_number":4858,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"2d0de1f2_c241a6c8","line":4855,"in_reply_to":"6cfa2a63_4b585a14","updated":"2023-01-24 17:43:05.000000000","message":"I\u0027m not against moving this to the OVO but I\u0027m don\u0027t know if we have a rule around it. @bauzas what do you think?","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"747e45509dc6122aada2fc1b7e22a333a89d4e23","unresolved":true,"context_lines":[{"line_number":4852,"context_line":""},{"line_number":4853,"context_line":"@require_context"},{"line_number":4854,"context_line":"@pick_context_manager_reader"},{"line_number":4855,"context_line":"def share_mapping_get_all(context):"},{"line_number":4856,"context_line":"    \"\"\"Get all share_mapping.\"\"\""},{"line_number":4857,"context_line":"    return context.session.query(models.ShareMapping).all()"},{"line_number":4858,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"2826f229_f2faf6d3","line":4855,"in_reply_to":"ee217121_8c4aacfe","updated":"2023-06-09 16:47:32.000000000","message":"Yeah we did specifically aim to put that into the objects for the API stuff. However, it was mostly because that was all new and it was in the \"we\u0027re never doing anything other than SQLA so we might as well dispense with the layers of DB API.\n\nWe can surely do either here, but it does feel very heavy to ad all these one-line methods here, personally.","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"95b03db3501a7b105700779fb484b5cf740087ea","unresolved":true,"context_lines":[{"line_number":4951,"context_line":""},{"line_number":4952,"context_line":"@require_context"},{"line_number":4953,"context_line":"@pick_context_manager_writer"},{"line_number":4954,"context_line":"def share_mapping_update("},{"line_number":4955,"context_line":"    context, uuid, instance_uuid, share_id, status, tag, export_location,"},{"line_number":4956,"context_line":"    share_proto"},{"line_number":4957,"context_line":"):"},{"line_number":4958,"context_line":"    \"\"\"Update share_mapping for a share"},{"line_number":4959,"context_line":"    Creates new record if needed."},{"line_number":4960,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":34,"id":"b8cff455_73eee96a","line":4957,"range":{"start_line":4954,"start_character":0,"end_line":4957,"end_character":2},"updated":"2023-11-15 12:59:50.000000000","message":"This is in a bit different style than our normal db update methods. There we rely on the OVO\u0027s obj_get_changes() method to gather the diff of the OVO and then use SQLA\u0027s model update() method and pass those updates, then do a model save() to persist. The direct benefit of that style is when a new field is added this interface does not need to be changed.","commit_id":"0b7b03025676dc201247a730f269f1dc14385a0c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"198f07a6bf484c6e2fd9c72b07690fbf2aecdfb0","unresolved":true,"context_lines":[{"line_number":4951,"context_line":""},{"line_number":4952,"context_line":"@require_context"},{"line_number":4953,"context_line":"@pick_context_manager_writer"},{"line_number":4954,"context_line":"def share_mapping_update("},{"line_number":4955,"context_line":"    context, uuid, instance_uuid, share_id, status, tag, export_location,"},{"line_number":4956,"context_line":"    share_proto"},{"line_number":4957,"context_line":"):"},{"line_number":4958,"context_line":"    \"\"\"Update share_mapping for a share"},{"line_number":4959,"context_line":"    Creates new record if needed."},{"line_number":4960,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":34,"id":"d8a2704b_6d46830d","line":4957,"range":{"start_line":4954,"start_character":0,"end_line":4957,"end_character":2},"in_reply_to":"b8cff455_73eee96a","updated":"2023-11-16 09:58:15.000000000","message":"Based on the IRC discussion so far we won\u0027t block on this.","commit_id":"0b7b03025676dc201247a730f269f1dc14385a0c"}],"nova/db/main/migrations/versions/13863f4e1612_create_share_mapping_table.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fec05be1d3e7cb0f22502220be59218b635cfa3e","unresolved":false,"context_lines":[{"line_number":40,"context_line":"            sa.ForeignKey("},{"line_number":41,"context_line":"                \u0027instances.uuid\u0027,"},{"line_number":42,"context_line":"                name\u003d\u0027share_mapping_instance_uuid_fkey\u0027)),"},{"line_number":43,"context_line":"        sa.Column(\u0027share_id\u0027, sa.String(length\u003d36)),"},{"line_number":44,"context_line":"        sa.Column(\u0027status\u0027, sa.String(length\u003d32)),"},{"line_number":45,"context_line":"        sa.Column(\u0027tag\u0027, sa.String(48)),"},{"line_number":46,"context_line":"        sa.Column(\u0027export_location\u0027, sa.String(255)),"}],"source_content_type":"text/x-python","patch_set":8,"id":"23a5dac3_c6df1155","line":43,"updated":"2022-08-04 13:40:00.000000000","message":"OK, so uuid the the id of the share mapping connecting a manila share identified by share_id to a nova instance identified by instance_uuid.","commit_id":"9a553f252bf640144973312893732fb38cc8eeaa"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6f0e00040d0b4b4fa7322a9cb524ece2440e2da7","unresolved":true,"context_lines":[{"line_number":37,"context_line":"        sa.Column(\u0027uuid\u0027, sa.String(36)),"},{"line_number":38,"context_line":"        sa.Column("},{"line_number":39,"context_line":"            \u0027instance_uuid\u0027, sa.String(length\u003d36),"},{"line_number":40,"context_line":"            sa.ForeignKey("},{"line_number":41,"context_line":"                \u0027instances.uuid\u0027,"},{"line_number":42,"context_line":"                name\u003d\u0027share_mapping_instance_uuid_fkey\u0027)),"},{"line_number":43,"context_line":"        sa.Column(\u0027share_id\u0027, sa.String(length\u003d36)),"}],"source_content_type":"text/x-python","patch_set":17,"id":"0b68a340_2575d3d6","line":40,"updated":"2022-10-10 17:46:09.000000000","message":"we probably should test that this is created correctly.","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"ed59d0793ec8705940973572bcb578054152c11c","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        sa.Column(\u0027uuid\u0027, sa.String(36)),"},{"line_number":38,"context_line":"        sa.Column("},{"line_number":39,"context_line":"            \u0027instance_uuid\u0027, sa.String(length\u003d36),"},{"line_number":40,"context_line":"            sa.ForeignKey("},{"line_number":41,"context_line":"                \u0027instances.uuid\u0027,"},{"line_number":42,"context_line":"                name\u003d\u0027share_mapping_instance_uuid_fkey\u0027)),"},{"line_number":43,"context_line":"        sa.Column(\u0027share_id\u0027, sa.String(length\u003d36)),"}],"source_content_type":"text/x-python","patch_set":17,"id":"c719a441_5dda0752","line":40,"in_reply_to":"0b68a340_2575d3d6","updated":"2023-02-02 14:52:09.000000000","message":"Done","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6f0e00040d0b4b4fa7322a9cb524ece2440e2da7","unresolved":true,"context_lines":[{"line_number":48,"context_line":"        sa.Index(\u0027share_id\u0027, \u0027share_id\u0027),"},{"line_number":49,"context_line":"        sa.Index("},{"line_number":50,"context_line":"            \u0027share_mapping_instance_uuid_share_id_idx\u0027,"},{"line_number":51,"context_line":"            \u0027instance_uuid\u0027, \u0027share_id\u0027),"},{"line_number":52,"context_line":"        mysql_engine\u003d\u0027InnoDB\u0027,"},{"line_number":53,"context_line":"        mysql_charset\u003d\u0027utf8\u0027"},{"line_number":54,"context_line":"    )"}],"source_content_type":"text/x-python","patch_set":17,"id":"bd323fe7_071ed8c7","line":51,"updated":"2022-10-10 17:46:09.000000000","message":"I don\u0027t see a test for these being created, its probably worth a test.","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"ed59d0793ec8705940973572bcb578054152c11c","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        sa.Index(\u0027share_id\u0027, \u0027share_id\u0027),"},{"line_number":49,"context_line":"        sa.Index("},{"line_number":50,"context_line":"            \u0027share_mapping_instance_uuid_share_id_idx\u0027,"},{"line_number":51,"context_line":"            \u0027instance_uuid\u0027, \u0027share_id\u0027),"},{"line_number":52,"context_line":"        mysql_engine\u003d\u0027InnoDB\u0027,"},{"line_number":53,"context_line":"        mysql_charset\u003d\u0027utf8\u0027"},{"line_number":54,"context_line":"    )"}],"source_content_type":"text/x-python","patch_set":17,"id":"f32ed385_c19d8a4f","line":51,"in_reply_to":"bd323fe7_071ed8c7","updated":"2023-02-02 14:52:09.000000000","message":"Done","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"eb937a00b65790f5361cd0d0f206e1624640e500","unresolved":true,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":"# revision identifiers, used by Alembic."},{"line_number":25,"context_line":"revision \u003d \u002713863f4e1612\u0027"},{"line_number":26,"context_line":"down_revision \u003d \u0027960aac0e09ea\u0027"},{"line_number":27,"context_line":"branch_labels \u003d None"},{"line_number":28,"context_line":"depends_on \u003d None"},{"line_number":29,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"2e51eae6_cf4ccad1","line":26,"range":{"start_line":26,"start_character":17,"end_line":26,"end_character":29},"updated":"2023-02-08 16:15:15.000000000","message":"seems legit :\n\nhttps://github.com/openstack/nova/commits/master/nova/db/main/migrations/versions last commit provides nova/db/main/migrations/versions/960aac0e09ea_de_duplicate_indexes_in_instances__.py","commit_id":"a484fc5434bafdb06887244f7e5e224f3845fd39"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"35d11a1b23d48af51bd5e932d624190e237e7a13","unresolved":false,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":"# revision identifiers, used by Alembic."},{"line_number":25,"context_line":"revision \u003d \u002713863f4e1612\u0027"},{"line_number":26,"context_line":"down_revision \u003d \u0027960aac0e09ea\u0027"},{"line_number":27,"context_line":"branch_labels \u003d None"},{"line_number":28,"context_line":"depends_on \u003d None"},{"line_number":29,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"c19aec18_2f46fa97","line":26,"range":{"start_line":26,"start_character":17,"end_line":26,"end_character":29},"in_reply_to":"2e51eae6_cf4ccad1","updated":"2023-04-14 09:48:02.000000000","message":"Ack","commit_id":"a484fc5434bafdb06887244f7e5e224f3845fd39"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"eb937a00b65790f5361cd0d0f206e1624640e500","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        sa.Column(\u0027id\u0027, sa.Integer, primary_key\u003dTrue, nullable\u003dFalse),"},{"line_number":37,"context_line":"        sa.Column(\u0027uuid\u0027, sa.String(36)),"},{"line_number":38,"context_line":"        sa.Column("},{"line_number":39,"context_line":"            \u0027instance_uuid\u0027, sa.String(length\u003d36),"},{"line_number":40,"context_line":"            sa.ForeignKey("},{"line_number":41,"context_line":"                \u0027instances.uuid\u0027,"},{"line_number":42,"context_line":"                name\u003d\u0027share_mapping_instance_uuid_fkey\u0027)),"}],"source_content_type":"text/x-python","patch_set":22,"id":"2b47916c_cd90678d","line":39,"range":{"start_line":39,"start_character":46,"end_line":39,"end_character":48},"updated":"2023-02-08 16:15:15.000000000","message":"classic UUID length, lgtm.","commit_id":"a484fc5434bafdb06887244f7e5e224f3845fd39"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"eb937a00b65790f5361cd0d0f206e1624640e500","unresolved":false,"context_lines":[{"line_number":40,"context_line":"            sa.ForeignKey("},{"line_number":41,"context_line":"                \u0027instances.uuid\u0027,"},{"line_number":42,"context_line":"                name\u003d\u0027share_mapping_instance_uuid_fkey\u0027)),"},{"line_number":43,"context_line":"        sa.Column(\u0027share_id\u0027, sa.String(length\u003d36)),"},{"line_number":44,"context_line":"        sa.Column(\u0027status\u0027, sa.String(length\u003d32)),"},{"line_number":45,"context_line":"        sa.Column(\u0027tag\u0027, sa.String(48)),"},{"line_number":46,"context_line":"        sa.Column(\u0027export_location\u0027, sa.String(255)),"}],"source_content_type":"text/x-python","patch_set":22,"id":"583287c3_5108b372","line":43,"range":{"start_line":43,"start_character":40,"end_line":43,"end_character":46},"updated":"2023-02-08 16:15:15.000000000","message":"nit: unnecessary","commit_id":"a484fc5434bafdb06887244f7e5e224f3845fd39"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"eb937a00b65790f5361cd0d0f206e1624640e500","unresolved":false,"context_lines":[{"line_number":42,"context_line":"                name\u003d\u0027share_mapping_instance_uuid_fkey\u0027)),"},{"line_number":43,"context_line":"        sa.Column(\u0027share_id\u0027, sa.String(length\u003d36)),"},{"line_number":44,"context_line":"        sa.Column(\u0027status\u0027, sa.String(length\u003d32)),"},{"line_number":45,"context_line":"        sa.Column(\u0027tag\u0027, sa.String(48)),"},{"line_number":46,"context_line":"        sa.Column(\u0027export_location\u0027, sa.String(255)),"},{"line_number":47,"context_line":"        sa.Column(\u0027share_proto\u0027, sa.String(32)),"},{"line_number":48,"context_line":"        sa.Index(\u0027share_id\u0027, \u0027share_id\u0027),"}],"source_content_type":"text/x-python","patch_set":22,"id":"28ba13b5_448f7687","line":45,"range":{"start_line":45,"start_character":35,"end_line":45,"end_character":37},"updated":"2023-02-08 16:15:15.000000000","message":"mmm, OK. Not sure about the reason of that length, but fair.","commit_id":"a484fc5434bafdb06887244f7e5e224f3845fd39"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"eb937a00b65790f5361cd0d0f206e1624640e500","unresolved":true,"context_lines":[{"line_number":45,"context_line":"        sa.Column(\u0027tag\u0027, sa.String(48)),"},{"line_number":46,"context_line":"        sa.Column(\u0027export_location\u0027, sa.String(255)),"},{"line_number":47,"context_line":"        sa.Column(\u0027share_proto\u0027, sa.String(32)),"},{"line_number":48,"context_line":"        sa.Index(\u0027share_id\u0027, \u0027share_id\u0027),"},{"line_number":49,"context_line":"        sa.Index("},{"line_number":50,"context_line":"            \u0027share_mapping_instance_uuid_share_id_idx\u0027,"},{"line_number":51,"context_line":"            \u0027instance_uuid\u0027, \u0027share_id\u0027),"}],"source_content_type":"text/x-python","patch_set":22,"id":"16ea4d98_f5efce08","line":48,"range":{"start_line":48,"start_character":18,"end_line":48,"end_character":26},"updated":"2023-02-08 16:15:15.000000000","message":"your index will be litterally named share_id: \nhttps://docs.sqlalchemy.org/en/20/core/constraints.html#sqlalchemy.schema.Index\n\nWe usually postfix the index name by _idx, I wonder whether it could cause problems if we were dropping the index by its name.","commit_id":"a484fc5434bafdb06887244f7e5e224f3845fd39"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"439559fae9311695d03f1e0b59dc50f6986d81e5","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        sa.Column(\u0027tag\u0027, sa.String(48)),"},{"line_number":46,"context_line":"        sa.Column(\u0027export_location\u0027, sa.String(255)),"},{"line_number":47,"context_line":"        sa.Column(\u0027share_proto\u0027, sa.String(32)),"},{"line_number":48,"context_line":"        sa.Index(\u0027share_id\u0027, \u0027share_id\u0027),"},{"line_number":49,"context_line":"        sa.Index("},{"line_number":50,"context_line":"            \u0027share_mapping_instance_uuid_share_id_idx\u0027,"},{"line_number":51,"context_line":"            \u0027instance_uuid\u0027, \u0027share_id\u0027),"}],"source_content_type":"text/x-python","patch_set":22,"id":"3803b223_41d411c7","line":48,"range":{"start_line":48,"start_character":18,"end_line":48,"end_character":26},"in_reply_to":"16ea4d98_f5efce08","updated":"2023-02-23 10:17:02.000000000","message":"Done","commit_id":"a484fc5434bafdb06887244f7e5e224f3845fd39"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4f646a0ef9298a5922e0100617cfb0259a918f03","unresolved":true,"context_lines":[{"line_number":33,"context_line":"        \u0027share_mapping\u0027,"},{"line_number":34,"context_line":"        sa.Column(\u0027created_at\u0027, sa.DateTime),"},{"line_number":35,"context_line":"        sa.Column(\u0027updated_at\u0027, sa.DateTime),"},{"line_number":36,"context_line":"        sa.Column(\u0027id\u0027, sa.Integer, primary_key\u003dTrue, nullable\u003dFalse),"},{"line_number":37,"context_line":"        sa.Column(\u0027uuid\u0027, sa.String(36)),"},{"line_number":38,"context_line":"        sa.Column("},{"line_number":39,"context_line":"            \u0027instance_uuid\u0027, sa.String(length\u003d36),"}],"source_content_type":"text/x-python","patch_set":24,"id":"d1c4dcd3_804130bc","line":36,"updated":"2023-03-10 14:40:11.000000000","message":"We learned recently that this can overflow in a real deployment. We are planning to change all the id column types to sa.BigInteger. So I would suggest to create this table with BigInt in the first place.","commit_id":"33492f6d186724f0587a4ffdad6e4c2cec5e812f"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"83b0a086a417839f413b0543979b1ac1d38149d6","unresolved":false,"context_lines":[{"line_number":33,"context_line":"        \u0027share_mapping\u0027,"},{"line_number":34,"context_line":"        sa.Column(\u0027created_at\u0027, sa.DateTime),"},{"line_number":35,"context_line":"        sa.Column(\u0027updated_at\u0027, sa.DateTime),"},{"line_number":36,"context_line":"        sa.Column(\u0027id\u0027, sa.Integer, primary_key\u003dTrue, nullable\u003dFalse),"},{"line_number":37,"context_line":"        sa.Column(\u0027uuid\u0027, sa.String(36)),"},{"line_number":38,"context_line":"        sa.Column("},{"line_number":39,"context_line":"            \u0027instance_uuid\u0027, sa.String(length\u003d36),"}],"source_content_type":"text/x-python","patch_set":24,"id":"5af960cf_36ed8646","line":36,"in_reply_to":"796da508_b64584a1","updated":"2023-04-11 17:47:03.000000000","message":"Done","commit_id":"33492f6d186724f0587a4ffdad6e4c2cec5e812f"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"d6dbe825b4e7fa81bf5085e715c499b30ec6239b","unresolved":true,"context_lines":[{"line_number":33,"context_line":"        \u0027share_mapping\u0027,"},{"line_number":34,"context_line":"        sa.Column(\u0027created_at\u0027, sa.DateTime),"},{"line_number":35,"context_line":"        sa.Column(\u0027updated_at\u0027, sa.DateTime),"},{"line_number":36,"context_line":"        sa.Column(\u0027id\u0027, sa.Integer, primary_key\u003dTrue, nullable\u003dFalse),"},{"line_number":37,"context_line":"        sa.Column(\u0027uuid\u0027, sa.String(36)),"},{"line_number":38,"context_line":"        sa.Column("},{"line_number":39,"context_line":"            \u0027instance_uuid\u0027, sa.String(length\u003d36),"}],"source_content_type":"text/x-python","patch_set":24,"id":"796da508_b64584a1","line":36,"in_reply_to":"d1c4dcd3_804130bc","updated":"2023-03-13 09:42:42.000000000","message":"That\u0027s a good question. That being said, the problem was due to the instances table. Here, I wonder whether an operator could have this same issue for the mappings. I think that we would have other tables being modified before that one, but I understand your point : once this one is merged, changing the type would be difficult, so OK, let\u0027s do this by now, and if we say in the PTG that we don\u0027t need to modify all the tables, then fine if we still have a BigInteger here.","commit_id":"33492f6d186724f0587a4ffdad6e4c2cec5e812f"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"35d11a1b23d48af51bd5e932d624190e237e7a13","unresolved":true,"context_lines":[{"line_number":35,"context_line":"        sa.Column(\u0027updated_at\u0027, sa.DateTime),"},{"line_number":36,"context_line":"        sa.Column("},{"line_number":37,"context_line":"            \"id\","},{"line_number":38,"context_line":"            sa.BigInteger().with_variant(sa.Integer, \"sqlite\"),"},{"line_number":39,"context_line":"            primary_key\u003dTrue,"},{"line_number":40,"context_line":"            autoincrement\u003dTrue,"},{"line_number":41,"context_line":"            nullable\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":25,"id":"aa678eee_810b6261","line":38,"updated":"2023-04-14 09:48:02.000000000","message":"this looks good to me.","commit_id":"43da83ea74d0da569ad25ae3fb157b562aadf7a7"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"023dd59c6f129753db8f6b0a5408a35700bdceda","unresolved":false,"context_lines":[{"line_number":35,"context_line":"        sa.Column(\u0027updated_at\u0027, sa.DateTime),"},{"line_number":36,"context_line":"        sa.Column("},{"line_number":37,"context_line":"            \"id\","},{"line_number":38,"context_line":"            sa.BigInteger().with_variant(sa.Integer, \"sqlite\"),"},{"line_number":39,"context_line":"            primary_key\u003dTrue,"},{"line_number":40,"context_line":"            autoincrement\u003dTrue,"},{"line_number":41,"context_line":"            nullable\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":25,"id":"6e119ab4_b3b21d10","line":38,"in_reply_to":"aa678eee_810b6261","updated":"2023-05-23 09:40:49.000000000","message":"Ack","commit_id":"43da83ea74d0da569ad25ae3fb157b562aadf7a7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"747e45509dc6122aada2fc1b7e22a333a89d4e23","unresolved":true,"context_lines":[{"line_number":42,"context_line":"        ),"},{"line_number":43,"context_line":"        sa.Column(\u0027uuid\u0027, sa.String(36)),"},{"line_number":44,"context_line":"        sa.Column("},{"line_number":45,"context_line":"            \u0027instance_uuid\u0027, sa.String(length\u003d36),"},{"line_number":46,"context_line":"            sa.ForeignKey("},{"line_number":47,"context_line":"                \u0027instances.uuid\u0027,"},{"line_number":48,"context_line":"                name\u003d\u0027share_mapping_instance_uuid_fkey\u0027)),"}],"source_content_type":"text/x-python","patch_set":30,"id":"80d57ceb_dd6f637f","line":45,"updated":"2023-06-09 16:47:32.000000000","message":"I think using instance.id would be better here. I know we join on (string) uuid in a lot of places but I think it\u0027s better to use the IDs, personally.","commit_id":"aef34518a3625b4320fda6618039fe5138cf1b13"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"db880686c9e6818befe80811528ba254b3a402a2","unresolved":true,"context_lines":[{"line_number":42,"context_line":"        ),"},{"line_number":43,"context_line":"        sa.Column(\u0027uuid\u0027, sa.String(36)),"},{"line_number":44,"context_line":"        sa.Column("},{"line_number":45,"context_line":"            \u0027instance_uuid\u0027, sa.String(length\u003d36),"},{"line_number":46,"context_line":"            sa.ForeignKey("},{"line_number":47,"context_line":"                \u0027instances.uuid\u0027,"},{"line_number":48,"context_line":"                name\u003d\u0027share_mapping_instance_uuid_fkey\u0027)),"}],"source_content_type":"text/x-python","patch_set":30,"id":"ae8db7d0_e5c665ba","line":45,"in_reply_to":"80d57ceb_dd6f637f","updated":"2023-06-13 17:32:44.000000000","message":"Well, in the spec, we agreed on joining on instance.uuid [1] but I\u0027m open to the change, provided it doesn\u0027t impact too much of the series, because this patch at least is already overdue for a long time.\n\n[1] https://specs.openstack.org/openstack/nova-specs/specs/2023.2/approved/libvirt-virtiofs-attach-manila-shares.html#data-model-impact","commit_id":"aef34518a3625b4320fda6618039fe5138cf1b13"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"747e45509dc6122aada2fc1b7e22a333a89d4e23","unresolved":true,"context_lines":[{"line_number":49,"context_line":"        sa.Column(\u0027share_id\u0027, sa.String(length\u003d36)),"},{"line_number":50,"context_line":"        sa.Column(\u0027status\u0027, sa.String(length\u003d32)),"},{"line_number":51,"context_line":"        sa.Column(\u0027tag\u0027, sa.String(48)),"},{"line_number":52,"context_line":"        sa.Column(\u0027export_location\u0027, sa.Text),"},{"line_number":53,"context_line":"        sa.Column(\u0027share_proto\u0027, sa.String(32)),"},{"line_number":54,"context_line":"        sa.Index(\u0027share_idx\u0027, \u0027share_id\u0027),"},{"line_number":55,"context_line":"        sa.Index("}],"source_content_type":"text/x-python","patch_set":30,"id":"c21614fc_714cb25a","line":52,"updated":"2023-06-09 16:47:32.000000000","message":"Is this a JSON blob or something?","commit_id":"aef34518a3625b4320fda6618039fe5138cf1b13"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"db880686c9e6818befe80811528ba254b3a402a2","unresolved":true,"context_lines":[{"line_number":49,"context_line":"        sa.Column(\u0027share_id\u0027, sa.String(length\u003d36)),"},{"line_number":50,"context_line":"        sa.Column(\u0027status\u0027, sa.String(length\u003d32)),"},{"line_number":51,"context_line":"        sa.Column(\u0027tag\u0027, sa.String(48)),"},{"line_number":52,"context_line":"        sa.Column(\u0027export_location\u0027, sa.Text),"},{"line_number":53,"context_line":"        sa.Column(\u0027share_proto\u0027, sa.String(32)),"},{"line_number":54,"context_line":"        sa.Index(\u0027share_idx\u0027, \u0027share_id\u0027),"},{"line_number":55,"context_line":"        sa.Index("}],"source_content_type":"text/x-python","patch_set":30,"id":"89481eff_b73e82d7","line":52,"in_reply_to":"c21614fc_714cb25a","updated":"2023-06-13 17:32:44.000000000","message":"Nah it just can be a very long URL afaik...","commit_id":"aef34518a3625b4320fda6618039fe5138cf1b13"}],"nova/db/main/models.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fec05be1d3e7cb0f22502220be59218b635cfa3e","unresolved":true,"context_lines":[{"line_number":656,"context_line":""},{"line_number":657,"context_line":""},{"line_number":658,"context_line":"# TODO(stephenfin): Remove once we drop the security_groups field from the"},{"line_number":659,"context_line":"# Instance table. Until then, this is tied to the SecurityGroup table"},{"line_number":660,"context_line":""},{"line_number":661,"context_line":""},{"line_number":662,"context_line":"class ShareMapping(BASE, NovaBase):"}],"source_content_type":"text/x-python","patch_set":8,"id":"945b1667_cee12299","line":659,"updated":"2022-08-04 13:40:00.000000000","message":"This comment belong to SecurityGroupInstanceAssociation","commit_id":"9a553f252bf640144973312893732fb38cc8eeaa"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"78fbb79f89c299882a5da67cbd5c883944330b54","unresolved":false,"context_lines":[{"line_number":656,"context_line":""},{"line_number":657,"context_line":""},{"line_number":658,"context_line":"# TODO(stephenfin): Remove once we drop the security_groups field from the"},{"line_number":659,"context_line":"# Instance table. Until then, this is tied to the SecurityGroup table"},{"line_number":660,"context_line":""},{"line_number":661,"context_line":""},{"line_number":662,"context_line":"class ShareMapping(BASE, NovaBase):"}],"source_content_type":"text/x-python","patch_set":8,"id":"93cbb07f_95526596","line":659,"in_reply_to":"945b1667_cee12299","updated":"2022-08-25 17:01:19.000000000","message":"Done","commit_id":"9a553f252bf640144973312893732fb38cc8eeaa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fec05be1d3e7cb0f22502220be59218b635cfa3e","unresolved":true,"context_lines":[{"line_number":670,"context_line":"    id \u003d sa.Column(sa.Integer, primary_key\u003dTrue, autoincrement\u003dTrue)"},{"line_number":671,"context_line":"    uuid \u003d sa.Column(sa.String(36))"},{"line_number":672,"context_line":"    instance_uuid \u003d sa.Column(sa.String(36), sa.ForeignKey(\u0027instances.uuid\u0027))"},{"line_number":673,"context_line":"    instance \u003d orm.relationship(\"Instance\", foreign_keys\u003dinstance_uuid,"},{"line_number":674,"context_line":"                            primaryjoin\u003d\u0027and_(ShareMapping.instance_uuid \u003d\u003d \u0027"},{"line_number":675,"context_line":"                                        \u0027Instance.uuid, Instance.deleted \u003d\u003d \u0027"},{"line_number":676,"context_line":"                                        \u00270)\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"713a4eeb_6d661419","line":673,"range":{"start_line":673,"start_character":31,"end_line":673,"end_character":33},"updated":"2022-08-04 13:40:00.000000000","message":"I would wrap the line here, after the (","commit_id":"9a553f252bf640144973312893732fb38cc8eeaa"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"78fbb79f89c299882a5da67cbd5c883944330b54","unresolved":false,"context_lines":[{"line_number":670,"context_line":"    id \u003d sa.Column(sa.Integer, primary_key\u003dTrue, autoincrement\u003dTrue)"},{"line_number":671,"context_line":"    uuid \u003d sa.Column(sa.String(36))"},{"line_number":672,"context_line":"    instance_uuid \u003d sa.Column(sa.String(36), sa.ForeignKey(\u0027instances.uuid\u0027))"},{"line_number":673,"context_line":"    instance \u003d orm.relationship(\"Instance\", foreign_keys\u003dinstance_uuid,"},{"line_number":674,"context_line":"                            primaryjoin\u003d\u0027and_(ShareMapping.instance_uuid \u003d\u003d \u0027"},{"line_number":675,"context_line":"                                        \u0027Instance.uuid, Instance.deleted \u003d\u003d \u0027"},{"line_number":676,"context_line":"                                        \u00270)\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"f49812bf_866f0ce5","line":673,"range":{"start_line":673,"start_character":31,"end_line":673,"end_character":33},"in_reply_to":"713a4eeb_6d661419","updated":"2022-08-25 17:01:19.000000000","message":"Done","commit_id":"9a553f252bf640144973312893732fb38cc8eeaa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fec05be1d3e7cb0f22502220be59218b635cfa3e","unresolved":true,"context_lines":[{"line_number":673,"context_line":"    instance \u003d orm.relationship(\"Instance\", foreign_keys\u003dinstance_uuid,"},{"line_number":674,"context_line":"                            primaryjoin\u003d\u0027and_(ShareMapping.instance_uuid \u003d\u003d \u0027"},{"line_number":675,"context_line":"                                        \u0027Instance.uuid, Instance.deleted \u003d\u003d \u0027"},{"line_number":676,"context_line":"                                        \u00270)\u0027)"},{"line_number":677,"context_line":"    share_id \u003d sa.Column(sa.String(36))"},{"line_number":678,"context_line":"    status \u003d sa.Column(sa.String(32))"},{"line_number":679,"context_line":"    tag \u003d sa.Column(sa.String(48))"}],"source_content_type":"text/x-python","patch_set":8,"id":"00727035_15f20a7b","line":676,"updated":"2022-08-04 13:40:00.000000000","message":"somebody with more sql and sqla knowledge should look at it","commit_id":"9a553f252bf640144973312893732fb38cc8eeaa"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6f0e00040d0b4b4fa7322a9cb524ece2440e2da7","unresolved":true,"context_lines":[{"line_number":666,"context_line":"    __table_args__ \u003d ("},{"line_number":667,"context_line":"        sa.Index(\u0027share_id\u0027, \u0027share_id\u0027),"},{"line_number":668,"context_line":"        sa.Index(\u0027share_mapping_instance_uuid_share_id_idx\u0027,"},{"line_number":669,"context_line":"              \u0027instance_uuid\u0027, \u0027share_id\u0027),"},{"line_number":670,"context_line":"    )"},{"line_number":671,"context_line":"    id \u003d sa.Column(sa.Integer, primary_key\u003dTrue, autoincrement\u003dTrue)"},{"line_number":672,"context_line":"    uuid \u003d sa.Column(sa.String(36))"}],"source_content_type":"text/x-python","patch_set":17,"id":"0f38456f_2d00eb9b","line":669,"updated":"2022-10-10 17:46:09.000000000","message":"So I think this needs to match the query on line 678, but it doesn\u0027t. I think the correct fix is to remove the instance.deleted \u003d\u003d 0 below.","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6f0e00040d0b4b4fa7322a9cb524ece2440e2da7","unresolved":true,"context_lines":[{"line_number":675,"context_line":"        \"Instance\","},{"line_number":676,"context_line":"        foreign_keys\u003dinstance_uuid,"},{"line_number":677,"context_line":"        primaryjoin\u003d\u0027and_(ShareMapping.instance_uuid \u003d\u003d Instance.uuid,\u0027"},{"line_number":678,"context_line":"                    \u0027Instance.deleted \u003d\u003d 0)\u0027"},{"line_number":679,"context_line":"    )"},{"line_number":680,"context_line":"    share_id \u003d sa.Column(sa.String(36))"},{"line_number":681,"context_line":"    status \u003d sa.Column(sa.String(32))"}],"source_content_type":"text/x-python","patch_set":17,"id":"a4a05997_ada7bb29","line":678,"updated":"2022-10-10 17:46:09.000000000","message":"Nit: Not sure we need the instance.deleted here, although I do see that used in quite a few of the models (shrugs). Probably just ignore me.","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"ea8d03e7d6001186b207fa8342a575f10730c239","unresolved":true,"context_lines":[{"line_number":675,"context_line":"        \"Instance\","},{"line_number":676,"context_line":"        foreign_keys\u003dinstance_uuid,"},{"line_number":677,"context_line":"        primaryjoin\u003d\u0027and_(ShareMapping.instance_uuid \u003d\u003d Instance.uuid,\u0027"},{"line_number":678,"context_line":"                    \u0027Instance.deleted \u003d\u003d 0)\u0027"},{"line_number":679,"context_line":"    )"},{"line_number":680,"context_line":"    share_id \u003d sa.Column(sa.String(36))"},{"line_number":681,"context_line":"    status \u003d sa.Column(sa.String(32))"}],"source_content_type":"text/x-python","patch_set":17,"id":"a8988dff_2c73c07c","line":678,"in_reply_to":"43da7f9c_a4b74e17","updated":"2023-03-24 08:45:36.000000000","message":"Yes, the assumption was confirmed with a test. If you delete an instance, the share_mapping attachment remains in the database.\nThe FK remains untouched as well.","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"ed59d0793ec8705940973572bcb578054152c11c","unresolved":true,"context_lines":[{"line_number":675,"context_line":"        \"Instance\","},{"line_number":676,"context_line":"        foreign_keys\u003dinstance_uuid,"},{"line_number":677,"context_line":"        primaryjoin\u003d\u0027and_(ShareMapping.instance_uuid \u003d\u003d Instance.uuid,\u0027"},{"line_number":678,"context_line":"                    \u0027Instance.deleted \u003d\u003d 0)\u0027"},{"line_number":679,"context_line":"    )"},{"line_number":680,"context_line":"    share_id \u003d sa.Column(sa.String(36))"},{"line_number":681,"context_line":"    status \u003d sa.Column(sa.String(32))"}],"source_content_type":"text/x-python","patch_set":17,"id":"d8535dce_0d34c1a7","line":678,"in_reply_to":"a4a05997_ada7bb29","updated":"2023-02-02 14:52:09.000000000","message":"Advices welcome.","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"35d11a1b23d48af51bd5e932d624190e237e7a13","unresolved":true,"context_lines":[{"line_number":675,"context_line":"        \"Instance\","},{"line_number":676,"context_line":"        foreign_keys\u003dinstance_uuid,"},{"line_number":677,"context_line":"        primaryjoin\u003d\u0027and_(ShareMapping.instance_uuid \u003d\u003d Instance.uuid,\u0027"},{"line_number":678,"context_line":"                    \u0027Instance.deleted \u003d\u003d 0)\u0027"},{"line_number":679,"context_line":"    )"},{"line_number":680,"context_line":"    share_id \u003d sa.Column(sa.String(36))"},{"line_number":681,"context_line":"    status \u003d sa.Column(sa.String(32))"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa5ac227_dc5eab11","line":678,"in_reply_to":"a8988dff_2c73c07c","updated":"2023-04-14 09:48:02.000000000","message":"So, then I assume we eventually delete the ShareMapping record by https://review.opendev.org/c/openstack/nova/+/839401/25/nova/objects/share_mapping.py#62\n\nBut my concern is, what happens if an user deletes an instance without previously detaching the share attachment ? Do we have any change that adds a verification in compute.api.py for an instance delete if we have a share mapping that we also need to remove ?","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"d6dbe825b4e7fa81bf5085e715c499b30ec6239b","unresolved":true,"context_lines":[{"line_number":675,"context_line":"        \"Instance\","},{"line_number":676,"context_line":"        foreign_keys\u003dinstance_uuid,"},{"line_number":677,"context_line":"        primaryjoin\u003d\u0027and_(ShareMapping.instance_uuid \u003d\u003d Instance.uuid,\u0027"},{"line_number":678,"context_line":"                    \u0027Instance.deleted \u003d\u003d 0)\u0027"},{"line_number":679,"context_line":"    )"},{"line_number":680,"context_line":"    share_id \u003d sa.Column(sa.String(36))"},{"line_number":681,"context_line":"    status \u003d sa.Column(sa.String(32))"}],"source_content_type":"text/x-python","patch_set":17,"id":"43da7f9c_a4b74e17","line":678,"in_reply_to":"d7337f99_6a958ea1","updated":"2023-03-13 09:42:42.000000000","message":"TBC, I still have this question. René, please try to verify about the deleted instances and the share mappings.","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"eb937a00b65790f5361cd0d0f206e1624640e500","unresolved":true,"context_lines":[{"line_number":675,"context_line":"        \"Instance\","},{"line_number":676,"context_line":"        foreign_keys\u003dinstance_uuid,"},{"line_number":677,"context_line":"        primaryjoin\u003d\u0027and_(ShareMapping.instance_uuid \u003d\u003d Instance.uuid,\u0027"},{"line_number":678,"context_line":"                    \u0027Instance.deleted \u003d\u003d 0)\u0027"},{"line_number":679,"context_line":"    )"},{"line_number":680,"context_line":"    share_id \u003d sa.Column(sa.String(36))"},{"line_number":681,"context_line":"    status \u003d sa.Column(sa.String(32))"}],"source_content_type":"text/x-python","patch_set":17,"id":"d7337f99_6a958ea1","line":678,"in_reply_to":"d8535dce_0d34c1a7","updated":"2023-02-08 16:15:15.000000000","message":"Here, the pattern is because this object isn\u0027t soft-deletable in the DB, AFAICU.\nGiven instances *are* soft-deletable records, this FK makes the record softly kept if the instance becomes soft-deleted, instead of having it deleted thru a cascade.\n\nThat means tho that we\u0027ll keep those records forever, unless someone cleans them up.\n\nDISCLAIMER : My whole theory could be wrong, but I don\u0027t see any SQLA parameter for saying about a delete cascade.","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"023dd59c6f129753db8f6b0a5408a35700bdceda","unresolved":false,"context_lines":[{"line_number":675,"context_line":"        \"Instance\","},{"line_number":676,"context_line":"        foreign_keys\u003dinstance_uuid,"},{"line_number":677,"context_line":"        primaryjoin\u003d\u0027and_(ShareMapping.instance_uuid \u003d\u003d Instance.uuid,\u0027"},{"line_number":678,"context_line":"                    \u0027Instance.deleted \u003d\u003d 0)\u0027"},{"line_number":679,"context_line":"    )"},{"line_number":680,"context_line":"    share_id \u003d sa.Column(sa.String(36))"},{"line_number":681,"context_line":"    status \u003d sa.Column(sa.String(32))"}],"source_content_type":"text/x-python","patch_set":17,"id":"f908abc1_2f84a9d2","line":678,"in_reply_to":"fa5ac227_dc5eab11","updated":"2023-05-23 09:40:49.000000000","message":"Ack","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6f0e00040d0b4b4fa7322a9cb524ece2440e2da7","unresolved":true,"context_lines":[{"line_number":680,"context_line":"    share_id \u003d sa.Column(sa.String(36))"},{"line_number":681,"context_line":"    status \u003d sa.Column(sa.String(32))"},{"line_number":682,"context_line":"    tag \u003d sa.Column(sa.String(48))"},{"line_number":683,"context_line":"    export_location \u003d sa.Column(sa.String(255))"},{"line_number":684,"context_line":"    share_proto \u003d sa.Column(sa.String(32))"},{"line_number":685,"context_line":""},{"line_number":686,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"941b1b44_d32d5c5e","line":683,"updated":"2022-10-10 17:46:09.000000000","message":"Are we happy that is big enough? Do we use smalltext somewhere similar? A DB person might tell me that is bad.\n\nFor ceph, I think this can include lots of monitor ip addresses, it gets quite long I guess? I can\u0027t remember if that 255 utf8 characters or ascii.","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"439559fae9311695d03f1e0b59dc50f6986d81e5","unresolved":false,"context_lines":[{"line_number":680,"context_line":"    share_id \u003d sa.Column(sa.String(36))"},{"line_number":681,"context_line":"    status \u003d sa.Column(sa.String(32))"},{"line_number":682,"context_line":"    tag \u003d sa.Column(sa.String(48))"},{"line_number":683,"context_line":"    export_location \u003d sa.Column(sa.String(255))"},{"line_number":684,"context_line":"    share_proto \u003d sa.Column(sa.String(32))"},{"line_number":685,"context_line":""},{"line_number":686,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"4688e16a_8366aa6c","line":683,"in_reply_to":"03337e49_d6476af5","updated":"2023-02-23 10:17:02.000000000","message":"Done","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"ed59d0793ec8705940973572bcb578054152c11c","unresolved":true,"context_lines":[{"line_number":680,"context_line":"    share_id \u003d sa.Column(sa.String(36))"},{"line_number":681,"context_line":"    status \u003d sa.Column(sa.String(32))"},{"line_number":682,"context_line":"    tag \u003d sa.Column(sa.String(48))"},{"line_number":683,"context_line":"    export_location \u003d sa.Column(sa.String(255))"},{"line_number":684,"context_line":"    share_proto \u003d sa.Column(sa.String(32))"},{"line_number":685,"context_line":""},{"line_number":686,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"f859aae4_5fa8bd78","line":683,"in_reply_to":"941b1b44_d32d5c5e","updated":"2023-02-02 14:52:09.000000000","message":"This is a good comment.\n@bauzas, @sean-k-mooney[m] what do you think ?","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"eb937a00b65790f5361cd0d0f206e1624640e500","unresolved":true,"context_lines":[{"line_number":680,"context_line":"    share_id \u003d sa.Column(sa.String(36))"},{"line_number":681,"context_line":"    status \u003d sa.Column(sa.String(32))"},{"line_number":682,"context_line":"    tag \u003d sa.Column(sa.String(48))"},{"line_number":683,"context_line":"    export_location \u003d sa.Column(sa.String(255))"},{"line_number":684,"context_line":"    share_proto \u003d sa.Column(sa.String(32))"},{"line_number":685,"context_line":""},{"line_number":686,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"03337e49_d6476af5","line":683,"in_reply_to":"f859aae4_5fa8bd78","updated":"2023-02-08 16:15:15.000000000","message":"Good question, for our connection URLs, we usually have a TEXT length IIRC.\n\nExamples :\nhttps://github.com/openstack/nova/blob/49aa40394a4857a06191b05ea3b15913f328a8d0/nova/db/api/models.py#L147\nhttps://github.com/openstack/nova/blob/49aa40394a4857a06191b05ea3b15913f328a8d0/nova/db/api/models.py#L148\n\nIn Manila, I briefly looked at they also have a large number being set :\nhttps://github.com/openstack/manila/blob/f6f93ff9516f0ae24801d7b422c0e504bfebd0f5/manila/db/sqlalchemy/models.py#L448\n\nSo, yeah, I\u0027d rather prefer to have a TEXT length here now and this is a blocker for accepting this change, or we would require a costly DB migration to alter this column later if we\u0027re asked to.","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"eb937a00b65790f5361cd0d0f206e1624640e500","unresolved":true,"context_lines":[{"line_number":663,"context_line":"    \"\"\"Represents share / instance mapping.\"\"\""},{"line_number":664,"context_line":"    __tablename__ \u003d \"share_mapping\""},{"line_number":665,"context_line":"    __table_args__ \u003d ("},{"line_number":666,"context_line":"        sa.Index(\u0027share_id\u0027, \u0027share_id\u0027),"},{"line_number":667,"context_line":"        sa.Index(\u0027share_mapping_instance_uuid_share_id_idx\u0027,"},{"line_number":668,"context_line":"              \u0027instance_uuid\u0027, \u0027share_id\u0027),"},{"line_number":669,"context_line":"    )"}],"source_content_type":"text/x-python","patch_set":22,"id":"90de64b1_6426446f","line":666,"updated":"2023-02-08 16:15:15.000000000","message":"same concern about the index naming.","commit_id":"a484fc5434bafdb06887244f7e5e224f3845fd39"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"cac16ff94fea92fea063ffc1bf37e86a9d214708","unresolved":false,"context_lines":[{"line_number":663,"context_line":"    \"\"\"Represents share / instance mapping.\"\"\""},{"line_number":664,"context_line":"    __tablename__ \u003d \"share_mapping\""},{"line_number":665,"context_line":"    __table_args__ \u003d ("},{"line_number":666,"context_line":"        sa.Index(\u0027share_id\u0027, \u0027share_id\u0027),"},{"line_number":667,"context_line":"        sa.Index(\u0027share_mapping_instance_uuid_share_id_idx\u0027,"},{"line_number":668,"context_line":"              \u0027instance_uuid\u0027, \u0027share_id\u0027),"},{"line_number":669,"context_line":"    )"}],"source_content_type":"text/x-python","patch_set":22,"id":"4e4c01e8_aecaa143","line":666,"in_reply_to":"90de64b1_6426446f","updated":"2023-02-23 10:17:55.000000000","message":"Done","commit_id":"a484fc5434bafdb06887244f7e5e224f3845fd39"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"747e45509dc6122aada2fc1b7e22a333a89d4e23","unresolved":true,"context_lines":[{"line_number":661,"context_line":"# TODO(stephenfin): Remove once we drop the security_groups field from the"},{"line_number":662,"context_line":"# Instance table. Until then, this is tied to the SecurityGroup table"},{"line_number":663,"context_line":""},{"line_number":664,"context_line":""},{"line_number":665,"context_line":"class SecurityGroupInstanceAssociation(BASE, NovaBase, models.SoftDeleteMixin):"},{"line_number":666,"context_line":"    __tablename__ \u003d \u0027security_group_instance_association\u0027"},{"line_number":667,"context_line":"    __table_args__ \u003d ("}],"source_content_type":"text/x-python","patch_set":30,"id":"271a0784_ff2c4408","side":"PARENT","line":664,"updated":"2023-06-09 16:47:32.000000000","message":"Unrelated whitespace damage here.","commit_id":"971809b4d4db39e98d6534531e55d5b8fcde49cb"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"db880686c9e6818befe80811528ba254b3a402a2","unresolved":true,"context_lines":[{"line_number":661,"context_line":"# TODO(stephenfin): Remove once we drop the security_groups field from the"},{"line_number":662,"context_line":"# Instance table. Until then, this is tied to the SecurityGroup table"},{"line_number":663,"context_line":""},{"line_number":664,"context_line":""},{"line_number":665,"context_line":"class SecurityGroupInstanceAssociation(BASE, NovaBase, models.SoftDeleteMixin):"},{"line_number":666,"context_line":"    __tablename__ \u003d \u0027security_group_instance_association\u0027"},{"line_number":667,"context_line":"    __table_args__ \u003d ("}],"source_content_type":"text/x-python","patch_set":30,"id":"bdd5aa88_b1f85f58","side":"PARENT","line":664,"in_reply_to":"271a0784_ff2c4408","updated":"2023-06-13 17:32:44.000000000","message":"Oh shit, probably coming from a rebase.","commit_id":"971809b4d4db39e98d6534531e55d5b8fcde49cb"}],"nova/tests/unit/db/main/test_api.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fec05be1d3e7cb0f22502220be59218b635cfa3e","unresolved":true,"context_lines":[{"line_number":4179,"context_line":"        self.project_id \u003d \u0027fake\u0027"},{"line_number":4180,"context_line":"        self.context \u003d context.RequestContext(self.user_id, self.project_id)"},{"line_number":4181,"context_line":""},{"line_number":4182,"context_line":"    def _compare(self, share_mapping, expected):"},{"line_number":4183,"context_line":"        for key, value in expected.items():"},{"line_number":4184,"context_line":"            self.assertEqual(share_mapping[key], value)"},{"line_number":4185,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"f89e0ab5_a31abac4","line":4182,"updated":"2022-08-04 13:40:00.000000000","message":"I\u0027m not 100% sure but I think you can use share_mapping._mapping to get a dict representation of the result from the DB and then you can simply use self.assertEqual(share_mapping._mapping, expected)","commit_id":"9a553f252bf640144973312893732fb38cc8eeaa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4a8c113270f7f4b874ed2ffd41f289b25976bd1a","unresolved":false,"context_lines":[{"line_number":4179,"context_line":"        self.project_id \u003d \u0027fake\u0027"},{"line_number":4180,"context_line":"        self.context \u003d context.RequestContext(self.user_id, self.project_id)"},{"line_number":4181,"context_line":""},{"line_number":4182,"context_line":"    def _compare(self, share_mapping, expected):"},{"line_number":4183,"context_line":"        for key, value in expected.items():"},{"line_number":4184,"context_line":"            self.assertEqual(share_mapping[key], value)"},{"line_number":4185,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"cce94e43_f9b8226d","line":4182,"in_reply_to":"74e054a1_1e9bcb5a","updated":"2022-08-26 07:31:55.000000000","message":"OK, lets keep this as is.","commit_id":"9a553f252bf640144973312893732fb38cc8eeaa"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"78fbb79f89c299882a5da67cbd5c883944330b54","unresolved":true,"context_lines":[{"line_number":4179,"context_line":"        self.project_id \u003d \u0027fake\u0027"},{"line_number":4180,"context_line":"        self.context \u003d context.RequestContext(self.user_id, self.project_id)"},{"line_number":4181,"context_line":""},{"line_number":4182,"context_line":"    def _compare(self, share_mapping, expected):"},{"line_number":4183,"context_line":"        for key, value in expected.items():"},{"line_number":4184,"context_line":"            self.assertEqual(share_mapping[key], value)"},{"line_number":4185,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"74e054a1_1e9bcb5a","line":4182,"in_reply_to":"f89e0ab5_a31abac4","updated":"2022-08-25 17:01:19.000000000","message":"Unless if I\u0027m wrong, there is no share_mapping._mapping.\nA _as_dict() method can return a dict representation.\nBut so far, I would rather not change it as it will break the pattern already used in this file elsewhere.\nPlease let me know your thought about it.","commit_id":"9a553f252bf640144973312893732fb38cc8eeaa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fec05be1d3e7cb0f22502220be59218b635cfa3e","unresolved":true,"context_lines":[{"line_number":4186,"context_line":"    @staticmethod"},{"line_number":4187,"context_line":"    def create_test_data(ctxt):"},{"line_number":4188,"context_line":"        expected_share_mappings \u003d {"},{"line_number":4189,"context_line":"            u\u00271\u0027: {"},{"line_number":4190,"context_line":"                \u0027uuid\u0027: \u0027fake-uuid1\u0027,"},{"line_number":4191,"context_line":"                \u0027instance_uuid\u0027: \u0027fake-instance-uuid1\u0027,"},{"line_number":4192,"context_line":"                \u0027share_id\u0027: u\u00271\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffe7c808_3b710bd6","line":4189,"range":{"start_line":4189,"start_character":12,"end_line":4189,"end_character":13},"updated":"2022-08-04 13:40:00.000000000","message":"this feels strange, py3 str is unicode by default. can we drop this?","commit_id":"9a553f252bf640144973312893732fb38cc8eeaa"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"78fbb79f89c299882a5da67cbd5c883944330b54","unresolved":false,"context_lines":[{"line_number":4186,"context_line":"    @staticmethod"},{"line_number":4187,"context_line":"    def create_test_data(ctxt):"},{"line_number":4188,"context_line":"        expected_share_mappings \u003d {"},{"line_number":4189,"context_line":"            u\u00271\u0027: {"},{"line_number":4190,"context_line":"                \u0027uuid\u0027: \u0027fake-uuid1\u0027,"},{"line_number":4191,"context_line":"                \u0027instance_uuid\u0027: \u0027fake-instance-uuid1\u0027,"},{"line_number":4192,"context_line":"                \u0027share_id\u0027: u\u00271\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"5e16b921_e9b5e48c","line":4189,"range":{"start_line":4189,"start_character":12,"end_line":4189,"end_character":13},"in_reply_to":"ffe7c808_3b710bd6","updated":"2022-08-25 17:01:19.000000000","message":"Done","commit_id":"9a553f252bf640144973312893732fb38cc8eeaa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fec05be1d3e7cb0f22502220be59218b635cfa3e","unresolved":true,"context_lines":[{"line_number":4232,"context_line":"    def test_share_mapping_update(self):"},{"line_number":4233,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":4234,"context_line":"        expected_share_mappings \u003d ("},{"line_number":4235,"context_line":"            ShareMappingDBApiTestCase.create_test_data(ctxt)"},{"line_number":4236,"context_line":"        )"},{"line_number":4237,"context_line":""},{"line_number":4238,"context_line":"        share_mappings \u003d db.share_mapping_get_all(ctxt)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3c75adad_1a81f29e","line":4235,"updated":"2022-08-04 13:40:00.000000000","message":"nit: you can call staticmethods via self too. that would be shorter :)\n\n$ python3\nPython 3.10.5 (main, Jun  8 2022, 09:26:22) [GCC 11.3.0] on linux\nType \"help\", \"copyright\", \"credits\" or \"license\" for more information.\n\u003e\u003e\u003e class A:\n...     @staticmethod\n...     def t():\n...         print(\"foo\")\n... \n\u003e\u003e\u003e A.t\n\u003cfunction A.t at 0x7ff811cf1ea0\u003e\n\u003e\u003e\u003e A.t()\nfoo\n\u003e\u003e\u003e a \u003d A()\n\u003e\u003e\u003e a.t\n\u003cfunction A.t at 0x7ff811cf1ea0\u003e\n\u003e\u003e\u003e a.t()\nfoo\n\u003e\u003e\u003e","commit_id":"9a553f252bf640144973312893732fb38cc8eeaa"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"78fbb79f89c299882a5da67cbd5c883944330b54","unresolved":false,"context_lines":[{"line_number":4232,"context_line":"    def test_share_mapping_update(self):"},{"line_number":4233,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":4234,"context_line":"        expected_share_mappings \u003d ("},{"line_number":4235,"context_line":"            ShareMappingDBApiTestCase.create_test_data(ctxt)"},{"line_number":4236,"context_line":"        )"},{"line_number":4237,"context_line":""},{"line_number":4238,"context_line":"        share_mappings \u003d db.share_mapping_get_all(ctxt)"}],"source_content_type":"text/x-python","patch_set":8,"id":"29792b47_fe4e13e3","line":4235,"in_reply_to":"3c75adad_1a81f29e","updated":"2022-08-25 17:01:19.000000000","message":"Done","commit_id":"9a553f252bf640144973312893732fb38cc8eeaa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fec05be1d3e7cb0f22502220be59218b635cfa3e","unresolved":true,"context_lines":[{"line_number":4264,"context_line":"        )"},{"line_number":4265,"context_line":""},{"line_number":4266,"context_line":"        expected_share_mappings_after_update \u003d {"},{"line_number":4267,"context_line":"            u\u00271\u0027: {"},{"line_number":4268,"context_line":"                \u0027uuid\u0027: \u0027fake-uuid1\u0027,"},{"line_number":4269,"context_line":"                \u0027instance_uuid\u0027: \u0027fake-instance-uuid1\u0027,"},{"line_number":4270,"context_line":"                \u0027share_id\u0027: u\u00271\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"2dbbac91_cb45c1c0","line":4267,"range":{"start_line":4267,"start_character":12,"end_line":4267,"end_character":13},"updated":"2022-08-04 13:40:00.000000000","message":"ditto","commit_id":"9a553f252bf640144973312893732fb38cc8eeaa"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"78fbb79f89c299882a5da67cbd5c883944330b54","unresolved":false,"context_lines":[{"line_number":4264,"context_line":"        )"},{"line_number":4265,"context_line":""},{"line_number":4266,"context_line":"        expected_share_mappings_after_update \u003d {"},{"line_number":4267,"context_line":"            u\u00271\u0027: {"},{"line_number":4268,"context_line":"                \u0027uuid\u0027: \u0027fake-uuid1\u0027,"},{"line_number":4269,"context_line":"                \u0027instance_uuid\u0027: \u0027fake-instance-uuid1\u0027,"},{"line_number":4270,"context_line":"                \u0027share_id\u0027: u\u00271\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"a89014fb_845f2104","line":4267,"range":{"start_line":4267,"start_character":12,"end_line":4267,"end_character":13},"in_reply_to":"2dbbac91_cb45c1c0","updated":"2022-08-25 17:01:19.000000000","message":"Done","commit_id":"9a553f252bf640144973312893732fb38cc8eeaa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fec05be1d3e7cb0f22502220be59218b635cfa3e","unresolved":true,"context_lines":[{"line_number":5927,"context_line":"            if table_name in ["},{"line_number":5928,"context_line":"                \u0027tags\u0027, \u0027resource_providers\u0027, \u0027allocations\u0027,"},{"line_number":5929,"context_line":"                \u0027inventories\u0027, \u0027resource_provider_aggregates\u0027,"},{"line_number":5930,"context_line":"                \u0027console_auth_tokens\u0027, \u0027share_mapping\u0027,"},{"line_number":5931,"context_line":"            ]:"},{"line_number":5932,"context_line":"                continue"},{"line_number":5933,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"39416459_4698f5e0","line":5930,"range":{"start_line":5930,"start_character":40,"end_line":5930,"end_character":53},"updated":"2022-08-04 13:40:00.000000000","message":"I think we want shadow table for share_mapping","commit_id":"9a553f252bf640144973312893732fb38cc8eeaa"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"78fbb79f89c299882a5da67cbd5c883944330b54","unresolved":false,"context_lines":[{"line_number":5927,"context_line":"            if table_name in ["},{"line_number":5928,"context_line":"                \u0027tags\u0027, \u0027resource_providers\u0027, \u0027allocations\u0027,"},{"line_number":5929,"context_line":"                \u0027inventories\u0027, \u0027resource_provider_aggregates\u0027,"},{"line_number":5930,"context_line":"                \u0027console_auth_tokens\u0027, \u0027share_mapping\u0027,"},{"line_number":5931,"context_line":"            ]:"},{"line_number":5932,"context_line":"                continue"},{"line_number":5933,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"179d09f1_645126be","line":5930,"range":{"start_line":5930,"start_character":40,"end_line":5930,"end_character":53},"in_reply_to":"39416459_4698f5e0","updated":"2022-08-25 17:01:19.000000000","message":"See comment shadow mapping is deprecated now.","commit_id":"9a553f252bf640144973312893732fb38cc8eeaa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fec05be1d3e7cb0f22502220be59218b635cfa3e","unresolved":true,"context_lines":[{"line_number":6156,"context_line":""},{"line_number":6157,"context_line":"    def _test_archive_deleted_rows_for_one_uuid_table(self, tablename):"},{"line_number":6158,"context_line":"        \"\"\":returns: 0 on success, 1 if no uuid column, 2 if insert failed.\"\"\""},{"line_number":6159,"context_line":"        # NOTE(cdent): migration 314 adds the resource_providers"},{"line_number":6160,"context_line":"        # table with a uuid column that does not archive, so skip."},{"line_number":6161,"context_line":"        skip_tables \u003d [\u0027resource_providers\u0027, \u0027share_mapping\u0027]"},{"line_number":6162,"context_line":"        if tablename in skip_tables:"},{"line_number":6163,"context_line":"            return 1"},{"line_number":6164,"context_line":"        main_table \u003d sqlalchemyutils.get_table(self.engine, tablename)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3dbc5546_94735fd0","line":6161,"range":{"start_line":6159,"start_character":0,"end_line":6161,"end_character":61},"updated":"2022-08-04 13:40:00.000000000","message":"I guess we want to have archiving for share_mapping","commit_id":"9a553f252bf640144973312893732fb38cc8eeaa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"aab81123d9d95f32d2b5a4be75fda45b60306c21","unresolved":false,"context_lines":[{"line_number":6156,"context_line":""},{"line_number":6157,"context_line":"    def _test_archive_deleted_rows_for_one_uuid_table(self, tablename):"},{"line_number":6158,"context_line":"        \"\"\":returns: 0 on success, 1 if no uuid column, 2 if insert failed.\"\"\""},{"line_number":6159,"context_line":"        # NOTE(cdent): migration 314 adds the resource_providers"},{"line_number":6160,"context_line":"        # table with a uuid column that does not archive, so skip."},{"line_number":6161,"context_line":"        skip_tables \u003d [\u0027resource_providers\u0027, \u0027share_mapping\u0027]"},{"line_number":6162,"context_line":"        if tablename in skip_tables:"},{"line_number":6163,"context_line":"            return 1"},{"line_number":6164,"context_line":"        main_table \u003d sqlalchemyutils.get_table(self.engine, tablename)"}],"source_content_type":"text/x-python","patch_set":8,"id":"8adc2fe1_9cc26e87","line":6161,"range":{"start_line":6159,"start_character":0,"end_line":6161,"end_character":61},"in_reply_to":"3dbc5546_94735fd0","updated":"2022-08-08 09:47:03.000000000","message":"I guessed wrong. in mitaka we deprecated soft delete. So we will hard delete rows from any new tables.","commit_id":"9a553f252bf640144973312893732fb38cc8eeaa"}],"nova/tests/unit/db/main/test_migrations.py":[{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"f661721273311810334f9c0c8b72dd8e5b8b3ab4","unresolved":true,"context_lines":[{"line_number":41,"context_line":"import sqlalchemy as sa"},{"line_number":42,"context_line":"import sqlalchemy.exc"},{"line_number":43,"context_line":"from sqlalchemy import Table"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"from nova.db.main import models"},{"line_number":47,"context_line":"from nova.db import migration"}],"source_content_type":"text/x-python","patch_set":12,"id":"c5d83be8_bd96d98f","line":44,"updated":"2022-08-28 06:26:58.000000000","message":"Extra blank line.","commit_id":"e4f3389038d8426e606a4ea36e7d64cb677c3baf"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"ed59d0793ec8705940973572bcb578054152c11c","unresolved":false,"context_lines":[{"line_number":41,"context_line":"import sqlalchemy as sa"},{"line_number":42,"context_line":"import sqlalchemy.exc"},{"line_number":43,"context_line":"from sqlalchemy import Table"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"from nova.db.main import models"},{"line_number":47,"context_line":"from nova.db import migration"}],"source_content_type":"text/x-python","patch_set":12,"id":"a976c5a3_157c31d9","line":44,"in_reply_to":"c5d83be8_bd96d98f","updated":"2023-02-02 14:52:09.000000000","message":"Done","commit_id":"e4f3389038d8426e606a4ea36e7d64cb677c3baf"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6f0e00040d0b4b4fa7322a9cb524ece2440e2da7","unresolved":true,"context_lines":[{"line_number":349,"context_line":""},{"line_number":350,"context_line":"        self.assertTableExists(connection, \u0027share_mapping\u0027)"},{"line_number":351,"context_line":"        for field in fields:"},{"line_number":352,"context_line":"            self.assertColumnExists(connection, \u0027share_mapping\u0027, field)"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"    def test_single_base_revision(self):"},{"line_number":355,"context_line":"        \"\"\"Ensure we only have a single base revision."}],"source_content_type":"text/x-python","patch_set":17,"id":"fe124316_a570c4bf","line":352,"updated":"2022-10-10 17:46:09.000000000","message":"Should we check for the foreign key constraint here? I guess there is an index or two we should check for as well?","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"ed59d0793ec8705940973572bcb578054152c11c","unresolved":false,"context_lines":[{"line_number":349,"context_line":""},{"line_number":350,"context_line":"        self.assertTableExists(connection, \u0027share_mapping\u0027)"},{"line_number":351,"context_line":"        for field in fields:"},{"line_number":352,"context_line":"            self.assertColumnExists(connection, \u0027share_mapping\u0027, field)"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"    def test_single_base_revision(self):"},{"line_number":355,"context_line":"        \"\"\"Ensure we only have a single base revision."}],"source_content_type":"text/x-python","patch_set":17,"id":"fda7502f_32c3c481","line":352,"in_reply_to":"fe124316_a570c4bf","updated":"2023-02-02 14:52:09.000000000","message":"Done","commit_id":"116e2ec7bd3749008e6e956190dcdfe6b2bd14a7"}]}