)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"93f9788b0c17f2f7ed75976e9b9fd92214296f36","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"ade92829_771c5dd9","updated":"2022-01-27 13:33:18.000000000","message":"I think Benny\u0027s patch is fine, he just needs to change his comments to TODOs (like the NOTEs shufl has left in all the base schema files).  See my comment in volume/v3_7/services.py for why I believe the refactoring into microversion directories should be done in a separate set of patches.","commit_id":"864022245f7088ce1ace699b594d07272e24f044"},{"author":{"_account_id":11075,"name":"Benny Kopilov","email":"bkopilov@redhat.com","username":"bkopilov"},"change_message_id":"7dec562e2122a1b3c465ec4c43f87bafdde348f6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"dee11650_c69bf0b7","updated":"2022-01-26 19:00:41.000000000","message":"Taking Gorka\u0027s comments and change the code to support same style as in nova (sperate files per micro-version)","commit_id":"864022245f7088ce1ace699b594d07272e24f044"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ebd0dc07ef1a476282b6d210024a6b67475a1eb7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"9de9ae8e_8f1c9d62","updated":"2022-01-26 12:31:19.000000000","message":"Thanks Benny for working on this.  I think we should add the v3_64 and v3_61 directories for those microversions instead of adding things to the bases.","commit_id":"864022245f7088ce1ace699b594d07272e24f044"},{"author":{"_account_id":11075,"name":"Benny Kopilov","email":"bkopilov@redhat.com","username":"bkopilov"},"change_message_id":"5922a9ff83a82d16618c048dc77c1cce5fe2e715","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"e22029e2_514d305f","updated":"2022-01-26 13:05:16.000000000","message":"for the record  , here are the changes that needed in cinder microversion:\n\nhttps://docs.openstack.org/cinder/latest/contributor/api_microversion_history.html","commit_id":"864022245f7088ce1ace699b594d07272e24f044"},{"author":{"_account_id":11075,"name":"Benny Kopilov","email":"bkopilov@redhat.com","username":"bkopilov"},"change_message_id":"c6b0d4905fe4ab9bfe55d5743e418df9e8ad1158","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"cc28db43_437c8b3e","in_reply_to":"9de9ae8e_8f1c9d62","updated":"2022-01-26 12:55:51.000000000","message":"Hi Gorka , \nThanks for your comments , i saw the volume schema without splitting to files per new micro-version .\nI think it will require to change the code for all microversions as you mentioned,\n\n\ni think it will be better to handle it in a seperate patch and not here.\nMeans we will have to update /lib/services/ clients code and add schema_versions_info means mapping between microversion to the corrent schema....\n\nAre we ok with  going forward with this patch and create another patch that will refactor the schema code for all micro-versions plus adding shcema_versions dictinary ?","commit_id":"864022245f7088ce1ace699b594d07272e24f044"},{"author":{"_account_id":11075,"name":"Benny Kopilov","email":"bkopilov@redhat.com","username":"bkopilov"},"change_message_id":"888df91f714fc73b6e486091f66ba01b576dd350","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"38bcaffa_441759e7","updated":"2022-01-28 20:39:51.000000000","message":"Hi ,\nGeneral comment about the changes in micro-version ,\ncurrent implementation may causes issues when deepcopy dictionaries,\nin volume.py api_schema we may have:\ncommon_show_volume which is a dictionary that has a reference to other dictionay like attachments and more in the same module....\nshow_command that refer to common_show_volume and \nlist_volumes_detail \u003d copy.deepcopy(common_show_volume)\n\nthe list_volume_detail call to deep copy from the module common_show_volume.\n\nWhen we create a new dirctory for mircoversion, current implementation deepcopy from last micro-version.\n\nThe problem that we may hit here , deepcopy from last file will copy evrething plus the local reference from the module but the reference changed in the new module.\nExample:\nin newer microverion we copy: \ncreate_volume \u003d copy.deepcopy(volumes.create_volume)\nbut the volumes.create_volume also copied the common_show_volume and in the current module we changed common_show_module, we must update create_volume reference to current common_show_module ....\n\nshow_volume \u003d copy.deepcopy(volumes.show_volume)\nshow_volume[\u0027response_body\u0027][\u0027properties\u0027][\u0027volume\u0027] \u003d common_show_volume\n\nMeans that deepcopy is not enough and we have dependencies to other dictionaries when microverion copies.\n\ni think we should come with a solution otherwise we have issues on the next changes.","commit_id":"b687980fde4e4f88e2177519b63e6c61db92697a"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"70934f2c960719e740eb88958d83ff46a5fe602e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"c4d5520d_8e39363e","updated":"2022-05-06 02:35:26.000000000","message":"lgtm thanks for update","commit_id":"b687980fde4e4f88e2177519b63e6c61db92697a"},{"author":{"_account_id":10459,"name":"Luigi Toscano","email":"ltoscano@redhat.com","username":"ltoscano"},"change_message_id":"95a20e99de9909eb8a5e22a98020834d93494aa6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"39061617_f950fa09","updated":"2022-06-24 09:01:21.000000000","message":"recheck\n\nflakiness?","commit_id":"b687980fde4e4f88e2177519b63e6c61db92697a"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"70934f2c960719e740eb88958d83ff46a5fe602e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"50d3ab65_0e725b6a","in_reply_to":"38bcaffa_441759e7","updated":"2022-05-06 02:35:26.000000000","message":"yeah, we need to be careful on deepcopy and make sure we copy the base and do additional changes on top of it or inbuild base schema.\n\nI agree that current way is time consuming and may end up in issue and ther is effort to improve it in this patch which i need to review - https://review.opendev.org/c/openstack/tempest/+/813459","commit_id":"b687980fde4e4f88e2177519b63e6c61db92697a"}],"tempest/lib/api_schema/response/volume/backups.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ebd0dc07ef1a476282b6d210024a6b67475a1eb7","unresolved":true,"context_lines":[{"line_number":37,"context_line":"        \u0027data_timestamp\u0027: parameter_types.date_time_or_null,"},{"line_number":38,"context_line":"        \u0027snapshot_id\u0027: {\u0027type\u0027: [\u0027string\u0027, \u0027null\u0027]},"},{"line_number":39,"context_line":"        # micro version 3.64 added encryption_key_id"},{"line_number":40,"context_line":"        \u0027encryption_key_id\u0027: parameter_types.uuid_or_null,"},{"line_number":41,"context_line":"        # TODO(zhufl): os-backup-project-attr:project_id is added"},{"line_number":42,"context_line":"        # in 3.18, we should move it to the 3.18 schema file when"},{"line_number":43,"context_line":"        # microversion is supported in volume interfaces."}],"source_content_type":"text/x-python","patch_set":4,"id":"341f4557_8243680b","line":40,"updated":"2022-01-26 12:31:19.000000000","message":"-1: In my opinion this should go into the v3_64 directory instead of here. Like we currently have for v3_7 and how compute has it for all their microversions.\n\nI see we have other microversions here that need to be moved (3.18, 3.43, and 3.56), but I\u0027d rather we didn\u0027t increase the technical debt by adding it here.","commit_id":"864022245f7088ce1ace699b594d07272e24f044"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"9b90c3425368a75ce39eb6e185ddeca8a8733250","unresolved":true,"context_lines":[{"line_number":37,"context_line":"        \u0027data_timestamp\u0027: parameter_types.date_time_or_null,"},{"line_number":38,"context_line":"        \u0027snapshot_id\u0027: {\u0027type\u0027: [\u0027string\u0027, \u0027null\u0027]},"},{"line_number":39,"context_line":"        # micro version 3.64 added encryption_key_id"},{"line_number":40,"context_line":"        \u0027encryption_key_id\u0027: parameter_types.uuid_or_null,"},{"line_number":41,"context_line":"        # TODO(zhufl): os-backup-project-attr:project_id is added"},{"line_number":42,"context_line":"        # in 3.18, we should move it to the 3.18 schema file when"},{"line_number":43,"context_line":"        # microversion is supported in volume interfaces."}],"source_content_type":"text/x-python","patch_set":4,"id":"9b28b862_6f96146f","line":40,"in_reply_to":"341f4557_8243680b","updated":"2022-01-26 20:03:37.000000000","message":"agree, all new microversion schema need to be versioned in different file. example: https://review.opendev.org/c/openstack/tempest/+/750269","commit_id":"864022245f7088ce1ace699b594d07272e24f044"}],"tempest/lib/api_schema/response/volume/v3_7/services.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"93f9788b0c17f2f7ed75976e9b9fd92214296f36","unresolved":true,"context_lines":[{"line_number":23,"context_line":"list_services[\u0027response_body\u0027][\u0027properties\u0027][\u0027services\u0027][\u0027items\u0027]["},{"line_number":24,"context_line":"    \u0027properties\u0027].update({\u0027cluster\u0027: {\u0027type\u0027: [\u0027string\u0027, \u0027null\u0027]}})"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"# NOTE(zhufl): Below are the unchanged schema in this microversion. We need"},{"line_number":27,"context_line":"# to keep this schema in this file to have the generic way to select the"},{"line_number":28,"context_line":"# right schema based on self.schema_versions_info mapping in service client."},{"line_number":29,"context_line":"# ****** Schemas unchanged since microversion 3.0 ******"},{"line_number":30,"context_line":"enable_service \u003d copy.deepcopy(services.enable_service)"},{"line_number":31,"context_line":"disable_service \u003d copy.deepcopy(services.disable_service)"},{"line_number":32,"context_line":"disable_log_reason \u003d copy.deepcopy(services.disable_log_reason)"},{"line_number":33,"context_line":"freeze_host \u003d copy.deepcopy(services.freeze_host)"},{"line_number":34,"context_line":"thaw_host \u003d copy.deepcopy(services.thaw_host)"}],"source_content_type":"text/x-python","patch_set":4,"id":"395d3871_e620afc1","line":34,"range":{"start_line":26,"start_character":0,"end_line":34,"end_character":45},"updated":"2022-01-27 13:33:18.000000000","message":"If I understand this correctly (after looking at the compute schemas, where all the microversions have their own directories), if I want to modify a response from the Services API for mv 3.xx, what I need to do is:\n\n1. find the most recent v_* directory that contains a services.py file\n2. import that module, so that I can ...\n3. copy *all* the schemas listed in that file into this one\n4. modify the schema that\u0027s changed (or add or remove schemas for radical changes)\n\nIf I am correct, it would be really counterproductive to just do mv 3.64 correctly and then go back and fix 3.7 through 3.63 because not only is this a maintenance nightmare, it will be very error prone.","commit_id":"864022245f7088ce1ace699b594d07272e24f044"},{"author":{"_account_id":11075,"name":"Benny Kopilov","email":"bkopilov@redhat.com","username":"bkopilov"},"change_message_id":"691cb5ba3c3dccc255dd2704a76006fae074813b","unresolved":true,"context_lines":[{"line_number":23,"context_line":"list_services[\u0027response_body\u0027][\u0027properties\u0027][\u0027services\u0027][\u0027items\u0027]["},{"line_number":24,"context_line":"    \u0027properties\u0027].update({\u0027cluster\u0027: {\u0027type\u0027: [\u0027string\u0027, \u0027null\u0027]}})"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"# NOTE(zhufl): Below are the unchanged schema in this microversion. We need"},{"line_number":27,"context_line":"# to keep this schema in this file to have the generic way to select the"},{"line_number":28,"context_line":"# right schema based on self.schema_versions_info mapping in service client."},{"line_number":29,"context_line":"# ****** Schemas unchanged since microversion 3.0 ******"},{"line_number":30,"context_line":"enable_service \u003d copy.deepcopy(services.enable_service)"},{"line_number":31,"context_line":"disable_service \u003d copy.deepcopy(services.disable_service)"},{"line_number":32,"context_line":"disable_log_reason \u003d copy.deepcopy(services.disable_log_reason)"},{"line_number":33,"context_line":"freeze_host \u003d copy.deepcopy(services.freeze_host)"},{"line_number":34,"context_line":"thaw_host \u003d copy.deepcopy(services.thaw_host)"}],"source_content_type":"text/x-python","patch_set":4,"id":"0da682f3_3ddd492b","line":34,"range":{"start_line":26,"start_character":0,"end_line":34,"end_character":45},"in_reply_to":"395d3871_e620afc1","updated":"2022-01-27 13:51:27.000000000","message":"Yes , the way its handled today is really pain for maintaince, i would change the code that each file with microversion contains only the changes and we should not do copy/paste work manually ... thats crazy but again its not this patch responsibility.\n\ni think instead of writing copy/paste work for each file we can do something like :\n\n\nEach file done it automatically ..... the only thing that should be written is\ndef copy_schema_keys(copy_from_module, exclude_schema\u003d[]):\n    dir \u003d vars(copy_from_module)\n    schema_keys \u003d list(filter(lambda key: not key.startswith(\u0027__\u0027)\n                              and key not in exclude_schema and\n                              isinstance(dir.get(key), dict), dir.keys()))\n    return schema_keys\n\n\ndef update_module_attribute(copy_from_module, current_module, keys):\n    module_dict \u003d current_module\n    # copy the schema\n    for key in keys:\n        copied \u003d copy.deepcopy(getattr(copy_from_module, key))\n        setattr(module_dict, key, copied)\n\n\nupdate_module_attribute(test2, sys.modules[__name__],\n                        copy_schema_keys(copy_from_module\u003dtest2))","commit_id":"864022245f7088ce1ace699b594d07272e24f044"}],"tempest/lib/api_schema/response/volume/volumes.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ebd0dc07ef1a476282b6d210024a6b67475a1eb7","unresolved":true,"context_lines":[{"line_number":79,"context_line":"        \u0027shared_targets\u0027: {\u0027type\u0027: \u0027boolean\u0027},"},{"line_number":80,"context_line":"        # from micro version 3.64 added to volume: volume_type_id,"},{"line_number":81,"context_line":"        # encryption_key_id and from 3.61 cluster_name"},{"line_number":82,"context_line":"        \u0027volume_type_id\u0027: parameter_types.uuid_or_null,"},{"line_number":83,"context_line":"        \u0027encryption_key_id\u0027: parameter_types.uuid_or_null,"},{"line_number":84,"context_line":"        \u0027cluster_name\u0027: parameter_types.uuid_or_null"},{"line_number":85,"context_line":"    },"},{"line_number":86,"context_line":"    \u0027additionalProperties\u0027: False,"}],"source_content_type":"text/x-python","patch_set":4,"id":"47041b63_474e2b5d","line":83,"range":{"start_line":82,"start_character":0,"end_line":83,"end_character":58},"updated":"2022-01-26 12:31:19.000000000","message":"-1: In my opinion this should go into the v3_64 directory instead of here. Like we currently have for v3_7 and how compute has it for all their microversions.","commit_id":"864022245f7088ce1ace699b594d07272e24f044"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ebd0dc07ef1a476282b6d210024a6b67475a1eb7","unresolved":true,"context_lines":[{"line_number":81,"context_line":"        # encryption_key_id and from 3.61 cluster_name"},{"line_number":82,"context_line":"        \u0027volume_type_id\u0027: parameter_types.uuid_or_null,"},{"line_number":83,"context_line":"        \u0027encryption_key_id\u0027: parameter_types.uuid_or_null,"},{"line_number":84,"context_line":"        \u0027cluster_name\u0027: parameter_types.uuid_or_null"},{"line_number":85,"context_line":"    },"},{"line_number":86,"context_line":"    \u0027additionalProperties\u0027: False,"},{"line_number":87,"context_line":"    \u0027required\u0027: [\u0027attachments\u0027, \u0027links\u0027, \u0027encrypted\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"64967a4b_f52aa6fa","line":84,"updated":"2022-01-26 12:31:19.000000000","message":"-1: In my opinion this should go into the v3_61 directory instead of here. Like we currently have for v3_7 and how compute has it for all their microversions.","commit_id":"864022245f7088ce1ace699b594d07272e24f044"}]}