)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"fd6722902dbeb0e35483ef827bc88c11575e990c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"41ee86bf_d0e5d11c","updated":"2023-09-15 14:15:25.000000000","message":"This is still a WIP, I didn\u0027t have the right OVN version on my Ubuntu box","commit_id":"d398a734f00cb4fe0c4f5a3356c819274bc3f612"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"e258899e47ff635aaef5bb640c6026c3fa17ec39","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d5b06741_3dab6cb7","updated":"2023-09-19 13:23:50.000000000","message":"Just a note to reviewers - the additional_chassis column is not present in the current OVS version used in functional testing. The test is skipped in the results.","commit_id":"208020746b6482940849a5edf638756c329ac2f5"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"c34eb408dd9df1b853b3bb0eee7ad0bdb4634ed5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f07ea19e_ef851f2f","updated":"2023-09-19 13:16:12.000000000","message":"recheck\nCan\u0027t SSH to the instance but I checked the metadata resources were provisioned just fine for the given port/network","commit_id":"208020746b6482940849a5edf638756c329ac2f5"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"045795eb772b6426541570b752a0f7f9b2235d3b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"434a15e9_08125146","updated":"2023-09-19 12:44:14.000000000","message":"recheck\ntest_multicast_between_vms_on_same_network is not related.","commit_id":"208020746b6482940849a5edf638756c329ac2f5"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"5d6f67902e749316e2842e2c4184bcafd2204347","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"065f3020_7a607e73","updated":"2023-09-19 21:12:35.000000000","message":"the functional test passes with OVN 22.06.1: https://storage.gra.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_e01/895849/1/check/neutron-functional-with-uwsgi/e0151c0/testr_results.html","commit_id":"208020746b6482940849a5edf638756c329ac2f5"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"fc9f980539b9cbd0853f0f412ea4a7da2c659636","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"b55b87d0_94ff1301","updated":"2023-09-29 14:18:05.000000000","message":"btw the functional tests added are skipped in gate. Can we bump ovn version in gate to reflect the new code? (or is there some other reason why they are skipped?)","commit_id":"e6de3164112af91e6a010974bd8f688918eb7622"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"081c8b064ea395c231e3986d36243aace15709ea","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"fa38aa6f_9d857e9a","in_reply_to":"b55b87d0_94ff1301","updated":"2023-09-29 15:20:51.000000000","message":"I did, it\u0027s bumped now https://review.opendev.org/c/openstack/neutron/+/895849","commit_id":"e6de3164112af91e6a010974bd8f688918eb7622"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"a4d278cebb852bd3b87b4e3e77c2f09db35d699a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"d7c544c0_f7790af7","updated":"2023-10-31 13:20:00.000000000","message":"I think this will need some refactoring in the tests due to the latest changes in the code base","commit_id":"1ed129aa78f03f1596b86929ca5baa656466d692"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"19e61bb59c79b5918e097b1255dfc5b324938d20","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"2d440f6c_ca146e34","updated":"2023-11-03 16:33:05.000000000","message":"Thanks Kuba! It LGTM!","commit_id":"3ec7f96cbb0571c401406781562160db75b3b9f5"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"26c60695c70c8307b8fea9af002009ac774cefac","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"8728e3a4_731458ba","updated":"2023-11-03 15:50:29.000000000","message":"lgtm","commit_id":"3ec7f96cbb0571c401406781562160db75b3b9f5"}],"neutron/agent/ovn/metadata/agent.py":[{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"588e16501ad89d75725e405ba5426d35e6915229","unresolved":true,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"class PortBindingChassisCreatedEvent(PortBindingChassisEvent):"},{"line_number":138,"context_line":"    LOG_MSG \u003d \"Port %s in datapath %s bound to our chassis\""},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    def __init__(self, metadata_agent):"},{"line_number":141,"context_line":"        events \u003d (self.ROW_UPDATE,)"}],"source_content_type":"text/x-python","patch_set":2,"id":"78ff1b05_10ebf2d1","line":138,"updated":"2023-09-20 01:21:12.000000000","message":"The port is not trully bound to the chassis in the case when you match the `additional_chassis` record(i.e. VM is getting migrated). Am I correct? If so I we should make this log more granular as it might throw somebody off when debugging agent issues and correlating event timestamps is important.\n\nSorry I dont have an enviroment to try this out","commit_id":"208020746b6482940849a5edf638756c329ac2f5"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"2cc192ca1c0f97fc0398dc643b004ae8ff9073ca","unresolved":true,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"class PortBindingChassisCreatedEvent(PortBindingChassisEvent):"},{"line_number":138,"context_line":"    LOG_MSG \u003d \"Port %s in datapath %s bound to our chassis\""},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    def __init__(self, metadata_agent):"},{"line_number":141,"context_line":"        events \u003d (self.ROW_UPDATE,)"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa0d319c_834dd639","line":138,"in_reply_to":"015ab86b_5cddf315","updated":"2023-09-20 14:47:57.000000000","message":"Thanks Ihar, did you make the blog post just to respond to my comment :)?  jk.\n+1 on having explicit log for \"main\" and \"additional\" chassis","commit_id":"208020746b6482940849a5edf638756c329ac2f5"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"b0c8aaea6160f21f0697a0ae2a7b3a83674c00dd","unresolved":true,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"class PortBindingChassisCreatedEvent(PortBindingChassisEvent):"},{"line_number":138,"context_line":"    LOG_MSG \u003d \"Port %s in datapath %s bound to our chassis\""},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    def __init__(self, metadata_agent):"},{"line_number":141,"context_line":"        events \u003d (self.ROW_UPDATE,)"}],"source_content_type":"text/x-python","patch_set":2,"id":"fac71770_b2cefc60","line":138,"in_reply_to":"78ff1b05_10ebf2d1","updated":"2023-09-20 12:40:31.000000000","message":"It is bound (the chassis is additional, so there\u0027s another chassis that also bound it - the main one).","commit_id":"208020746b6482940849a5edf638756c329ac2f5"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"ae681a97f37b9b5febae91f1c334613ebaf60b43","unresolved":true,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"class PortBindingChassisCreatedEvent(PortBindingChassisEvent):"},{"line_number":138,"context_line":"    LOG_MSG \u003d \"Port %s in datapath %s bound to our chassis\""},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    def __init__(self, metadata_agent):"},{"line_number":141,"context_line":"        events \u003d (self.ROW_UPDATE,)"}],"source_content_type":"text/x-python","patch_set":2,"id":"015ab86b_5cddf315","line":138,"in_reply_to":"84304378_dc0ad81d","updated":"2023-09-20 12:42:08.000000000","message":"That said, we could probably update the message so that it explicitly logs that the chassis is \"main\" or \"additional\".","commit_id":"208020746b6482940849a5edf638756c329ac2f5"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"d95aeb7b96e81e3aba2fd8bd9ff92468a179d7cb","unresolved":false,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"class PortBindingChassisCreatedEvent(PortBindingChassisEvent):"},{"line_number":138,"context_line":"    LOG_MSG \u003d \"Port %s in datapath %s bound to our chassis\""},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    def __init__(self, metadata_agent):"},{"line_number":141,"context_line":"        events \u003d (self.ROW_UPDATE,)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fdf17da_1951fd82","line":138,"in_reply_to":"fa0d319c_834dd639","updated":"2023-09-22 18:01:39.000000000","message":"Done","commit_id":"208020746b6482940849a5edf638756c329ac2f5"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"a695d5148d8b442a01c06dae2bdf4842c101d4d4","unresolved":true,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"class PortBindingChassisCreatedEvent(PortBindingChassisEvent):"},{"line_number":138,"context_line":"    LOG_MSG \u003d \"Port %s in datapath %s bound to our chassis\""},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    def __init__(self, metadata_agent):"},{"line_number":141,"context_line":"        events \u003d (self.ROW_UPDATE,)"}],"source_content_type":"text/x-python","patch_set":2,"id":"84304378_dc0ad81d","line":138,"in_reply_to":"fac71770_b2cefc60","updated":"2023-09-20 12:41:08.000000000","message":"In case you need more background as to how ovn binds these ports, please take a look: https://ihar.dev/posts/ovn-chassis-binding-walkthru/","commit_id":"208020746b6482940849a5edf638756c329ac2f5"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"2abe98b84dce64340460a3a8eab70d5ff5eaebb7","unresolved":true,"context_lines":[{"line_number":143,"context_line":"            metadata_agent, events)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    def match_fn(self, event, row, old):"},{"line_number":146,"context_line":"        try:"},{"line_number":147,"context_line":"            if row.chassis[0].name \u003d\u003d self.agent.chassis and not old.chassis:"},{"line_number":148,"context_line":"                return True"},{"line_number":149,"context_line":"        except (IndexError, AttributeError):"}],"source_content_type":"text/x-python","patch_set":2,"id":"b0f5cab1_a7943e90","line":146,"updated":"2023-09-19 23:13:20.000000000","message":"you don\u0027t handle the case here where the chassis was (in the `old`) additional but now became \"main\". I think this may be fine since in this case the metadata service as provisioned when the chassis was \"additional\" is as good, so you ignore the case. Is this your thinking?\n\nIf so, may I ask you to capture this in a comment here, so that we know that we haven\u0027t just forgotten this transition scenario but gave it a consideration?","commit_id":"208020746b6482940849a5edf638756c329ac2f5"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"d95aeb7b96e81e3aba2fd8bd9ff92468a179d7cb","unresolved":false,"context_lines":[{"line_number":143,"context_line":"            metadata_agent, events)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    def match_fn(self, event, row, old):"},{"line_number":146,"context_line":"        try:"},{"line_number":147,"context_line":"            if row.chassis[0].name \u003d\u003d self.agent.chassis and not old.chassis:"},{"line_number":148,"context_line":"                return True"},{"line_number":149,"context_line":"        except (IndexError, AttributeError):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5eb8b5a7_66a95d77","line":146,"in_reply_to":"53626915_17c2f42e","updated":"2023-09-22 18:01:39.000000000","message":"Done","commit_id":"208020746b6482940849a5edf638756c329ac2f5"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"4c3f2935b2dc4d9e78f91d3020db9047560e01c3","unresolved":true,"context_lines":[{"line_number":143,"context_line":"            metadata_agent, events)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    def match_fn(self, event, row, old):"},{"line_number":146,"context_line":"        try:"},{"line_number":147,"context_line":"            if row.chassis[0].name \u003d\u003d self.agent.chassis and not old.chassis:"},{"line_number":148,"context_line":"                return True"},{"line_number":149,"context_line":"        except (IndexError, AttributeError):"}],"source_content_type":"text/x-python","patch_set":2,"id":"e0a52516_87f37bf0","line":146,"in_reply_to":"b0f5cab1_a7943e90","updated":"2023-09-20 13:46:13.000000000","message":"Yes, I will add a comment. My thinking is that this event handles 2 scenarios in which the result is to check provisioned resources:\n\n1) New instance created -\u003e port is bound to a chassis (lines 146-150).\n2) Live migration -\u003e the use of additional chassis (lines 152-159)\n\nFor the second scenario, when the chassis from additional_chassis column becomes chassis, the resources are already provisioned.","commit_id":"208020746b6482940849a5edf638756c329ac2f5"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7e11acbdfc7ffa111aeb2a3b50d98054d140c938","unresolved":true,"context_lines":[{"line_number":143,"context_line":"            metadata_agent, events)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    def match_fn(self, event, row, old):"},{"line_number":146,"context_line":"        try:"},{"line_number":147,"context_line":"            if row.chassis[0].name \u003d\u003d self.agent.chassis and not old.chassis:"},{"line_number":148,"context_line":"                return True"},{"line_number":149,"context_line":"        except (IndexError, AttributeError):"}],"source_content_type":"text/x-python","patch_set":2,"id":"53626915_17c2f42e","line":146,"in_reply_to":"e0a52516_87f37bf0","updated":"2023-09-20 13:48:47.000000000","message":"Yes, so we are on the same page. I just figured that it\u0027s not an obvious thing and a reader may want to explicitly know that this is a scenario that should be handled by doing nothing. Thanks for adding the comment.","commit_id":"208020746b6482940849a5edf638756c329ac2f5"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"2abe98b84dce64340460a3a8eab70d5ff5eaebb7","unresolved":true,"context_lines":[{"line_number":171,"context_line":"    def match_fn(self, event, row, old):"},{"line_number":172,"context_line":"        try:"},{"line_number":173,"context_line":"            if event \u003d\u003d self.ROW_UPDATE:"},{"line_number":174,"context_line":"                return (old.chassis[0].name \u003d\u003d self.agent.chassis and"},{"line_number":175,"context_line":"                        not row.chassis)"},{"line_number":176,"context_line":"            else:"},{"line_number":177,"context_line":"                if row.chassis[0].name \u003d\u003d self.agent.chassis:"}],"source_content_type":"text/x-python","patch_set":2,"id":"f4c4084a_5c3d4b1b","line":174,"updated":"2023-09-19 23:13:20.000000000","message":"should this also be updated for in case when a current chassis was \"additional\" but was removed from the list without transitioning to \"main\" (.chassis) status? (e.g. because the migration failed)","commit_id":"208020746b6482940849a5edf638756c329ac2f5"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"2f1e19c52e1fc41a70b7b7631e1494e110a78e24","unresolved":false,"context_lines":[{"line_number":171,"context_line":"    def match_fn(self, event, row, old):"},{"line_number":172,"context_line":"        try:"},{"line_number":173,"context_line":"            if event \u003d\u003d self.ROW_UPDATE:"},{"line_number":174,"context_line":"                return (old.chassis[0].name \u003d\u003d self.agent.chassis and"},{"line_number":175,"context_line":"                        not row.chassis)"},{"line_number":176,"context_line":"            else:"},{"line_number":177,"context_line":"                if row.chassis[0].name \u003d\u003d self.agent.chassis:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1e10a406_f19a2699","line":174,"in_reply_to":"3f4f7fe7_df0b859a","updated":"2023-09-22 18:01:50.000000000","message":"Done","commit_id":"208020746b6482940849a5edf638756c329ac2f5"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"4c3f2935b2dc4d9e78f91d3020db9047560e01c3","unresolved":true,"context_lines":[{"line_number":171,"context_line":"    def match_fn(self, event, row, old):"},{"line_number":172,"context_line":"        try:"},{"line_number":173,"context_line":"            if event \u003d\u003d self.ROW_UPDATE:"},{"line_number":174,"context_line":"                return (old.chassis[0].name \u003d\u003d self.agent.chassis and"},{"line_number":175,"context_line":"                        not row.chassis)"},{"line_number":176,"context_line":"            else:"},{"line_number":177,"context_line":"                if row.chassis[0].name \u003d\u003d self.agent.chassis:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4f7fe7_df0b859a","line":174,"in_reply_to":"54bba51c_d4f8d05f","updated":"2023-09-20 13:46:13.000000000","message":"Thanks, good point!","commit_id":"208020746b6482940849a5edf638756c329ac2f5"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"588e16501ad89d75725e405ba5426d35e6915229","unresolved":true,"context_lines":[{"line_number":171,"context_line":"    def match_fn(self, event, row, old):"},{"line_number":172,"context_line":"        try:"},{"line_number":173,"context_line":"            if event \u003d\u003d self.ROW_UPDATE:"},{"line_number":174,"context_line":"                return (old.chassis[0].name \u003d\u003d self.agent.chassis and"},{"line_number":175,"context_line":"                        not row.chassis)"},{"line_number":176,"context_line":"            else:"},{"line_number":177,"context_line":"                if row.chassis[0].name \u003d\u003d self.agent.chassis:"}],"source_content_type":"text/x-python","patch_set":2,"id":"54bba51c_d4f8d05f","line":174,"in_reply_to":"f4c4084a_5c3d4b1b","updated":"2023-09-20 01:21:12.000000000","message":"+1 on this topic. I have been wondering about the same as if what happens if the migration fails.","commit_id":"208020746b6482940849a5edf638756c329ac2f5"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"1dee43ba495abfd6de7ecd15954d0ea8bf431340","unresolved":true,"context_lines":[{"line_number":178,"context_line":"            additional_chassis \u003d {ch for ch in row.additional_chassis"},{"line_number":179,"context_line":"                                  if ch.name \u003d\u003d self.agent.chassis}"},{"line_number":180,"context_line":"            try:"},{"line_number":181,"context_line":"                # Return if the addtional chassis contains new chassis compared"},{"line_number":182,"context_line":"                # to the old value."},{"line_number":183,"context_line":"                self.log_msg \u003d ("},{"line_number":184,"context_line":"                    \"Live migrating port %s from network %s was added to this \""}],"source_content_type":"text/x-python","patch_set":3,"id":"2aed3d14_0ab24cdf","line":181,"range":{"start_line":181,"start_character":32,"end_line":181,"end_character":41},"updated":"2023-09-28 19:57:19.000000000","message":"additional","commit_id":"e6de3164112af91e6a010974bd8f688918eb7622"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6ee4325c27e5b83c85e9fc540b0b19a67e39d28d","unresolved":false,"context_lines":[{"line_number":178,"context_line":"            additional_chassis \u003d {ch for ch in row.additional_chassis"},{"line_number":179,"context_line":"                                  if ch.name \u003d\u003d self.agent.chassis}"},{"line_number":180,"context_line":"            try:"},{"line_number":181,"context_line":"                # Return if the addtional chassis contains new chassis compared"},{"line_number":182,"context_line":"                # to the old value."},{"line_number":183,"context_line":"                self.log_msg \u003d ("},{"line_number":184,"context_line":"                    \"Live migrating port %s from network %s was added to this \""}],"source_content_type":"text/x-python","patch_set":3,"id":"eb6da9f6_8b42b657","line":181,"range":{"start_line":181,"start_character":32,"end_line":181,"end_character":41},"in_reply_to":"2aed3d14_0ab24cdf","updated":"2023-09-29 15:20:16.000000000","message":"Done","commit_id":"e6de3164112af91e6a010974bd8f688918eb7622"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"1dee43ba495abfd6de7ecd15954d0ea8bf431340","unresolved":true,"context_lines":[{"line_number":191,"context_line":""},{"line_number":192,"context_line":"    def _additional_chassis_removed(self, row, old):"},{"line_number":193,"context_line":"        if ovn_utils.is_additional_chassis_supported(self.agent.sb_idl):"},{"line_number":194,"context_line":"            # Check if addtional_chassis that was ours has been removed."},{"line_number":195,"context_line":"            # At the same time we have to check that only"},{"line_number":196,"context_line":"            # the additional_chassis has been removed and current chassis"},{"line_number":197,"context_line":"            # was not set to ours chassis."}],"source_content_type":"text/x-python","patch_set":3,"id":"d67ee904_dfa64a91","line":194,"range":{"start_line":194,"start_character":23,"end_line":194,"end_character":40},"updated":"2023-09-28 19:57:19.000000000","message":"additional_chassis","commit_id":"e6de3164112af91e6a010974bd8f688918eb7622"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6ee4325c27e5b83c85e9fc540b0b19a67e39d28d","unresolved":false,"context_lines":[{"line_number":191,"context_line":""},{"line_number":192,"context_line":"    def _additional_chassis_removed(self, row, old):"},{"line_number":193,"context_line":"        if ovn_utils.is_additional_chassis_supported(self.agent.sb_idl):"},{"line_number":194,"context_line":"            # Check if addtional_chassis that was ours has been removed."},{"line_number":195,"context_line":"            # At the same time we have to check that only"},{"line_number":196,"context_line":"            # the additional_chassis has been removed and current chassis"},{"line_number":197,"context_line":"            # was not set to ours chassis."}],"source_content_type":"text/x-python","patch_set":3,"id":"8ab4d907_6f861d67","line":194,"range":{"start_line":194,"start_character":23,"end_line":194,"end_character":40},"in_reply_to":"d67ee904_dfa64a91","updated":"2023-09-29 15:20:16.000000000","message":"Done","commit_id":"e6de3164112af91e6a010974bd8f688918eb7622"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"a4879502bbb8d5fdaacef97e355977f12d83fb93","unresolved":true,"context_lines":[{"line_number":198,"context_line":"            # If it was, it could mean the live migration has ended and the"},{"line_number":199,"context_line":"            # chassis was switched, in which case we should not trigger"},{"line_number":200,"context_line":"            # provisioning."},{"line_number":201,"context_line":"            old_a_chassis \u003d {ch for ch in old.additional_chassis"},{"line_number":202,"context_line":"                             if ch.name \u003d\u003d self.agent.chassis}"},{"line_number":203,"context_line":"            try:"},{"line_number":204,"context_line":"                return bool("}],"source_content_type":"text/x-python","patch_set":3,"id":"acf1d26f_7195d456","line":201,"updated":"2023-09-29 14:12:33.000000000","message":"as discussed elsewhere, this may raise an exception when additional_chassis is not touched.","commit_id":"e6de3164112af91e6a010974bd8f688918eb7622"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"3b15c666c967de4e598c142df33ba145e158331e","unresolved":false,"context_lines":[{"line_number":198,"context_line":"            # If it was, it could mean the live migration has ended and the"},{"line_number":199,"context_line":"            # chassis was switched, in which case we should not trigger"},{"line_number":200,"context_line":"            # provisioning."},{"line_number":201,"context_line":"            old_a_chassis \u003d {ch for ch in old.additional_chassis"},{"line_number":202,"context_line":"                             if ch.name \u003d\u003d self.agent.chassis}"},{"line_number":203,"context_line":"            try:"},{"line_number":204,"context_line":"                return bool("}],"source_content_type":"text/x-python","patch_set":3,"id":"3269b0fb_31156bc3","line":201,"in_reply_to":"acf1d26f_7195d456","updated":"2023-09-29 18:14:54.000000000","message":"Done","commit_id":"e6de3164112af91e6a010974bd8f688918eb7622"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"2d47a4aa8983bca4e09bd61eb70762ccf3967127","unresolved":true,"context_lines":[{"line_number":201,"context_line":"            old_a_chassis \u003d {ch for ch in old.additional_chassis"},{"line_number":202,"context_line":"                             if ch.name \u003d\u003d self.agent.chassis}"},{"line_number":203,"context_line":"            try:"},{"line_number":204,"context_line":"                return bool("},{"line_number":205,"context_line":"                    old_a_chassis.difference(row.additional_chassis) and"},{"line_number":206,"context_line":"                    not hasattr(old, \u0027chassis\u0027))"},{"line_number":207,"context_line":"            except AttributeError:"}],"source_content_type":"text/x-python","patch_set":3,"id":"eac4cdd7_c907ad83","line":204,"updated":"2023-09-29 14:15:24.000000000","message":"as discussed elsewhere, this code won\u0027t raise AttributeError at all since it doesn\u0027t access attributes on old.","commit_id":"e6de3164112af91e6a010974bd8f688918eb7622"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"3b15c666c967de4e598c142df33ba145e158331e","unresolved":false,"context_lines":[{"line_number":201,"context_line":"            old_a_chassis \u003d {ch for ch in old.additional_chassis"},{"line_number":202,"context_line":"                             if ch.name \u003d\u003d self.agent.chassis}"},{"line_number":203,"context_line":"            try:"},{"line_number":204,"context_line":"                return bool("},{"line_number":205,"context_line":"                    old_a_chassis.difference(row.additional_chassis) and"},{"line_number":206,"context_line":"                    not hasattr(old, \u0027chassis\u0027))"},{"line_number":207,"context_line":"            except AttributeError:"}],"source_content_type":"text/x-python","patch_set":3,"id":"40064151_8b57bfbc","line":204,"in_reply_to":"eac4cdd7_c907ad83","updated":"2023-09-29 18:14:54.000000000","message":"Done","commit_id":"e6de3164112af91e6a010974bd8f688918eb7622"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"1dee43ba495abfd6de7ecd15954d0ea8bf431340","unresolved":true,"context_lines":[{"line_number":203,"context_line":"            try:"},{"line_number":204,"context_line":"                return bool("},{"line_number":205,"context_line":"                    old_a_chassis.difference(row.additional_chassis) and"},{"line_number":206,"context_line":"                    not hasattr(old, \u0027chassis\u0027))"},{"line_number":207,"context_line":"            except AttributeError:"},{"line_number":208,"context_line":"                return False"},{"line_number":209,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":3,"id":"35b189f1_a2fd1724","line":206,"updated":"2023-09-28 19:57:19.000000000","message":"I am a bit confused here.\n\n1. the first conditional (`old_a_chassis.difference(row.additional_chassis)`) can be interpreted as: \"The current agent chassis was in additional_chassis but is not longer there\". (Which happens either in case of successful live migration - in which case the current agent chassis will be set to row.chassis and removed from row.additional_chassis - or in case of LM failure - in which case the current chassis will be removed from row.additional_chassis.)\n\n2. the second conditional (`not hasattr(old, \u0027chassis\u0027)`) can be interpreted as: \"The old version of the PB record had the attribute called chassis.\" Which I think has not much to do with the actual PB state or transition?\n\nWouldn\u0027t a proper second conditional check that the self.agent.chassis !\u003d row.chassis?\n\nPerhaps this code relies on an (internal implementation?) detail that when row.chassis was not touched, then `hasattr(old, \u0027chassis\u0027) \u003d\u003d False`? Then what if `chassis` *was* set (to a different value than the previous `chassis`?)? Will this code return False then?\n\n---\n\nThe scenario would be:\n\n1. set chassis\u003dCH1, additional_chassis\u003dCH2\n2. set chassis\u003dCH3, additional_chassis\u003d[]\n\nAFAIU then `not hassattr(old, \u0027chassis\u0027)` will be \u003d\u003d False, no?\n\nPerhaps I am misreading it. Can we add a test case for such scenario to prove this code handles it gracefully?\n\n---\n\nI understand that maybe this example is a bit synthetic, but I think this code should not rely on particular order of updates to a port binding. WDYT?","commit_id":"e6de3164112af91e6a010974bd8f688918eb7622"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6ee4325c27e5b83c85e9fc540b0b19a67e39d28d","unresolved":false,"context_lines":[{"line_number":203,"context_line":"            try:"},{"line_number":204,"context_line":"                return bool("},{"line_number":205,"context_line":"                    old_a_chassis.difference(row.additional_chassis) and"},{"line_number":206,"context_line":"                    not hasattr(old, \u0027chassis\u0027))"},{"line_number":207,"context_line":"            except AttributeError:"},{"line_number":208,"context_line":"                return False"},{"line_number":209,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":3,"id":"33b2880b_3fb14b8e","line":206,"in_reply_to":"35b189f1_a2fd1724","updated":"2023-09-29 15:20:16.000000000","message":"Done","commit_id":"e6de3164112af91e6a010974bd8f688918eb7622"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"9178de9418c0d4de382fe978153c515174961f98","unresolved":true,"context_lines":[{"line_number":195,"context_line":"        try:"},{"line_number":196,"context_line":"            # Return True if the agent chassis was added to additional_chassis"},{"line_number":197,"context_line":"            # column"},{"line_number":198,"context_line":"            return bool("},{"line_number":199,"context_line":"                additional_chassis.difference(old.additional_chassis))"},{"line_number":200,"context_line":"        except AttributeError:"},{"line_number":201,"context_line":"            # If addtional_chassis column was not changed then the old object"}],"source_content_type":"text/x-python","patch_set":5,"id":"c5417ed7_84f9f19f","line":198,"updated":"2023-10-03 17:26:40.000000000","message":"To capture discussion elsewhere, it\u0027s unclear how the handler (provision_datapath) will distinguish between the need to tear down and to bring up.\n\nAs you pointed out, get_ports_on_chassis may be the filter used to distinguish between ports, but it doesn\u0027t consider `additional_chassis`, so it may need an adjustment.\n\nThis also made me think that maybe functional tests are only scratching surface - they validate the decision of whether an event is \"of interest\" (in which case provision_datapath is called - which is checked with mock), but not whether the event handled results in the expected state of the service (which could only be checked if the mock is removed and we instead inspect the state of the netns and interfaces in it.) [As said elsewhere, I don\u0027t suggest that you must extend the suite to validate the end state - I don\u0027t want to add more work on your plate. But that\u0027s just something that could reveal any additional issues in the layer below.]","commit_id":"5e630ef5fffbd5270d3969da460451579a3d29f5"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"8462b0e11bba2c5acf8ea2e54cbec00acde1385f","unresolved":false,"context_lines":[{"line_number":195,"context_line":"        try:"},{"line_number":196,"context_line":"            # Return True if the agent chassis was added to additional_chassis"},{"line_number":197,"context_line":"            # column"},{"line_number":198,"context_line":"            return bool("},{"line_number":199,"context_line":"                additional_chassis.difference(old.additional_chassis))"},{"line_number":200,"context_line":"        except AttributeError:"},{"line_number":201,"context_line":"            # If addtional_chassis column was not changed then the old object"}],"source_content_type":"text/x-python","patch_set":5,"id":"da3ce692_436b60a3","line":198,"in_reply_to":"c5417ed7_84f9f19f","updated":"2023-10-04 20:48:00.000000000","message":"Done","commit_id":"5e630ef5fffbd5270d3969da460451579a3d29f5"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"9178de9418c0d4de382fe978153c515174961f98","unresolved":true,"context_lines":[{"line_number":198,"context_line":"            return bool("},{"line_number":199,"context_line":"                additional_chassis.difference(old.additional_chassis))"},{"line_number":200,"context_line":"        except AttributeError:"},{"line_number":201,"context_line":"            # If addtional_chassis column was not changed then the old object"},{"line_number":202,"context_line":"            # raises AttributeError when reading the column"},{"line_number":203,"context_line":"            return False"},{"line_number":204,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7e9f07cc_f10b78b6","line":201,"range":{"start_line":201,"start_character":17,"end_line":201,"end_character":34},"updated":"2023-10-03 17:26:40.000000000","message":"typo","commit_id":"5e630ef5fffbd5270d3969da460451579a3d29f5"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"8b7112926c4076b17046e7abf7fbc8ce0044c5bf","unresolved":false,"context_lines":[{"line_number":198,"context_line":"            return bool("},{"line_number":199,"context_line":"                additional_chassis.difference(old.additional_chassis))"},{"line_number":200,"context_line":"        except AttributeError:"},{"line_number":201,"context_line":"            # If addtional_chassis column was not changed then the old object"},{"line_number":202,"context_line":"            # raises AttributeError when reading the column"},{"line_number":203,"context_line":"            return False"},{"line_number":204,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"ed1200d8_b6bcb1cf","line":201,"range":{"start_line":201,"start_character":17,"end_line":201,"end_character":34},"in_reply_to":"7e9f07cc_f10b78b6","updated":"2023-10-04 20:46:28.000000000","message":"Done","commit_id":"5e630ef5fffbd5270d3969da460451579a3d29f5"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"26c60695c70c8307b8fea9af002009ac774cefac","unresolved":true,"context_lines":[{"line_number":225,"context_line":"            # additional_chassis attribute and raises an AttributeError"},{"line_number":226,"context_line":"            return False"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"        # If was changed to the agent chassis then we do not need to teardown"},{"line_number":229,"context_line":"        # the resources"},{"line_number":230,"context_line":"        try:"},{"line_number":231,"context_line":"            if (hasattr(old, \u0027chassis\u0027) and"}],"source_content_type":"text/x-python","patch_set":9,"id":"a9a80043_7e228460","line":228,"updated":"2023-11-03 15:50:29.000000000","message":"nit: If it was...","commit_id":"3ec7f96cbb0571c401406781562160db75b3b9f5"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"57198c667c7164fdb9de6f06b0df05ac83910d4a","unresolved":true,"context_lines":[{"line_number":225,"context_line":"            # additional_chassis attribute and raises an AttributeError"},{"line_number":226,"context_line":"            return False"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"        # If was changed to the agent chassis then we do not need to teardown"},{"line_number":229,"context_line":"        # the resources"},{"line_number":230,"context_line":"        try:"},{"line_number":231,"context_line":"            if (hasattr(old, \u0027chassis\u0027) and"}],"source_content_type":"text/x-python","patch_set":9,"id":"8d221e62_b48354ea","line":228,"in_reply_to":"a9a80043_7e228460","updated":"2023-11-03 16:05:55.000000000","message":"Sharp eyes! Thanks! Will fix if respin is needed","commit_id":"3ec7f96cbb0571c401406781562160db75b3b9f5"}],"neutron/common/ovn/utils.py":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"514c94436d2b22fc762cd5912a993cc475232c21","unresolved":true,"context_lines":[{"line_number":1093,"context_line":""},{"line_number":1094,"context_line":""},{"line_number":1095,"context_line":"def is_additional_chassis_supported(idl):"},{"line_number":1096,"context_line":"    return idl.is_col_present(\u0027Port_Binding\u0027, \u0027additional_chassis\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"b820f29f_c824d7f7","line":1096,"updated":"2023-10-25 10:25:55.000000000","message":"nit: this is a one liner, do we really need it a helper method for it ?\n\nAlso a TODO would be good to remove it in the future as additional_chassis will always be present since older OVN versions won\u0027t be supported anymore","commit_id":"1ed129aa78f03f1596b86929ca5baa656466d692"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"430d4f544bf6671ea75bf45c0d321c46826987e9","unresolved":true,"context_lines":[{"line_number":1093,"context_line":""},{"line_number":1094,"context_line":""},{"line_number":1095,"context_line":"def is_additional_chassis_supported(idl):"},{"line_number":1096,"context_line":"    return idl.is_col_present(\u0027Port_Binding\u0027, \u0027additional_chassis\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ce9f2efb_9e5cc45c","line":1096,"in_reply_to":"323abd2a_9a61ae8c","updated":"2023-10-27 15:02:26.000000000","message":"It can definitely be removed later when OVN version is no longer supported. One doesn\u0027t preclude the other.","commit_id":"1ed129aa78f03f1596b86929ca5baa656466d692"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c8dd2ca28fa996c9566281ea331bb4f1b4df2a2e","unresolved":true,"context_lines":[{"line_number":1093,"context_line":""},{"line_number":1094,"context_line":""},{"line_number":1095,"context_line":"def is_additional_chassis_supported(idl):"},{"line_number":1096,"context_line":"    return idl.is_col_present(\u0027Port_Binding\u0027, \u0027additional_chassis\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"c4488e85_89188428","line":1096,"in_reply_to":"b820f29f_c824d7f7","updated":"2023-10-26 13:36:25.000000000","message":"The one liner hides knowledge about the column, so yes, I think it\u0027s helpful.","commit_id":"1ed129aa78f03f1596b86929ca5baa656466d692"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"d6a721626389b01d6fd8674c8a3c2258a62e33bc","unresolved":true,"context_lines":[{"line_number":1093,"context_line":""},{"line_number":1094,"context_line":""},{"line_number":1095,"context_line":"def is_additional_chassis_supported(idl):"},{"line_number":1096,"context_line":"    return idl.is_col_present(\u0027Port_Binding\u0027, \u0027additional_chassis\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"323abd2a_9a61ae8c","line":1096,"in_reply_to":"c4488e85_89188428","updated":"2023-10-26 13:44:17.000000000","message":"I disagree because we have plenty of is_col_present() around the code for various other columns. Usually these checks are temporary and should be removed later once older versions of OVN are no longer supported.\n\nBut this is just my 0.2c and a nit so...","commit_id":"1ed129aa78f03f1596b86929ca5baa656466d692"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"093bcbb5fd8542cfb42b60c37b35e85dea16067b","unresolved":true,"context_lines":[{"line_number":1093,"context_line":""},{"line_number":1094,"context_line":""},{"line_number":1095,"context_line":"def is_additional_chassis_supported(idl):"},{"line_number":1096,"context_line":"    return idl.is_col_present(\u0027Port_Binding\u0027, \u0027additional_chassis\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ebad25f5_d147408c","line":1096,"in_reply_to":"ce9f2efb_9e5cc45c","updated":"2023-11-02 20:38:40.000000000","message":"I added just because I was asked to. I don\u0027t care much to be honest about this, if I need to pick I would leave the helper here as I find it a bit more readable and it encapsulates the table name inside.","commit_id":"1ed129aa78f03f1596b86929ca5baa656466d692"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"19e61bb59c79b5918e097b1255dfc5b324938d20","unresolved":false,"context_lines":[{"line_number":1093,"context_line":""},{"line_number":1094,"context_line":""},{"line_number":1095,"context_line":"def is_additional_chassis_supported(idl):"},{"line_number":1096,"context_line":"    return idl.is_col_present(\u0027Port_Binding\u0027, \u0027additional_chassis\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"958138b8_fdfe0b84","line":1096,"in_reply_to":"ebad25f5_d147408c","updated":"2023-11-03 16:33:05.000000000","message":"All good :D","commit_id":"1ed129aa78f03f1596b86929ca5baa656466d692"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"865b4c05bca74631a364908488501b88d2007daa","unresolved":true,"context_lines":[{"line_number":963,"context_line":"        return self.db_set(\u0027Port_Binding\u0027, name, \u0027external_ids\u0027,"},{"line_number":964,"context_line":"                           {\u0027neutron-port-cidrs\u0027: cidrs})"},{"line_number":965,"context_line":""},{"line_number":966,"context_line":"    def get_ports_on_chassis(self, chassis, include_additional_chassis\u003dFalse):"},{"line_number":967,"context_line":"        # TODO(twilson) Some day it would be nice to stop passing names around"},{"line_number":968,"context_line":"        # and just start using chassis objects so db_find_rows could be used"},{"line_number":969,"context_line":"        rows \u003d self.db_list_rows(\u0027Port_Binding\u0027).execute(check_error\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3c7149fa_55e7178a","line":966,"range":{"start_line":966,"start_character":44,"end_line":966,"end_character":70},"updated":"2023-10-03 22:03:56.000000000","message":"why is it not the default and only (hardcoded) behavior of the function? I don\u0027t see it used anywhere but in the two other places where you explicitly pass \u003dTrue","commit_id":"1ed129aa78f03f1596b86929ca5baa656466d692"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"87ed383796c4dd8dd5bf85d97244929e5d10a4c7","unresolved":true,"context_lines":[{"line_number":963,"context_line":"        return self.db_set(\u0027Port_Binding\u0027, name, \u0027external_ids\u0027,"},{"line_number":964,"context_line":"                           {\u0027neutron-port-cidrs\u0027: cidrs})"},{"line_number":965,"context_line":""},{"line_number":966,"context_line":"    def get_ports_on_chassis(self, chassis, include_additional_chassis\u003dFalse):"},{"line_number":967,"context_line":"        # TODO(twilson) Some day it would be nice to stop passing names around"},{"line_number":968,"context_line":"        # and just start using chassis objects so db_find_rows could be used"},{"line_number":969,"context_line":"        rows \u003d self.db_list_rows(\u0027Port_Binding\u0027).execute(check_error\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7c5cd174_c551d052","line":966,"range":{"start_line":966,"start_character":44,"end_line":966,"end_character":70},"in_reply_to":"3c7149fa_55e7178a","updated":"2023-10-04 13:51:35.000000000","message":"It\u0027s a \"public\" method of an idl class, I didn\u0027t want to break anybody who can potentially use it.","commit_id":"1ed129aa78f03f1596b86929ca5baa656466d692"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"359475128e93e57eb319ed70c2a01ddcde92f992","unresolved":false,"context_lines":[{"line_number":963,"context_line":"        return self.db_set(\u0027Port_Binding\u0027, name, \u0027external_ids\u0027,"},{"line_number":964,"context_line":"                           {\u0027neutron-port-cidrs\u0027: cidrs})"},{"line_number":965,"context_line":""},{"line_number":966,"context_line":"    def get_ports_on_chassis(self, chassis, include_additional_chassis\u003dFalse):"},{"line_number":967,"context_line":"        # TODO(twilson) Some day it would be nice to stop passing names around"},{"line_number":968,"context_line":"        # and just start using chassis objects so db_find_rows could be used"},{"line_number":969,"context_line":"        rows \u003d self.db_list_rows(\u0027Port_Binding\u0027).execute(check_error\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":8,"id":"1358bdea_01e1aef8","line":966,"range":{"start_line":966,"start_character":44,"end_line":966,"end_character":70},"in_reply_to":"7c5cd174_c551d052","updated":"2023-10-04 14:02:40.000000000","message":"Ack","commit_id":"1ed129aa78f03f1596b86929ca5baa656466d692"}],"neutron/tests/functional/agent/ovn/metadata/test_metadata_agent.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"1dee43ba495abfd6de7ecd15954d0ea8bf431340","unresolved":true,"context_lines":[{"line_number":479,"context_line":"            with testtools.ExpectedException(NoDatapathProvision):"},{"line_number":480,"context_line":"                n_utils.wait_until_true("},{"line_number":481,"context_line":"                    lambda: m_provision.called,"},{"line_number":482,"context_line":"                    timeout\u003d1,"},{"line_number":483,"context_line":"                    exception\u003dNoDatapathProvision("},{"line_number":484,"context_line":"                        \"Provisioning wasn\u0027t triggered\"))"},{"line_number":485,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"2ead4160_c0b4f062","line":482,"updated":"2023-09-28 19:57:19.000000000","message":"(I don\u0027t know if this could be easily fixed somehow, but let\u0027s see if you have some immediate idea)\n\nthis is error prone, isn\u0027t it? what if the machine running the test is a bit too slow to trigger the event handler? Is there a way to enforce event consumption?","commit_id":"e6de3164112af91e6a010974bd8f688918eb7622"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"9178de9418c0d4de382fe978153c515174961f98","unresolved":true,"context_lines":[{"line_number":479,"context_line":"            with testtools.ExpectedException(NoDatapathProvision):"},{"line_number":480,"context_line":"                n_utils.wait_until_true("},{"line_number":481,"context_line":"                    lambda: m_provision.called,"},{"line_number":482,"context_line":"                    timeout\u003d1,"},{"line_number":483,"context_line":"                    exception\u003dNoDatapathProvision("},{"line_number":484,"context_line":"                        \"Provisioning wasn\u0027t triggered\"))"},{"line_number":485,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"91a7f79f_30aaa361","line":482,"in_reply_to":"1f43afd8_e25b5715","updated":"2023-10-03 17:26:40.000000000","message":"What we are interested here would be - whether the event 1) happened and 2) was passed to event handlers. If there were a way to confirm ovsdbapp \"version\" of the state is \u003e\u003d what we\u0027d expect...","commit_id":"e6de3164112af91e6a010974bd8f688918eb7622"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6ee4325c27e5b83c85e9fc540b0b19a67e39d28d","unresolved":true,"context_lines":[{"line_number":479,"context_line":"            with testtools.ExpectedException(NoDatapathProvision):"},{"line_number":480,"context_line":"                n_utils.wait_until_true("},{"line_number":481,"context_line":"                    lambda: m_provision.called,"},{"line_number":482,"context_line":"                    timeout\u003d1,"},{"line_number":483,"context_line":"                    exception\u003dNoDatapathProvision("},{"line_number":484,"context_line":"                        \"Provisioning wasn\u0027t triggered\"))"},{"line_number":485,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1f43afd8_e25b5715","line":482,"in_reply_to":"2ead4160_c0b4f062","updated":"2023-09-29 15:20:16.000000000","message":"I don\u0027t think in general there is a way to say confidently something is NOT going to happen. That means if we expect an event to not be triggered, how can we tell it won\u0027t triggered later after we considered the system to be fine and not triggering the event.\n\nWith the number of jobs we execute per day if it gets triggered after 1 second, I think there is a high chance this would fail.\n\nI\u0027m open to ideas","commit_id":"e6de3164112af91e6a010974bd8f688918eb7622"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"98a47be473d487e4a5e746723aa26062071ff0cf","unresolved":true,"context_lines":[{"line_number":479,"context_line":"            with testtools.ExpectedException(NoDatapathProvision):"},{"line_number":480,"context_line":"                n_utils.wait_until_true("},{"line_number":481,"context_line":"                    lambda: m_provision.called,"},{"line_number":482,"context_line":"                    timeout\u003d1,"},{"line_number":483,"context_line":"                    exception\u003dNoDatapathProvision("},{"line_number":484,"context_line":"                        \"Provisioning wasn\u0027t triggered\"))"},{"line_number":485,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"cadd93e3_ad18d799","line":482,"in_reply_to":"91a7f79f_30aaa361","updated":"2023-10-03 20:38:30.000000000","message":"Talked about it with Terry and for what I understood the only exact way to confirm that an event was received and processed would be to replace one of match_fn handlers with a mock that\n\n1. registers the fact that the event was processed and\n2. if needed, pass the control to the actual match_fn handler\n\nI don\u0027t know if I want to push towards more mocks here though for a theoretical timeout problem. Consider it a non-issue.","commit_id":"e6de3164112af91e6a010974bd8f688918eb7622"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"79288b592a0e7ad3324058bc77f8e1dcbda16594","unresolved":false,"context_lines":[{"line_number":479,"context_line":"            with testtools.ExpectedException(NoDatapathProvision):"},{"line_number":480,"context_line":"                n_utils.wait_until_true("},{"line_number":481,"context_line":"                    lambda: m_provision.called,"},{"line_number":482,"context_line":"                    timeout\u003d1,"},{"line_number":483,"context_line":"                    exception\u003dNoDatapathProvision("},{"line_number":484,"context_line":"                        \"Provisioning wasn\u0027t triggered\"))"},{"line_number":485,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"675daaaf_0189acc6","line":482,"in_reply_to":"cadd93e3_ad18d799","updated":"2023-10-06 15:56:34.000000000","message":"Done","commit_id":"e6de3164112af91e6a010974bd8f688918eb7622"}],"neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl.py":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"a4d278cebb852bd3b87b4e3e77c2f09db35d699a","unresolved":true,"context_lines":[{"line_number":193,"context_line":"    def test_get_ports_on_chassis_with_additional_chassis(self):"},{"line_number":194,"context_line":"        chassis, switch, port, binding \u003d self._add_switch_port("},{"line_number":195,"context_line":"            self.data[\u0027chassis\u0027][0][\u0027name\u0027])"},{"line_number":196,"context_line":"        chassis2, switch2, port2, binding2 \u003d self._add_switch_port("},{"line_number":197,"context_line":"            self.data[\u0027chassis\u0027][1][\u0027name\u0027])"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"        self._test_get_ports_on_chassis_with_additional_chassis("}],"source_content_type":"text/x-python","patch_set":8,"id":"83922fad_c918f38b","line":196,"updated":"2023-10-31 13:20:00.000000000","message":"Apparently the _add_switch_port method was changed as part of [0] and now the tests are failing with: \n\n```\nAttributeError: \u0027TestSbApi\u0027 object has no attribute \u0027_add_switch_port\n```\n\nSee: https://storage.gra.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_60b/895402/8/gate/neutron-functional-with-uwsgi/60ba6c1/testr_results.html\n\n[0] https://review.opendev.org/c/openstack/neutron/+/897345/8/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl.py","commit_id":"1ed129aa78f03f1596b86929ca5baa656466d692"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"b20a264d73c644a966bb08305a4c0a6b080f1b2f","unresolved":false,"context_lines":[{"line_number":193,"context_line":"    def test_get_ports_on_chassis_with_additional_chassis(self):"},{"line_number":194,"context_line":"        chassis, switch, port, binding \u003d self._add_switch_port("},{"line_number":195,"context_line":"            self.data[\u0027chassis\u0027][0][\u0027name\u0027])"},{"line_number":196,"context_line":"        chassis2, switch2, port2, binding2 \u003d self._add_switch_port("},{"line_number":197,"context_line":"            self.data[\u0027chassis\u0027][1][\u0027name\u0027])"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"        self._test_get_ports_on_chassis_with_additional_chassis("}],"source_content_type":"text/x-python","patch_set":8,"id":"b51e7d1b_05b40874","line":196,"in_reply_to":"83922fad_c918f38b","updated":"2023-11-02 20:36:11.000000000","message":"Done","commit_id":"1ed129aa78f03f1596b86929ca5baa656466d692"}]}