)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":33648,"name":"Nahim Alves de Souza","email":"nahimsouza@outlook.com","username":"nahimsouza"},"change_message_id":"41c6698eb05f4f2330762f58c936e7b9729e5946","unresolved":true,"context_lines":[{"line_number":14,"context_line":"the `netapp_network_type` and `netapp_vlan` must be set."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"If share network subnet does not have those metadata fields,"},{"line_number":17,"context_line":"the segmentation configuration keeps working as before, that\u0027s it,"},{"line_number":18,"context_line":"it is set according to the values provided by Manila network plugin."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Change-Id: I69731b187527d8d443893dcc94c604688d73e696"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"68a22e02_71ccee12","line":17,"range":{"start_line":17,"start_character":56,"end_line":17,"end_character":65},"updated":"2023-02-07 14:31:52.000000000","message":"nit: Did you mean \"that is\"?","commit_id":"7f86d02bf85249a9b3a1a18dab5b5e727be4b008"},{"author":{"_account_id":31721,"name":"Felipe Rodrigues","email":"felipefuty01@gmail.com","username":"felipefutty"},"change_message_id":"d727674b09e4abda4593e0ab589b90b3256f2e74","unresolved":false,"context_lines":[{"line_number":14,"context_line":"the `netapp_network_type` and `netapp_vlan` must be set."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"If share network subnet does not have those metadata fields,"},{"line_number":17,"context_line":"the segmentation configuration keeps working as before, that\u0027s it,"},{"line_number":18,"context_line":"it is set according to the values provided by Manila network plugin."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Change-Id: I69731b187527d8d443893dcc94c604688d73e696"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3ead2966_62e42dc5","line":17,"range":{"start_line":17,"start_character":56,"end_line":17,"end_character":65},"in_reply_to":"68a22e02_71ccee12","updated":"2023-02-15 02:48:35.000000000","message":"Done","commit_id":"7f86d02bf85249a9b3a1a18dab5b5e727be4b008"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"296df4f1ef648dc251e058cf1217fba0c17edf70","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"46ccfaa7_1047ec5f","updated":"2023-02-09 14:09:20.000000000","message":"Also, could you please resolve this merge conflict?\n","commit_id":"7f86d02bf85249a9b3a1a18dab5b5e727be4b008"},{"author":{"_account_id":33648,"name":"Nahim Alves de Souza","email":"nahimsouza@outlook.com","username":"nahimsouza"},"change_message_id":"41c6698eb05f4f2330762f58c936e7b9729e5946","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c654804a_1031778c","updated":"2023-02-07 14:31:52.000000000","message":"Overall, patch looks good. Please, just check Billy\u0027s comment and NetApp CI. Thanks, Felipe.","commit_id":"7f86d02bf85249a9b3a1a18dab5b5e727be4b008"},{"author":{"_account_id":8992,"name":"Billy Olsen","email":"billy.olsen@canonical.com","username":"billy-olsen"},"change_message_id":"8e00dd36e279094752112467a507485ff81fc068","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9683c13a_de34be95","updated":"2023-01-13 20:23:20.000000000","message":"Some comments inline regarding the implications of the property keys defined.","commit_id":"7f86d02bf85249a9b3a1a18dab5b5e727be4b008"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"3feeeb7597eb0256dcaa7206ba403bf1bc17cb15","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"04fc2a15_6e3e038c","updated":"2023-02-09 14:09:07.000000000","message":"Thanks for proposing this change - please take a look at the question inline.","commit_id":"7f86d02bf85249a9b3a1a18dab5b5e727be4b008"},{"author":{"_account_id":18816,"name":"Maurice Escher","display_name":"carthaca","email":"maurice.escher@sap.com","username":"mapocace"},"change_message_id":"a71bd9e091eaf66bc5480b119b4f931944cfd319","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e75d7eb3_59f6eab1","updated":"2023-02-09 16:47:55.000000000","message":"Thanks for this flexibility. Thinking out loud: Is there any other meaningful place where to document this feature other than the releasenotes?\n\nAnd see my inline notes.","commit_id":"7f86d02bf85249a9b3a1a18dab5b5e727be4b008"},{"author":{"_account_id":33301,"name":"Caique Mello","email":"caiquemellosbo@gmail.com","username":"MelloCaique"},"change_message_id":"b34f252aa9ff5cce3332f1ea7802b7b7b24ea61d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"895243a2_a57a426f","updated":"2023-02-06 14:35:52.000000000","message":"Thanks, Felipe. Besides Billy\u0027s comment, the rest LGTM.","commit_id":"7f86d02bf85249a9b3a1a18dab5b5e727be4b008"},{"author":{"_account_id":33648,"name":"Nahim Alves de Souza","email":"nahimsouza@outlook.com","username":"nahimsouza"},"change_message_id":"68e2b21a33dd493c05959af6fbddcc9e8f874f29","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9f55964c_f62be02c","updated":"2023-02-07 16:07:41.000000000","message":"run-NetApp CI","commit_id":"7f86d02bf85249a9b3a1a18dab5b5e727be4b008"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"a9e58508aa49ecdc4a712f8432d15e761c71ae38","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"135f9c2e_4288f6b5","updated":"2023-02-15 13:55:50.000000000","message":"Also, please fix the broken unit test and ensure NetApp CI is voting on this change :)","commit_id":"79d5d866be50625cf114fa583b3164231ce32c77"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"804aa6dd8941300d8c0d278b1b3ac8ad70de8c76","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"d133331b_5d832ff1","updated":"2023-02-15 13:55:10.000000000","message":"Looking good - thank you for working on this feature.","commit_id":"79d5d866be50625cf114fa583b3164231ce32c77"},{"author":{"_account_id":31721,"name":"Felipe Rodrigues","email":"felipefuty01@gmail.com","username":"felipefutty"},"change_message_id":"d727674b09e4abda4593e0ab589b90b3256f2e74","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"4eb183b1_7ce6536b","updated":"2023-02-15 02:48:35.000000000","message":"Thannks billy, carlos and carthaca. Fixed in the newest patchset.","commit_id":"79d5d866be50625cf114fa583b3164231ce32c77"},{"author":{"_account_id":31721,"name":"Felipe Rodrigues","email":"felipefuty01@gmail.com","username":"felipefutty"},"change_message_id":"6dc5d83b88327d4095625cd6b45af7496c3e8337","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"408cc759_2ac66b0a","updated":"2023-02-16 11:42:47.000000000","message":"recheck failed by overloaded CI","commit_id":"65c5c84e4cabd9a0469e22a10cc51e1da944ee08"},{"author":{"_account_id":31721,"name":"Felipe Rodrigues","email":"felipefuty01@gmail.com","username":"felipefutty"},"change_message_id":"f47fd00f42f63c0566cf886f763c0c92296a61ab","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"0c87b5f4_8065ff8c","updated":"2023-02-15 23:47:15.000000000","message":"run-NetApp CI","commit_id":"65c5c84e4cabd9a0469e22a10cc51e1da944ee08"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"0586f62e02e0a6d7c0d0ffc43b74da65cd513fe8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"4dc9392d_844ccc73","updated":"2023-02-23 19:27:13.000000000","message":"Hey Felipe, \n\nplease see my comments/suggestions inline","commit_id":"ccb6bccce67fb7deab33cb7afd122f44dca999af"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"c8c50da89e9b50759bc0ee3f7936cb75e05ebe11","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"4310f38f_1d20363f","updated":"2023-02-23 19:29:11.000000000","message":"I\u0027m hoping you were able to test this change - i don\u0027t think a fancy vendor ml2 plugin is necessary for this; we could test that whatever the user sets is honored no matter whether the network plugin has the data we care about... Does this make sense? ","commit_id":"ccb6bccce67fb7deab33cb7afd122f44dca999af"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"6e3c4918ab514b6564e6862907d916d0c5ea7386","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"db3480d4_71ef0c06","updated":"2023-02-23 17:26:22.000000000","message":"LGTM, thanks","commit_id":"ccb6bccce67fb7deab33cb7afd122f44dca999af"},{"author":{"_account_id":31721,"name":"Felipe Rodrigues","email":"felipefuty01@gmail.com","username":"felipefutty"},"change_message_id":"df913f2fb9fbd2aa2593c61c73a1d8db41b028b9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"2a925b81_a5053932","updated":"2023-02-17 22:37:43.000000000","message":"run-NetApp CI","commit_id":"ccb6bccce67fb7deab33cb7afd122f44dca999af"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"68f0dd4fcd2335b2fdb5279abad7d8ca07cc9654","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"e45f2cc6_07c4015b","updated":"2023-02-24 01:59:34.000000000","message":"LGTM, thanks Felipe","commit_id":"e2729946fc4860095eaa37e7bb47eaf692bb586b"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"5dfb43b48d9e8cccc9cd6fde873c873b00cf80eb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"e30df2db_74b4fdea","updated":"2023-02-24 06:35:54.000000000","message":"thanks for the changes; i think the error handling and the user message can come in a subsequent patch","commit_id":"e2729946fc4860095eaa37e7bb47eaf692bb586b"}],"manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py":[{"author":{"_account_id":8992,"name":"Billy Olsen","email":"billy.olsen@canonical.com","username":"billy-olsen"},"change_message_id":"8e00dd36e279094752112467a507485ff81fc068","unresolved":true,"context_lines":[{"line_number":46,"context_line":"CLUSTER_IPSPACES \u003d (\u0027Cluster\u0027, \u0027Default\u0027)"},{"line_number":47,"context_line":"SERVER_MIGRATE_SVM_DR \u003d \u0027svm_dr\u0027"},{"line_number":48,"context_line":"SERVER_MIGRATE_SVM_MIGRATE \u003d \u0027svm_migrate\u0027"},{"line_number":49,"context_line":"METADATA_NET_TYPE \u003d \u0027netapp_network_type\u0027"},{"line_number":50,"context_line":"METADATA_VLAN \u003d \u0027netapp_vlan\u0027"},{"line_number":51,"context_line":"METADATA_MTU \u003d \u0027netapp_mtu\u0027"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"class NetAppCmodeMultiSVMFileStorageLibrary("}],"source_content_type":"text/x-python","patch_set":2,"id":"1b513927_fadfa0e9","line":51,"range":{"start_line":49,"start_character":0,"end_line":51,"end_character":27},"updated":"2023-01-13 20:23:20.000000000","message":"AIUI, these are property keys that are intended to be exposed to tenants. Including the netapp_ prefix in these keys would expose cloud implementation details to the tenant - which seems undesirable from a cloud provider scenario.","commit_id":"7f86d02bf85249a9b3a1a18dab5b5e727be4b008"},{"author":{"_account_id":31721,"name":"Felipe Rodrigues","email":"felipefuty01@gmail.com","username":"felipefutty"},"change_message_id":"d727674b09e4abda4593e0ab589b90b3256f2e74","unresolved":true,"context_lines":[{"line_number":46,"context_line":"CLUSTER_IPSPACES \u003d (\u0027Cluster\u0027, \u0027Default\u0027)"},{"line_number":47,"context_line":"SERVER_MIGRATE_SVM_DR \u003d \u0027svm_dr\u0027"},{"line_number":48,"context_line":"SERVER_MIGRATE_SVM_MIGRATE \u003d \u0027svm_migrate\u0027"},{"line_number":49,"context_line":"METADATA_NET_TYPE \u003d \u0027netapp_network_type\u0027"},{"line_number":50,"context_line":"METADATA_VLAN \u003d \u0027netapp_vlan\u0027"},{"line_number":51,"context_line":"METADATA_MTU \u003d \u0027netapp_mtu\u0027"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"class NetAppCmodeMultiSVMFileStorageLibrary("}],"source_content_type":"text/x-python","patch_set":2,"id":"848ba154_b2c32801","line":51,"range":{"start_line":49,"start_character":0,"end_line":51,"end_character":27},"in_reply_to":"1b513927_fadfa0e9","updated":"2023-02-15 02:48:35.000000000","message":"I see. Replaced the \"netapp\" by \"set\". I don\u0027t want to configure as \"network_type\" or simply \"vlan\", since there is a low possibility of a miss behavior where the user sets the metadata just to save extra information of the subnet and it ends up configuring those net resources","commit_id":"7f86d02bf85249a9b3a1a18dab5b5e727be4b008"},{"author":{"_account_id":31721,"name":"Felipe Rodrigues","email":"felipefuty01@gmail.com","username":"felipefutty"},"change_message_id":"27a40ba66c47351b749f69426c793651c53b7b0e","unresolved":false,"context_lines":[{"line_number":46,"context_line":"CLUSTER_IPSPACES \u003d (\u0027Cluster\u0027, \u0027Default\u0027)"},{"line_number":47,"context_line":"SERVER_MIGRATE_SVM_DR \u003d \u0027svm_dr\u0027"},{"line_number":48,"context_line":"SERVER_MIGRATE_SVM_MIGRATE \u003d \u0027svm_migrate\u0027"},{"line_number":49,"context_line":"METADATA_NET_TYPE \u003d \u0027netapp_network_type\u0027"},{"line_number":50,"context_line":"METADATA_VLAN \u003d \u0027netapp_vlan\u0027"},{"line_number":51,"context_line":"METADATA_MTU \u003d \u0027netapp_mtu\u0027"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"class NetAppCmodeMultiSVMFileStorageLibrary("}],"source_content_type":"text/x-python","patch_set":2,"id":"1bc45ac8_a77503d6","line":51,"range":{"start_line":49,"start_character":0,"end_line":51,"end_character":27},"in_reply_to":"848ba154_b2c32801","updated":"2023-02-23 17:25:06.000000000","message":"Done","commit_id":"7f86d02bf85249a9b3a1a18dab5b5e727be4b008"},{"author":{"_account_id":18816,"name":"Maurice Escher","display_name":"carthaca","email":"maurice.escher@sap.com","username":"mapocace"},"change_message_id":"a71bd9e091eaf66bc5480b119b4f931944cfd319","unresolved":true,"context_lines":[{"line_number":177,"context_line":"                raise exception.NetworkBadConfigurationException("},{"line_number":178,"context_line":"                    reason\u003dmsg % metadata_vlan)"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"            network[\u0027network_type\u0027] \u003d metadata.get(METADATA_NET_TYPE)"},{"line_number":181,"context_line":"            network[\u0027segmentation_id\u0027] \u003d metadata_vlan"},{"line_number":182,"context_line":"            for allocation in network[\u0027network_allocations\u0027]:"},{"line_number":183,"context_line":"                allocation[\u0027network_type\u0027] \u003d metadata.get(METADATA_NET_TYPE)"}],"source_content_type":"text/x-python","patch_set":2,"id":"b2483479_a43340ae","line":180,"updated":"2023-02-09 16:47:55.000000000","message":"This needs some validation. Currently only \u0027vlan\u0027 is allowed, right?","commit_id":"7f86d02bf85249a9b3a1a18dab5b5e727be4b008"},{"author":{"_account_id":18816,"name":"Maurice Escher","display_name":"carthaca","email":"maurice.escher@sap.com","username":"mapocace"},"change_message_id":"a4a1b23e3e8f5f3a97210e4b3c419e2fba369083","unresolved":false,"context_lines":[{"line_number":177,"context_line":"                raise exception.NetworkBadConfigurationException("},{"line_number":178,"context_line":"                    reason\u003dmsg % metadata_vlan)"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"            network[\u0027network_type\u0027] \u003d metadata.get(METADATA_NET_TYPE)"},{"line_number":181,"context_line":"            network[\u0027segmentation_id\u0027] \u003d metadata_vlan"},{"line_number":182,"context_line":"            for allocation in network[\u0027network_allocations\u0027]:"},{"line_number":183,"context_line":"                allocation[\u0027network_type\u0027] \u003d metadata.get(METADATA_NET_TYPE)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7be10648_265319b2","line":180,"in_reply_to":"10611587_7b6c5753","updated":"2023-02-15 07:29:18.000000000","message":"Good","commit_id":"7f86d02bf85249a9b3a1a18dab5b5e727be4b008"},{"author":{"_account_id":31721,"name":"Felipe Rodrigues","email":"felipefuty01@gmail.com","username":"felipefutty"},"change_message_id":"d727674b09e4abda4593e0ab589b90b3256f2e74","unresolved":true,"context_lines":[{"line_number":177,"context_line":"                raise exception.NetworkBadConfigurationException("},{"line_number":178,"context_line":"                    reason\u003dmsg % metadata_vlan)"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"            network[\u0027network_type\u0027] \u003d metadata.get(METADATA_NET_TYPE)"},{"line_number":181,"context_line":"            network[\u0027segmentation_id\u0027] \u003d metadata_vlan"},{"line_number":182,"context_line":"            for allocation in network[\u0027network_allocations\u0027]:"},{"line_number":183,"context_line":"                allocation[\u0027network_type\u0027] \u003d metadata.get(METADATA_NET_TYPE)"}],"source_content_type":"text/x-python","patch_set":2,"id":"10611587_7b6c5753","line":180,"in_reply_to":"b2483479_a43340ae","updated":"2023-02-15 02:48:35.000000000","message":"The check is done by \"_validate_network_type\" right after this function call (see line 214). So we don\u0027t need to redo it here.","commit_id":"7f86d02bf85249a9b3a1a18dab5b5e727be4b008"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"3feeeb7597eb0256dcaa7206ba403bf1bc17cb15","unresolved":true,"context_lines":[{"line_number":182,"context_line":"            for allocation in network[\u0027network_allocations\u0027]:"},{"line_number":183,"context_line":"                allocation[\u0027network_type\u0027] \u003d metadata.get(METADATA_NET_TYPE)"},{"line_number":184,"context_line":"                allocation[\u0027segmentation_id\u0027] \u003d metadata_vlan"},{"line_number":185,"context_line":"                allocation[\u0027mtu\u0027] \u003d metadata.get(METADATA_MTU)"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    @na_utils.trace"},{"line_number":188,"context_line":"    def setup_server(self, network_info, metadata\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"827cdea1_ddcf9f57","line":185,"range":{"start_line":185,"start_character":36,"end_line":185,"end_character":62},"updated":"2023-02-09 14:09:07.000000000","message":"if metadata_mtu is not set and the one in the network_info is specified would we be ignoring the MTU set?\nShouldn\u0027t this be:\nallocation[\u0027mtu\u0027] \u003d metadata.get(METADATA_MTU, network.get(\u0027mtu\u0027))\n\nI know network.get(\u0027mtu\u0027) might not return something too, but was just thinking if we are not ignoring the value for MTU when not set in metadata.","commit_id":"7f86d02bf85249a9b3a1a18dab5b5e727be4b008"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"804aa6dd8941300d8c0d278b1b3ac8ad70de8c76","unresolved":false,"context_lines":[{"line_number":182,"context_line":"            for allocation in network[\u0027network_allocations\u0027]:"},{"line_number":183,"context_line":"                allocation[\u0027network_type\u0027] \u003d metadata.get(METADATA_NET_TYPE)"},{"line_number":184,"context_line":"                allocation[\u0027segmentation_id\u0027] \u003d metadata_vlan"},{"line_number":185,"context_line":"                allocation[\u0027mtu\u0027] \u003d metadata.get(METADATA_MTU)"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    @na_utils.trace"},{"line_number":188,"context_line":"    def setup_server(self, network_info, metadata\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"e5d27069_8418a9a6","line":185,"range":{"start_line":185,"start_character":36,"end_line":185,"end_character":62},"in_reply_to":"1fe06dae_7a125d7e","updated":"2023-02-15 13:55:10.000000000","message":"makes sense, thanks!","commit_id":"7f86d02bf85249a9b3a1a18dab5b5e727be4b008"},{"author":{"_account_id":18816,"name":"Maurice Escher","display_name":"carthaca","email":"maurice.escher@sap.com","username":"mapocace"},"change_message_id":"a71bd9e091eaf66bc5480b119b4f931944cfd319","unresolved":true,"context_lines":[{"line_number":182,"context_line":"            for allocation in network[\u0027network_allocations\u0027]:"},{"line_number":183,"context_line":"                allocation[\u0027network_type\u0027] \u003d metadata.get(METADATA_NET_TYPE)"},{"line_number":184,"context_line":"                allocation[\u0027segmentation_id\u0027] \u003d metadata_vlan"},{"line_number":185,"context_line":"                allocation[\u0027mtu\u0027] \u003d metadata.get(METADATA_MTU)"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    @na_utils.trace"},{"line_number":188,"context_line":"    def setup_server(self, network_info, metadata\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"b6055174_a99468b0","line":185,"range":{"start_line":185,"start_character":36,"end_line":185,"end_character":62},"in_reply_to":"827cdea1_ddcf9f57","updated":"2023-02-09 16:47:55.000000000","message":"... and this scenario should go right into a unit test :)\n\nAdditionally I think we need input validation for METADATA_MTU, too. At least make sure it is an integer.","commit_id":"7f86d02bf85249a9b3a1a18dab5b5e727be4b008"},{"author":{"_account_id":18816,"name":"Maurice Escher","display_name":"carthaca","email":"maurice.escher@sap.com","username":"mapocace"},"change_message_id":"a4a1b23e3e8f5f3a97210e4b3c419e2fba369083","unresolved":false,"context_lines":[{"line_number":182,"context_line":"            for allocation in network[\u0027network_allocations\u0027]:"},{"line_number":183,"context_line":"                allocation[\u0027network_type\u0027] \u003d metadata.get(METADATA_NET_TYPE)"},{"line_number":184,"context_line":"                allocation[\u0027segmentation_id\u0027] \u003d metadata_vlan"},{"line_number":185,"context_line":"                allocation[\u0027mtu\u0027] \u003d metadata.get(METADATA_MTU)"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    @na_utils.trace"},{"line_number":188,"context_line":"    def setup_server(self, network_info, metadata\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1fe06dae_7a125d7e","line":185,"range":{"start_line":185,"start_character":36,"end_line":185,"end_character":62},"in_reply_to":"9332ba75_42cbc4bf","updated":"2023-02-15 07:29:18.000000000","message":"Makes sense, thanks for the explanation","commit_id":"7f86d02bf85249a9b3a1a18dab5b5e727be4b008"},{"author":{"_account_id":31721,"name":"Felipe Rodrigues","email":"felipefuty01@gmail.com","username":"felipefutty"},"change_message_id":"d727674b09e4abda4593e0ab589b90b3256f2e74","unresolved":true,"context_lines":[{"line_number":182,"context_line":"            for allocation in network[\u0027network_allocations\u0027]:"},{"line_number":183,"context_line":"                allocation[\u0027network_type\u0027] \u003d metadata.get(METADATA_NET_TYPE)"},{"line_number":184,"context_line":"                allocation[\u0027segmentation_id\u0027] \u003d metadata_vlan"},{"line_number":185,"context_line":"                allocation[\u0027mtu\u0027] \u003d metadata.get(METADATA_MTU)"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    @na_utils.trace"},{"line_number":188,"context_line":"    def setup_server(self, network_info, metadata\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9332ba75_42cbc4bf","line":185,"range":{"start_line":185,"start_character":36,"end_line":185,"end_character":62},"in_reply_to":"b6055174_a99468b0","updated":"2023-02-15 02:48:35.000000000","message":"Not setting the mtu means: I don\u0027t want to change its value. Retrieving the value from \"network.get(\u0027mtu\u0027) means that the vlan is set manually but the mtu is set from network plugin. I think it is not desired. The goal of this feature is manually configure the network. If the operator wants to manually set the vlan, he/she should set the mtu too manually. If he/she does not set it, it means that they want to keep as the default one. This behavior will be documented (release note and netapp documentation).\n\n\nAbout the MTU integer check, I agree. Added to next patchset with its UT.","commit_id":"7f86d02bf85249a9b3a1a18dab5b5e727be4b008"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"0586f62e02e0a6d7c0d0ffc43b74da65cd513fe8","unresolved":false,"context_lines":[{"line_number":182,"context_line":"            for allocation in network[\u0027network_allocations\u0027]:"},{"line_number":183,"context_line":"                allocation[\u0027network_type\u0027] \u003d metadata.get(METADATA_NET_TYPE)"},{"line_number":184,"context_line":"                allocation[\u0027segmentation_id\u0027] \u003d metadata_vlan"},{"line_number":185,"context_line":"                allocation[\u0027mtu\u0027] \u003d metadata.get(METADATA_MTU)"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    @na_utils.trace"},{"line_number":188,"context_line":"    def setup_server(self, network_info, metadata\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"8327bd36_dc96ce26","line":185,"range":{"start_line":185,"start_character":36,"end_line":185,"end_character":62},"in_reply_to":"e5d27069_8418a9a6","updated":"2023-02-23 19:27:13.000000000","message":"It\u0027s nuance, it is definitely possible that the network plugin can give us the MTU we need to set - and if set, it is much more reliable than the DEFAULT_MTU that the driver has hardcoded - how would that be incorrect behavior? \n\n\" If the operator wants to manually set the vlan, he/she should set the mtu too manually. \" -- that\u0027d then be true of everything else that we\u0027re relying on neutron to provide!\n\n\nI don\u0027t want to hold up this change, but we could make the user experience slightly better by processing \"set_mtu\" selectively and only relying on \"set_vlan\". See comments in the latest patchset","commit_id":"7f86d02bf85249a9b3a1a18dab5b5e727be4b008"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"0586f62e02e0a6d7c0d0ffc43b74da65cd513fe8","unresolved":true,"context_lines":[{"line_number":152,"context_line":""},{"line_number":153,"context_line":"        for network in network_info:"},{"line_number":154,"context_line":"            metadata \u003d network.get(\u0027subnet_metadata\u0027)"},{"line_number":155,"context_line":"            if not metadata:"},{"line_number":156,"context_line":"                continue"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"            # metadata is only used if network_type or vlan or  mtu are set."},{"line_number":159,"context_line":"            if (METADATA_NET_TYPE not in metadata and"},{"line_number":160,"context_line":"                    METADATA_VLAN not in metadata and"},{"line_number":161,"context_line":"                    METADATA_MTU not in metadata):"},{"line_number":162,"context_line":"                continue"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"            if (METADATA_NET_TYPE not in metadata or"},{"line_number":165,"context_line":"                    METADATA_VLAN not in metadata):"},{"line_number":166,"context_line":"                msg \u003d _(\u0027The network configuration through metadata must \u0027"},{"line_number":167,"context_line":"                        \u0027contain both %s and %s fields.\u0027)"},{"line_number":168,"context_line":"                raise exception.NetworkBadConfigurationException("},{"line_number":169,"context_line":"                    reason\u003dmsg % (METADATA_NET_TYPE, METADATA_VLAN))"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"            metadata_vlan \u003d metadata.get(METADATA_VLAN)"},{"line_number":172,"context_line":"            if int(metadata_vlan) \u003e 4094 or int(metadata_vlan) \u003c 1:"},{"line_number":173,"context_line":"                msg \u003d _("},{"line_number":174,"context_line":"                    \u0027A segmentation ID %s was specified but is not valid for \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"4d6a164b_e2d32758","line":171,"range":{"start_line":155,"start_character":12,"end_line":171,"end_character":55},"updated":"2023-02-23 19:27:13.000000000","message":"IMHO, this is weird user experience on top of an unusual requirement that we\u0027re already dealing with. We\u0027re working around a problem with a vendor ML2 plugin that doesn\u0027t provide VLAN IDs via the API - and in that scenario, we want the cloud administrator to hand the VLAN ID out of band to a tenant user that\u0027s creating their share network. Great, we can live with that. Why would we force them to specify the network type here? We know it can\u0027t be anything other than VLAN especially if they have \"set_vlan\" as their metadata - VLAN is in the name. \n\nBesides, I responded to the earlier discussion between carloss/carthaca/you on the MTU.. I feel it\u0027d be incorrect not to fall back to the network plugin provided MTU. \nIMHO, our goal should be on reducing the number of extraneous data fields that a user needs to input:\n \n\n   metadata_vlan \u003d metadata.get(METADATA_VLAN)\n   if not metadata_vlan:\n       continue\n   elif int(metadata_vlan) \u003e 4094 or int(metadata_vlan) \u003c 1:\n       msg \u003d _(\n               \u0027A segmentation ID %s was specified but is not valid for \u0027\n               \u0027a VLAN network type; the segmentation ID must be an \u0027\n               \u0027integer value in the range of [1,4094]\u0027\n       )\n       raise exception.NetworkBadConfigurationException(\n           reason\u003dmsg % metadata_vlan)\n#\n       # PLEASE RAISE A USER MESSAGE HERE. Without it we\u0027re leaving it to the logs and have the user\u0027s share go to \"error\" - a head scratcher when they may have just made a typo! \n#   \n   network[\u0027network_type\u0027] \u003d \u0027vlan\u0027\n   network[\u0027segmentation_id\u0027] \u003d metadata_vlan\n   if metadata.get(METADATA_MTU) is not None:\n      try:\n         int(metadata.get(METADATA_MTU))\n      except ValueError:\n          msg \u003d _(\u0027Metadata network MTU must be an integer value.\u0027)\n          raise exception.NetworkBadConfigurationException(msg)\n   mtu \u003d metadata.get(METADATA_MTU) or network[\u0027mtu\u0027]\n   for allocation in network[\u0027network_allocations\u0027]:\n          allocation[\u0027network_type\u0027] \u003d \u0027vlan\u0027\n          allocation[\u0027segmentation_id\u0027] \u003d metadata_vlan\n          allocation[\u0027mtu\u0027] \u003d mtu","commit_id":"ccb6bccce67fb7deab33cb7afd122f44dca999af"},{"author":{"_account_id":31721,"name":"Felipe Rodrigues","email":"felipefuty01@gmail.com","username":"felipefutty"},"change_message_id":"1f4e80ad1885b252ddd3a9e6a05ef802ca999fb1","unresolved":true,"context_lines":[{"line_number":152,"context_line":""},{"line_number":153,"context_line":"        for network in network_info:"},{"line_number":154,"context_line":"            metadata \u003d network.get(\u0027subnet_metadata\u0027)"},{"line_number":155,"context_line":"            if not metadata:"},{"line_number":156,"context_line":"                continue"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"            # metadata is only used if network_type or vlan or  mtu are set."},{"line_number":159,"context_line":"            if (METADATA_NET_TYPE not in metadata and"},{"line_number":160,"context_line":"                    METADATA_VLAN not in metadata and"},{"line_number":161,"context_line":"                    METADATA_MTU not in metadata):"},{"line_number":162,"context_line":"                continue"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"            if (METADATA_NET_TYPE not in metadata or"},{"line_number":165,"context_line":"                    METADATA_VLAN not in metadata):"},{"line_number":166,"context_line":"                msg \u003d _(\u0027The network configuration through metadata must \u0027"},{"line_number":167,"context_line":"                        \u0027contain both %s and %s fields.\u0027)"},{"line_number":168,"context_line":"                raise exception.NetworkBadConfigurationException("},{"line_number":169,"context_line":"                    reason\u003dmsg % (METADATA_NET_TYPE, METADATA_VLAN))"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"            metadata_vlan \u003d metadata.get(METADATA_VLAN)"},{"line_number":172,"context_line":"            if int(metadata_vlan) \u003e 4094 or int(metadata_vlan) \u003c 1:"},{"line_number":173,"context_line":"                msg \u003d _("},{"line_number":174,"context_line":"                    \u0027A segmentation ID %s was specified but is not valid for \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"a62fb3fe_506102ff","line":171,"range":{"start_line":155,"start_character":12,"end_line":171,"end_character":55},"in_reply_to":"4d6a164b_e2d32758","updated":"2023-02-24 01:31:11.000000000","message":"Thank you goutham for the review. As we discussed in the channel. I submitted a new patchset removing the \"set_network_type\" field and using the MTU from plugin when it is not set from metadata. About the user message, we can try to add this later and backport.","commit_id":"ccb6bccce67fb7deab33cb7afd122f44dca999af"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"5dfb43b48d9e8cccc9cd6fde873c873b00cf80eb","unresolved":true,"context_lines":[{"line_number":157,"context_line":"            metadata_vlan \u003d metadata.get(METADATA_VLAN)"},{"line_number":158,"context_line":"            if not metadata_vlan:"},{"line_number":159,"context_line":"                continue"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"            if int(metadata_vlan) \u003e 4094 or int(metadata_vlan) \u003c 1:"},{"line_number":162,"context_line":"                msg \u003d _("},{"line_number":163,"context_line":"                    \u0027A segmentation ID %s was specified but is not valid for \u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"d51538f7_82c1c251","line":160,"updated":"2023-02-24 06:35:54.000000000","message":"what if metadata_vlan isn\u0027t an integer? need a try..except here, along with an appropriate message","commit_id":"e2729946fc4860095eaa37e7bb47eaf692bb586b"}]}