)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d197f26de6634972ff7bdff8b638db4bb2ff3fd7","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch introduce the ShareMapping and ShareMappingList top level objects"},{"line_number":10,"context_line":"and associated attach and detach methods that interact with the DB"},{"line_number":11,"context_line":"It also introduce ShareMappingLibvirt and ShareMappingLibvirtNFS"},{"line_number":12,"context_line":"children objects that will be used by the driver part."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Manila is the OpenStack Shared Filesystems service."},{"line_number":15,"context_line":"These series of patches implement changes required in Nova to allow the shares"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"29d35f33_80436a77","line":12,"range":{"start_line":11,"start_character":0,"end_line":12,"end_character":54},"updated":"2022-10-10 17:47:42.000000000","message":"So I was expecting to see the generic object stuff in the previous patch, and to see these libvirt specific bits in a follow on patch. That might just be me though. Just asking for the other reviewers to correct me.","commit_id":"ce6f8e1055dfa0995fd58ee1fa2d541d718bacd5"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"d43766424b9e04259c62608a82e402931be02be1","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch introduce the ShareMapping and ShareMappingList top level objects"},{"line_number":10,"context_line":"and associated attach and detach methods that interact with the DB"},{"line_number":11,"context_line":"It also introduce ShareMappingLibvirt and ShareMappingLibvirtNFS"},{"line_number":12,"context_line":"children objects that will be used by the driver part."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Manila is the OpenStack Shared Filesystems service."},{"line_number":15,"context_line":"These series of patches implement changes required in Nova to allow the shares"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"c0830e35_e01e92e2","line":12,"range":{"start_line":11,"start_character":0,"end_line":12,"end_character":54},"in_reply_to":"29d35f33_80436a77","updated":"2023-01-25 14:47:49.000000000","message":"The specification probably biased me. The spec mentioned that ShareMappingLibvirt and ShareMappingLibvirtNFS children objects were needed, and I worked in this direction. I tried to put all code related to share_mapping, whatever driver or protocol used, into the share_mapping.py file. I also had to add code to build these ShareMapping\u003cdriver\u003e\u003cprotocol\u003e objects. I\u0027m not sure it brings enough value vs complexity. However, I\u0027d like not to change it now unless everybody thinks it is not a good approach.","commit_id":"ce6f8e1055dfa0995fd58ee1fa2d541d718bacd5"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"93c90e978063a17e9e33a4ffbc43836ca34334b2","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch introduce the ShareMapping and ShareMappingList top level objects"},{"line_number":10,"context_line":"and associated attach and detach methods that interact with the DB"},{"line_number":11,"context_line":"It also introduce ShareMappingLibvirt and ShareMappingLibvirtNFS"},{"line_number":12,"context_line":"children objects that will be used by the driver part."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Manila is the OpenStack Shared Filesystems service."},{"line_number":15,"context_line":"These series of patches implement changes required in Nova to allow the shares"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"d024681e_09b4b1ee","line":12,"range":{"start_line":11,"start_character":0,"end_line":12,"end_character":54},"in_reply_to":"c0830e35_e01e92e2","updated":"2023-02-23 10:20:11.000000000","message":"Done","commit_id":"ce6f8e1055dfa0995fd58ee1fa2d541d718bacd5"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"528a8e16a4ea4ab58c86c323398b1c8d42099b04","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"ef47c4d8_bb9c0a7a","updated":"2022-08-04 15:16:44.000000000","message":"I just partially reviewed this change before I run out of time","commit_id":"22f057595a900102cf455a26800c599fa9c9e4d1"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"85e574cb_aee6895c","updated":"2022-08-17 10:40:12.000000000","message":"OK, now I finished reviewing the whole patch. See my comments inlin","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"91a2146143d3553e75a72100dba18d60c39ae113","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"e90429bf_99f8ca09","updated":"2022-08-27 21:09:52.000000000","message":"recheck","commit_id":"e321d57bee6e10ef9aba8dc1e548876844ba2690"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4b84d3ff95287d2478e01b16f6eeaae3cda3d826","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"cdf10884_aae4f0aa","updated":"2022-08-29 12:54:34.000000000","message":"the only remaining thing in this patch from me is about the status param and status handling of the share mapping during attach/deatch","commit_id":"aa7029d191909a800c75df26538df492c4913445"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"00c200392dce4d2c8676b736bb7afa634bd628f1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"3f2368a9_a6671763","updated":"2023-01-24 17:53:29.000000000","message":"My comment about state handling was resolved. But John has some comments / questions. -1 for visibility.","commit_id":"12cd024387a609450de181aecd45cce178aed57d"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6f2dd463d1d46198fea750e39821f278aa848e31","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":22,"id":"242e2aad_a3d47f3c","updated":"2023-02-08 16:47:39.000000000","message":"I could be wrong but I don\u0027t see the need for a libvirt-specific interface, maybe we could discuss on IRC if you really want us to draft how to do what you want.","commit_id":"e3a914759553d26b9d40955dc6a928b820a48d32"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8d7e42099c7ec12fa900f4eaab33f23c732bc7c7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"2166c5f2_d218bad5","updated":"2023-03-13 09:51:36.000000000","message":"I honestly think we need to sit down and discuss about your interfaces and the workflow between them.","commit_id":"46a857729d0b2f6d44c9759bbb703af0d0bd1ffe"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"10730f571492acec3020d215504d932b6db9ce40","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":29,"id":"ddd49f77_c385601b","updated":"2023-05-23 13:35:55.000000000","message":"thanks for continued to understand us 😊","commit_id":"279093a175000a97af5e4cda93247f54f3beaf65"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"aced06ed0b4b81c56a290979ab5149ece8c0b20e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":36,"id":"9743cb10_cd7d3ca5","updated":"2024-01-29 13:31:18.000000000","message":"looks good to me","commit_id":"0680f6868272ce888f252202ecb5e4a48c7eb0df"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5f2d93b0d9d70228179af325a128fb8c7e7a579e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":36,"id":"c1199031_a803015a","updated":"2024-01-23 15:16:45.000000000","message":"looks to me good but in case you need to rebase, please change what I said below.","commit_id":"0680f6868272ce888f252202ecb5e4a48c7eb0df"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"c1ff83f944105ebf4dde8441ee555d7fcbe5d64b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":36,"id":"745cbf22_94ee8b95","updated":"2024-01-23 15:17:48.000000000","message":"recheck bug 2026345","commit_id":"0680f6868272ce888f252202ecb5e4a48c7eb0df"}],"nova/objects/__init__.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6f2dd463d1d46198fea750e39821f278aa848e31","unresolved":false,"context_lines":[{"line_number":69,"context_line":"    __import__(\u0027nova.objects.virt_cpu_topology\u0027)"},{"line_number":70,"context_line":"    __import__(\u0027nova.objects.virtual_interface\u0027)"},{"line_number":71,"context_line":"    __import__(\u0027nova.objects.volume_usage\u0027)"},{"line_number":72,"context_line":"    __import__(\u0027nova.objects.share_mapping\u0027)"}],"source_content_type":"text/x-python","patch_set":22,"id":"704f5c7b_c17b14e3","line":72,"updated":"2023-02-08 16:47:39.000000000","message":"++","commit_id":"e3a914759553d26b9d40955dc6a928b820a48d32"}],"nova/objects/fields.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d197f26de6634972ff7bdff8b638db4bb2ff3fd7","unresolved":true,"context_lines":[{"line_number":540,"context_line":"    \"\"\"Represents the possible protocol used by a share mapping\"\"\""},{"line_number":541,"context_line":""},{"line_number":542,"context_line":"    NFS \u003d \"NFS\""},{"line_number":543,"context_line":"    CEPHFS \u003d \"CEPHFS\""},{"line_number":544,"context_line":""},{"line_number":545,"context_line":"    ALL \u003d (NFS, CEPHFS)"},{"line_number":546,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"fd72619d_02b1c5a8","line":543,"updated":"2022-10-10 17:47:42.000000000","message":"There are more listed here:\nhttps://docs.openstack.org/api-ref/shared-file-system/#shares\n\nNFS. Network File System (NFS).\nCIFS. Common Internet File System (CIFS).\nGLUSTERFS. Gluster file system (GlusterFS).\nHDFS. Hadoop Distributed File System (HDFS).\nCEPHFS. Ceph File System (CephFS).\nMAPRFS. MapR File System (MAPRFS).\n\nBut I guess we only want supported ones. I have a feeling the kernel just got updated CIFS support mind.\n\n... probably just ignore me. I just worry about the version mess as we add extra ones.","commit_id":"ce6f8e1055dfa0995fd58ee1fa2d541d718bacd5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6f2dd463d1d46198fea750e39821f278aa848e31","unresolved":false,"context_lines":[{"line_number":540,"context_line":"    \"\"\"Represents the possible protocol used by a share mapping\"\"\""},{"line_number":541,"context_line":""},{"line_number":542,"context_line":"    NFS \u003d \"NFS\""},{"line_number":543,"context_line":"    CEPHFS \u003d \"CEPHFS\""},{"line_number":544,"context_line":""},{"line_number":545,"context_line":"    ALL \u003d (NFS, CEPHFS)"},{"line_number":546,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"2f5fe1a1_8081636e","line":543,"in_reply_to":"0ac72809_b3e2c0c3","updated":"2023-02-08 16:47:39.000000000","message":"Yeah, we agreed on the spec to limit to NFS and CephFS backends for the first phase.\nhttps://specs.openstack.org/openstack/nova-specs/specs/2023.1/approved/libvirt-virtiofs-attach-manila-shares.html#proposed-change\n\nMarking this comment thread as resolved.","commit_id":"ce6f8e1055dfa0995fd58ee1fa2d541d718bacd5"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"d43766424b9e04259c62608a82e402931be02be1","unresolved":true,"context_lines":[{"line_number":540,"context_line":"    \"\"\"Represents the possible protocol used by a share mapping\"\"\""},{"line_number":541,"context_line":""},{"line_number":542,"context_line":"    NFS \u003d \"NFS\""},{"line_number":543,"context_line":"    CEPHFS \u003d \"CEPHFS\""},{"line_number":544,"context_line":""},{"line_number":545,"context_line":"    ALL \u003d (NFS, CEPHFS)"},{"line_number":546,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"0ac72809_b3e2c0c3","line":543,"in_reply_to":"fd72619d_02b1c5a8","updated":"2023-01-25 14:47:49.000000000","message":"No that\u0027s a good comment. It is just the specification that stick with NFS and CephFS as a first shot.","commit_id":"ce6f8e1055dfa0995fd58ee1fa2d541d718bacd5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6f2dd463d1d46198fea750e39821f278aa848e31","unresolved":false,"context_lines":[{"line_number":533,"context_line":"    INACTIVE \u003d \"inactive\""},{"line_number":534,"context_line":"    ERROR \u003d \"error\""},{"line_number":535,"context_line":""},{"line_number":536,"context_line":"    ALL \u003d (ACTIVE, INACTIVE, ERROR)"},{"line_number":537,"context_line":""},{"line_number":538,"context_line":""},{"line_number":539,"context_line":"class ShareMappingProto(BaseNovaEnum):"}],"source_content_type":"text/x-python","patch_set":22,"id":"303cfc8c_b5bbc4d1","line":536,"updated":"2023-02-08 16:47:39.000000000","message":"Statuses correspond to what we agreed on the spec https://specs.openstack.org/openstack/nova-specs/specs/2023.1/approved/libvirt-virtiofs-attach-manila-shares.html#proposed-change","commit_id":"e3a914759553d26b9d40955dc6a928b820a48d32"}],"nova/objects/share_mapping.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"528a8e16a4ea4ab58c86c323398b1c8d42099b04","unresolved":true,"context_lines":[{"line_number":34,"context_line":"        \u0027id\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":35,"context_line":"        \u0027uuid\u0027: fields.UUIDField(nullable\u003dFalse),"},{"line_number":36,"context_line":"        \u0027instance_uuid\u0027: fields.UUIDField(nullable\u003dFalse),"},{"line_number":37,"context_line":"        \u0027share_id\u0027: fields.UUIDField(nullable\u003dTrue),"},{"line_number":38,"context_line":"        \u0027status\u0027: fields.EnumField(valid_values\u003dSHARE_MAPPING_STATUSES),"},{"line_number":39,"context_line":"        \u0027tag\u0027: fields.StringField(nullable\u003dFalse),"},{"line_number":40,"context_line":"        \u0027export_location\u0027: fields.StringField(nullable\u003dFalse),"}],"source_content_type":"text/x-python","patch_set":6,"id":"60f1bf42_6ec7258c","line":37,"range":{"start_line":37,"start_character":37,"end_line":37,"end_character":50},"updated":"2022-08-04 15:16:44.000000000","message":"when will be this null?","commit_id":"22f057595a900102cf455a26800c599fa9c9e4d1"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"b5eec71b57697a3350c1922ce46d82608c6b8860","unresolved":false,"context_lines":[{"line_number":34,"context_line":"        \u0027id\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":35,"context_line":"        \u0027uuid\u0027: fields.UUIDField(nullable\u003dFalse),"},{"line_number":36,"context_line":"        \u0027instance_uuid\u0027: fields.UUIDField(nullable\u003dFalse),"},{"line_number":37,"context_line":"        \u0027share_id\u0027: fields.UUIDField(nullable\u003dTrue),"},{"line_number":38,"context_line":"        \u0027status\u0027: fields.EnumField(valid_values\u003dSHARE_MAPPING_STATUSES),"},{"line_number":39,"context_line":"        \u0027tag\u0027: fields.StringField(nullable\u003dFalse),"},{"line_number":40,"context_line":"        \u0027export_location\u0027: fields.StringField(nullable\u003dFalse),"}],"source_content_type":"text/x-python","patch_set":6,"id":"66a4095e_35778af4","line":37,"range":{"start_line":37,"start_character":37,"end_line":37,"end_character":50},"in_reply_to":"60f1bf42_6ec7258c","updated":"2022-08-25 17:01:28.000000000","message":"Done","commit_id":"22f057595a900102cf455a26800c599fa9c9e4d1"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"528a8e16a4ea4ab58c86c323398b1c8d42099b04","unresolved":true,"context_lines":[{"line_number":35,"context_line":"        \u0027uuid\u0027: fields.UUIDField(nullable\u003dFalse),"},{"line_number":36,"context_line":"        \u0027instance_uuid\u0027: fields.UUIDField(nullable\u003dFalse),"},{"line_number":37,"context_line":"        \u0027share_id\u0027: fields.UUIDField(nullable\u003dTrue),"},{"line_number":38,"context_line":"        \u0027status\u0027: fields.EnumField(valid_values\u003dSHARE_MAPPING_STATUSES),"},{"line_number":39,"context_line":"        \u0027tag\u0027: fields.StringField(nullable\u003dFalse),"},{"line_number":40,"context_line":"        \u0027export_location\u0027: fields.StringField(nullable\u003dFalse),"},{"line_number":41,"context_line":"        \u0027share_proto\u0027: fields.EnumField(valid_values\u003dSHARE_MAPPING_PROTO),"}],"source_content_type":"text/x-python","patch_set":6,"id":"a95b57e3_ea1db141","line":38,"updated":"2022-08-04 15:16:44.000000000","message":"We tend to create Enum fields via like https://github.com/openstack/nova/blob/3b4378c1890337a096b26f1a5335620d475cb778/nova/objects/fields.py#L240-L248","commit_id":"22f057595a900102cf455a26800c599fa9c9e4d1"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"b5eec71b57697a3350c1922ce46d82608c6b8860","unresolved":false,"context_lines":[{"line_number":35,"context_line":"        \u0027uuid\u0027: fields.UUIDField(nullable\u003dFalse),"},{"line_number":36,"context_line":"        \u0027instance_uuid\u0027: fields.UUIDField(nullable\u003dFalse),"},{"line_number":37,"context_line":"        \u0027share_id\u0027: fields.UUIDField(nullable\u003dTrue),"},{"line_number":38,"context_line":"        \u0027status\u0027: fields.EnumField(valid_values\u003dSHARE_MAPPING_STATUSES),"},{"line_number":39,"context_line":"        \u0027tag\u0027: fields.StringField(nullable\u003dFalse),"},{"line_number":40,"context_line":"        \u0027export_location\u0027: fields.StringField(nullable\u003dFalse),"},{"line_number":41,"context_line":"        \u0027share_proto\u0027: fields.EnumField(valid_values\u003dSHARE_MAPPING_PROTO),"}],"source_content_type":"text/x-python","patch_set":6,"id":"cef87e53_b5d2b5d2","line":38,"in_reply_to":"a95b57e3_ea1db141","updated":"2022-08-25 17:01:28.000000000","message":"Done","commit_id":"22f057595a900102cf455a26800c599fa9c9e4d1"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"528a8e16a4ea4ab58c86c323398b1c8d42099b04","unresolved":true,"context_lines":[{"line_number":46,"context_line":"        for field in share_mapping.fields:"},{"line_number":47,"context_line":"            # TODO(rribaud): Try to refactor object to remove next line"},{"line_number":48,"context_line":"            # Next line is a dirty hack"},{"line_number":49,"context_line":"            if not (field \u003d\u003d \u0027deleted\u0027 or field \u003d\u003d \u0027deleted_at\u0027):"},{"line_number":50,"context_line":"                setattr(share_mapping, field, db_share_mapping[field])"},{"line_number":51,"context_line":"        share_mapping._context \u003d context"},{"line_number":52,"context_line":"        share_mapping.obj_reset_changes()"}],"source_content_type":"text/x-python","patch_set":6,"id":"b760655f_646972d5","line":49,"updated":"2022-08-04 15:16:44.000000000","message":"why we cannot copy these two fields?","commit_id":"22f057595a900102cf455a26800c599fa9c9e4d1"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2c2ec2b01c41f5a9193cbe4d721bc9d63a424476","unresolved":false,"context_lines":[{"line_number":46,"context_line":"        for field in share_mapping.fields:"},{"line_number":47,"context_line":"            # TODO(rribaud): Try to refactor object to remove next line"},{"line_number":48,"context_line":"            # Next line is a dirty hack"},{"line_number":49,"context_line":"            if not (field \u003d\u003d \u0027deleted\u0027 or field \u003d\u003d \u0027deleted_at\u0027):"},{"line_number":50,"context_line":"                setattr(share_mapping, field, db_share_mapping[field])"},{"line_number":51,"context_line":"        share_mapping._context \u003d context"},{"line_number":52,"context_line":"        share_mapping.obj_reset_changes()"}],"source_content_type":"text/x-python","patch_set":6,"id":"ac756239_d73df0d3","line":49,"in_reply_to":"130a48de_7a143b78","updated":"2022-08-26 08:10:33.000000000","message":"OK, lets do that separately","commit_id":"22f057595a900102cf455a26800c599fa9c9e4d1"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"b5eec71b57697a3350c1922ce46d82608c6b8860","unresolved":true,"context_lines":[{"line_number":46,"context_line":"        for field in share_mapping.fields:"},{"line_number":47,"context_line":"            # TODO(rribaud): Try to refactor object to remove next line"},{"line_number":48,"context_line":"            # Next line is a dirty hack"},{"line_number":49,"context_line":"            if not (field \u003d\u003d \u0027deleted\u0027 or field \u003d\u003d \u0027deleted_at\u0027):"},{"line_number":50,"context_line":"                setattr(share_mapping, field, db_share_mapping[field])"},{"line_number":51,"context_line":"        share_mapping._context \u003d context"},{"line_number":52,"context_line":"        share_mapping.obj_reset_changes()"}],"source_content_type":"text/x-python","patch_set":6,"id":"130a48de_7a143b78","line":49,"in_reply_to":"2fd03e81_b12d5265","updated":"2022-08-25 17:01:28.000000000","message":"Absolutely, but I wanted to do it in a separate patch.\nI would like also to make NovaPersistantObject the \"default\" object without soft delete. So more or less create a NovaPersistantObject and a NovaPersistantSoftDeleteObject.\n\nProposed patch: https://review.opendev.org/c/openstack/nova/+/854355","commit_id":"22f057595a900102cf455a26800c599fa9c9e4d1"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":true,"context_lines":[{"line_number":46,"context_line":"        for field in share_mapping.fields:"},{"line_number":47,"context_line":"            # TODO(rribaud): Try to refactor object to remove next line"},{"line_number":48,"context_line":"            # Next line is a dirty hack"},{"line_number":49,"context_line":"            if not (field \u003d\u003d \u0027deleted\u0027 or field \u003d\u003d \u0027deleted_at\u0027):"},{"line_number":50,"context_line":"                setattr(share_mapping, field, db_share_mapping[field])"},{"line_number":51,"context_line":"        share_mapping._context \u003d context"},{"line_number":52,"context_line":"        share_mapping.obj_reset_changes()"}],"source_content_type":"text/x-python","patch_set":6,"id":"2fd03e81_b12d5265","line":49,"in_reply_to":"b760655f_646972d5","updated":"2022-08-17 10:40:12.000000000","message":"This is due to the fact that ShareMapping is not soft deletable. So you need to create a new variant of NovaPersistentObject base class that does not declare the deleted and deleted_at fields","commit_id":"22f057595a900102cf455a26800c599fa9c9e4d1"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":true,"context_lines":[{"line_number":60,"context_line":"        self._from_db_object(self._context, self, db_share_mapping)"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def attach(self, status):"},{"line_number":63,"context_line":"        if status \u003d\u003d \u0027inactive\u0027:"},{"line_number":64,"context_line":"            LOG.info("},{"line_number":65,"context_line":"            \"Share \u0027%s\u0027 about to be attached to instance \u0027%s\u0027.\","},{"line_number":66,"context_line":"                self.share_id, self.instance_uuid)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        elif status \u003d\u003d \u0027active\u0027:"},{"line_number":69,"context_line":"            LOG.info("},{"line_number":70,"context_line":"            \"Associate share \u0027%s\u0027 to instance \u0027%s\u0027.\","},{"line_number":71,"context_line":"                self.share_id, self.instance_uuid)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        self._change_status(status)"},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"bad39198_7b9739fb","line":71,"range":{"start_line":63,"start_character":0,"end_line":71,"end_character":50},"updated":"2022-08-17 10:40:12.000000000","message":"nit: I don\u0027t see much of the difference. In both case we the instance will get the share at the next hard reboot.\n\n//later\n\nor is this called from two different places? When it is called from the libvirt driver then we are really attaching the share, but also called it from the REST API when the user just associates a share?\n\n\n//later\n\nProbably need a documentation to this function what does the status parameter mean.","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"c3e7af0ae5eecef27bf04884bec74fbc904de298","unresolved":true,"context_lines":[{"line_number":60,"context_line":"        self._from_db_object(self._context, self, db_share_mapping)"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def attach(self, status):"},{"line_number":63,"context_line":"        if status \u003d\u003d \u0027inactive\u0027:"},{"line_number":64,"context_line":"            LOG.info("},{"line_number":65,"context_line":"            \"Share \u0027%s\u0027 about to be attached to instance \u0027%s\u0027.\","},{"line_number":66,"context_line":"                self.share_id, self.instance_uuid)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        elif status \u003d\u003d \u0027active\u0027:"},{"line_number":69,"context_line":"            LOG.info("},{"line_number":70,"context_line":"            \"Associate share \u0027%s\u0027 to instance \u0027%s\u0027.\","},{"line_number":71,"context_line":"                self.share_id, self.instance_uuid)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        self._change_status(status)"},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fd87b68d_7c718fa9","line":71,"range":{"start_line":63,"start_character":0,"end_line":71,"end_character":50},"in_reply_to":"bad39198_7b9739fb","updated":"2022-08-31 13:48:23.000000000","message":"So the first call to attach happens creating the share mapping in the database, and it will set the share \u0027inactive\u0027 (API part).\nThe \"next\" call is powering on the instance and sets the share mapping to active.\n\nThe intention here was to have different logs according to our state.","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"93c90e978063a17e9e33a4ffbc43836ca34334b2","unresolved":false,"context_lines":[{"line_number":60,"context_line":"        self._from_db_object(self._context, self, db_share_mapping)"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def attach(self, status):"},{"line_number":63,"context_line":"        if status \u003d\u003d \u0027inactive\u0027:"},{"line_number":64,"context_line":"            LOG.info("},{"line_number":65,"context_line":"            \"Share \u0027%s\u0027 about to be attached to instance \u0027%s\u0027.\","},{"line_number":66,"context_line":"                self.share_id, self.instance_uuid)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        elif status \u003d\u003d \u0027active\u0027:"},{"line_number":69,"context_line":"            LOG.info("},{"line_number":70,"context_line":"            \"Associate share \u0027%s\u0027 to instance \u0027%s\u0027.\","},{"line_number":71,"context_line":"                self.share_id, self.instance_uuid)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        self._change_status(status)"},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5699bc4b_86177296","line":71,"range":{"start_line":63,"start_character":0,"end_line":71,"end_character":50},"in_reply_to":"cecfe57a_c7236db0","updated":"2023-02-23 10:20:11.000000000","message":"Done","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"582e2c5da3282793e42ac0ee5cd41c99f15026c2","unresolved":true,"context_lines":[{"line_number":60,"context_line":"        self._from_db_object(self._context, self, db_share_mapping)"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def attach(self, status):"},{"line_number":63,"context_line":"        if status \u003d\u003d \u0027inactive\u0027:"},{"line_number":64,"context_line":"            LOG.info("},{"line_number":65,"context_line":"            \"Share \u0027%s\u0027 about to be attached to instance \u0027%s\u0027.\","},{"line_number":66,"context_line":"                self.share_id, self.instance_uuid)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        elif status \u003d\u003d \u0027active\u0027:"},{"line_number":69,"context_line":"            LOG.info("},{"line_number":70,"context_line":"            \"Associate share \u0027%s\u0027 to instance \u0027%s\u0027.\","},{"line_number":71,"context_line":"                self.share_id, self.instance_uuid)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        self._change_status(status)"},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"cecfe57a_c7236db0","line":71,"range":{"start_line":63,"start_character":0,"end_line":71,"end_character":50},"in_reply_to":"f0e79cbd_c8509850","updated":"2022-09-08 09:19:47.000000000","message":"`So the attach call from the API is basically a noop as far as I see.`\nYou are completely right.\n\nSo I have added a create() method and this is clearer. I think it is what you expect. Please let me know if it is ok.","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"99f6558ce84177fb70e31a96cf6bb2150f7033a3","unresolved":true,"context_lines":[{"line_number":60,"context_line":"        self._from_db_object(self._context, self, db_share_mapping)"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def attach(self, status):"},{"line_number":63,"context_line":"        if status \u003d\u003d \u0027inactive\u0027:"},{"line_number":64,"context_line":"            LOG.info("},{"line_number":65,"context_line":"            \"Share \u0027%s\u0027 about to be attached to instance \u0027%s\u0027.\","},{"line_number":66,"context_line":"                self.share_id, self.instance_uuid)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        elif status \u003d\u003d \u0027active\u0027:"},{"line_number":69,"context_line":"            LOG.info("},{"line_number":70,"context_line":"            \"Associate share \u0027%s\u0027 to instance \u0027%s\u0027.\","},{"line_number":71,"context_line":"                self.share_id, self.instance_uuid)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        self._change_status(status)"},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"f0e79cbd_c8509850","line":71,"range":{"start_line":63,"start_character":0,"end_line":71,"end_character":50},"in_reply_to":"fd87b68d_7c718fa9","updated":"2022-09-01 09:22:37.000000000","message":"Thanks for the clarification. I would separate the two code paths. When the API creates the ShareMapping then the state of the mapping can be set already to inactive, so there I don\u0027t see why we need an extra attach call from the API. Then the virt driver will call the attach to bring the inactive ShareMapping to active state.\n\n//later\n\nI checked in the API patch and you initialize the state of a new sharemapping to inactive at create. So the attach call from the API is basically a noop as far as I see.","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":true,"context_lines":[{"line_number":85,"context_line":"            \"Force removal share \u0027%s\u0027 from instance \u0027%s\u0027.\","},{"line_number":86,"context_line":"                self.share_id, self.instance_uuid)"},{"line_number":87,"context_line":"            self._detach()"},{"line_number":88,"context_line":"        elif self.status \u003d\u003d \u0027active\u0027:"},{"line_number":89,"context_line":"            LOG.info("},{"line_number":90,"context_line":"            \"Share \u0027%s\u0027 about to be detached from instance \u0027%s\u0027.\","},{"line_number":91,"context_line":"                self.share_id, self.instance_uuid)"},{"line_number":92,"context_line":"            self._change_status(\u0027inactive\u0027)"},{"line_number":93,"context_line":"        else:"},{"line_number":94,"context_line":"            LOG.info("},{"line_number":95,"context_line":"            \"Dissociate share \u0027%s\u0027 from instance \u0027%s\u0027.\","},{"line_number":96,"context_line":"                self.share_id, self.instance_uuid)"},{"line_number":97,"context_line":"            self._detach()"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    @base.remotable"},{"line_number":100,"context_line":"    def _detach(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"a7d9689a_63ea38ec","line":97,"range":{"start_line":88,"start_character":0,"end_line":97,"end_character":26},"updated":"2022-08-17 10:40:12.000000000","message":"nit: ditto here. The logging a specific thing about force make sense but separating the active / inactive case carries a lot less information","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"c3e7af0ae5eecef27bf04884bec74fbc904de298","unresolved":true,"context_lines":[{"line_number":85,"context_line":"            \"Force removal share \u0027%s\u0027 from instance \u0027%s\u0027.\","},{"line_number":86,"context_line":"                self.share_id, self.instance_uuid)"},{"line_number":87,"context_line":"            self._detach()"},{"line_number":88,"context_line":"        elif self.status \u003d\u003d \u0027active\u0027:"},{"line_number":89,"context_line":"            LOG.info("},{"line_number":90,"context_line":"            \"Share \u0027%s\u0027 about to be detached from instance \u0027%s\u0027.\","},{"line_number":91,"context_line":"                self.share_id, self.instance_uuid)"},{"line_number":92,"context_line":"            self._change_status(\u0027inactive\u0027)"},{"line_number":93,"context_line":"        else:"},{"line_number":94,"context_line":"            LOG.info("},{"line_number":95,"context_line":"            \"Dissociate share \u0027%s\u0027 from instance \u0027%s\u0027.\","},{"line_number":96,"context_line":"                self.share_id, self.instance_uuid)"},{"line_number":97,"context_line":"            self._detach()"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    @base.remotable"},{"line_number":100,"context_line":"    def _detach(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"b6f43d9e_867eaadb","line":97,"range":{"start_line":88,"start_character":0,"end_line":97,"end_character":26},"in_reply_to":"a7d9689a_63ea38ec","updated":"2022-08-31 13:48:23.000000000","message":"Same logic as attach, different logs to better understand the state.\nAt a force option, because I thought it would have been required, but so far I did not use it.","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"582e2c5da3282793e42ac0ee5cd41c99f15026c2","unresolved":false,"context_lines":[{"line_number":85,"context_line":"            \"Force removal share \u0027%s\u0027 from instance \u0027%s\u0027.\","},{"line_number":86,"context_line":"                self.share_id, self.instance_uuid)"},{"line_number":87,"context_line":"            self._detach()"},{"line_number":88,"context_line":"        elif self.status \u003d\u003d \u0027active\u0027:"},{"line_number":89,"context_line":"            LOG.info("},{"line_number":90,"context_line":"            \"Share \u0027%s\u0027 about to be detached from instance \u0027%s\u0027.\","},{"line_number":91,"context_line":"                self.share_id, self.instance_uuid)"},{"line_number":92,"context_line":"            self._change_status(\u0027inactive\u0027)"},{"line_number":93,"context_line":"        else:"},{"line_number":94,"context_line":"            LOG.info("},{"line_number":95,"context_line":"            \"Dissociate share \u0027%s\u0027 from instance \u0027%s\u0027.\","},{"line_number":96,"context_line":"                self.share_id, self.instance_uuid)"},{"line_number":97,"context_line":"            self._detach()"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    @base.remotable"},{"line_number":100,"context_line":"    def _detach(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"44bb116f_408be289","line":97,"range":{"start_line":88,"start_character":0,"end_line":97,"end_character":26},"in_reply_to":"b6f43d9e_867eaadb","updated":"2022-09-08 09:19:47.000000000","message":"Done","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":true,"context_lines":[{"line_number":109,"context_line":"            context, instance_uuid, share_id)"},{"line_number":110,"context_line":"        if not db_share_mapping:"},{"line_number":111,"context_line":"            raise exception.ShareNotFound(share_id\u003dshare_id)"},{"line_number":112,"context_line":"        # We should have a list with only one element as a share can be"},{"line_number":113,"context_line":"        # associated only one time to an instance."},{"line_number":114,"context_line":"        return ShareMapping._from_db_object("},{"line_number":115,"context_line":"                context,"}],"source_content_type":"text/x-python","patch_set":7,"id":"12edc48b_5a0b189f","line":112,"updated":"2022-08-17 10:40:12.000000000","message":"I think not have a list with a single element is totally OK here.","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"b5eec71b57697a3350c1922ce46d82608c6b8860","unresolved":false,"context_lines":[{"line_number":109,"context_line":"            context, instance_uuid, share_id)"},{"line_number":110,"context_line":"        if not db_share_mapping:"},{"line_number":111,"context_line":"            raise exception.ShareNotFound(share_id\u003dshare_id)"},{"line_number":112,"context_line":"        # We should have a list with only one element as a share can be"},{"line_number":113,"context_line":"        # associated only one time to an instance."},{"line_number":114,"context_line":"        return ShareMapping._from_db_object("},{"line_number":115,"context_line":"                context,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1516fe52_3869eed2","line":112,"in_reply_to":"12edc48b_5a0b189f","updated":"2022-08-25 17:01:28.000000000","message":"Improve comment and add an assert to enforce this rule.","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":true,"context_lines":[{"line_number":125,"context_line":"            \u0027tag\u0027: self.tag,"},{"line_number":126,"context_line":"            \u0027export_location\u0027: self.export_location,"},{"line_number":127,"context_line":"            \u0027share_proto\u0027: self.share_proto,"},{"line_number":128,"context_line":"        }"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    def get_share_host_provider(self):"},{"line_number":131,"context_line":"        if not self.export_location:"}],"source_content_type":"text/x-python","patch_set":7,"id":"c5246506_9d10a1ee","line":128,"updated":"2022-08-17 10:40:12.000000000","message":"can obj_to_primitive be reused here?","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2c2ec2b01c41f5a9193cbe4d721bc9d63a424476","unresolved":true,"context_lines":[{"line_number":125,"context_line":"            \u0027tag\u0027: self.tag,"},{"line_number":126,"context_line":"            \u0027export_location\u0027: self.export_location,"},{"line_number":127,"context_line":"            \u0027share_proto\u0027: self.share_proto,"},{"line_number":128,"context_line":"        }"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    def get_share_host_provider(self):"},{"line_number":131,"context_line":"        if not self.export_location:"}],"source_content_type":"text/x-python","patch_set":7,"id":"cec4def6_cd26a8a0","line":128,"in_reply_to":"b67ba8aa_480ab6db","updated":"2022-08-26 08:10:33.000000000","message":"I don\u0027t think we need this by default for a ovo. So if you haven\u0027t found any need for it in a later patch then please drop it.","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"b5eec71b57697a3350c1922ce46d82608c6b8860","unresolved":true,"context_lines":[{"line_number":125,"context_line":"            \u0027tag\u0027: self.tag,"},{"line_number":126,"context_line":"            \u0027export_location\u0027: self.export_location,"},{"line_number":127,"context_line":"            \u0027share_proto\u0027: self.share_proto,"},{"line_number":128,"context_line":"        }"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    def get_share_host_provider(self):"},{"line_number":131,"context_line":"        if not self.export_location:"}],"source_content_type":"text/x-python","patch_set":7,"id":"b67ba8aa_480ab6db","line":128,"in_reply_to":"c5246506_9d10a1ee","updated":"2022-08-25 17:01:28.000000000","message":"I don\u0027t know. I noticed that the to_dict() method was implemented on a lot of objects. So I simply thought that it is a method required by objects mechanism and implemented it on share_mapping.","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"90e01c462c86fdd29169f197217e60f09448dbeb","unresolved":false,"context_lines":[{"line_number":125,"context_line":"            \u0027tag\u0027: self.tag,"},{"line_number":126,"context_line":"            \u0027export_location\u0027: self.export_location,"},{"line_number":127,"context_line":"            \u0027share_proto\u0027: self.share_proto,"},{"line_number":128,"context_line":"        }"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    def get_share_host_provider(self):"},{"line_number":131,"context_line":"        if not self.export_location:"}],"source_content_type":"text/x-python","patch_set":7,"id":"387eadc8_6863df56","line":128,"in_reply_to":"cec4def6_cd26a8a0","updated":"2022-08-29 09:13:43.000000000","message":"Done","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"90e01c462c86fdd29169f197217e60f09448dbeb","unresolved":false,"context_lines":[{"line_number":125,"context_line":"            \u0027tag\u0027: self.tag,"},{"line_number":126,"context_line":"            \u0027export_location\u0027: self.export_location,"},{"line_number":127,"context_line":"            \u0027share_proto\u0027: self.share_proto,"},{"line_number":128,"context_line":"        }"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    def get_share_host_provider(self):"},{"line_number":131,"context_line":"        if not self.export_location:"}],"source_content_type":"text/x-python","patch_set":7,"id":"e39cb142_47a77be8","line":128,"in_reply_to":"cec4def6_cd26a8a0","updated":"2022-08-29 09:13:43.000000000","message":"Done","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":false,"context_lines":[{"line_number":153,"context_line":"            context, cls(context), ShareMapping, db_share_mappings)"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    @base.remotable_classmethod"},{"line_number":156,"context_line":"    def get_by_share_id(cls, context, share_id):"},{"line_number":157,"context_line":"        db_share_mappings \u003d db.share_mapping_get_by_share_id("},{"line_number":158,"context_line":"            context, share_id)"},{"line_number":159,"context_line":"        return base.obj_make_list("}],"source_content_type":"text/x-python","patch_set":7,"id":"8c389334_227e26a0","line":156,"updated":"2022-08-17 10:40:12.000000000","message":"So this returns a list of share mappings that uses the same manila share. Make sense.","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":true,"context_lines":[{"line_number":160,"context_line":"            context, cls(context), ShareMapping, db_share_mappings)"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"class ShareMappingLibvirt(ShareMapping, metaclass\u003dABCMeta):"},{"line_number":164,"context_line":"    @classmethod"},{"line_number":165,"context_line":"    def from_share_mapping(cls, context, instance, share_mapping):"},{"line_number":166,"context_line":"        share_mapping_libvirt \u003d cls(context)"}],"source_content_type":"text/x-python","patch_set":7,"id":"aee5be29_c7116741","line":163,"range":{"start_line":163,"start_character":40,"end_line":163,"end_character":57},"updated":"2022-08-17 10:40:12.000000000","message":"do we need this if we don\u0027t define abstract methods? I guess you want to make some abstract methods here and add specific implementations in derived classes. You can make the ShareMappingLibvirtCephFS conform to the interface by implementing the abstract methods with a raise NotImplemented()","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2c2ec2b01c41f5a9193cbe4d721bc9d63a424476","unresolved":true,"context_lines":[{"line_number":160,"context_line":"            context, cls(context), ShareMapping, db_share_mappings)"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"class ShareMappingLibvirt(ShareMapping, metaclass\u003dABCMeta):"},{"line_number":164,"context_line":"    @classmethod"},{"line_number":165,"context_line":"    def from_share_mapping(cls, context, instance, share_mapping):"},{"line_number":166,"context_line":"        share_mapping_libvirt \u003d cls(context)"}],"source_content_type":"text/x-python","patch_set":7,"id":"24fefe4b_0bcf2984","line":163,"range":{"start_line":163,"start_character":40,"end_line":163,"end_character":57},"in_reply_to":"1b5c4771_57385c78","updated":"2022-08-26 08:10:33.000000000","message":"Does the children classes need to conform to some kind of interface? I.e. each child should have attach() and detach()? If so then I would add attach() and detach() to this abstract base class as abstract methods.","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"90e01c462c86fdd29169f197217e60f09448dbeb","unresolved":true,"context_lines":[{"line_number":160,"context_line":"            context, cls(context), ShareMapping, db_share_mappings)"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"class ShareMappingLibvirt(ShareMapping, metaclass\u003dABCMeta):"},{"line_number":164,"context_line":"    @classmethod"},{"line_number":165,"context_line":"    def from_share_mapping(cls, context, instance, share_mapping):"},{"line_number":166,"context_line":"        share_mapping_libvirt \u003d cls(context)"}],"source_content_type":"text/x-python","patch_set":7,"id":"743346ba_a8df52e0","line":163,"range":{"start_line":163,"start_character":40,"end_line":163,"end_character":57},"in_reply_to":"24fefe4b_0bcf2984","updated":"2022-08-29 09:13:43.000000000","message":"Ok, but it seems to misbehave. As soon as I add the attach and detach abstract method, it looks like the inheritance is broken. I suspect \u0027@base.NovaObjectRegistry.register\u0027 to do that but unsure.","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6f87ed694f242ca00ba4ccc6a809a017a748620e","unresolved":false,"context_lines":[{"line_number":160,"context_line":"            context, cls(context), ShareMapping, db_share_mappings)"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"class ShareMappingLibvirt(ShareMapping, metaclass\u003dABCMeta):"},{"line_number":164,"context_line":"    @classmethod"},{"line_number":165,"context_line":"    def from_share_mapping(cls, context, instance, share_mapping):"},{"line_number":166,"context_line":"        share_mapping_libvirt \u003d cls(context)"}],"source_content_type":"text/x-python","patch_set":7,"id":"66064754_33ff8912","line":163,"range":{"start_line":163,"start_character":40,"end_line":163,"end_character":57},"in_reply_to":"743346ba_a8df52e0","updated":"2022-08-29 12:41:37.000000000","message":"I see why. So attach and detach is not really abstract methods here as the base class ShareMapping already has implementation for them. So ignore my comment as is.","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"b5eec71b57697a3350c1922ce46d82608c6b8860","unresolved":true,"context_lines":[{"line_number":160,"context_line":"            context, cls(context), ShareMapping, db_share_mappings)"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"class ShareMappingLibvirt(ShareMapping, metaclass\u003dABCMeta):"},{"line_number":164,"context_line":"    @classmethod"},{"line_number":165,"context_line":"    def from_share_mapping(cls, context, instance, share_mapping):"},{"line_number":166,"context_line":"        share_mapping_libvirt \u003d cls(context)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1b5c4771_57385c78","line":163,"range":{"start_line":163,"start_character":40,"end_line":163,"end_character":57},"in_reply_to":"aee5be29_c7116741","updated":"2022-08-25 17:01:28.000000000","message":"My intention here was to create an abstract class to prevent users from instantiating this class and using the children\u0027s ones.","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":true,"context_lines":[{"line_number":176,"context_line":"        # required driver class based on share protocol."},{"line_number":177,"context_line":"        # e.g. for NFS, call the following class:"},{"line_number":178,"context_line":"        # nova.virt.libvirt.volume.nfs.LibvirtNFSVolumeDriver"},{"line_number":179,"context_line":"        module_name \u003d importlib.import_module("},{"line_number":180,"context_line":"            \u0027nova.virt.libvirt.volume.\u0027 +"},{"line_number":181,"context_line":"            share_mapping.share_proto.lower())"},{"line_number":182,"context_line":"        class_name \u003d getattr("}],"source_content_type":"text/x-python","patch_set":7,"id":"f5f74f11_7f7c1563","line":179,"range":{"start_line":179,"start_character":8,"end_line":179,"end_character":19},"updated":"2022-08-17 10:40:12.000000000","message":"this is not the name of the module, it is the actual module (object) itself","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"b5eec71b57697a3350c1922ce46d82608c6b8860","unresolved":false,"context_lines":[{"line_number":176,"context_line":"        # required driver class based on share protocol."},{"line_number":177,"context_line":"        # e.g. for NFS, call the following class:"},{"line_number":178,"context_line":"        # nova.virt.libvirt.volume.nfs.LibvirtNFSVolumeDriver"},{"line_number":179,"context_line":"        module_name \u003d importlib.import_module("},{"line_number":180,"context_line":"            \u0027nova.virt.libvirt.volume.\u0027 +"},{"line_number":181,"context_line":"            share_mapping.share_proto.lower())"},{"line_number":182,"context_line":"        class_name \u003d getattr("}],"source_content_type":"text/x-python","patch_set":7,"id":"9c95ac90_d713a495","line":179,"range":{"start_line":179,"start_character":8,"end_line":179,"end_character":19},"in_reply_to":"f5f74f11_7f7c1563","updated":"2022-08-25 17:01:28.000000000","message":"Done","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":true,"context_lines":[{"line_number":179,"context_line":"        module_name \u003d importlib.import_module("},{"line_number":180,"context_line":"            \u0027nova.virt.libvirt.volume.\u0027 +"},{"line_number":181,"context_line":"            share_mapping.share_proto.lower())"},{"line_number":182,"context_line":"        class_name \u003d getattr("},{"line_number":183,"context_line":"            module_name, \u0027Libvirt\u0027 +"},{"line_number":184,"context_line":"            share_mapping.share_proto +"},{"line_number":185,"context_line":"            \u0027VolumeDriver\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"0797320b_d110f881","line":182,"range":{"start_line":182,"start_character":8,"end_line":182,"end_character":18},"updated":"2022-08-17 10:40:12.000000000","message":"this is not the name of the class, this is the actual class (the Type instance)","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"492dca29bb092568eb7abe41063aaf1d02e74dda","unresolved":false,"context_lines":[{"line_number":179,"context_line":"        module_name \u003d importlib.import_module("},{"line_number":180,"context_line":"            \u0027nova.virt.libvirt.volume.\u0027 +"},{"line_number":181,"context_line":"            share_mapping.share_proto.lower())"},{"line_number":182,"context_line":"        class_name \u003d getattr("},{"line_number":183,"context_line":"            module_name, \u0027Libvirt\u0027 +"},{"line_number":184,"context_line":"            share_mapping.share_proto +"},{"line_number":185,"context_line":"            \u0027VolumeDriver\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f42ff94_7ae8a5e3","line":182,"range":{"start_line":182,"start_character":8,"end_line":182,"end_character":18},"in_reply_to":"02bb9efd_5feb9f35","updated":"2022-08-26 08:13:05.000000000","message":"look good now.","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"b5eec71b57697a3350c1922ce46d82608c6b8860","unresolved":true,"context_lines":[{"line_number":179,"context_line":"        module_name \u003d importlib.import_module("},{"line_number":180,"context_line":"            \u0027nova.virt.libvirt.volume.\u0027 +"},{"line_number":181,"context_line":"            share_mapping.share_proto.lower())"},{"line_number":182,"context_line":"        class_name \u003d getattr("},{"line_number":183,"context_line":"            module_name, \u0027Libvirt\u0027 +"},{"line_number":184,"context_line":"            share_mapping.share_proto +"},{"line_number":185,"context_line":"            \u0027VolumeDriver\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"02bb9efd_5feb9f35","line":182,"range":{"start_line":182,"start_character":8,"end_line":182,"end_character":18},"in_reply_to":"0797320b_d110f881","updated":"2022-08-25 17:01:28.000000000","message":"You are totally right. I changed to class_object. Not sure that\u0027s the best name.\nPlease let me know what you think about it ?","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":true,"context_lines":[{"line_number":201,"context_line":"        return mount_path"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def attach(self, status):"},{"line_number":204,"context_line":"        try:"},{"line_number":205,"context_line":"            self.libvirt_driver.connect_volume("},{"line_number":206,"context_line":"                    self._get_connection_info(), self.instance)"},{"line_number":207,"context_line":"        except processutils.ProcessExecutionError:"}],"source_content_type":"text/x-python","patch_set":7,"id":"9abad579_e9da4a3a","line":204,"updated":"2022-08-17 10:40:12.000000000","message":"do we have to do different things for different status values?","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"582e2c5da3282793e42ac0ee5cd41c99f15026c2","unresolved":false,"context_lines":[{"line_number":201,"context_line":"        return mount_path"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def attach(self, status):"},{"line_number":204,"context_line":"        try:"},{"line_number":205,"context_line":"            self.libvirt_driver.connect_volume("},{"line_number":206,"context_line":"                    self._get_connection_info(), self.instance)"},{"line_number":207,"context_line":"        except processutils.ProcessExecutionError:"}],"source_content_type":"text/x-python","patch_set":7,"id":"819f9551_d5f42f44","line":204,"in_reply_to":"3b65c739_dfb99385","updated":"2022-09-08 09:19:47.000000000","message":"Done","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"88621fc252664534021c62be78ae6d2369307e53","unresolved":true,"context_lines":[{"line_number":201,"context_line":"        return mount_path"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def attach(self, status):"},{"line_number":204,"context_line":"        try:"},{"line_number":205,"context_line":"            self.libvirt_driver.connect_volume("},{"line_number":206,"context_line":"                    self._get_connection_info(), self.instance)"},{"line_number":207,"context_line":"        except processutils.ProcessExecutionError:"}],"source_content_type":"text/x-python","patch_set":7,"id":"3b65c739_dfb99385","line":204,"in_reply_to":"9abad579_e9da4a3a","updated":"2022-08-31 13:51:06.000000000","message":"Only the logging part explained in other comments.","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":true,"context_lines":[{"line_number":210,"context_line":""},{"line_number":211,"context_line":"        super().attach(status)"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"    def detach(self, force\u003dNone):"},{"line_number":214,"context_line":"        try:"},{"line_number":215,"context_line":"            self.libvirt_driver.disconnect_volume("},{"line_number":216,"context_line":"                    self._get_connection_info(), self.instance)"}],"source_content_type":"text/x-python","patch_set":7,"id":"2d397ed4_8b3dce85","line":213,"updated":"2022-08-17 10:40:12.000000000","message":"1) please do not user tertiary binary logic if it is not strictly necessary. I think False is OK here.\n\n2) I think it is unused now. I don\u0027t know if a later patch will use it or not.","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"b5eec71b57697a3350c1922ce46d82608c6b8860","unresolved":false,"context_lines":[{"line_number":210,"context_line":""},{"line_number":211,"context_line":"        super().attach(status)"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"    def detach(self, force\u003dNone):"},{"line_number":214,"context_line":"        try:"},{"line_number":215,"context_line":"            self.libvirt_driver.disconnect_volume("},{"line_number":216,"context_line":"                    self._get_connection_info(), self.instance)"}],"source_content_type":"text/x-python","patch_set":7,"id":"7205806a_d95cf5d8","line":213,"in_reply_to":"2d397ed4_8b3dce85","updated":"2022-08-25 17:01:28.000000000","message":"2) I\u0027m not sure it will be used. The idea was to force the deletion of the share mapping if we have an error.","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":true,"context_lines":[{"line_number":221,"context_line":"        super().detach(force)"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"class ShareMappingLibvirtCephFS(ShareMappingList):"},{"line_number":225,"context_line":"    pass"}],"source_content_type":"text/x-python","patch_set":7,"id":"c4c5e1b7_02bed71e","line":224,"range":{"start_line":224,"start_character":32,"end_line":224,"end_character":48},"updated":"2022-08-17 10:40:12.000000000","message":"isn\u0027t it ShareMappingLibvirt ?","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"b5eec71b57697a3350c1922ce46d82608c6b8860","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        super().detach(force)"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"class ShareMappingLibvirtCephFS(ShareMappingList):"},{"line_number":225,"context_line":"    pass"}],"source_content_type":"text/x-python","patch_set":7,"id":"71b024b4_436251da","line":224,"range":{"start_line":224,"start_character":32,"end_line":224,"end_character":48},"in_reply_to":"c4c5e1b7_02bed71e","updated":"2022-08-25 17:01:28.000000000","message":"Done","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"047353018c61979fe48aa1ed58facdbc37fedd6c","unresolved":true,"context_lines":[{"line_number":176,"context_line":"        # Dynamically import the appropriate module and call the"},{"line_number":177,"context_line":"        # required driver class based on share protocol."},{"line_number":178,"context_line":"        # e.g. for NFS, call the following class:"},{"line_number":179,"context_line":"        # nova.virt.libvirt.volume.nfs.LibvirtNFSVolumeDriver"},{"line_number":180,"context_line":"        module_object \u003d importlib.import_module("},{"line_number":181,"context_line":"            \u0027nova.virt.libvirt.volume.\u0027 +"},{"line_number":182,"context_line":"            share_mapping.share_proto.lower())"}],"source_content_type":"text/x-python","patch_set":8,"id":"3455fcdd_7570fc9e","line":179,"range":{"start_line":179,"start_character":29,"end_line":179,"end_character":61},"updated":"2022-08-26 09:45:08.000000000","message":"copy paste","commit_id":"42829eb38394f687d6c9d72187dc8da69753a5ba"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"90e01c462c86fdd29169f197217e60f09448dbeb","unresolved":true,"context_lines":[{"line_number":176,"context_line":"        # Dynamically import the appropriate module and call the"},{"line_number":177,"context_line":"        # required driver class based on share protocol."},{"line_number":178,"context_line":"        # e.g. for NFS, call the following class:"},{"line_number":179,"context_line":"        # nova.virt.libvirt.volume.nfs.LibvirtNFSVolumeDriver"},{"line_number":180,"context_line":"        module_object \u003d importlib.import_module("},{"line_number":181,"context_line":"            \u0027nova.virt.libvirt.volume.\u0027 +"},{"line_number":182,"context_line":"            share_mapping.share_proto.lower())"}],"source_content_type":"text/x-python","patch_set":8,"id":"540168bc_6c6f8347","line":179,"range":{"start_line":179,"start_character":29,"end_line":179,"end_character":61},"in_reply_to":"3455fcdd_7570fc9e","updated":"2022-08-29 09:13:43.000000000","message":"If you don\u0027t mind, I would like to keep this comment because having the class name expended here helps me.","commit_id":"42829eb38394f687d6c9d72187dc8da69753a5ba"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6f87ed694f242ca00ba4ccc6a809a017a748620e","unresolved":false,"context_lines":[{"line_number":176,"context_line":"        # Dynamically import the appropriate module and call the"},{"line_number":177,"context_line":"        # required driver class based on share protocol."},{"line_number":178,"context_line":"        # e.g. for NFS, call the following class:"},{"line_number":179,"context_line":"        # nova.virt.libvirt.volume.nfs.LibvirtNFSVolumeDriver"},{"line_number":180,"context_line":"        module_object \u003d importlib.import_module("},{"line_number":181,"context_line":"            \u0027nova.virt.libvirt.volume.\u0027 +"},{"line_number":182,"context_line":"            share_mapping.share_proto.lower())"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f9d4f65_7e6c5320","line":179,"range":{"start_line":179,"start_character":29,"end_line":179,"end_character":61},"in_reply_to":"540168bc_6c6f8347","updated":"2022-08-29 12:41:37.000000000","message":"Ack","commit_id":"42829eb38394f687d6c9d72187dc8da69753a5ba"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d197f26de6634972ff7bdff8b638db4bb2ff3fd7","unresolved":true,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    @base.remotable"},{"line_number":55,"context_line":"    def save(self):"},{"line_number":56,"context_line":"        db_share_mapping \u003d db.share_mapping_update("},{"line_number":57,"context_line":"            self._context, self.uuid, self.instance_uuid, self.share_id,"},{"line_number":58,"context_line":"            self.status, self.tag, self.export_location, self.share_proto)"},{"line_number":59,"context_line":"        self._from_db_object(self._context, self, db_share_mapping)"}],"source_content_type":"text/x-python","patch_set":17,"id":"b19aec84_fad29b7c","line":56,"updated":"2022-10-10 17:47:42.000000000","message":"I think we said all new DB code would be like this, a static method embedded in the object:\nhttps://github.com/openstack/nova/blob/03d2715ed492350fa11908aea0fdd0265993e284/nova/objects/instance_mapping.py#L98\n\n... but not sure its worth reworking, just curious why it wasn\u0027t done that way. The unit testing is much easier when you test the DB and object logic all in one go.","commit_id":"ce6f8e1055dfa0995fd58ee1fa2d541d718bacd5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6f2dd463d1d46198fea750e39821f278aa848e31","unresolved":true,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    @base.remotable"},{"line_number":55,"context_line":"    def save(self):"},{"line_number":56,"context_line":"        db_share_mapping \u003d db.share_mapping_update("},{"line_number":57,"context_line":"            self._context, self.uuid, self.instance_uuid, self.share_id,"},{"line_number":58,"context_line":"            self.status, self.tag, self.export_location, self.share_proto)"},{"line_number":59,"context_line":"        self._from_db_object(self._context, self, db_share_mapping)"}],"source_content_type":"text/x-python","patch_set":17,"id":"3eab77d2_2aa48772","line":56,"in_reply_to":"62524cd7_6f64af75","updated":"2023-02-08 16:47:39.000000000","message":"As I wrote in the precedent change, this can be an optimization, but I\u0027m OK to leave the helper methods in the DB modules.\n\nAnyway, this can be fixed in a seamless follow-up later if we agree on that pattern.","commit_id":"ce6f8e1055dfa0995fd58ee1fa2d541d718bacd5"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"d43766424b9e04259c62608a82e402931be02be1","unresolved":true,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    @base.remotable"},{"line_number":55,"context_line":"    def save(self):"},{"line_number":56,"context_line":"        db_share_mapping \u003d db.share_mapping_update("},{"line_number":57,"context_line":"            self._context, self.uuid, self.instance_uuid, self.share_id,"},{"line_number":58,"context_line":"            self.status, self.tag, self.export_location, self.share_proto)"},{"line_number":59,"context_line":"        self._from_db_object(self._context, self, db_share_mapping)"}],"source_content_type":"text/x-python","patch_set":17,"id":"62524cd7_6f64af75","line":56,"in_reply_to":"8ec38733_157e9306","updated":"2023-01-25 14:47:49.000000000","message":"Hello John, I apologize for not having answered before. I did not notice you reviewed the patches. :(\nI did not know about this rule. An older code inspired me, which was not using it. I will be able to change that code, but I think I could do it later, as it is not a blocker. This is the first implementation of virtiofs, which will require improvements, especially because virtiofs online attachment has been merged. So I will be able to fix that in a later iteration.","commit_id":"ce6f8e1055dfa0995fd58ee1fa2d541d718bacd5"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"00c200392dce4d2c8676b736bb7afa634bd628f1","unresolved":true,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    @base.remotable"},{"line_number":55,"context_line":"    def save(self):"},{"line_number":56,"context_line":"        db_share_mapping \u003d db.share_mapping_update("},{"line_number":57,"context_line":"            self._context, self.uuid, self.instance_uuid, self.share_id,"},{"line_number":58,"context_line":"            self.status, self.tag, self.export_location, self.share_proto)"},{"line_number":59,"context_line":"        self._from_db_object(self._context, self, db_share_mapping)"}],"source_content_type":"text/x-python","patch_set":17,"id":"8ec38733_157e9306","line":56,"in_reply_to":"b19aec84_fad29b7c","updated":"2023-01-24 17:53:29.000000000","message":"I have nothing against moving the db call into the OVO","commit_id":"ce6f8e1055dfa0995fd58ee1fa2d541d718bacd5"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d197f26de6634972ff7bdff8b638db4bb2ff3fd7","unresolved":true,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        return share_drv_list"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"class ShareMappingDrvList(list):"},{"line_number":167,"context_line":"    def mount_all(self):"},{"line_number":168,"context_line":"        for share in self:"}],"source_content_type":"text/x-python","patch_set":17,"id":"bfb87479_bed5b853","line":165,"updated":"2022-10-10 17:47:42.000000000","message":"I am confused about all the bits below this line, they look like they should live in the libvirt driver. I guess it must be something relating to the RPC objects similar to the livemigration data objects. I need to revisit this.","commit_id":"ce6f8e1055dfa0995fd58ee1fa2d541d718bacd5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6f2dd463d1d46198fea750e39821f278aa848e31","unresolved":true,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        return share_drv_list"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"class ShareMappingDrvList(list):"},{"line_number":167,"context_line":"    def mount_all(self):"},{"line_number":168,"context_line":"        for share in self:"}],"source_content_type":"text/x-python","patch_set":17,"id":"782b7d78_e5fe6a21","line":165,"in_reply_to":"704550d9_73ebbb9b","updated":"2023-02-08 16:47:39.000000000","message":"Agreed unfortunately, we shouldn\u0027t have libvirt-specific methods in ovo objects. That being said, we agreed on the spec to have specific objects named \"Libvirt\".\nSo maybe the spec is wrong, but I\u0027m not happy to import libvirt here.","commit_id":"ce6f8e1055dfa0995fd58ee1fa2d541d718bacd5"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"93c90e978063a17e9e33a4ffbc43836ca34334b2","unresolved":false,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        return share_drv_list"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"class ShareMappingDrvList(list):"},{"line_number":167,"context_line":"    def mount_all(self):"},{"line_number":168,"context_line":"        for share in self:"}],"source_content_type":"text/x-python","patch_set":17,"id":"29108e38_e0967766","line":165,"in_reply_to":"782b7d78_e5fe6a21","updated":"2023-02-23 10:20:11.000000000","message":"Done","commit_id":"ce6f8e1055dfa0995fd58ee1fa2d541d718bacd5"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"00c200392dce4d2c8676b736bb7afa634bd628f1","unresolved":true,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        return share_drv_list"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"class ShareMappingDrvList(list):"},{"line_number":167,"context_line":"    def mount_all(self):"},{"line_number":168,"context_line":"        for share in self:"}],"source_content_type":"text/x-python","patch_set":17,"id":"704550d9_73ebbb9b","line":165,"in_reply_to":"bfb87479_bed5b853","updated":"2023-01-24 17:53:29.000000000","message":"I think this is a valid question as some code below is libvirt driver specific.","commit_id":"ce6f8e1055dfa0995fd58ee1fa2d541d718bacd5"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d197f26de6634972ff7bdff8b638db4bb2ff3fd7","unresolved":true,"context_lines":[{"line_number":232,"context_line":"            share_mapping.share_proto +"},{"line_number":233,"context_line":"            \u0027VolumeDriver\u0027)"},{"line_number":234,"context_line":"        self.libvirt_driver \u003d class_object(instance.host)"},{"line_number":235,"context_line":"        return self"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"@base.NovaObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":17,"id":"89c5413d_69361236","line":235,"updated":"2022-10-10 17:47:42.000000000","message":"I am not understanding the approach here, can\u0027t this code all live in the libvirt driver code near similar code there? This seems very like the disk code that can be volume vs local disk.","commit_id":"ce6f8e1055dfa0995fd58ee1fa2d541d718bacd5"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"93c90e978063a17e9e33a4ffbc43836ca34334b2","unresolved":false,"context_lines":[{"line_number":232,"context_line":"            share_mapping.share_proto +"},{"line_number":233,"context_line":"            \u0027VolumeDriver\u0027)"},{"line_number":234,"context_line":"        self.libvirt_driver \u003d class_object(instance.host)"},{"line_number":235,"context_line":"        return self"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"@base.NovaObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":17,"id":"68f0acd0_818782c7","line":235,"in_reply_to":"3619bd9c_2fb992ce","updated":"2023-02-23 10:20:11.000000000","message":"Done","commit_id":"ce6f8e1055dfa0995fd58ee1fa2d541d718bacd5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6f2dd463d1d46198fea750e39821f278aa848e31","unresolved":true,"context_lines":[{"line_number":232,"context_line":"            share_mapping.share_proto +"},{"line_number":233,"context_line":"            \u0027VolumeDriver\u0027)"},{"line_number":234,"context_line":"        self.libvirt_driver \u003d class_object(instance.host)"},{"line_number":235,"context_line":"        return self"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"@base.NovaObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":17,"id":"3619bd9c_2fb992ce","line":235,"in_reply_to":"89c5413d_69361236","updated":"2023-02-08 16:47:39.000000000","message":"Yup, I have a concern here. Objects can be used in any service, not only the compute service. If the libvirt driver wants to use an ovo object, that\u0027s OK if we pass the object record to it, but that object shouldn\u0027t have a specific libvirt method.","commit_id":"ce6f8e1055dfa0995fd58ee1fa2d541d718bacd5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8d7e42099c7ec12fa900f4eaab33f23c732bc7c7","unresolved":true,"context_lines":[{"line_number":69,"context_line":"        LOG.info("},{"line_number":70,"context_line":"            \"Share \u0027%s\u0027 about to be attached to instance \u0027%s\u0027.\","},{"line_number":71,"context_line":"            self.share_id, self.instance_uuid)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        self._change_status(fields.ShareMappingStatus.ACTIVE)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    @base.remotable"}],"source_content_type":"text/x-python","patch_set":22,"id":"007abb92_010c04af","line":72,"updated":"2023-03-13 09:51:36.000000000","message":"better to have :\n  self.status \u003d fields.ShareMappingStatus.ACTIVE\n  self.save()","commit_id":"e3a914759553d26b9d40955dc6a928b820a48d32"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"4ddf982ce823bfdafddb5684306c1c8dde4d31b5","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        LOG.info("},{"line_number":70,"context_line":"            \"Share \u0027%s\u0027 about to be attached to instance \u0027%s\u0027.\","},{"line_number":71,"context_line":"            self.share_id, self.instance_uuid)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        self._change_status(fields.ShareMappingStatus.ACTIVE)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    @base.remotable"}],"source_content_type":"text/x-python","patch_set":22,"id":"56b8f20e_6613688a","line":72,"in_reply_to":"007abb92_010c04af","updated":"2023-04-11 17:47:10.000000000","message":"Done","commit_id":"e3a914759553d26b9d40955dc6a928b820a48d32"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6f2dd463d1d46198fea750e39821f278aa848e31","unresolved":true,"context_lines":[{"line_number":76,"context_line":"    def _change_status(self, status):"},{"line_number":77,"context_line":"        db_share_mapping \u003d db.share_mapping_update("},{"line_number":78,"context_line":"            self._context, self.uuid, self.instance_uuid, self.share_id,"},{"line_number":79,"context_line":"            status, self.tag, self.export_location, self.share_proto)"},{"line_number":80,"context_line":"        self._from_db_object(self._context, self, db_share_mapping)"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def detach(self, force\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":22,"id":"13ecce48_f1ecc989","line":79,"updated":"2023-02-08 16:47:39.000000000","message":"any reason why you\u0027re not following your public interface with create() and save() ?","commit_id":"e3a914759553d26b9d40955dc6a928b820a48d32"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"93c90e978063a17e9e33a4ffbc43836ca34334b2","unresolved":false,"context_lines":[{"line_number":76,"context_line":"    def _change_status(self, status):"},{"line_number":77,"context_line":"        db_share_mapping \u003d db.share_mapping_update("},{"line_number":78,"context_line":"            self._context, self.uuid, self.instance_uuid, self.share_id,"},{"line_number":79,"context_line":"            status, self.tag, self.export_location, self.share_proto)"},{"line_number":80,"context_line":"        self._from_db_object(self._context, self, db_share_mapping)"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def detach(self, force\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":22,"id":"19e5f029_b28c76a5","line":79,"in_reply_to":"13ecce48_f1ecc989","updated":"2023-02-23 10:20:11.000000000","message":"Done","commit_id":"e3a914759553d26b9d40955dc6a928b820a48d32"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8d7e42099c7ec12fa900f4eaab33f23c732bc7c7","unresolved":true,"context_lines":[{"line_number":89,"context_line":"            LOG.info("},{"line_number":90,"context_line":"            \"Share \u0027%s\u0027 about to be detached from instance \u0027%s\u0027.\","},{"line_number":91,"context_line":"                self.share_id, self.instance_uuid)"},{"line_number":92,"context_line":"            self._change_status(fields.ShareMappingStatus.INACTIVE)"},{"line_number":93,"context_line":"        else:"},{"line_number":94,"context_line":"            LOG.info("},{"line_number":95,"context_line":"            \"Dissociate share \u0027%s\u0027 from instance \u0027%s\u0027.\","}],"source_content_type":"text/x-python","patch_set":22,"id":"693c476d_bb6dadf1","line":92,"updated":"2023-03-13 09:51:36.000000000","message":"instead of this, that :\n  self.status \u003d fields.[..].status\n  self.save()","commit_id":"e3a914759553d26b9d40955dc6a928b820a48d32"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"4ddf982ce823bfdafddb5684306c1c8dde4d31b5","unresolved":false,"context_lines":[{"line_number":89,"context_line":"            LOG.info("},{"line_number":90,"context_line":"            \"Share \u0027%s\u0027 about to be detached from instance \u0027%s\u0027.\","},{"line_number":91,"context_line":"                self.share_id, self.instance_uuid)"},{"line_number":92,"context_line":"            self._change_status(fields.ShareMappingStatus.INACTIVE)"},{"line_number":93,"context_line":"        else:"},{"line_number":94,"context_line":"            LOG.info("},{"line_number":95,"context_line":"            \"Dissociate share \u0027%s\u0027 from instance \u0027%s\u0027.\","}],"source_content_type":"text/x-python","patch_set":22,"id":"01d50e63_d05d631c","line":92,"in_reply_to":"693c476d_bb6dadf1","updated":"2023-04-11 17:47:10.000000000","message":"Done","commit_id":"e3a914759553d26b9d40955dc6a928b820a48d32"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6f2dd463d1d46198fea750e39821f278aa848e31","unresolved":true,"context_lines":[{"line_number":99,"context_line":"    @base.remotable"},{"line_number":100,"context_line":"    def _detach(self):"},{"line_number":101,"context_line":"        db.share_mapping_delete_by_instance_uuid_and_share_id("},{"line_number":102,"context_line":"        self._context, self.instance_uuid, self.share_id)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    @base.remotable_classmethod"},{"line_number":105,"context_line":"    def get_by_instance_uuid_and_share_id("}],"source_content_type":"text/x-python","patch_set":22,"id":"e986ba0f_7783305f","line":102,"updated":"2023-02-08 16:47:39.000000000","message":"See, I\u0027m quite unhappy with the existing interfaces you created.\n\nWhat I\u0027d prefer is to follow the same pattern we have for other objects : \n1/ we have object methods for creating, updating and deleting objects\n2/ we can persist the object by a specific save() method.\n3/ if a method modifies an object field, it can call save() to persist the modification into the DB.\n\nThat way, we can use an object and modify it directly without needing it to be persisted, unless we want to.","commit_id":"e3a914759553d26b9d40955dc6a928b820a48d32"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"4ddf982ce823bfdafddb5684306c1c8dde4d31b5","unresolved":false,"context_lines":[{"line_number":99,"context_line":"    @base.remotable"},{"line_number":100,"context_line":"    def _detach(self):"},{"line_number":101,"context_line":"        db.share_mapping_delete_by_instance_uuid_and_share_id("},{"line_number":102,"context_line":"        self._context, self.instance_uuid, self.share_id)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    @base.remotable_classmethod"},{"line_number":105,"context_line":"    def get_by_instance_uuid_and_share_id("}],"source_content_type":"text/x-python","patch_set":22,"id":"9aad5dbd_5237f991","line":102,"in_reply_to":"12d443b9_5b3601c8","updated":"2023-04-11 17:47:10.000000000","message":"Done","commit_id":"e3a914759553d26b9d40955dc6a928b820a48d32"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"93c90e978063a17e9e33a4ffbc43836ca34334b2","unresolved":false,"context_lines":[{"line_number":99,"context_line":"    @base.remotable"},{"line_number":100,"context_line":"    def _detach(self):"},{"line_number":101,"context_line":"        db.share_mapping_delete_by_instance_uuid_and_share_id("},{"line_number":102,"context_line":"        self._context, self.instance_uuid, self.share_id)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    @base.remotable_classmethod"},{"line_number":105,"context_line":"    def get_by_instance_uuid_and_share_id("}],"source_content_type":"text/x-python","patch_set":22,"id":"715d461d_e307bf07","line":102,"in_reply_to":"e986ba0f_7783305f","updated":"2023-02-23 10:20:11.000000000","message":"Done","commit_id":"e3a914759553d26b9d40955dc6a928b820a48d32"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8d7e42099c7ec12fa900f4eaab33f23c732bc7c7","unresolved":true,"context_lines":[{"line_number":99,"context_line":"    @base.remotable"},{"line_number":100,"context_line":"    def _detach(self):"},{"line_number":101,"context_line":"        db.share_mapping_delete_by_instance_uuid_and_share_id("},{"line_number":102,"context_line":"        self._context, self.instance_uuid, self.share_id)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    @base.remotable_classmethod"},{"line_number":105,"context_line":"    def get_by_instance_uuid_and_share_id("}],"source_content_type":"text/x-python","patch_set":22,"id":"12d443b9_5b3601c8","line":102,"in_reply_to":"e986ba0f_7783305f","updated":"2023-03-13 09:51:36.000000000","message":"OK, maybe looks better to have a delete() method that calls it so here you directly call .delete()","commit_id":"e3a914759553d26b9d40955dc6a928b820a48d32"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6f2dd463d1d46198fea750e39821f278aa848e31","unresolved":true,"context_lines":[{"line_number":249,"context_line":""},{"line_number":250,"context_line":"    def mount(self):"},{"line_number":251,"context_line":"        try:"},{"line_number":252,"context_line":"            self.libvirt_driver.connect_volume("},{"line_number":253,"context_line":"                    self._get_connection_info(), self.instance)"},{"line_number":254,"context_line":"        except processutils.ProcessExecutionError as exc:"},{"line_number":255,"context_line":"            self._change_status(fields.ShareMappingStatus.ERROR)"}],"source_content_type":"text/x-python","patch_set":22,"id":"18702979_e9772b8a","line":252,"updated":"2023-02-08 16:47:39.000000000","message":"No, you shouldn\u0027t be doing it this way. I don\u0027t know yet which service will call .mount() of that object, but I guess this is the API. If so, the API should rather call the compute RPC API with a mount() RPC call by passing the object as a paramter, rather than doing it this way.","commit_id":"e3a914759553d26b9d40955dc6a928b820a48d32"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"93c90e978063a17e9e33a4ffbc43836ca34334b2","unresolved":false,"context_lines":[{"line_number":249,"context_line":""},{"line_number":250,"context_line":"    def mount(self):"},{"line_number":251,"context_line":"        try:"},{"line_number":252,"context_line":"            self.libvirt_driver.connect_volume("},{"line_number":253,"context_line":"                    self._get_connection_info(), self.instance)"},{"line_number":254,"context_line":"        except processutils.ProcessExecutionError as exc:"},{"line_number":255,"context_line":"            self._change_status(fields.ShareMappingStatus.ERROR)"}],"source_content_type":"text/x-python","patch_set":22,"id":"6cec9216_89b4d96a","line":252,"in_reply_to":"18702979_e9772b8a","updated":"2023-02-23 10:20:11.000000000","message":"Done","commit_id":"e3a914759553d26b9d40955dc6a928b820a48d32"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d3254df86a5e9e335dfaaa5668e6de1a221d244c","unresolved":true,"context_lines":[{"line_number":11,"context_line":"#    under the License."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"# from abc import ABCMeta"},{"line_number":14,"context_line":"# from abc import abstractmethod"},{"line_number":15,"context_line":"import logging"},{"line_number":16,"context_line":"from nova.compute import api as compute"},{"line_number":17,"context_line":"from nova.db.main import api as db"}],"source_content_type":"text/x-python","patch_set":24,"id":"09ef62b5_85eabb44","line":14,"updated":"2023-03-10 16:53:13.000000000","message":"please delete these","commit_id":"46a857729d0b2f6d44c9759bbb703af0d0bd1ffe"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"4ddf982ce823bfdafddb5684306c1c8dde4d31b5","unresolved":false,"context_lines":[{"line_number":11,"context_line":"#    under the License."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"# from abc import ABCMeta"},{"line_number":14,"context_line":"# from abc import abstractmethod"},{"line_number":15,"context_line":"import logging"},{"line_number":16,"context_line":"from nova.compute import api as compute"},{"line_number":17,"context_line":"from nova.db.main import api as db"}],"source_content_type":"text/x-python","patch_set":24,"id":"4f013597_e066387f","line":14,"in_reply_to":"09ef62b5_85eabb44","updated":"2023-04-11 17:47:10.000000000","message":"Done","commit_id":"46a857729d0b2f6d44c9759bbb703af0d0bd1ffe"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d3254df86a5e9e335dfaaa5668e6de1a221d244c","unresolved":true,"context_lines":[{"line_number":75,"context_line":"        self.status \u003d fields.ShareMappingStatus.ACTIVE"},{"line_number":76,"context_line":"        self.save()"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def detach(self, force\u003dFalse):"},{"line_number":79,"context_line":"        if force:"},{"line_number":80,"context_line":"            LOG.info("},{"line_number":81,"context_line":"                \"Force removal share \u0027%s\u0027 from instance \u0027%s\u0027.\","}],"source_content_type":"text/x-python","patch_set":24,"id":"6b9247f4_f1919335","line":78,"updated":"2023-03-10 16:53:13.000000000","message":"Do I see it correctly that the expected usage of ShareMapping is the following:\n\n1) ShareMapping.create(): to create an mapping in INACTIVE state. I think this is called from the API when the user requests a share to be associated with the instance.\n\n2) ShareMapping.attach() to make the attachment ACTIVE. It is called via the ShareMappingList.attach_all() from the libvirt driver during hard reboot.\n\n3) ShareMapping.detach() is called from two places.\n* From the libvirt driver in power_off() via the ShareMappingList.detach_all()\n* From the API when the user wants to remove the share association\n\nThe detach() code then based on the state of the ShareMapping either modifies the state or deletes the mapping. \n\nI think this is confusing. I think the api code can simply call delete() as it already has a check that the instance is in stopped state and therefore the mapping is already inactive. This way the detach() does not need to do two different things based on the state of the mapping. Also it creates a nice symmetry. The api will always call create / delete, and the virt driver will call attach / detach.\n\nAlso I don\u0027t see force is ever used so that can also be removed from the picture.\n\n// later\n\nthe current detach is probably error prone too. If the virt driver calls detach() twice on the same ShareMapping then the second call will delete the ShareMapping. I think we can have cases when the compute manager tries to blindly power off an already powered off VMs.","commit_id":"46a857729d0b2f6d44c9759bbb703af0d0bd1ffe"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"4ddf982ce823bfdafddb5684306c1c8dde4d31b5","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        self.status \u003d fields.ShareMappingStatus.ACTIVE"},{"line_number":76,"context_line":"        self.save()"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def detach(self, force\u003dFalse):"},{"line_number":79,"context_line":"        if force:"},{"line_number":80,"context_line":"            LOG.info("},{"line_number":81,"context_line":"                \"Force removal share \u0027%s\u0027 from instance \u0027%s\u0027.\","}],"source_content_type":"text/x-python","patch_set":24,"id":"50dc1b62_aae985df","line":78,"in_reply_to":"614fc6a0_e7e0f135","updated":"2023-04-11 17:47:10.000000000","message":"Done","commit_id":"46a857729d0b2f6d44c9759bbb703af0d0bd1ffe"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8d7e42099c7ec12fa900f4eaab33f23c732bc7c7","unresolved":true,"context_lines":[{"line_number":75,"context_line":"        self.status \u003d fields.ShareMappingStatus.ACTIVE"},{"line_number":76,"context_line":"        self.save()"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def detach(self, force\u003dFalse):"},{"line_number":79,"context_line":"        if force:"},{"line_number":80,"context_line":"            LOG.info("},{"line_number":81,"context_line":"                \"Force removal share \u0027%s\u0027 from instance \u0027%s\u0027.\","}],"source_content_type":"text/x-python","patch_set":24,"id":"614fc6a0_e7e0f135","line":78,"in_reply_to":"6b9247f4_f1919335","updated":"2023-03-13 09:51:36.000000000","message":"Agreed with Gibi, you should keep detach() as simple as possible and rather call another object method in the API.\nThat\u0027s why I tried to explain in https://review.opendev.org/c/openstack/nova/+/839401/comments/e986ba0f_7783305f : we should maybe discuss the object workflow with you in some gmeet.\nAlso, see my comment below L167, I wonder why you\u0027d want to call the compute API to get something you already have in the DB. The object is just some interface between a DB and the services, so you should directly use it.","commit_id":"46a857729d0b2f6d44c9759bbb703af0d0bd1ffe"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8d7e42099c7ec12fa900f4eaab33f23c732bc7c7","unresolved":true,"context_lines":[{"line_number":164,"context_line":"        smlist \u003d ShareMappingList.get_by_share_id(context, share_id)"},{"line_number":165,"context_line":"        for share_mapping in smlist:"},{"line_number":166,"context_line":"            node_sm_instance \u003d ("},{"line_number":167,"context_line":"                compute_api.get(context, share_mapping.instance_uuid).node"},{"line_number":168,"context_line":"            )"},{"line_number":169,"context_line":"            if node_sm_instance \u003d\u003d node:"},{"line_number":170,"context_line":"                instance_ids.append(share_mapping.instance_uuid)"}],"source_content_type":"text/x-python","patch_set":24,"id":"fc526d9a_2b341cb9","line":167,"updated":"2023-03-13 09:51:36.000000000","message":"No, you shouldn\u0027t get the list of instances by calling the compute API but rather by calling the DB using the share mappings table.","commit_id":"46a857729d0b2f6d44c9759bbb703af0d0bd1ffe"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"529c145cc2afacb0db04f94b28238860e541de19","unresolved":true,"context_lines":[{"line_number":164,"context_line":"        smlist \u003d ShareMappingList.get_by_share_id(context, share_id)"},{"line_number":165,"context_line":"        for share_mapping in smlist:"},{"line_number":166,"context_line":"            node_sm_instance \u003d ("},{"line_number":167,"context_line":"                compute_api.get(context, share_mapping.instance_uuid).node"},{"line_number":168,"context_line":"            )"},{"line_number":169,"context_line":"            if node_sm_instance \u003d\u003d node:"},{"line_number":170,"context_line":"                instance_ids.append(share_mapping.instance_uuid)"}],"source_content_type":"text/x-python","patch_set":24,"id":"383fac11_54480832","line":167,"in_reply_to":"e031653c_b24ea1ea","updated":"2023-03-14 09:38:15.000000000","message":"I discussed this with René. IMHO, instead of trying to look here at which instances are used by this node, we should rather get the instances *before* calling the object, and maybe just asking to get all the shares from a list of instances (ie. instead of having this method, just having a remotable classmethod like \u0027get_by_instance_uuids(list)\u0027","commit_id":"46a857729d0b2f6d44c9759bbb703af0d0bd1ffe"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d5ba0a97888e52f63edda7b1fc785baabc986b34","unresolved":true,"context_lines":[{"line_number":164,"context_line":"        smlist \u003d ShareMappingList.get_by_share_id(context, share_id)"},{"line_number":165,"context_line":"        for share_mapping in smlist:"},{"line_number":166,"context_line":"            node_sm_instance \u003d ("},{"line_number":167,"context_line":"                compute_api.get(context, share_mapping.instance_uuid).node"},{"line_number":168,"context_line":"            )"},{"line_number":169,"context_line":"            if node_sm_instance \u003d\u003d node:"},{"line_number":170,"context_line":"                instance_ids.append(share_mapping.instance_uuid)"}],"source_content_type":"text/x-python","patch_set":24,"id":"e031653c_b24ea1ea","line":167,"in_reply_to":"fc526d9a_2b341cb9","updated":"2023-03-13 19:32:56.000000000","message":"the share_mapping table does not have a node stored in so if we need to know the node the share is mounted on then the whole instance needs to be loaded.","commit_id":"46a857729d0b2f6d44c9759bbb703af0d0bd1ffe"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ad7737724f733c79c6c93693b3c4547c6904d189","unresolved":true,"context_lines":[{"line_number":94,"context_line":"            context, instance_uuid, share_id)"},{"line_number":95,"context_line":"        if not db_share_mapping:"},{"line_number":96,"context_line":"            raise exception.ShareNotFound(share_id\u003dshare_id)"},{"line_number":97,"context_line":"        # This query should return only one element as a share can be"},{"line_number":98,"context_line":"        # associated only one time to an instance."},{"line_number":99,"context_line":"        # The REST API prevent the user to create duplicate share mapping by"},{"line_number":100,"context_line":"        # raising an exception.ShareMappingAlreadyExists."},{"line_number":101,"context_line":"        assert isinstance(db_share_mapping, models.ShareMapping)"},{"line_number":102,"context_line":"        return ShareMapping._from_db_object("},{"line_number":103,"context_line":"                context,"},{"line_number":104,"context_line":"                share_mapping,"}],"source_content_type":"text/x-python","patch_set":34,"id":"be7cd48c_967b927d","line":101,"range":{"start_line":97,"start_character":1,"end_line":101,"end_character":64},"updated":"2023-11-15 13:07:44.000000000","message":"I don\u0027t follow. Are the code comment and the assert related? How? Why we need the type assert here? The db impl of `share_mapping_get_by_instance_uuid_and_share_id` uses `.first()` at the end of the query so we definitely get an ShareMapping model object and not a list of it.","commit_id":"10d935dc74483deba615e49c38c5e08731196dcd"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"aced06ed0b4b81c56a290979ab5149ece8c0b20e","unresolved":false,"context_lines":[{"line_number":94,"context_line":"            context, instance_uuid, share_id)"},{"line_number":95,"context_line":"        if not db_share_mapping:"},{"line_number":96,"context_line":"            raise exception.ShareNotFound(share_id\u003dshare_id)"},{"line_number":97,"context_line":"        # This query should return only one element as a share can be"},{"line_number":98,"context_line":"        # associated only one time to an instance."},{"line_number":99,"context_line":"        # The REST API prevent the user to create duplicate share mapping by"},{"line_number":100,"context_line":"        # raising an exception.ShareMappingAlreadyExists."},{"line_number":101,"context_line":"        assert isinstance(db_share_mapping, models.ShareMapping)"},{"line_number":102,"context_line":"        return ShareMapping._from_db_object("},{"line_number":103,"context_line":"                context,"},{"line_number":104,"context_line":"                share_mapping,"}],"source_content_type":"text/x-python","patch_set":34,"id":"d867b4d6_4fefaa91","line":101,"range":{"start_line":97,"start_character":1,"end_line":101,"end_character":64},"in_reply_to":"39e64802_b29ee92f","updated":"2024-01-29 13:31:18.000000000","message":"Done","commit_id":"10d935dc74483deba615e49c38c5e08731196dcd"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"cbf3f4388ba45921dd39a88096bfd5d5c59de567","unresolved":true,"context_lines":[{"line_number":94,"context_line":"            context, instance_uuid, share_id)"},{"line_number":95,"context_line":"        if not db_share_mapping:"},{"line_number":96,"context_line":"            raise exception.ShareNotFound(share_id\u003dshare_id)"},{"line_number":97,"context_line":"        # This query should return only one element as a share can be"},{"line_number":98,"context_line":"        # associated only one time to an instance."},{"line_number":99,"context_line":"        # The REST API prevent the user to create duplicate share mapping by"},{"line_number":100,"context_line":"        # raising an exception.ShareMappingAlreadyExists."},{"line_number":101,"context_line":"        assert isinstance(db_share_mapping, models.ShareMapping)"},{"line_number":102,"context_line":"        return ShareMapping._from_db_object("},{"line_number":103,"context_line":"                context,"},{"line_number":104,"context_line":"                share_mapping,"}],"source_content_type":"text/x-python","patch_set":34,"id":"e7e18a69_e48d6bcf","line":101,"range":{"start_line":97,"start_character":1,"end_line":101,"end_character":64},"in_reply_to":"be7cd48c_967b927d","updated":"2023-12-12 10:39:44.000000000","message":"Actually, you\u0027re right, the assert won\u0027t fail even if we have duplicate shares.\nRené, you don\u0027t need to use `assert` for that, you should rather just either checking the number of duplicate shares or just remove this.","commit_id":"10d935dc74483deba615e49c38c5e08731196dcd"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"a46a2d23021dbb6a9836a69eb5dbbc0afe3eab17","unresolved":true,"context_lines":[{"line_number":94,"context_line":"            context, instance_uuid, share_id)"},{"line_number":95,"context_line":"        if not db_share_mapping:"},{"line_number":96,"context_line":"            raise exception.ShareNotFound(share_id\u003dshare_id)"},{"line_number":97,"context_line":"        # This query should return only one element as a share can be"},{"line_number":98,"context_line":"        # associated only one time to an instance."},{"line_number":99,"context_line":"        # The REST API prevent the user to create duplicate share mapping by"},{"line_number":100,"context_line":"        # raising an exception.ShareMappingAlreadyExists."},{"line_number":101,"context_line":"        assert isinstance(db_share_mapping, models.ShareMapping)"},{"line_number":102,"context_line":"        return ShareMapping._from_db_object("},{"line_number":103,"context_line":"                context,"},{"line_number":104,"context_line":"                share_mapping,"}],"source_content_type":"text/x-python","patch_set":34,"id":"39e64802_b29ee92f","line":101,"range":{"start_line":97,"start_character":1,"end_line":101,"end_character":64},"in_reply_to":"e7e18a69_e48d6bcf","updated":"2024-01-17 14:05:22.000000000","message":"Ok, guys, I\u0027m going to remove this.\nI thought using assert in this case was a best practice, as explained here: https://realpython.com/python-assert-statement.\nThe intent was to be sure that this method would raise an issue if someone changed the DB method and supplied a list.\nAnyway, that\u0027s not a big deal.","commit_id":"10d935dc74483deba615e49c38c5e08731196dcd"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ad7737724f733c79c6c93693b3c4547c6904d189","unresolved":true,"context_lines":[{"line_number":108,"context_line":"        if not self.export_location:"},{"line_number":109,"context_line":"            return None"},{"line_number":110,"context_line":"        if self.share_proto \u003d\u003d \u0027NFS\u0027:"},{"line_number":111,"context_line":"            rhost, rpath \u003d self.export_location.strip().split(\u0027:\u0027)"},{"line_number":112,"context_line":"        else:"},{"line_number":113,"context_line":"            raise NotImplementedError()"},{"line_number":114,"context_line":"        return rhost"}],"source_content_type":"text/x-python","patch_set":34,"id":"03c07d6d_8b93f363","line":111,"updated":"2023-11-15 13:07:44.000000000","message":"nit: you can use `_` instead of `rpath` to signal that it is unused.","commit_id":"10d935dc74483deba615e49c38c5e08731196dcd"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"cbf3f4388ba45921dd39a88096bfd5d5c59de567","unresolved":true,"context_lines":[{"line_number":108,"context_line":"        if not self.export_location:"},{"line_number":109,"context_line":"            return None"},{"line_number":110,"context_line":"        if self.share_proto \u003d\u003d \u0027NFS\u0027:"},{"line_number":111,"context_line":"            rhost, rpath \u003d self.export_location.strip().split(\u0027:\u0027)"},{"line_number":112,"context_line":"        else:"},{"line_number":113,"context_line":"            raise NotImplementedError()"},{"line_number":114,"context_line":"        return rhost"}],"source_content_type":"text/x-python","patch_set":34,"id":"e9867631_13f4cdd2","line":111,"in_reply_to":"03c07d6d_8b93f363","updated":"2023-12-12 10:39:44.000000000","message":"yup","commit_id":"10d935dc74483deba615e49c38c5e08731196dcd"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"a46a2d23021dbb6a9836a69eb5dbbc0afe3eab17","unresolved":true,"context_lines":[{"line_number":108,"context_line":"        if not self.export_location:"},{"line_number":109,"context_line":"            return None"},{"line_number":110,"context_line":"        if self.share_proto \u003d\u003d \u0027NFS\u0027:"},{"line_number":111,"context_line":"            rhost, rpath \u003d self.export_location.strip().split(\u0027:\u0027)"},{"line_number":112,"context_line":"        else:"},{"line_number":113,"context_line":"            raise NotImplementedError()"},{"line_number":114,"context_line":"        return rhost"}],"source_content_type":"text/x-python","patch_set":34,"id":"48883e50_a412d63c","line":111,"in_reply_to":"e9867631_13f4cdd2","updated":"2024-01-17 14:05:22.000000000","message":"Done","commit_id":"10d935dc74483deba615e49c38c5e08731196dcd"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5f2d93b0d9d70228179af325a128fb8c7e7a579e","unresolved":true,"context_lines":[{"line_number":90,"context_line":"        # This query returns only one element as a share can be"},{"line_number":91,"context_line":"        # associated only one time to an instance."},{"line_number":92,"context_line":"        # Note: the REST API prevent the user to create duplicate share"},{"line_number":93,"context_line":"        # mapping by raising an exception.ShareMappingAlreadyExists."},{"line_number":94,"context_line":"            cls, context, instance_uuid, share_id):"},{"line_number":95,"context_line":"        share_mapping \u003d ShareMapping(context)"},{"line_number":96,"context_line":"        db_share_mapping \u003d db.share_mapping_get_by_instance_uuid_and_share_id("}],"source_content_type":"text/x-python","patch_set":36,"id":"1dea7a45_04319667","line":93,"updated":"2024-01-23 15:16:45.000000000","message":"sounds the bad place where to document here... Could you move the comments above the decorator or rather use docstrings (\"\"\" \"\"\") ?","commit_id":"0680f6868272ce888f252202ecb5e4a48c7eb0df"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"38e71b8f816f35c62f8d9600adf09b3f3286561b","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        # This query returns only one element as a share can be"},{"line_number":91,"context_line":"        # associated only one time to an instance."},{"line_number":92,"context_line":"        # Note: the REST API prevent the user to create duplicate share"},{"line_number":93,"context_line":"        # mapping by raising an exception.ShareMappingAlreadyExists."},{"line_number":94,"context_line":"            cls, context, instance_uuid, share_id):"},{"line_number":95,"context_line":"        share_mapping \u003d ShareMapping(context)"},{"line_number":96,"context_line":"        db_share_mapping \u003d db.share_mapping_get_by_instance_uuid_and_share_id("}],"source_content_type":"text/x-python","patch_set":36,"id":"958b7cd0_c999706f","line":93,"in_reply_to":"1dea7a45_04319667","updated":"2024-02-02 15:35:51.000000000","message":"Done","commit_id":"0680f6868272ce888f252202ecb5e4a48c7eb0df"}],"nova/tests/unit/objects/test_objects.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":true,"context_lines":[{"line_number":1149,"context_line":"    \u0027VirtualInterfaceList\u0027: \u00271.0-9750e2074437b3077e46359102779fc6\u0027,"},{"line_number":1150,"context_line":"    \u0027VolumeUsage\u0027: \u00271.0-6c8190c46ce1469bb3286a1f21c2e475\u0027,"},{"line_number":1151,"context_line":"    \u0027XenDeviceBus\u0027: \u00271.0-272a4f899b24e31e42b2b9a7ed7e9194\u0027,"},{"line_number":1152,"context_line":"    # TODO(efried): re-alphabetize this"},{"line_number":1153,"context_line":"    \u0027LibvirtVPMEMDevice\u0027: \u00271.0-17ffaf47585199eeb9a2b83d6bde069f\u0027,"},{"line_number":1154,"context_line":"}"},{"line_number":1155,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"ac4c61dc_58706677","side":"PARENT","line":1152,"updated":"2022-08-17 10:40:12.000000000","message":"Please do the re-alphabetization in a separate commit, to make this change cleaner","commit_id":"4b8e00e34faba5f7f0c6fa2ec7ad2febf1f1d138"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"b5eec71b57697a3350c1922ce46d82608c6b8860","unresolved":false,"context_lines":[{"line_number":1149,"context_line":"    \u0027VirtualInterfaceList\u0027: \u00271.0-9750e2074437b3077e46359102779fc6\u0027,"},{"line_number":1150,"context_line":"    \u0027VolumeUsage\u0027: \u00271.0-6c8190c46ce1469bb3286a1f21c2e475\u0027,"},{"line_number":1151,"context_line":"    \u0027XenDeviceBus\u0027: \u00271.0-272a4f899b24e31e42b2b9a7ed7e9194\u0027,"},{"line_number":1152,"context_line":"    # TODO(efried): re-alphabetize this"},{"line_number":1153,"context_line":"    \u0027LibvirtVPMEMDevice\u0027: \u00271.0-17ffaf47585199eeb9a2b83d6bde069f\u0027,"},{"line_number":1154,"context_line":"}"},{"line_number":1155,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"17f7c47b_20f0d9a2","side":"PARENT","line":1152,"in_reply_to":"ac4c61dc_58706677","updated":"2022-08-25 17:01:28.000000000","message":"Done","commit_id":"4b8e00e34faba5f7f0c6fa2ec7ad2febf1f1d138"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":true,"context_lines":[{"line_number":1137,"context_line":"    \u0027ServiceList\u0027: \u00271.19-5325bce13eebcbf22edc9678285270cc\u0027,"},{"line_number":1138,"context_line":"    \u0027ShareMapping\u0027: \u00271.0-efd7ee71a12549f8507d0d1f7fb060b0\u0027,"},{"line_number":1139,"context_line":"    \u0027ShareMappingList\u0027: \u00271.0-634980d5efdf3656e28c8dec3d862ab9\u0027,"},{"line_number":1140,"context_line":"    \u0027ShareMappingLibvirtNFS\u0027: \u00271.0-efd7ee71a12549f8507d0d1f7fb060b0\u0027,"},{"line_number":1141,"context_line":"    \u0027Tag\u0027: \u00271.1-8b8d7d5b48887651a0e01241672e2963\u0027,"},{"line_number":1142,"context_line":"    \u0027TagList\u0027: \u00271.1-55231bdb671ecf7641d6a2e9109b5d8e\u0027,"},{"line_number":1143,"context_line":"    \u0027TaskLog\u0027: \u00271.0-78b0534366f29aa3eebb01860fbe18fe\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"254fa410_6f52db6d","line":1140,"updated":"2022-08-17 10:40:12.000000000","message":"I think you need to add ShareMappingLibvirtCephFS as well","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"b5eec71b57697a3350c1922ce46d82608c6b8860","unresolved":true,"context_lines":[{"line_number":1137,"context_line":"    \u0027ServiceList\u0027: \u00271.19-5325bce13eebcbf22edc9678285270cc\u0027,"},{"line_number":1138,"context_line":"    \u0027ShareMapping\u0027: \u00271.0-efd7ee71a12549f8507d0d1f7fb060b0\u0027,"},{"line_number":1139,"context_line":"    \u0027ShareMappingList\u0027: \u00271.0-634980d5efdf3656e28c8dec3d862ab9\u0027,"},{"line_number":1140,"context_line":"    \u0027ShareMappingLibvirtNFS\u0027: \u00271.0-efd7ee71a12549f8507d0d1f7fb060b0\u0027,"},{"line_number":1141,"context_line":"    \u0027Tag\u0027: \u00271.1-8b8d7d5b48887651a0e01241672e2963\u0027,"},{"line_number":1142,"context_line":"    \u0027TagList\u0027: \u00271.1-55231bdb671ecf7641d6a2e9109b5d8e\u0027,"},{"line_number":1143,"context_line":"    \u0027TaskLog\u0027: \u00271.0-78b0534366f29aa3eebb01860fbe18fe\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"b7b0b813_a5a9a83a","line":1140,"in_reply_to":"254fa410_6f52db6d","updated":"2022-08-25 17:01:28.000000000","message":"If you don\u0027t mind, I\u0027ll do that in a next patch that will implement the CephFS protocol.","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2c2ec2b01c41f5a9193cbe4d721bc9d63a424476","unresolved":true,"context_lines":[{"line_number":1137,"context_line":"    \u0027ServiceList\u0027: \u00271.19-5325bce13eebcbf22edc9678285270cc\u0027,"},{"line_number":1138,"context_line":"    \u0027ShareMapping\u0027: \u00271.0-efd7ee71a12549f8507d0d1f7fb060b0\u0027,"},{"line_number":1139,"context_line":"    \u0027ShareMappingList\u0027: \u00271.0-634980d5efdf3656e28c8dec3d862ab9\u0027,"},{"line_number":1140,"context_line":"    \u0027ShareMappingLibvirtNFS\u0027: \u00271.0-efd7ee71a12549f8507d0d1f7fb060b0\u0027,"},{"line_number":1141,"context_line":"    \u0027Tag\u0027: \u00271.1-8b8d7d5b48887651a0e01241672e2963\u0027,"},{"line_number":1142,"context_line":"    \u0027TagList\u0027: \u00271.1-55231bdb671ecf7641d6a2e9109b5d8e\u0027,"},{"line_number":1143,"context_line":"    \u0027TaskLog\u0027: \u00271.0-78b0534366f29aa3eebb01860fbe18fe\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"e8ed7658_fe9e93c8","line":1140,"in_reply_to":"b7b0b813_a5a9a83a","updated":"2022-08-26 08:10:33.000000000","message":"interesting, I thought that the unit test would complain that it does not have a hash associated here. Btw, in any case you can drop the ShareMappingLibvirtCephFS class from this patch and add it later when you implement it","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"90e01c462c86fdd29169f197217e60f09448dbeb","unresolved":false,"context_lines":[{"line_number":1137,"context_line":"    \u0027ServiceList\u0027: \u00271.19-5325bce13eebcbf22edc9678285270cc\u0027,"},{"line_number":1138,"context_line":"    \u0027ShareMapping\u0027: \u00271.0-efd7ee71a12549f8507d0d1f7fb060b0\u0027,"},{"line_number":1139,"context_line":"    \u0027ShareMappingList\u0027: \u00271.0-634980d5efdf3656e28c8dec3d862ab9\u0027,"},{"line_number":1140,"context_line":"    \u0027ShareMappingLibvirtNFS\u0027: \u00271.0-efd7ee71a12549f8507d0d1f7fb060b0\u0027,"},{"line_number":1141,"context_line":"    \u0027Tag\u0027: \u00271.1-8b8d7d5b48887651a0e01241672e2963\u0027,"},{"line_number":1142,"context_line":"    \u0027TagList\u0027: \u00271.1-55231bdb671ecf7641d6a2e9109b5d8e\u0027,"},{"line_number":1143,"context_line":"    \u0027TaskLog\u0027: \u00271.0-78b0534366f29aa3eebb01860fbe18fe\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"189bd292_97e40f5c","line":1140,"in_reply_to":"e8ed7658_fe9e93c8","updated":"2022-08-29 09:13:43.000000000","message":"Done","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"492dca29bb092568eb7abe41063aaf1d02e74dda","unresolved":true,"context_lines":[{"line_number":1138,"context_line":"    \u0027ShareMapping\u0027: \u00271.0-f7c5536a70ddb691d80c633fca3ddd59\u0027,"},{"line_number":1139,"context_line":"    \u0027ShareMappingList\u0027: \u00271.0-634980d5efdf3656e28c8dec3d862ab9\u0027,"},{"line_number":1140,"context_line":"    \u0027ShareMappingLibvirtNFS\u0027: \u00271.0-f7c5536a70ddb691d80c633fca3ddd59\u0027,"},{"line_number":1141,"context_line":"    \u0027ShareMetadata\u0027: \u00271.0-09f69ac0bd47371417b5477a277e43af\u0027,"},{"line_number":1142,"context_line":"    \u0027Tag\u0027: \u00271.1-8b8d7d5b48887651a0e01241672e2963\u0027,"},{"line_number":1143,"context_line":"    \u0027TagList\u0027: \u00271.1-55231bdb671ecf7641d6a2e9109b5d8e\u0027,"},{"line_number":1144,"context_line":"    \u0027TaskLog\u0027: \u00271.0-78b0534366f29aa3eebb01860fbe18fe\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"98200e16_ce4ba57f","line":1141,"updated":"2022-08-26 08:13:05.000000000","message":"this does not seem to be defined in this patch","commit_id":"42829eb38394f687d6c9d72187dc8da69753a5ba"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"90e01c462c86fdd29169f197217e60f09448dbeb","unresolved":false,"context_lines":[{"line_number":1138,"context_line":"    \u0027ShareMapping\u0027: \u00271.0-f7c5536a70ddb691d80c633fca3ddd59\u0027,"},{"line_number":1139,"context_line":"    \u0027ShareMappingList\u0027: \u00271.0-634980d5efdf3656e28c8dec3d862ab9\u0027,"},{"line_number":1140,"context_line":"    \u0027ShareMappingLibvirtNFS\u0027: \u00271.0-f7c5536a70ddb691d80c633fca3ddd59\u0027,"},{"line_number":1141,"context_line":"    \u0027ShareMetadata\u0027: \u00271.0-09f69ac0bd47371417b5477a277e43af\u0027,"},{"line_number":1142,"context_line":"    \u0027Tag\u0027: \u00271.1-8b8d7d5b48887651a0e01241672e2963\u0027,"},{"line_number":1143,"context_line":"    \u0027TagList\u0027: \u00271.1-55231bdb671ecf7641d6a2e9109b5d8e\u0027,"},{"line_number":1144,"context_line":"    \u0027TaskLog\u0027: \u00271.0-78b0534366f29aa3eebb01860fbe18fe\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"0657a438_49e482d6","line":1141,"in_reply_to":"98200e16_ce4ba57f","updated":"2022-08-29 09:13:43.000000000","message":"Sorry it was an issue I introduced by fixing a conflict. Removed so far.","commit_id":"42829eb38394f687d6c9d72187dc8da69753a5ba"}],"nova/tests/unit/objects/test_share_mapping.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":true,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"from unittest import mock"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"NOW \u003d timeutils.utcnow().replace(microsecond\u003d0)"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"fake_share_mapping \u003d {"},{"line_number":27,"context_line":"    \u0027created_at\u0027: NOW,"}],"source_content_type":"text/x-python","patch_set":7,"id":"d02f7409_9beaa936","line":24,"updated":"2022-08-17 10:40:12.000000000","message":"please don\u0027t do this, this will make the test time dependent. You should use timeutils.set_time_override to set the current time instead. There is a TimeOverride fixture available as well","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"90e01c462c86fdd29169f197217e60f09448dbeb","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"from unittest import mock"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"NOW \u003d timeutils.utcnow().replace(microsecond\u003d0)"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"fake_share_mapping \u003d {"},{"line_number":27,"context_line":"    \u0027created_at\u0027: NOW,"}],"source_content_type":"text/x-python","patch_set":7,"id":"392ce7d9_42cdfd18","line":24,"in_reply_to":"d02f7409_9beaa936","updated":"2022-08-29 09:13:43.000000000","message":"Done","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":true,"context_lines":[{"line_number":67,"context_line":"            \u0027NFS\u0027"},{"line_number":68,"context_line":"        )"},{"line_number":69,"context_line":"        self.compare_obj(share_mapping, fake_share_mapping,"},{"line_number":70,"context_line":"                allow_missing\u003d[\u0027deleted\u0027, \u0027deleted_at\u0027])"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    def test_get_share_host_provider(self):"},{"line_number":73,"context_line":"        share_mapping \u003d objects.ShareMapping(self.context)"}],"source_content_type":"text/x-python","patch_set":7,"id":"63e57406_d5bf9fea","line":70,"range":{"start_line":70,"start_character":16,"end_line":70,"end_character":29},"updated":"2022-08-17 10:40:12.000000000","message":"this can also be factored out when you make a separate ovo baseclass for object that are not softdeletable","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"90e01c462c86fdd29169f197217e60f09448dbeb","unresolved":true,"context_lines":[{"line_number":67,"context_line":"            \u0027NFS\u0027"},{"line_number":68,"context_line":"        )"},{"line_number":69,"context_line":"        self.compare_obj(share_mapping, fake_share_mapping,"},{"line_number":70,"context_line":"                allow_missing\u003d[\u0027deleted\u0027, \u0027deleted_at\u0027])"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    def test_get_share_host_provider(self):"},{"line_number":73,"context_line":"        share_mapping \u003d objects.ShareMapping(self.context)"}],"source_content_type":"text/x-python","patch_set":7,"id":"c6b16166_0c388a84","line":70,"range":{"start_line":70,"start_character":16,"end_line":70,"end_character":29},"in_reply_to":"63e57406_d5bf9fea","updated":"2022-08-29 09:13:43.000000000","message":"Maybe need to be reviewed because I factorize but not in the base class.","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4b84d3ff95287d2478e01b16f6eeaae3cda3d826","unresolved":false,"context_lines":[{"line_number":67,"context_line":"            \u0027NFS\u0027"},{"line_number":68,"context_line":"        )"},{"line_number":69,"context_line":"        self.compare_obj(share_mapping, fake_share_mapping,"},{"line_number":70,"context_line":"                allow_missing\u003d[\u0027deleted\u0027, \u0027deleted_at\u0027])"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    def test_get_share_host_provider(self):"},{"line_number":73,"context_line":"        share_mapping \u003d objects.ShareMapping(self.context)"}],"source_content_type":"text/x-python","patch_set":7,"id":"18295458_6d82c4b1","line":70,"range":{"start_line":70,"start_character":16,"end_line":70,"end_character":29},"in_reply_to":"c6b16166_0c388a84","updated":"2022-08-29 12:54:34.000000000","message":"Ack","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":true,"context_lines":[{"line_number":122,"context_line":"    @mock.patch("},{"line_number":123,"context_line":"        \u0027nova.db.main.api.share_mapping_update\u0027,"},{"line_number":124,"context_line":"        return_value\u003dfake_share_mapping_attached)"},{"line_number":125,"context_line":"    def test_attach_attaching_status(self, mock_upd):"},{"line_number":126,"context_line":"        share_mapping \u003d objects.ShareMapping(self.context)"},{"line_number":127,"context_line":"        share_mapping.uuid \u003d uuids.share_mapping"},{"line_number":128,"context_line":"        share_mapping.instance_uuid \u003d uuids.instance"}],"source_content_type":"text/x-python","patch_set":7,"id":"b1a20bf0_1e175dd2","line":125,"updated":"2022-08-17 10:40:12.000000000","message":"I don\u0027t see how this differs from test_attach","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"b5eec71b57697a3350c1922ce46d82608c6b8860","unresolved":false,"context_lines":[{"line_number":122,"context_line":"    @mock.patch("},{"line_number":123,"context_line":"        \u0027nova.db.main.api.share_mapping_update\u0027,"},{"line_number":124,"context_line":"        return_value\u003dfake_share_mapping_attached)"},{"line_number":125,"context_line":"    def test_attach_attaching_status(self, mock_upd):"},{"line_number":126,"context_line":"        share_mapping \u003d objects.ShareMapping(self.context)"},{"line_number":127,"context_line":"        share_mapping.uuid \u003d uuids.share_mapping"},{"line_number":128,"context_line":"        share_mapping.instance_uuid \u003d uuids.instance"}],"source_content_type":"text/x-python","patch_set":7,"id":"ebf12ac9_dab553fe","line":125,"in_reply_to":"b1a20bf0_1e175dd2","updated":"2022-08-25 17:01:28.000000000","message":"Done","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":true,"context_lines":[{"line_number":166,"context_line":"        return_value\u003dfake_share_mapping)"},{"line_number":167,"context_line":"    def test_get_by_instance_uuid_and_share_id(self, mock_get):"},{"line_number":168,"context_line":"        share_mapping \u003d sm.ShareMapping.get_by_instance_uuid_and_share_id("},{"line_number":169,"context_line":"                self.context,"},{"line_number":170,"context_line":"                uuids.instance,"},{"line_number":171,"context_line":"                uuids.share)"},{"line_number":172,"context_line":"        mock_get.assert_called_once_with("}],"source_content_type":"text/x-python","patch_set":7,"id":"d484d082_13652206","line":169,"range":{"start_line":169,"start_character":0,"end_line":169,"end_character":16},"updated":"2022-08-17 10:40:12.000000000","message":"nit: over indented. A single level of indent is enough","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"b5eec71b57697a3350c1922ce46d82608c6b8860","unresolved":false,"context_lines":[{"line_number":166,"context_line":"        return_value\u003dfake_share_mapping)"},{"line_number":167,"context_line":"    def test_get_by_instance_uuid_and_share_id(self, mock_get):"},{"line_number":168,"context_line":"        share_mapping \u003d sm.ShareMapping.get_by_instance_uuid_and_share_id("},{"line_number":169,"context_line":"                self.context,"},{"line_number":170,"context_line":"                uuids.instance,"},{"line_number":171,"context_line":"                uuids.share)"},{"line_number":172,"context_line":"        mock_get.assert_called_once_with("}],"source_content_type":"text/x-python","patch_set":7,"id":"d8744d62_aecab639","line":169,"range":{"start_line":169,"start_character":0,"end_line":169,"end_character":16},"in_reply_to":"d484d082_13652206","updated":"2022-08-25 17:01:28.000000000","message":"Done","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":true,"context_lines":[{"line_number":171,"context_line":"                uuids.share)"},{"line_number":172,"context_line":"        mock_get.assert_called_once_with("},{"line_number":173,"context_line":"            self.context, uuids.instance, uuids.share)"},{"line_number":174,"context_line":"        self.compare_obj(share_mapping, fake_share_mapping,"},{"line_number":175,"context_line":"                allow_missing\u003d[\u0027deleted\u0027, \u0027deleted_at\u0027])"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    @mock.patch("}],"source_content_type":"text/x-python","patch_set":7,"id":"cab1de2d_f4272ce9","line":174,"range":{"start_line":174,"start_character":24,"end_line":174,"end_character":26},"updated":"2022-08-17 10:40:12.000000000","message":"wrap the line here. the below line is now overindented","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"b5eec71b57697a3350c1922ce46d82608c6b8860","unresolved":false,"context_lines":[{"line_number":171,"context_line":"                uuids.share)"},{"line_number":172,"context_line":"        mock_get.assert_called_once_with("},{"line_number":173,"context_line":"            self.context, uuids.instance, uuids.share)"},{"line_number":174,"context_line":"        self.compare_obj(share_mapping, fake_share_mapping,"},{"line_number":175,"context_line":"                allow_missing\u003d[\u0027deleted\u0027, \u0027deleted_at\u0027])"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    @mock.patch("}],"source_content_type":"text/x-python","patch_set":7,"id":"6391c61a_2dd37aa9","line":174,"range":{"start_line":174,"start_character":24,"end_line":174,"end_character":26},"in_reply_to":"cab1de2d_f4272ce9","updated":"2022-08-25 17:01:28.000000000","message":"Done","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":true,"context_lines":[{"line_number":190,"context_line":"class _TestShareMappingList(object):"},{"line_number":191,"context_line":"    def test_get_by_instance_uuid(self):"},{"line_number":192,"context_line":"        with mock.patch.object("},{"line_number":193,"context_line":"            db, \u0027share_mapping_get_by_instance_uuid\u0027) as get:"},{"line_number":194,"context_line":"            get.return_value \u003d [fake_share_mapping]"},{"line_number":195,"context_line":"            share_mappings \u003d sm.ShareMappingList.get_by_instance_uuid("},{"line_number":196,"context_line":"            self.context, uuids.instance)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9e5d92f9_14db847e","line":193,"updated":"2022-08-17 10:40:12.000000000","message":"this fits the previous line","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"b5eec71b57697a3350c1922ce46d82608c6b8860","unresolved":false,"context_lines":[{"line_number":190,"context_line":"class _TestShareMappingList(object):"},{"line_number":191,"context_line":"    def test_get_by_instance_uuid(self):"},{"line_number":192,"context_line":"        with mock.patch.object("},{"line_number":193,"context_line":"            db, \u0027share_mapping_get_by_instance_uuid\u0027) as get:"},{"line_number":194,"context_line":"            get.return_value \u003d [fake_share_mapping]"},{"line_number":195,"context_line":"            share_mappings \u003d sm.ShareMappingList.get_by_instance_uuid("},{"line_number":196,"context_line":"            self.context, uuids.instance)"}],"source_content_type":"text/x-python","patch_set":7,"id":"65f98311_302f2f7b","line":193,"in_reply_to":"9e5d92f9_14db847e","updated":"2022-08-25 17:01:28.000000000","message":"not it is too long by only 1 char. 😞\nSo I try to rewrite it differently to better highlight blocks.","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":true,"context_lines":[{"line_number":193,"context_line":"            db, \u0027share_mapping_get_by_instance_uuid\u0027) as get:"},{"line_number":194,"context_line":"            get.return_value \u003d [fake_share_mapping]"},{"line_number":195,"context_line":"            share_mappings \u003d sm.ShareMappingList.get_by_instance_uuid("},{"line_number":196,"context_line":"            self.context, uuids.instance)"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"            self.assertEqual(1, len(share_mappings))"},{"line_number":199,"context_line":"            self.assertIsInstance(share_mappings[0], sm.ShareMapping)"}],"source_content_type":"text/x-python","patch_set":7,"id":"aacf37d6_ee71cd72","line":196,"updated":"2022-08-17 10:40:12.000000000","message":"this is underindented. you need one level of indent","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"b5eec71b57697a3350c1922ce46d82608c6b8860","unresolved":false,"context_lines":[{"line_number":193,"context_line":"            db, \u0027share_mapping_get_by_instance_uuid\u0027) as get:"},{"line_number":194,"context_line":"            get.return_value \u003d [fake_share_mapping]"},{"line_number":195,"context_line":"            share_mappings \u003d sm.ShareMappingList.get_by_instance_uuid("},{"line_number":196,"context_line":"            self.context, uuids.instance)"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"            self.assertEqual(1, len(share_mappings))"},{"line_number":199,"context_line":"            self.assertIsInstance(share_mappings[0], sm.ShareMapping)"}],"source_content_type":"text/x-python","patch_set":7,"id":"94f25fd3_efaf7430","line":196,"in_reply_to":"aacf37d6_ee71cd72","updated":"2022-08-25 17:01:28.000000000","message":"Done","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":true,"context_lines":[{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    def test_get_by_share_id(self):"},{"line_number":202,"context_line":"        with mock.patch.object("},{"line_number":203,"context_line":"            db, \u0027share_mapping_get_by_share_id\u0027) as get:"},{"line_number":204,"context_line":"            get.return_value \u003d [fake_share_mapping]"},{"line_number":205,"context_line":"            share_mappings \u003d sm.ShareMappingList.get_by_share_id("},{"line_number":206,"context_line":"            self.context, uuids.share)"}],"source_content_type":"text/x-python","patch_set":7,"id":"180a27e0_76665ef2","line":203,"updated":"2022-08-17 10:40:12.000000000","message":"this fits the previous line\n\n(https://github.com/gibizer/blacken_selection can help formatting pieces of code from an IDE)","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"b5eec71b57697a3350c1922ce46d82608c6b8860","unresolved":false,"context_lines":[{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    def test_get_by_share_id(self):"},{"line_number":202,"context_line":"        with mock.patch.object("},{"line_number":203,"context_line":"            db, \u0027share_mapping_get_by_share_id\u0027) as get:"},{"line_number":204,"context_line":"            get.return_value \u003d [fake_share_mapping]"},{"line_number":205,"context_line":"            share_mappings \u003d sm.ShareMappingList.get_by_share_id("},{"line_number":206,"context_line":"            self.context, uuids.share)"}],"source_content_type":"text/x-python","patch_set":7,"id":"0255e4af_d08894f7","line":203,"in_reply_to":"180a27e0_76665ef2","updated":"2022-08-25 17:01:28.000000000","message":"Done","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":true,"context_lines":[{"line_number":209,"context_line":"            self.assertIsInstance(share_mappings[0], sm.ShareMapping)"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"class TestShareMapping(test_objects._LocalTest, _TestShareMapping):"},{"line_number":213,"context_line":"    pass"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"c7d93569_ce82cc67","line":212,"updated":"2022-08-17 10:40:12.000000000","message":"I guess we do all these hoops to be able to also call these tests remotely via test_objects._RemoteTest","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2c2ec2b01c41f5a9193cbe4d721bc9d63a424476","unresolved":true,"context_lines":[{"line_number":209,"context_line":"            self.assertIsInstance(share_mappings[0], sm.ShareMapping)"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"class TestShareMapping(test_objects._LocalTest, _TestShareMapping):"},{"line_number":213,"context_line":"    pass"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"ee6e6dd2_eeca7772","line":212,"in_reply_to":"b9aa9217_1c7e8f3e","updated":"2022-08-26 08:10:33.000000000","message":"sorry I wasn\u0027t specific enough.\n\nSo I think these tests are created in a base class _TestShareMapping to be able to call them twice. First with _LocalTest where the functions are called directly on the ovo. And second with _RemoteTest where the remotable methods are called via the RPC proxy (simulating that the function is called from in nova-compute but actually exectued in the nova-conductor to access the DB)\n\nThe volume_usage.py has the example for _RemoteTest too:\n\nclass TestRemoteVolumeUsage(test_objects._RemoteTest, _TestVolumeUsage):\n    pass\n\nSo you need similar one for ShareMapping too","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"b5eec71b57697a3350c1922ce46d82608c6b8860","unresolved":true,"context_lines":[{"line_number":209,"context_line":"            self.assertIsInstance(share_mappings[0], sm.ShareMapping)"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"class TestShareMapping(test_objects._LocalTest, _TestShareMapping):"},{"line_number":213,"context_line":"    pass"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"b9aa9217_1c7e8f3e","line":212,"in_reply_to":"c7d93569_ce82cc67","updated":"2022-08-25 17:01:28.000000000","message":"To be honest, I don\u0027t know 😋. This code is inspired by nova/tests/unit/objects/test_volume_usage.py, I did that several months ago, and I more or less tried to mimic it.","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"90e01c462c86fdd29169f197217e60f09448dbeb","unresolved":false,"context_lines":[{"line_number":209,"context_line":"            self.assertIsInstance(share_mappings[0], sm.ShareMapping)"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"class TestShareMapping(test_objects._LocalTest, _TestShareMapping):"},{"line_number":213,"context_line":"    pass"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"e445ffc1_96267e67","line":212,"in_reply_to":"ee6e6dd2_eeca7772","updated":"2022-08-29 09:13:43.000000000","message":"Done","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63556d50d0ee9c14128ce3282b81e8bb9bcd55cf","unresolved":true,"context_lines":[{"line_number":213,"context_line":"    pass"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"class TestShareMappingList(test_objects._LocalTest, _TestShareMappingList):"},{"line_number":217,"context_line":"    pass"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"07901b68_e6b0540f","line":216,"updated":"2022-08-17 10:40:12.000000000","message":"ditto","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"b5eec71b57697a3350c1922ce46d82608c6b8860","unresolved":false,"context_lines":[{"line_number":213,"context_line":"    pass"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"class TestShareMappingList(test_objects._LocalTest, _TestShareMappingList):"},{"line_number":217,"context_line":"    pass"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"012ff1ff_1d1ffa2f","line":216,"in_reply_to":"07901b68_e6b0540f","updated":"2022-08-25 17:01:28.000000000","message":"Done","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"818e8681aa16657357acab3fe0decd9cbf45e5aa","unresolved":true,"context_lines":[{"line_number":217,"context_line":"    pass"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"class TestShareMappingLibvirtNFS(test_objects._LocalTest):"},{"line_number":221,"context_line":"    @mock.patch("},{"line_number":222,"context_line":"        \u0027nova.virt.libvirt.volume.nfs.LibvirtNFSVolumeDriver\u0027)"},{"line_number":223,"context_line":"    @mock.patch("}],"source_content_type":"text/x-python","patch_set":7,"id":"5f85297a_804ec5b3","line":220,"updated":"2022-08-17 10:42:18.000000000","message":"coverage is missing for detach","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"b5eec71b57697a3350c1922ce46d82608c6b8860","unresolved":false,"context_lines":[{"line_number":217,"context_line":"    pass"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"class TestShareMappingLibvirtNFS(test_objects._LocalTest):"},{"line_number":221,"context_line":"    @mock.patch("},{"line_number":222,"context_line":"        \u0027nova.virt.libvirt.volume.nfs.LibvirtNFSVolumeDriver\u0027)"},{"line_number":223,"context_line":"    @mock.patch("}],"source_content_type":"text/x-python","patch_set":7,"id":"d8e3932e_036d41ef","line":220,"in_reply_to":"5f85297a_804ec5b3","updated":"2022-08-25 17:01:28.000000000","message":"Done","commit_id":"3a14e0ec00d8eaeea25907531aec17a1921934b6"}]}