)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"b7b1cab9e9127dca83201bf94d10ed65d4cf162e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"1f1429c8_6dfe27e9","updated":"2023-02-22 16:24:32.000000000","message":"Thanks, question inline regarding the joined loading of the share servers model","commit_id":"b57fd4608d268507f3b4505967d7fc65c0449e90"},{"author":{"_account_id":30025,"name":"Vida Haririan","display_name":"Vida Haririan","email":"vhariria@redhat.com","username":"vhari"},"change_message_id":"2c3c8e5851ed12f336d58e459d53fdbba83b8d20","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"3b866f73_164626a5","updated":"2023-03-03 17:47:10.000000000","message":"LGTM, barring verification test.","commit_id":"f62cbd6c743e8fd17677c2eeeb41fdfa06d22c95"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"c4eee53687419422cafa9b42e3bc198eb1ccc33a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"5e179e21_48838ad8","updated":"2023-03-01 19:16:44.000000000","message":"Thanks for the reviews and please take a look in the proposed solution.","commit_id":"f62cbd6c743e8fd17677c2eeeb41fdfa06d22c95"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"793e252d9e64514740ecb68805344cce1f58831d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"82bbe9d6_f92472cd","updated":"2023-03-01 22:25:34.000000000","message":"Thanks, LGTM now.. looks like we need to modify tempest tests to get this to merge. ","commit_id":"f62cbd6c743e8fd17677c2eeeb41fdfa06d22c95"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"b98f34b4eeb9280a4a09fe92486946e82cddd771","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"3e0bc3b9_14781484","updated":"2024-01-26 05:04:11.000000000","message":"LGTM\n\nrecheck\n\nlogs need refresh","commit_id":"2b52056721297b494a1d09b773b3d48666cc159f"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"c7441a397b4cfa42969d49ae2abb009bf0079720","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"b2b64e66_e14838d6","updated":"2023-08-21 17:50:36.000000000","message":"Looks like some more test adjustments are needed; we have merged a few more tests now with potential issues","commit_id":"2b52056721297b494a1d09b773b3d48666cc159f"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"d92f3b7b9a0a6d672a220f97fe58b2a086af20c1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"5a90e756_fe7be6d3","updated":"2023-08-17 21:33:59.000000000","message":"recheck\n\nlvm driver job failed on a couple of nova VM readiness issues","commit_id":"2b52056721297b494a1d09b773b3d48666cc159f"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"5c7c6abeb568efcf169072ce7c06400af28d5a9f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"f3bfd33c_f7956dce","updated":"2023-08-17 17:13:24.000000000","message":"recheck\n\nrefreshing CI","commit_id":"2b52056721297b494a1d09b773b3d48666cc159f"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"6497b7109b33c9e56a75d51cb02ba056a54b1530","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"638a75b8_04b03a1d","updated":"2024-04-23 22:24:23.000000000","message":"recheck\n\nthis is starting to look like deja vu","commit_id":"2b52056721297b494a1d09b773b3d48666cc159f"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"49b3758be67913d4ffc7cbe0b46da8948ddf80c0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"5017a6f3_232acd15","updated":"2023-10-16 17:14:42.000000000","message":"recheck\nlogs are gone","commit_id":"2b52056721297b494a1d09b773b3d48666cc159f"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"9d197dfcf7f9ab62c201239eccdc4010f7f744c3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"30cc74e7_c9ebeb9e","updated":"2024-04-23 20:16:19.000000000","message":"this has waited for long; lets get this in to improve gate stability","commit_id":"2b52056721297b494a1d09b773b3d48666cc159f"}],"manila/api/v2/share_network_subnets.py":[{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"793e252d9e64514740ecb68805344cce1f58831d","unresolved":true,"context_lines":[{"line_number":89,"context_line":"                LOG.error(msg)"},{"line_number":90,"context_line":"                raise exc.HTTPConflict(explanation\u003dmsg)"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"            share_groups \u003d db_api.share_group_get_all_by_share_server("},{"line_number":93,"context_line":"                context, share_server[\u0027id\u0027])"},{"line_number":94,"context_line":"            if share_groups:"},{"line_number":95,"context_line":"                msg \u003d _(\"Cannot delete share network subnet %(id)s, it has \""},{"line_number":96,"context_line":"                        \"one or more share groups.\") % {"},{"line_number":97,"context_line":"                            \u0027id\u0027: share_network_subnet_id}"},{"line_number":98,"context_line":"                LOG.error(msg)"},{"line_number":99,"context_line":"                raise exc.HTTPConflict(explanation\u003dmsg)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"        # NOTE(silvacarlose): Do not allow the deletion of any share server"},{"line_number":102,"context_line":"        # if any of them has the flag is_auto_deletable \u003d False"}],"source_content_type":"text/x-python","patch_set":3,"id":"dc0ffd76_8630f4fc","line":99,"range":{"start_line":92,"start_character":0,"end_line":99,"end_character":55},"updated":"2023-03-01 22:25:34.000000000","message":"probably add a unit test for this?","commit_id":"f62cbd6c743e8fd17677c2eeeb41fdfa06d22c95"}],"manila/db/sqlalchemy/models.py":[{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"b7b1cab9e9127dca83201bf94d10ed65d4cf162e","unresolved":true,"context_lines":[{"line_number":1095,"context_line":""},{"line_number":1096,"context_line":"    share_groups \u003d orm.relationship("},{"line_number":1097,"context_line":"        \"ShareGroup\","},{"line_number":1098,"context_line":"        backref\u003d\u0027share_server\u0027,"},{"line_number":1099,"context_line":"        lazy\u003d\u0027joined\u0027,"},{"line_number":1100,"context_line":"        primaryjoin\u003d\u0027and_(\u0027"},{"line_number":1101,"context_line":"                    \u0027ShareServer.id \u003d\u003d ShareGroup.share_server_id,\u0027"},{"line_number":1102,"context_line":"                    \u0027ShareGroup.deleted \u003d\u003d \"False\")\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"34172481_21d37214","line":1099,"range":{"start_line":1098,"start_character":8,"end_line":1099,"end_character":22},"updated":"2023-02-22 16:24:32.000000000","message":"if we need this only for the query we\u0027re currently making, couldn\u0027t we just set it in the db query?","commit_id":"b57fd4608d268507f3b4505967d7fc65c0449e90"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"1a70721b80038cefe4d339bf69b21c377bbfa8ec","unresolved":true,"context_lines":[{"line_number":1095,"context_line":""},{"line_number":1096,"context_line":"    share_groups \u003d orm.relationship("},{"line_number":1097,"context_line":"        \"ShareGroup\","},{"line_number":1098,"context_line":"        backref\u003d\u0027share_server\u0027,"},{"line_number":1099,"context_line":"        lazy\u003d\u0027joined\u0027,"},{"line_number":1100,"context_line":"        primaryjoin\u003d\u0027and_(\u0027"},{"line_number":1101,"context_line":"                    \u0027ShareServer.id \u003d\u003d ShareGroup.share_server_id,\u0027"},{"line_number":1102,"context_line":"                    \u0027ShareGroup.deleted \u003d\u003d \"False\")\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"b4f97f31_83f7203b","line":1099,"range":{"start_line":1098,"start_character":8,"end_line":1099,"end_character":22},"in_reply_to":"34172481_21d37214","updated":"2023-02-24 17:42:41.000000000","message":"we are not querying share servers directly... Instead, we are:\n- getting share servers in a subnet\n- fetching through the share servers\n- trying to get the share group for each share server\nI was trying to avoid doing a db call for each share server, so that\u0027s why we have this here.\nIf we were querying the share server directly, then we could have a joinedload in the query.  I might be missing something though :)","commit_id":"b57fd4608d268507f3b4505967d7fc65c0449e90"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"29b266f324d46c00aa2a543a12e79688192db2d4","unresolved":true,"context_lines":[{"line_number":1095,"context_line":""},{"line_number":1096,"context_line":"    share_groups \u003d orm.relationship("},{"line_number":1097,"context_line":"        \"ShareGroup\","},{"line_number":1098,"context_line":"        backref\u003d\u0027share_server\u0027,"},{"line_number":1099,"context_line":"        lazy\u003d\u0027joined\u0027,"},{"line_number":1100,"context_line":"        primaryjoin\u003d\u0027and_(\u0027"},{"line_number":1101,"context_line":"                    \u0027ShareServer.id \u003d\u003d ShareGroup.share_server_id,\u0027"},{"line_number":1102,"context_line":"                    \u0027ShareGroup.deleted \u003d\u003d \"False\")\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"e9026ea4_93d4c96f","line":1099,"range":{"start_line":1098,"start_character":8,"end_line":1099,"end_character":22},"in_reply_to":"b4f97f31_83f7203b","updated":"2023-02-24 19:43:08.000000000","message":"The approach is correct, but slows down all other share server queries because we\u0027re \nloading the following when querying share groups:\n - availability zones\n - share group snapshots\n - share snapshot instances\n - share snapshot instance export locations\n - share instances\n - snapshots\n - share snapshot instance access mapping\n ....\n\nSo essentially, the query is loading most of manila\u0027s DB :( \nThere are certainly inefficiencies here that can be resolved in the sqlalchemy query layer.. we\u0027ve sprinked joins in the models more than necessary over time.. I mean, sometimes it might make sense - you want shares and instances to load together always; but it feels like many others here have just been done for code conveniences, don\u0027t you think?\n\nso my suggestion is: if we\u0027re only using it in the API service query just now and have never needed it, we can fix sqlalchemy/api query [1] to load share groups instead selectively.\n\n[1] https://opendev.org/openstack/manila/src/commit/8de44f11d0cae35b0cc869919457e0f9c0bb5270/manila/db/sqlalchemy/api.py#L4628","commit_id":"b57fd4608d268507f3b4505967d7fc65c0449e90"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"a1d13a76a51657f4710be3d45367189d5024dcb8","unresolved":false,"context_lines":[{"line_number":1095,"context_line":""},{"line_number":1096,"context_line":"    share_groups \u003d orm.relationship("},{"line_number":1097,"context_line":"        \"ShareGroup\","},{"line_number":1098,"context_line":"        backref\u003d\u0027share_server\u0027,"},{"line_number":1099,"context_line":"        lazy\u003d\u0027joined\u0027,"},{"line_number":1100,"context_line":"        primaryjoin\u003d\u0027and_(\u0027"},{"line_number":1101,"context_line":"                    \u0027ShareServer.id \u003d\u003d ShareGroup.share_server_id,\u0027"},{"line_number":1102,"context_line":"                    \u0027ShareGroup.deleted \u003d\u003d \"False\")\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"f226d900_9251703c","line":1099,"range":{"start_line":1098,"start_character":8,"end_line":1099,"end_character":22},"in_reply_to":"e9026ea4_93d4c96f","updated":"2023-03-01 19:04:30.000000000","message":"It makes sense - thanks for sharing your thoughts. I was initially avoiding to send another db call but at the end it sounds better than loading most of the Manila database, so I modified that in the latest PS.","commit_id":"b57fd4608d268507f3b4505967d7fc65c0449e90"}]}