)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"041aafa2f9a5b08b78407303a69721d178b0174e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"a356a7c5_2b474cc1","updated":"2025-07-11 08:12:10.000000000","message":"I tested this patch, \n\n1, initial state\n\n$ openstack port show test-port-val-spec-my_port-m3stbv7m7byy |grep binding_profile\n| binding_profile         | spoof\u003d\u0027false\u0027, trusted\u003d\u0027true\u0027   \n\n2, update template\n\nvalue_specs: {\"binding:profile\": {\u0027spoof\u0027: \u0027false\u0027, \u0027trusted\u0027:\u0027false\u0027}} \n\n3, the result\n\n$ openstack port show test-port-val-spec-my_port-m3stbv7m7byy |grep binding_profile\n| binding_profile         | trusted\u003d\u0027false\u0027\n\nThe result aligns with the behavior of the original code, when a field has the same value as before, it gets removed (I just moved this logic to  in binding:profile dict as well).\n\nHowever, I don\u0027t think it\u0027s reasonable for the filed (\u0027spoof\u0027: \u0027false\u0027) to be removed as well in this case.\n\nIt feels like neither the current code nor the previous logic behaves in a truly declarative way. Should I change this orignal logic?","commit_id":"674cbe5b58fb40975e3a8c67fbe58a112d6301a3"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"e900d752f4fdcea0a8b101822ddc0627aa3950e6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"f2f89de7_daf12926","updated":"2025-08-08 01:49:59.000000000","message":"recheck","commit_id":"ca512c2f2c30492073e4124765ce48fa3d911aec"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"5db4752065103f924c5256a4588add42ccb020f2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"bfda4c04_537bbf42","updated":"2025-11-13 07:55:44.000000000","message":"Looks good to me.","commit_id":"8f1bd2646a9106012ea49a0f3f5872c371c52f80"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"1d6a65b4d938114d8040d46551ebb30833c58371","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"b81a881e_43d3ae05","updated":"2025-11-21 14:06:04.000000000","message":"The CI will not run green until this change is merged: https://review.opendev.org/c/openstack/heat/+/966092 - wait for that to merge, then rebase this change.","commit_id":"59931c88dde3c734f618194618c4391425ed4056"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"c217beb4aa087ee3894c5f23d7a8244207f4dbe5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"5f7aa35d_412dbbed","updated":"2025-11-21 12:09:04.000000000","message":"recheck","commit_id":"59931c88dde3c734f618194618c4391425ed4056"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"d37b040b913dc58c4be1dcfd30794481a975bf7f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"cc97e3eb_f1795b59","updated":"2025-11-21 10:17:40.000000000","message":"recheck","commit_id":"59931c88dde3c734f618194618c4391425ed4056"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"a2b39b44162113cc93790cf9671d69c9ba8ba07a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"6e1d9c4e_ec49fc43","updated":"2025-11-21 05:48:05.000000000","message":"this is the test result for patch set 6 - https://paste.ubuntu.com/p/mZvWmshtV4/","commit_id":"59931c88dde3c734f618194618c4391425ed4056"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"3939160b8eea65a151f009c0219abdda5b4a8702","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"03200fc2_a7577e68","updated":"2025-11-21 10:17:30.000000000","message":"timeout issue, will recheck\n\nheat_tempest_plugin.common.exceptions.TimeoutException: Request timed out\nDetails: Stack ParallelDeploymentsTest-1011293136/7ae23d29-e71e-4da7-8ed7-d2ba3979b40a failed to reach CREATE_COMPLETE status within the required time (1200 s)","commit_id":"59931c88dde3c734f618194618c4391425ed4056"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"15f3052f3c03a4f771975842451bb46bb68267fe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"73178c28_a90be81c","updated":"2025-12-05 07:20:41.000000000","message":"Hi Zane, thank you for the review. I have refactored the code and this is the latest test result - https://paste.openstack.org/show/bna6KBX9JJqzNTjcpC44/","commit_id":"9db3415a1114c3a3a578024c4864330aefbce1ca"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"5f8627417cf9fcb5e5239b9407759448ff4900a4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"9dd0ef2f_d120f4ac","updated":"2025-12-15 07:36:57.000000000","message":"Hi Takashi, thanks so much for your feedback, I\u0027ve addressed all your comments, and this is the latest test result for patch set 9 - https://paste.openstack.org/show/brpwxlgX5F9U2UScSvyI/","commit_id":"90073905e9086545ca4f3a2e6aa83001f9a21022"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"0a0dd1f6ee60ac4a085ce188c928b502b628dca5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"b2b2b9fb_bfd02403","updated":"2026-03-09 11:26:05.000000000","message":"Hi Takashi, thank you so much for your feedback! I’ve addressed all your comments.","commit_id":"7179fdb576c8583fce988024b762a0784a9f30f2"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"4f42ce2f0d32657ea4c41601fb6e9bf8d8dbff14","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"b9e0cc7b_57568574","updated":"2026-03-11 08:19:39.000000000","message":"Hi Takashi, here is the latest test result for patchset 12 - https://paste.openstack.org/show/bd9564Qbz0r69o4gfV9z/\nI also specifically tested the scenario where neutron has transient issue which caused this specfic call to fail, it works.\nThanks a lot for your review.","commit_id":"577ebbfbcf810462f376db49305c8f43094d6985"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"91a5a18d951f6fd92857943292e3088eb73c03f0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"3e5e6ea1_f55c431a","updated":"2026-03-09 11:40:35.000000000","message":"Hi Takashi, thank you for your prompt review. I\u0027ve deleted these comments as you suggested.","commit_id":"577ebbfbcf810462f376db49305c8f43094d6985"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"57380270c4d9d3eee12b01e808869a1e38f99b99","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"27469bea_0ff94b84","updated":"2026-03-20 09:23:23.000000000","message":"Hi Rabi, thanks a lot for your review. I\u0027ve addressed all your comments. and this is test result for this patchset 13 - https://paste.openstack.org/show/bUeIQ1FTQgev9U3HEbA9/","commit_id":"16fbce7c9fadda9deeb432f8ad5bfdf161af8113"},{"author":{"_account_id":8833,"name":"Rabi Mishra","email":"ramishra@redhat.com","username":"rabi"},"change_message_id":"a58cccd41f68b4bb85bcc479a273dd5636967e87","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"e64fcf70_48acfe0a","updated":"2026-03-23 06:02:54.000000000","message":"I think there are too many unit tests added. We just have to test the 3 way merge and nothing specific to SRIOV etc. Otherwise LGTM.","commit_id":"16fbce7c9fadda9deeb432f8ad5bfdf161af8113"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"d3f3467eac4ebc2ce14919361582c39a0b83f84a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"b18c68b4_4eb5ecc8","updated":"2026-03-24 10:54:52.000000000","message":"Hi @ramishra@redhat.com , \n\nI\u0027ve addressed your comments by updating the tests:\n\n1, Keeping only the three-way merge unit test (test_prepare_update_properties_binding_profile).\n2, Removing the additional scenario-style tests (including test_update_port_with_sriov_binding_profile, test_update_port_non_binding_profile_no_show, and the exception-path case).\n3, Moving test_prepare_update_properties_binding_profile from UpdatePortTest to NeutronPortTest to avoid scenario parameterization.\n\nMany thanks for your review and suggestions!","commit_id":"099b6dea629df424006bd13469b4ca53aa036082"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"30e167d69411cc5f058133550667e45983a2dbdb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"d0714a24_8b5dfa85","updated":"2026-03-24 14:15:54.000000000","message":"The failed grenade-heat-multinode-skip-level does not provide any logs, so let\u0027s try again","commit_id":"099b6dea629df424006bd13469b4ca53aa036082"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"cd85bb84aac3ec63c32ae74e97946265e98eaec9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"58b0227c_8163aa8b","updated":"2026-03-24 14:21:05.000000000","message":"recheck - grenade-heat-multinode-skip-level failed with no logs, likely infrastructure issue.","commit_id":"099b6dea629df424006bd13469b4ca53aa036082"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"476ce72deae3842a61e41d70801200adfcf78853","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"ee655804_779a62b3","updated":"2026-03-25 01:14:43.000000000","message":"recheck - still no any log from https://zuul.opendev.org/t/openstack/build/f60a45ff88e44a349e3b7b79c935851a","commit_id":"099b6dea629df424006bd13469b4ca53aa036082"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"ea53149f54a74d825813aa808247541095855929","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"0f723c11_706bd71a","updated":"2026-03-26 10:23:49.000000000","message":"recheck: likely transient Neutron/Nova race; previous run passed.","commit_id":"099b6dea629df424006bd13469b4ca53aa036082"}],"heat/engine/resources/openstack/neutron/neutron.py":[{"author":{"_account_id":8833,"name":"Rabi Mishra","email":"ramishra@redhat.com","username":"rabi"},"change_message_id":"f6a95df6c72f37f7d27f096574b188726bfd74c0","unresolved":true,"context_lines":[{"line_number":99,"context_line":"                for k in list(value_spec_props):"},{"line_number":100,"context_line":"                    # Preserve existing fields not specified in the template"},{"line_number":101,"context_line":"                    old_val \u003d before_value_specs.get(k, None)"},{"line_number":102,"context_line":"                    if (k \u003d\u003d \u0027binding:profile\u0027"},{"line_number":103,"context_line":"                            and isinstance(value_spec_props[k], dict)"},{"line_number":104,"context_line":"                            and isinstance(old_val, dict)):"},{"line_number":105,"context_line":"                        merged_profile \u003d old_val.copy()"}],"source_content_type":"text/x-python","patch_set":4,"id":"42303e23_8e1f0b8b","line":102,"updated":"2026-03-16 10:23:20.000000000","message":"Though looks ok, I\u0027m not a f","commit_id":"ca512c2f2c30492073e4124765ce48fa3d911aec"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"57380270c4d9d3eee12b01e808869a1e38f99b99","unresolved":false,"context_lines":[{"line_number":99,"context_line":"                for k in list(value_spec_props):"},{"line_number":100,"context_line":"                    # Preserve existing fields not specified in the template"},{"line_number":101,"context_line":"                    old_val \u003d before_value_specs.get(k, None)"},{"line_number":102,"context_line":"                    if (k \u003d\u003d \u0027binding:profile\u0027"},{"line_number":103,"context_line":"                            and isinstance(value_spec_props[k], dict)"},{"line_number":104,"context_line":"                            and isinstance(old_val, dict)):"},{"line_number":105,"context_line":"                        merged_profile \u003d old_val.copy()"}],"source_content_type":"text/x-python","patch_set":4,"id":"2245ad81_75392aa7","line":102,"in_reply_to":"42303e23_8e1f0b8b","updated":"2026-03-20 09:23:23.000000000","message":"Done","commit_id":"ca512c2f2c30492073e4124765ce48fa3d911aec"},{"author":{"_account_id":8833,"name":"Rabi Mishra","email":"ramishra@redhat.com","username":"rabi"},"change_message_id":"a58cccd41f68b4bb85bcc479a273dd5636967e87","unresolved":false,"context_lines":[{"line_number":99,"context_line":"                for k in list(value_spec_props):"},{"line_number":100,"context_line":"                    # Preserve existing fields not specified in the template"},{"line_number":101,"context_line":"                    old_val \u003d before_value_specs.get(k, None)"},{"line_number":102,"context_line":"                    if (k \u003d\u003d \u0027binding:profile\u0027"},{"line_number":103,"context_line":"                            and isinstance(value_spec_props[k], dict)"},{"line_number":104,"context_line":"                            and isinstance(old_val, dict)):"},{"line_number":105,"context_line":"                        merged_profile \u003d old_val.copy()"}],"source_content_type":"text/x-python","patch_set":4,"id":"5133f2fe_a2b5eecb","line":102,"in_reply_to":"42303e23_8e1f0b8b","updated":"2026-03-23 06:02:54.000000000","message":"Ignore this..Maybe I had some draft comment earlier.","commit_id":"ca512c2f2c30492073e4124765ce48fa3d911aec"},{"author":{"_account_id":4257,"name":"Zane Bitter","email":"zbitter@redhat.com","username":"zaneb"},"change_message_id":"7cb4a163e20cb2cd5cd62d4c22469cbae0064210","unresolved":true,"context_lines":[{"line_number":30,"context_line":"    # are often used by users to request deletion of keys inside"},{"line_number":31,"context_line":"    # value_specs-based fields such as binding:profile."},{"line_number":32,"context_line":"    # This list contains deletion sentinel values recognized by Heat."},{"line_number":33,"context_line":"    DELETE_SENTINELS \u003d {None, \u0027\u0027, \u0027null\u0027, \u0027Null\u0027, \u0027NULL\u0027, \u0027~\u0027}"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def get_resource_plural(self):"},{"line_number":36,"context_line":"        \"\"\"Return the plural of resource type."}],"source_content_type":"text/x-python","patch_set":7,"id":"9d170c6b_0ef8511d","line":33,"updated":"2025-11-24 00:29:15.000000000","message":"`null` and `~` are yaml-level nulls (that will be parsed as `None` in Python), but `\"null\"` and `\"~\"` are not. We should not use magic strings for in-band signalling of stuff that YAML natively supports.","commit_id":"59931c88dde3c734f618194618c4391425ed4056"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"15f3052f3c03a4f771975842451bb46bb68267fe","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    # are often used by users to request deletion of keys inside"},{"line_number":31,"context_line":"    # value_specs-based fields such as binding:profile."},{"line_number":32,"context_line":"    # This list contains deletion sentinel values recognized by Heat."},{"line_number":33,"context_line":"    DELETE_SENTINELS \u003d {None, \u0027\u0027, \u0027null\u0027, \u0027Null\u0027, \u0027NULL\u0027, \u0027~\u0027}"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def get_resource_plural(self):"},{"line_number":36,"context_line":"        \"\"\"Return the plural of resource type."}],"source_content_type":"text/x-python","patch_set":7,"id":"b08898be_af33d826","line":33,"in_reply_to":"9d170c6b_0ef8511d","updated":"2025-12-05 07:20:41.000000000","message":"I have deleted DELETE_SENTINELS now","commit_id":"59931c88dde3c734f618194618c4391425ed4056"},{"author":{"_account_id":4257,"name":"Zane Bitter","email":"zbitter@redhat.com","username":"zaneb"},"change_message_id":"7cb4a163e20cb2cd5cd62d4c22469cbae0064210","unresolved":true,"context_lines":[{"line_number":111,"context_line":"                        merged_profile \u003d old_val.copy()"},{"line_number":112,"context_line":"                        for pk, pv in value_spec_props[k].items():"},{"line_number":113,"context_line":"                            # Skip unhashable types (list, dict)"},{"line_number":114,"context_line":"                            if (not isinstance(pv, (list, dict)) and"},{"line_number":115,"context_line":"                                    pv in NeutronResource.DELETE_SENTINELS):"},{"line_number":116,"context_line":"                                # Handle deletion semantics for value_specs"},{"line_number":117,"context_line":"                                merged_profile.pop(pk, None)"}],"source_content_type":"text/x-python","patch_set":7,"id":"ef40d676_b2b4a6df","line":114,"updated":"2025-11-24 00:29:15.000000000","message":"In every other place in Heat, removing a field in an update is equivalent to changing the value. Here is the only place I am aware of where you have to explicitly set it to null.\n\nAs mentioned in https://storyboard.openstack.org/#!/story/2011481#comment-230785 two sources of data is not enough to do this correctly, you have to have a 3-way diff:\n\n```\nset_val \u003d {}\nfor pk in set(old_val) | set(value_spec_props[k]):\n   if old_val.get(pk) !\u003d value_spec_props[k].get(pk):\n     set_val \u003d value_spec_props[k].get(pk)\nmerged_profile \u003d current_profile.copy()\nfor pk, pv in set_val.items():\n    if pv is None:\n        merged_profile.pop(pk, None)\n    else:\n        merged_profile[pk] \u003d [pv]\n```\n\nHowever, as noted in the comment linked above, this still relies on accurate knowledge of `old_val` (by which I mean the value from the previous template), which cannot be guaranteed after a previous update failure because we don\u0027t record enough information to know if this part of the update succeeded. Diffing against _every_ previous template in the first loop would work, but from memory I don\u0027t think we have access to that information here.\n\nAs I mentioned, if the list of parameters inserted by Neutron is known, then we should probably just special-case those ones. If not then the 3-way merge is probably better than the current behaviour in almost every case at least.","commit_id":"59931c88dde3c734f618194618c4391425ed4056"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"15f3052f3c03a4f771975842451bb46bb68267fe","unresolved":false,"context_lines":[{"line_number":111,"context_line":"                        merged_profile \u003d old_val.copy()"},{"line_number":112,"context_line":"                        for pk, pv in value_spec_props[k].items():"},{"line_number":113,"context_line":"                            # Skip unhashable types (list, dict)"},{"line_number":114,"context_line":"                            if (not isinstance(pv, (list, dict)) and"},{"line_number":115,"context_line":"                                    pv in NeutronResource.DELETE_SENTINELS):"},{"line_number":116,"context_line":"                                # Handle deletion semantics for value_specs"},{"line_number":117,"context_line":"                                merged_profile.pop(pk, None)"}],"source_content_type":"text/x-python","patch_set":7,"id":"df510e0f_173c22f3","line":114,"in_reply_to":"ef40d676_b2b4a6df","updated":"2025-12-05 07:20:41.000000000","message":"Zane, thanks for the detailed guidance. You\u0027re absolutely right that 2-way diff \nis insufficient here.\n\nI\u0027ve implemented the 3-way merge as you suggested, but placed it in \nprepare_update_properties() instead of merge_value_specs() to minimize \nthe impact on other value_specs fields.","commit_id":"59931c88dde3c734f618194618c4391425ed4056"},{"author":{"_account_id":4257,"name":"Zane Bitter","email":"zbitter@redhat.com","username":"zaneb"},"change_message_id":"7cb4a163e20cb2cd5cd62d4c22469cbae0064210","unresolved":true,"context_lines":[{"line_number":122,"context_line":"                    elif (not isinstance(value_spec_props[k], (list, dict))"},{"line_number":123,"context_line":"                          and value_spec_props[k]"},{"line_number":124,"context_line":"                          in NeutronResource.DELETE_SENTINELS):"},{"line_number":125,"context_line":"                        value_spec_props.pop(k, None)"},{"line_number":126,"context_line":"            props.update(value_spec_props)"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"    def prepare_update_properties(self, prop_diff):"}],"source_content_type":"text/x-python","patch_set":7,"id":"ccf64c8c_e3ab38b6","line":125,"updated":"2025-11-24 00:29:15.000000000","message":"nit: no default is required because k is by definition in value_spec_props here.","commit_id":"59931c88dde3c734f618194618c4391425ed4056"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"15f3052f3c03a4f771975842451bb46bb68267fe","unresolved":false,"context_lines":[{"line_number":122,"context_line":"                    elif (not isinstance(value_spec_props[k], (list, dict))"},{"line_number":123,"context_line":"                          and value_spec_props[k]"},{"line_number":124,"context_line":"                          in NeutronResource.DELETE_SENTINELS):"},{"line_number":125,"context_line":"                        value_spec_props.pop(k, None)"},{"line_number":126,"context_line":"            props.update(value_spec_props)"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"    def prepare_update_properties(self, prop_diff):"}],"source_content_type":"text/x-python","patch_set":7,"id":"5f739e77_449e3d49","line":125,"in_reply_to":"ccf64c8c_e3ab38b6","updated":"2025-12-05 07:20:41.000000000","message":"I\u0027ve implemented the 3-way merge in prepare_update_properties, so I have restored merge_value_specs, it\u0027s the same as before","commit_id":"59931c88dde3c734f618194618c4391425ed4056"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"8d0a2706701bafe38be227b2cd00762207289a07","unresolved":true,"context_lines":[{"line_number":114,"context_line":"            # Neutron API to preserve auto-populated fields (e.g., SR-IOV"},{"line_number":115,"context_line":"            # hardware info)."},{"line_number":116,"context_line":"            binding_profile_handled \u003d False"},{"line_number":117,"context_line":"            if (\u0027binding:profile\u0027 in prop_diff[\u0027value_specs\u0027] and"},{"line_number":118,"context_line":"                    self.resource_id):"},{"line_number":119,"context_line":"                try:"},{"line_number":120,"context_line":"                    current_attrs \u003d self._show_resource()"}],"source_content_type":"text/x-python","patch_set":8,"id":"407c693b_c6fb2790","line":117,"range":{"start_line":117,"start_character":16,"end_line":117,"end_character":33},"updated":"2025-12-14 14:53:07.000000000","message":"The prepare_update_properties method is used by multiple resources, and adding this logic quite specific to port does not look correct. If we do this then we should provide this key as an arg or we should override this method in a specific child classes.","commit_id":"9db3415a1114c3a3a578024c4864330aefbce1ca"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"11d95066a13c13e773d93bb915f2c7b2d8883320","unresolved":false,"context_lines":[{"line_number":114,"context_line":"            # Neutron API to preserve auto-populated fields (e.g., SR-IOV"},{"line_number":115,"context_line":"            # hardware info)."},{"line_number":116,"context_line":"            binding_profile_handled \u003d False"},{"line_number":117,"context_line":"            if (\u0027binding:profile\u0027 in prop_diff[\u0027value_specs\u0027] and"},{"line_number":118,"context_line":"                    self.resource_id):"},{"line_number":119,"context_line":"                try:"},{"line_number":120,"context_line":"                    current_attrs \u003d self._show_resource()"}],"source_content_type":"text/x-python","patch_set":8,"id":"023ecf62_c16428ab","line":117,"range":{"start_line":117,"start_character":16,"end_line":117,"end_character":33},"in_reply_to":"12589f1d_540b3315","updated":"2025-12-15 07:54:29.000000000","message":"Done","commit_id":"9db3415a1114c3a3a578024c4864330aefbce1ca"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"5f8627417cf9fcb5e5239b9407759448ff4900a4","unresolved":true,"context_lines":[{"line_number":114,"context_line":"            # Neutron API to preserve auto-populated fields (e.g., SR-IOV"},{"line_number":115,"context_line":"            # hardware info)."},{"line_number":116,"context_line":"            binding_profile_handled \u003d False"},{"line_number":117,"context_line":"            if (\u0027binding:profile\u0027 in prop_diff[\u0027value_specs\u0027] and"},{"line_number":118,"context_line":"                    self.resource_id):"},{"line_number":119,"context_line":"                try:"},{"line_number":120,"context_line":"                    current_attrs \u003d self._show_resource()"}],"source_content_type":"text/x-python","patch_set":8,"id":"12589f1d_540b3315","line":117,"range":{"start_line":117,"start_character":16,"end_line":117,"end_character":33},"in_reply_to":"407c693b_c6fb2790","updated":"2025-12-15 07:36:57.000000000","message":"Good catch, Takashi, thanks, I\u0027ve moved the binding:profile 3-way merge logic from the generic NeutronResource base class to the Port subclass by overriding prepare_update_properties(). This keeps port-specific logic out of the base class and makes it clearer.","commit_id":"9db3415a1114c3a3a578024c4864330aefbce1ca"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"8d0a2706701bafe38be227b2cd00762207289a07","unresolved":true,"context_lines":[{"line_number":121,"context_line":"                    if current_attrs and \u0027binding:profile\u0027 in current_attrs:"},{"line_number":122,"context_line":"                        api_profile \u003d current_attrs[\u0027binding:profile\u0027]"},{"line_number":123,"context_line":"                        # Ensure we have dicts"},{"line_number":124,"context_line":"                        if isinstance(api_profile, dict):"},{"line_number":125,"context_line":"                            old_specs \u003d before_value_specs or {}"},{"line_number":126,"context_line":"                            old_profile \u003d old_specs.get(\u0027binding:profile\u0027, {})"},{"line_number":127,"context_line":"                            new_profile \u003d prop_diff[\u0027value_specs\u0027]["}],"source_content_type":"text/x-python","patch_set":8,"id":"af89b7dc_0f509c38","line":124,"range":{"start_line":124,"start_character":24,"end_line":124,"end_character":57},"updated":"2025-12-14 14:53:07.000000000","message":"If neutron does not accept anything other than dict then IMO this is just redundant.","commit_id":"9db3415a1114c3a3a578024c4864330aefbce1ca"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"11d95066a13c13e773d93bb915f2c7b2d8883320","unresolved":false,"context_lines":[{"line_number":121,"context_line":"                    if current_attrs and \u0027binding:profile\u0027 in current_attrs:"},{"line_number":122,"context_line":"                        api_profile \u003d current_attrs[\u0027binding:profile\u0027]"},{"line_number":123,"context_line":"                        # Ensure we have dicts"},{"line_number":124,"context_line":"                        if isinstance(api_profile, dict):"},{"line_number":125,"context_line":"                            old_specs \u003d before_value_specs or {}"},{"line_number":126,"context_line":"                            old_profile \u003d old_specs.get(\u0027binding:profile\u0027, {})"},{"line_number":127,"context_line":"                            new_profile \u003d prop_diff[\u0027value_specs\u0027]["}],"source_content_type":"text/x-python","patch_set":8,"id":"a3a044a5_efd38730","line":124,"range":{"start_line":124,"start_character":24,"end_line":124,"end_character":57},"in_reply_to":"63f3d5f4_e4076371","updated":"2025-12-15 07:54:29.000000000","message":"Done","commit_id":"9db3415a1114c3a3a578024c4864330aefbce1ca"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"5f8627417cf9fcb5e5239b9407759448ff4900a4","unresolved":true,"context_lines":[{"line_number":121,"context_line":"                    if current_attrs and \u0027binding:profile\u0027 in current_attrs:"},{"line_number":122,"context_line":"                        api_profile \u003d current_attrs[\u0027binding:profile\u0027]"},{"line_number":123,"context_line":"                        # Ensure we have dicts"},{"line_number":124,"context_line":"                        if isinstance(api_profile, dict):"},{"line_number":125,"context_line":"                            old_specs \u003d before_value_specs or {}"},{"line_number":126,"context_line":"                            old_profile \u003d old_specs.get(\u0027binding:profile\u0027, {})"},{"line_number":127,"context_line":"                            new_profile \u003d prop_diff[\u0027value_specs\u0027]["}],"source_content_type":"text/x-python","patch_set":8,"id":"63f3d5f4_e4076371","line":124,"range":{"start_line":124,"start_character":24,"end_line":124,"end_character":57},"in_reply_to":"af89b7dc_0f509c38","updated":"2025-12-15 07:36:57.000000000","message":"You\u0027re right. I\u0027ve removed it. The Neutron API guarantees binding:profile is always a dict.","commit_id":"9db3415a1114c3a3a578024c4864330aefbce1ca"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"8d0a2706701bafe38be227b2cd00762207289a07","unresolved":true,"context_lines":[{"line_number":126,"context_line":"                            old_profile \u003d old_specs.get(\u0027binding:profile\u0027, {})"},{"line_number":127,"context_line":"                            new_profile \u003d prop_diff[\u0027value_specs\u0027]["},{"line_number":128,"context_line":"                                \u0027binding:profile\u0027]"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"                            if (isinstance(old_profile, dict) and"},{"line_number":131,"context_line":"                                    isinstance(new_profile, dict)):"},{"line_number":132,"context_line":"                                # 3-way merge logic"},{"line_number":133,"context_line":"                                current_profile \u003d api_profile"},{"line_number":134,"context_line":"                                set_val \u003d {}"}],"source_content_type":"text/x-python","patch_set":8,"id":"9fe52ef8_476e3fbf","line":131,"range":{"start_line":129,"start_character":0,"end_line":131,"end_character":67},"updated":"2025-12-14 14:53:07.000000000","message":"this means that setting non dict value for binding:profile is silently ignored. We should at least add a validation to avoid non-string for new.","commit_id":"9db3415a1114c3a3a578024c4864330aefbce1ca"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"5f8627417cf9fcb5e5239b9407759448ff4900a4","unresolved":true,"context_lines":[{"line_number":126,"context_line":"                            old_profile \u003d old_specs.get(\u0027binding:profile\u0027, {})"},{"line_number":127,"context_line":"                            new_profile \u003d prop_diff[\u0027value_specs\u0027]["},{"line_number":128,"context_line":"                                \u0027binding:profile\u0027]"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"                            if (isinstance(old_profile, dict) and"},{"line_number":131,"context_line":"                                    isinstance(new_profile, dict)):"},{"line_number":132,"context_line":"                                # 3-way merge logic"},{"line_number":133,"context_line":"                                current_profile \u003d api_profile"},{"line_number":134,"context_line":"                                set_val \u003d {}"}],"source_content_type":"text/x-python","patch_set":8,"id":"af66911d_d8817de4","line":131,"range":{"start_line":129,"start_character":0,"end_line":131,"end_character":67},"in_reply_to":"9fe52ef8_476e3fbf","updated":"2025-12-15 07:36:57.000000000","message":"Removed all isinstance checks for old_profile and new_profile as well. Heat\u0027s schema validation already ensures binding:profile is a dict.","commit_id":"9db3415a1114c3a3a578024c4864330aefbce1ca"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"11d95066a13c13e773d93bb915f2c7b2d8883320","unresolved":false,"context_lines":[{"line_number":126,"context_line":"                            old_profile \u003d old_specs.get(\u0027binding:profile\u0027, {})"},{"line_number":127,"context_line":"                            new_profile \u003d prop_diff[\u0027value_specs\u0027]["},{"line_number":128,"context_line":"                                \u0027binding:profile\u0027]"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"                            if (isinstance(old_profile, dict) and"},{"line_number":131,"context_line":"                                    isinstance(new_profile, dict)):"},{"line_number":132,"context_line":"                                # 3-way merge logic"},{"line_number":133,"context_line":"                                current_profile \u003d api_profile"},{"line_number":134,"context_line":"                                set_val \u003d {}"}],"source_content_type":"text/x-python","patch_set":8,"id":"5fcd0a86_e5ab8bad","line":131,"range":{"start_line":129,"start_character":0,"end_line":131,"end_character":67},"in_reply_to":"af66911d_d8817de4","updated":"2025-12-15 07:54:29.000000000","message":"Done","commit_id":"9db3415a1114c3a3a578024c4864330aefbce1ca"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"8d0a2706701bafe38be227b2cd00762207289a07","unresolved":true,"context_lines":[{"line_number":135,"context_line":"                                for pk in set(old_profile) | set(new_profile):"},{"line_number":136,"context_line":"                                    # Only process keys that appear in"},{"line_number":137,"context_line":"                                    # new_profile (omitted keys are kept)"},{"line_number":138,"context_line":"                                    if pk in new_profile:"},{"line_number":139,"context_line":"                                        old_val \u003d old_profile.get(pk)"},{"line_number":140,"context_line":"                                        new_val \u003d new_profile.get(pk)"},{"line_number":141,"context_line":"                                        if old_val !\u003d new_val:"}],"source_content_type":"text/x-python","patch_set":8,"id":"5da50d5d_1c5543cf","line":138,"range":{"start_line":138,"start_character":36,"end_line":138,"end_character":57},"updated":"2025-12-14 14:53:07.000000000","message":"If you use keys from new_profiles then what is the point of L135 ?","commit_id":"9db3415a1114c3a3a578024c4864330aefbce1ca"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"5f8627417cf9fcb5e5239b9407759448ff4900a4","unresolved":true,"context_lines":[{"line_number":135,"context_line":"                                for pk in set(old_profile) | set(new_profile):"},{"line_number":136,"context_line":"                                    # Only process keys that appear in"},{"line_number":137,"context_line":"                                    # new_profile (omitted keys are kept)"},{"line_number":138,"context_line":"                                    if pk in new_profile:"},{"line_number":139,"context_line":"                                        old_val \u003d old_profile.get(pk)"},{"line_number":140,"context_line":"                                        new_val \u003d new_profile.get(pk)"},{"line_number":141,"context_line":"                                        if old_val !\u003d new_val:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ab6f63b3_7384056f","line":138,"range":{"start_line":138,"start_character":36,"end_line":138,"end_character":57},"in_reply_to":"5da50d5d_1c5543cf","updated":"2025-12-15 07:36:57.000000000","message":"Good catch, I\u0027ve removed this line now.","commit_id":"9db3415a1114c3a3a578024c4864330aefbce1ca"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"11d95066a13c13e773d93bb915f2c7b2d8883320","unresolved":false,"context_lines":[{"line_number":135,"context_line":"                                for pk in set(old_profile) | set(new_profile):"},{"line_number":136,"context_line":"                                    # Only process keys that appear in"},{"line_number":137,"context_line":"                                    # new_profile (omitted keys are kept)"},{"line_number":138,"context_line":"                                    if pk in new_profile:"},{"line_number":139,"context_line":"                                        old_val \u003d old_profile.get(pk)"},{"line_number":140,"context_line":"                                        new_val \u003d new_profile.get(pk)"},{"line_number":141,"context_line":"                                        if old_val !\u003d new_val:"}],"source_content_type":"text/x-python","patch_set":8,"id":"00e1ab29_8d7a6043","line":138,"range":{"start_line":138,"start_character":36,"end_line":138,"end_character":57},"in_reply_to":"ab6f63b3_7384056f","updated":"2025-12-15 07:54:29.000000000","message":"Done","commit_id":"9db3415a1114c3a3a578024c4864330aefbce1ca"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"8d0a2706701bafe38be227b2cd00762207289a07","unresolved":true,"context_lines":[{"line_number":153,"context_line":"                                    \u0027binding:profile\u0027] \u003d merged_profile"},{"line_number":154,"context_line":"                                binding_profile_handled \u003d True"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"                except Exception as ex:"},{"line_number":157,"context_line":"                    LOG.warning(\"Failed to fetch current resource attributes \""},{"line_number":158,"context_line":"                                \"for value_specs merging: %s. Falling back to \""},{"line_number":159,"context_line":"                                \"template-based merge for binding:profile, \""}],"source_content_type":"text/x-python","patch_set":8,"id":"b0d9e8c2_4f35a659","line":156,"range":{"start_line":156,"start_character":16,"end_line":156,"end_character":39},"updated":"2025-12-14 14:53:07.000000000","message":"What is the excat case when this exception is used ? I\u0027m unsure if ignoring an exception is a safe choise here.","commit_id":"9db3415a1114c3a3a578024c4864330aefbce1ca"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"5f8627417cf9fcb5e5239b9407759448ff4900a4","unresolved":true,"context_lines":[{"line_number":153,"context_line":"                                    \u0027binding:profile\u0027] \u003d merged_profile"},{"line_number":154,"context_line":"                                binding_profile_handled \u003d True"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"                except Exception as ex:"},{"line_number":157,"context_line":"                    LOG.warning(\"Failed to fetch current resource attributes \""},{"line_number":158,"context_line":"                                \"for value_specs merging: %s. Falling back to \""},{"line_number":159,"context_line":"                                \"template-based merge for binding:profile, \""}],"source_content_type":"text/x-python","patch_set":8,"id":"b37ac684_f7f459c1","line":156,"range":{"start_line":156,"start_character":16,"end_line":156,"end_character":39},"in_reply_to":"b0d9e8c2_4f35a659","updated":"2025-12-15 07:36:57.000000000","message":"Hi Takashi, I think ignoring an exception here should be safe, because:\n\n3-way merge is an optimization, not essential for the update to succeed\nFallback to template-based merge (pre-existing behavior)\nTransient API failures shouldn\u0027t block the entire update\nAdded clear warning logs so users know when fallback occurs.\n\nI\u0027ve added more comments here, thank you.","commit_id":"9db3415a1114c3a3a578024c4864330aefbce1ca"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"11d95066a13c13e773d93bb915f2c7b2d8883320","unresolved":false,"context_lines":[{"line_number":153,"context_line":"                                    \u0027binding:profile\u0027] \u003d merged_profile"},{"line_number":154,"context_line":"                                binding_profile_handled \u003d True"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"                except Exception as ex:"},{"line_number":157,"context_line":"                    LOG.warning(\"Failed to fetch current resource attributes \""},{"line_number":158,"context_line":"                                \"for value_specs merging: %s. Falling back to \""},{"line_number":159,"context_line":"                                \"template-based merge for binding:profile, \""}],"source_content_type":"text/x-python","patch_set":8,"id":"450f8d95_88c07959","line":156,"range":{"start_line":156,"start_character":16,"end_line":156,"end_character":39},"in_reply_to":"b37ac684_f7f459c1","updated":"2025-12-15 07:54:29.000000000","message":"Done","commit_id":"9db3415a1114c3a3a578024c4864330aefbce1ca"}],"heat/engine/resources/openstack/neutron/port.py":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"829cbf6a266f81829071c0142955602e41c2ca0b","unresolved":true,"context_lines":[{"line_number":657,"context_line":"                            \u0027binding:profile\u0027] \u003d merged_profile"},{"line_number":658,"context_line":"                        binding_profile_handled \u003d True"},{"line_number":659,"context_line":""},{"line_number":660,"context_line":"                except Exception as ex:"},{"line_number":661,"context_line":"                    # API call failed, fall back to template-based merge."},{"line_number":662,"context_line":"                    # This is safe because 3-way merge is an optimization,"},{"line_number":663,"context_line":"                    # and template-based merge is the pre-existing behavior."},{"line_number":664,"context_line":"                    LOG.warning(\"Failed to fetch current binding:profile \""},{"line_number":665,"context_line":"                                \"from Neutron API for 3-way merge: %s. \""},{"line_number":666,"context_line":"                                \"Falling back to template-based merge. \""}],"source_content_type":"text/x-python","patch_set":10,"id":"92a10bbe_3d6063cd","line":663,"range":{"start_line":660,"start_character":39,"end_line":663,"end_character":76},"updated":"2026-03-08 10:21:12.000000000","message":"If you expect API call error specifically and don\u0027t expect any from the other logs then please do wrap only L630 by try-except. \n\nToo Broad try-except is not really safe really because it can swallow the error.","commit_id":"1ce8ccc6314fa70800c0d415756d973662eaca82"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"0a0dd1f6ee60ac4a085ce188c928b502b628dca5","unresolved":false,"context_lines":[{"line_number":657,"context_line":"                            \u0027binding:profile\u0027] \u003d merged_profile"},{"line_number":658,"context_line":"                        binding_profile_handled \u003d True"},{"line_number":659,"context_line":""},{"line_number":660,"context_line":"                except Exception as ex:"},{"line_number":661,"context_line":"                    # API call failed, fall back to template-based merge."},{"line_number":662,"context_line":"                    # This is safe because 3-way merge is an optimization,"},{"line_number":663,"context_line":"                    # and template-based merge is the pre-existing behavior."},{"line_number":664,"context_line":"                    LOG.warning(\"Failed to fetch current binding:profile \""},{"line_number":665,"context_line":"                                \"from Neutron API for 3-way merge: %s. \""},{"line_number":666,"context_line":"                                \"Falling back to template-based merge. \""}],"source_content_type":"text/x-python","patch_set":10,"id":"11c9820a_f6f9a108","line":663,"range":{"start_line":660,"start_character":39,"end_line":663,"end_character":76},"in_reply_to":"92a10bbe_3d6063cd","updated":"2026-03-09 11:26:05.000000000","message":"Hi Takashi, thank you. You\u0027re absolutely right. I\u0027ve removed the try-except entirely. The code now lets any exception from _show_resource() propagate naturally, making failures visible.","commit_id":"1ce8ccc6314fa70800c0d415756d973662eaca82"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"829cbf6a266f81829071c0142955602e41c2ca0b","unresolved":true,"context_lines":[{"line_number":661,"context_line":"                    # API call failed, fall back to template-based merge."},{"line_number":662,"context_line":"                    # This is safe because 3-way merge is an optimization,"},{"line_number":663,"context_line":"                    # and template-based merge is the pre-existing behavior."},{"line_number":664,"context_line":"                    LOG.warning(\"Failed to fetch current binding:profile \""},{"line_number":665,"context_line":"                                \"from Neutron API for 3-way merge: %s. \""},{"line_number":666,"context_line":"                                \"Falling back to template-based merge. \""},{"line_number":667,"context_line":"                                \"Auto-populated fields may not be preserved.\","},{"line_number":668,"context_line":"                                ex)"}],"source_content_type":"text/x-python","patch_set":10,"id":"71ed4535_965947e8","line":665,"range":{"start_line":664,"start_character":33,"end_line":665,"end_character":66},"updated":"2026-03-08 10:21:12.000000000","message":"I don\u0027t think this is really correct. Imagine the situation where neutron has transient issue which caused this specific call to fail. That results in \"unexpected\" situation where binding profile is purged. The error is not visible to users. IMO we should rather let the update fail.","commit_id":"1ce8ccc6314fa70800c0d415756d973662eaca82"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"0a0dd1f6ee60ac4a085ce188c928b502b628dca5","unresolved":false,"context_lines":[{"line_number":661,"context_line":"                    # API call failed, fall back to template-based merge."},{"line_number":662,"context_line":"                    # This is safe because 3-way merge is an optimization,"},{"line_number":663,"context_line":"                    # and template-based merge is the pre-existing behavior."},{"line_number":664,"context_line":"                    LOG.warning(\"Failed to fetch current binding:profile \""},{"line_number":665,"context_line":"                                \"from Neutron API for 3-way merge: %s. \""},{"line_number":666,"context_line":"                                \"Falling back to template-based merge. \""},{"line_number":667,"context_line":"                                \"Auto-populated fields may not be preserved.\","},{"line_number":668,"context_line":"                                ex)"}],"source_content_type":"text/x-python","patch_set":10,"id":"29a90683_6b007861","line":665,"range":{"start_line":664,"start_character":33,"end_line":665,"end_character":66},"in_reply_to":"71ed4535_965947e8","updated":"2026-03-09 11:26:05.000000000","message":"Agreed. I\u0027ve removed the catch-and-fallback logic. Now if the API call fails, the update fails visibly. and I also added the following inline comment for it:\n\n                # Don\u0027t catch exceptions here. If neutron has a transient\n                # issue causing this call to fail, that would result in\n                # binding:profile being purged (losing auto-populated\n                # SR-IOV fields). We should rather let the update fail.","commit_id":"1ce8ccc6314fa70800c0d415756d973662eaca82"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"bba60b458527167017a898c28234c47848090d88","unresolved":true,"context_lines":[{"line_number":625,"context_line":"            # Neutron API to preserve auto-populated fields."},{"line_number":626,"context_line":"            binding_profile_handled \u003d False"},{"line_number":627,"context_line":"            if (\u0027binding:profile\u0027 in prop_diff[\u0027value_specs\u0027] and"},{"line_number":628,"context_line":"                    self.resource_id):"},{"line_number":629,"context_line":"                # Don\u0027t catch exceptions here. If neutron has a transient"},{"line_number":630,"context_line":"                # issue causing this call to fail, that would result in"},{"line_number":631,"context_line":"                # binding:profile being purged (losing auto-populated"},{"line_number":632,"context_line":"                # SR-IOV fields). We should rather let the update fail."},{"line_number":633,"context_line":"                current_attrs \u003d self._show_resource()"},{"line_number":634,"context_line":"                if current_attrs and \u0027binding:profile\u0027 in current_attrs:"},{"line_number":635,"context_line":"                    api_profile \u003d current_attrs[\u0027binding:profile\u0027]"}],"source_content_type":"text/x-python","patch_set":11,"id":"0a17b9c8_bf6314f4","line":632,"range":{"start_line":628,"start_character":38,"end_line":632,"end_character":71},"updated":"2026-03-09 11:29:11.000000000","message":"IMO this is too much details left in the comment. We don\u0027t document such note unless it\u0027s not clear at first glance, but I don\u0027t think this is unclear. Can we remove it ?","commit_id":"7179fdb576c8583fce988024b762a0784a9f30f2"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"91a5a18d951f6fd92857943292e3088eb73c03f0","unresolved":false,"context_lines":[{"line_number":625,"context_line":"            # Neutron API to preserve auto-populated fields."},{"line_number":626,"context_line":"            binding_profile_handled \u003d False"},{"line_number":627,"context_line":"            if (\u0027binding:profile\u0027 in prop_diff[\u0027value_specs\u0027] and"},{"line_number":628,"context_line":"                    self.resource_id):"},{"line_number":629,"context_line":"                # Don\u0027t catch exceptions here. If neutron has a transient"},{"line_number":630,"context_line":"                # issue causing this call to fail, that would result in"},{"line_number":631,"context_line":"                # binding:profile being purged (losing auto-populated"},{"line_number":632,"context_line":"                # SR-IOV fields). We should rather let the update fail."},{"line_number":633,"context_line":"                current_attrs \u003d self._show_resource()"},{"line_number":634,"context_line":"                if current_attrs and \u0027binding:profile\u0027 in current_attrs:"},{"line_number":635,"context_line":"                    api_profile \u003d current_attrs[\u0027binding:profile\u0027]"}],"source_content_type":"text/x-python","patch_set":11,"id":"114aa0cf_533e039b","line":632,"range":{"start_line":628,"start_character":38,"end_line":632,"end_character":71},"in_reply_to":"0a17b9c8_bf6314f4","updated":"2026-03-09 11:40:35.000000000","message":"thanks, I\u0027ve removed it now","commit_id":"7179fdb576c8583fce988024b762a0784a9f30f2"},{"author":{"_account_id":8833,"name":"Rabi Mishra","email":"ramishra@redhat.com","username":"rabi"},"change_message_id":"f6a95df6c72f37f7d27f096574b188726bfd74c0","unresolved":true,"context_lines":[{"line_number":634,"context_line":"                    new_profile \u003d prop_diff[\u0027value_specs\u0027]["},{"line_number":635,"context_line":"                        \u0027binding:profile\u0027]"},{"line_number":636,"context_line":""},{"line_number":637,"context_line":"                    # 3-way merge logic"},{"line_number":638,"context_line":"                    current_profile \u003d api_profile"},{"line_number":639,"context_line":"                    set_val \u003d {}"},{"line_number":640,"context_line":"                    for pk in new_profile:"},{"line_number":641,"context_line":"                        # Check if this key has changed"},{"line_number":642,"context_line":"                        old_val \u003d old_profile.get(pk)"},{"line_number":643,"context_line":"                        new_val \u003d new_profile.get(pk)"},{"line_number":644,"context_line":"                        if old_val !\u003d new_val:"},{"line_number":645,"context_line":"                            set_val[pk] \u003d new_val"},{"line_number":646,"context_line":""},{"line_number":647,"context_line":"                    merged_profile \u003d current_profile.copy()"},{"line_number":648,"context_line":"                    for pk, pv in set_val.items():"},{"line_number":649,"context_line":"                        if pv is None:"},{"line_number":650,"context_line":"                            merged_profile.pop(pk, None)"},{"line_number":651,"context_line":"                        else:"},{"line_number":652,"context_line":"                            merged_profile[pk] \u003d pv"},{"line_number":653,"context_line":""},{"line_number":654,"context_line":"                    # Update value_specs with merged result"},{"line_number":655,"context_line":"                    prop_diff[\u0027value_specs\u0027]["}],"source_content_type":"text/x-python","patch_set":12,"id":"13d61e75_ca8c5cdd","line":652,"range":{"start_line":637,"start_character":11,"end_line":652,"end_character":51},"updated":"2026-03-16 10:23:20.000000000","message":"I think we can do this with a single for loop? We don\u0027t need the binding_profile_handled flag or current_profile alias.\n\n\ndef prepare_update_properties(self, prop_diff):\n    if \u0027value_specs\u0027 in prop_diff:\n        before_value_specs \u003d self.properties.get(self.VALUE_SPECS)\n\n        if (\u0027binding:profile\u0027 in prop_diff[\u0027value_specs\u0027]\n                and self.resource_id):\n            current_attrs \u003d self._show_resource()\n            if current_attrs and \u0027binding:profile\u0027 in current_attrs:\n                old_profile \u003d (before_value_specs or {}).get(\n                    \u0027binding:profile\u0027, {})\n                new_profile \u003d prop_diff[\u0027value_specs\u0027][\u0027binding:profile\u0027]\n                merged \u003d current_attrs[\u0027binding:profile\u0027].copy()\n\n                for key, new_val in new_profile.items():\n                    if new_val !\u003d old_profile.get(key):\n                        if new_val is None:\n                            merged.pop(key, None)\n                        else:\n                            merged[key] \u003d new_val\n\n                prop_diff[\u0027value_specs\u0027][\u0027binding:profile\u0027] \u003d merged\n                if (before_value_specs\n                        and \u0027binding:profile\u0027 in before_value_specs):\n                    before_value_specs \u003d {\n                        k: v for k, v in before_value_specs.items()\n                        if k !\u003d \u0027binding:profile\u0027}\n\n        neutron.NeutronResource.merge_value_specs(\n            prop_diff, before_value_specs)\n    if \u0027name\u0027 in prop_diff and prop_diff[\u0027name\u0027] is None:\n        prop_diff[\u0027name\u0027] \u003d self.physical_resource_name()","commit_id":"577ebbfbcf810462f376db49305c8f43094d6985"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"57380270c4d9d3eee12b01e808869a1e38f99b99","unresolved":false,"context_lines":[{"line_number":634,"context_line":"                    new_profile \u003d prop_diff[\u0027value_specs\u0027]["},{"line_number":635,"context_line":"                        \u0027binding:profile\u0027]"},{"line_number":636,"context_line":""},{"line_number":637,"context_line":"                    # 3-way merge logic"},{"line_number":638,"context_line":"                    current_profile \u003d api_profile"},{"line_number":639,"context_line":"                    set_val \u003d {}"},{"line_number":640,"context_line":"                    for pk in new_profile:"},{"line_number":641,"context_line":"                        # Check if this key has changed"},{"line_number":642,"context_line":"                        old_val \u003d old_profile.get(pk)"},{"line_number":643,"context_line":"                        new_val \u003d new_profile.get(pk)"},{"line_number":644,"context_line":"                        if old_val !\u003d new_val:"},{"line_number":645,"context_line":"                            set_val[pk] \u003d new_val"},{"line_number":646,"context_line":""},{"line_number":647,"context_line":"                    merged_profile \u003d current_profile.copy()"},{"line_number":648,"context_line":"                    for pk, pv in set_val.items():"},{"line_number":649,"context_line":"                        if pv is None:"},{"line_number":650,"context_line":"                            merged_profile.pop(pk, None)"},{"line_number":651,"context_line":"                        else:"},{"line_number":652,"context_line":"                            merged_profile[pk] \u003d pv"},{"line_number":653,"context_line":""},{"line_number":654,"context_line":"                    # Update value_specs with merged result"},{"line_number":655,"context_line":"                    prop_diff[\u0027value_specs\u0027]["}],"source_content_type":"text/x-python","patch_set":12,"id":"e5e5f27e_c896ffed","line":652,"range":{"start_line":637,"start_character":11,"end_line":652,"end_character":51},"in_reply_to":"13d61e75_ca8c5cdd","updated":"2026-03-20 09:23:23.000000000","message":"Hi @ramishra@redhat.com , good catch, thanks a lot, I\u0027ve changed it to a single for loop, and also removed binding_profile_handled flag and current_profile alias.","commit_id":"577ebbfbcf810462f376db49305c8f43094d6985"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"57380270c4d9d3eee12b01e808869a1e38f99b99","unresolved":false,"context_lines":[{"line_number":634,"context_line":"                    new_profile \u003d prop_diff[\u0027value_specs\u0027]["},{"line_number":635,"context_line":"                        \u0027binding:profile\u0027]"},{"line_number":636,"context_line":""},{"line_number":637,"context_line":"                    # 3-way merge logic"},{"line_number":638,"context_line":"                    current_profile \u003d api_profile"},{"line_number":639,"context_line":"                    set_val \u003d {}"},{"line_number":640,"context_line":"                    for pk in new_profile:"},{"line_number":641,"context_line":"                        # Check if this key has changed"},{"line_number":642,"context_line":"                        old_val \u003d old_profile.get(pk)"},{"line_number":643,"context_line":"                        new_val \u003d new_profile.get(pk)"},{"line_number":644,"context_line":"                        if old_val !\u003d new_val:"},{"line_number":645,"context_line":"                            set_val[pk] \u003d new_val"},{"line_number":646,"context_line":""},{"line_number":647,"context_line":"                    merged_profile \u003d current_profile.copy()"},{"line_number":648,"context_line":"                    for pk, pv in set_val.items():"},{"line_number":649,"context_line":"                        if pv is None:"},{"line_number":650,"context_line":"                            merged_profile.pop(pk, None)"},{"line_number":651,"context_line":"                        else:"},{"line_number":652,"context_line":"                            merged_profile[pk] \u003d pv"},{"line_number":653,"context_line":""},{"line_number":654,"context_line":"                    # Update value_specs with merged result"},{"line_number":655,"context_line":"                    prop_diff[\u0027value_specs\u0027]["}],"source_content_type":"text/x-python","patch_set":12,"id":"136d2267_6c9ab722","line":652,"range":{"start_line":637,"start_character":11,"end_line":652,"end_character":51},"in_reply_to":"13d61e75_ca8c5cdd","updated":"2026-03-20 09:23:23.000000000","message":"Hi @ramishra@redhat.com ,good catch, thanks a lot. I\u0027ve changed it to a single for loop, and also removed binding_profile_handled flag and current_profile alias.","commit_id":"577ebbfbcf810462f376db49305c8f43094d6985"}]}