)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"8b35a8e4b34efcec15479a3bd211e8c86026b977","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6ad1c6d2_f10fea1e","updated":"2022-05-18 13:55:05.000000000","message":"If I understand the code correctly, you set min_rate as part of Qos northbound db object. The OVN piece we merged doesn\u0027t use Qos objects. Instead, you should update options: column for the Logical_Switch_Port to include the required qos_min_rate option.","commit_id":"bfce8dcfe501fa27e8c2e7cd31b7cea8bfc9fc84"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"530de0e31617fa27fe0abe7699da6a3f32997c65","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"17409a02_95e5ae64","updated":"2022-07-19 00:57:00.000000000","message":"Hi. What\u0027s missing with this patch?\n\nIs it possible to add a \u0027fullstack\u0027 (as in: \u0027fullstack\u0027 test suite in-tree) test scenario here that would:\n\n1) create min rate rule in neutron db\n2) check in ovndb that the rule is set\n3) perhaps also check that the rate is set on bridge netdev device underlying the port","commit_id":"ce5763d4827e630d9c85b8ae114d627b3e073f37"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"ea46eec5ab011b524a8a550b1e774dbd4afd1ac2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"f10ce4b6_73b51a9e","updated":"2022-07-28 00:11:18.000000000","message":"Release note needed I believe. Otherwise it\u0027s great.","commit_id":"5f3c330440f6dcb9716ab5b42c68f23ebe1c8c4c"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"b0a1e1dee43d98a0e44293c332f57aefcf2876c6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"42070758_1482a6e5","updated":"2022-08-18 12:31:53.000000000","message":"Looks good to me. Not a huge deal, but I\u0027d like to see the row_by_value() changed to the more efficient lookup() that already more efficiently handles the lookup by uuid or name/BaseCommand stuff.","commit_id":"cc136f73150f0f5a3e9e9c17d8c1125025a97adb"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"0d0f2ac900744e04b001564c777bb58201d633a2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"f58adaa2_7161d8d9","updated":"2022-08-19 19:12:40.000000000","message":"Looks great, thanks!","commit_id":"846737dac4acb0fd47fb20058663513c4e054ee6"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"08b6b5f58e7cd42715e68c383da2e981a536bf3d","unresolved":true,"context_lines":[{"line_number":200,"context_line":""},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"class UpdateLSwitchPortQosOptionsCommand(command.BaseCommand):"},{"line_number":203,"context_line":"    def __init__(self, api, lport, if_exists, **qos_options):"},{"line_number":204,"context_line":"        super(UpdateLSwitchPortQosOptionsCommand, self).__init__(api)"},{"line_number":205,"context_line":"        self.lport \u003d lport"},{"line_number":206,"context_line":"        self.qos_options \u003d qos_options"}],"source_content_type":"text/x-python","patch_set":4,"id":"780d5368_48435cba","line":203,"range":{"start_line":203,"start_character":48,"end_line":203,"end_character":59},"updated":"2022-07-19 14:55:30.000000000","message":"Please describe in the method description how many parameters can we provide (qos_min_rate, qos_max_rate, qos_burst, qdisc_queue_id).","commit_id":"f7255710dce76365d9bc1f7c714ff1c1dfc92ae9"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a801bb461549f1b6fbaa463a373c8cd2d42a856e","unresolved":false,"context_lines":[{"line_number":200,"context_line":""},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"class UpdateLSwitchPortQosOptionsCommand(command.BaseCommand):"},{"line_number":203,"context_line":"    def __init__(self, api, lport, if_exists, **qos_options):"},{"line_number":204,"context_line":"        super(UpdateLSwitchPortQosOptionsCommand, self).__init__(api)"},{"line_number":205,"context_line":"        self.lport \u003d lport"},{"line_number":206,"context_line":"        self.qos_options \u003d qos_options"}],"source_content_type":"text/x-python","patch_set":4,"id":"c090ec6a_4b5ffb92","line":203,"range":{"start_line":203,"start_character":48,"end_line":203,"end_character":59},"in_reply_to":"780d5368_48435cba","updated":"2022-08-04 10:40:50.000000000","message":"Done","commit_id":"f7255710dce76365d9bc1f7c714ff1c1dfc92ae9"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"08b6b5f58e7cd42715e68c383da2e981a536bf3d","unresolved":true,"context_lines":[{"line_number":222,"context_line":"            if options[key] is None:"},{"line_number":223,"context_line":"                del options[key]"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"        setattr(lsp, \u0027options\u0027, options)"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"class DelLSwitchPortCommand(command.BaseCommand):"}],"source_content_type":"text/x-python","patch_set":4,"id":"15facf06_a8f55911","line":225,"range":{"start_line":225,"start_character":7,"end_line":225,"end_character":40},"updated":"2022-07-19 14:55:30.000000000","message":"I\u0027m not sure if that is needed. You have updated lsp.options in the previous lines.","commit_id":"f7255710dce76365d9bc1f7c714ff1c1dfc92ae9"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a801bb461549f1b6fbaa463a373c8cd2d42a856e","unresolved":false,"context_lines":[{"line_number":222,"context_line":"            if options[key] is None:"},{"line_number":223,"context_line":"                del options[key]"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"        setattr(lsp, \u0027options\u0027, options)"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"class DelLSwitchPortCommand(command.BaseCommand):"}],"source_content_type":"text/x-python","patch_set":4,"id":"d5d00353_73d2842c","line":225,"range":{"start_line":225,"start_character":7,"end_line":225,"end_character":40},"in_reply_to":"15facf06_a8f55911","updated":"2022-08-04 10:40:50.000000000","message":"Done","commit_id":"f7255710dce76365d9bc1f7c714ff1c1dfc92ae9"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"04ab6c9b76755940da09577ba84ec102b02853de","unresolved":true,"context_lines":[{"line_number":200,"context_line":""},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"class UpdateLSwitchPortQosOptionsCommand(command.BaseCommand):"},{"line_number":203,"context_line":"    def __init__(self, api, port, if_exists, **qos):"},{"line_number":204,"context_line":"        super().__init__(api)"},{"line_number":205,"context_line":"        self.port \u003d port"},{"line_number":206,"context_line":"        self.if_exists \u003d if_exists"}],"source_content_type":"text/x-python","patch_set":8,"id":"e4e0f475_0ecc8c48","line":203,"range":{"start_line":203,"start_character":28,"end_line":203,"end_character":32},"updated":"2022-08-10 14:40:35.000000000","message":"perhaps in order to be consistent in naming with other LSP commands, this should be named \u0027lport\u0027","commit_id":"1071de73b31599933cf89fd53eb85c474d9d8dc7"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"8667c83b42b99e059cf9f4896a2708bde0324549","unresolved":false,"context_lines":[{"line_number":200,"context_line":""},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"class UpdateLSwitchPortQosOptionsCommand(command.BaseCommand):"},{"line_number":203,"context_line":"    def __init__(self, api, port, if_exists, **qos):"},{"line_number":204,"context_line":"        super().__init__(api)"},{"line_number":205,"context_line":"        self.port \u003d port"},{"line_number":206,"context_line":"        self.if_exists \u003d if_exists"}],"source_content_type":"text/x-python","patch_set":8,"id":"e144e397_58f62a40","line":203,"range":{"start_line":203,"start_character":28,"end_line":203,"end_character":32},"in_reply_to":"e4e0f475_0ecc8c48","updated":"2022-08-10 15:59:23.000000000","message":"yes, \"lport\" name is better here.","commit_id":"1071de73b31599933cf89fd53eb85c474d9d8dc7"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"04ab6c9b76755940da09577ba84ec102b02853de","unresolved":true,"context_lines":[{"line_number":207,"context_line":"        self.qos \u003d qos"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    def run_idl(self, txn):"},{"line_number":210,"context_line":"        if isinstance(self.port, command.BaseCommand):"},{"line_number":211,"context_line":"            port_id_or_name \u003d self.port.result"},{"line_number":212,"context_line":"            column \u003d \u0027uuid\u0027"},{"line_number":213,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"96989781_6f4f3a3e","line":210,"updated":"2022-08-10 14:40:35.000000000","message":"Just out of curiosity, why would we pass a command as a port parameter? I see it\u0027s used when the port is create with the qos rule but can\u0027t we always use port UUID to avoid having this condition?","commit_id":"1071de73b31599933cf89fd53eb85c474d9d8dc7"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"8667c83b42b99e059cf9f4896a2708bde0324549","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        self.qos \u003d qos"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    def run_idl(self, txn):"},{"line_number":210,"context_line":"        if isinstance(self.port, command.BaseCommand):"},{"line_number":211,"context_line":"            port_id_or_name \u003d self.port.result"},{"line_number":212,"context_line":"            column \u003d \u0027uuid\u0027"},{"line_number":213,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"f2ab224c_6c42f016","line":210,"in_reply_to":"96989781_6f4f3a3e","updated":"2022-08-10 15:59:23.000000000","message":"Yeah, good question. This is similar to [1]. During the port creation call, the QoS creation method is called *inside* the OVN DB transaction. At this point we need to pass the port creation command (executed just before this code) and retrieve the .result value.\n\nWe can move the QoS method call outside the OVN DB transaction but it will be slower and unsafe.\n\n[1]https://github.com/openstack/ovsdbapp/blob/96cf8d6288587423e65d5149016e07fb51430724/ovsdbapp/schema/ovn_northbound/commands.py#L1691-L1700","commit_id":"1071de73b31599933cf89fd53eb85c474d9d8dc7"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"04ab6c9b76755940da09577ba84ec102b02853de","unresolved":true,"context_lines":[{"line_number":222,"context_line":"            if self.if_exists:"},{"line_number":223,"context_line":"                return"},{"line_number":224,"context_line":"            raise RuntimeError(_(\u0027Logical Switch Port %s does not exist\u0027) %"},{"line_number":225,"context_line":"                               self.port)"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"        for key, value in self.qos.items():"},{"line_number":228,"context_line":"            if value is None:"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a60960e_03f29088","line":225,"range":{"start_line":225,"start_character":31,"end_line":225,"end_character":40},"updated":"2022-08-10 14:40:35.000000000","message":"Should this be port_id_or_name ?","commit_id":"1071de73b31599933cf89fd53eb85c474d9d8dc7"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"8667c83b42b99e059cf9f4896a2708bde0324549","unresolved":false,"context_lines":[{"line_number":222,"context_line":"            if self.if_exists:"},{"line_number":223,"context_line":"                return"},{"line_number":224,"context_line":"            raise RuntimeError(_(\u0027Logical Switch Port %s does not exist\u0027) %"},{"line_number":225,"context_line":"                               self.port)"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"        for key, value in self.qos.items():"},{"line_number":228,"context_line":"            if value is None:"}],"source_content_type":"text/x-python","patch_set":8,"id":"b2726792_cd83dd32","line":225,"range":{"start_line":225,"start_character":31,"end_line":225,"end_character":40},"in_reply_to":"9a60960e_03f29088","updated":"2022-08-10 15:59:23.000000000","message":"Exactly","commit_id":"1071de73b31599933cf89fd53eb85c474d9d8dc7"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"b0a1e1dee43d98a0e44293c332f57aefcf2876c6","unresolved":true,"context_lines":[{"line_number":207,"context_line":"        self.qos \u003d qos"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    def run_idl(self, txn):"},{"line_number":210,"context_line":"        if isinstance(self.lport, command.BaseCommand):"},{"line_number":211,"context_line":"            port_id_or_name \u003d self.lport.result"},{"line_number":212,"context_line":"            column \u003d \u0027uuid\u0027"},{"line_number":213,"context_line":"        else:"},{"line_number":214,"context_line":"            port_id_or_name \u003d self.lport.name"},{"line_number":215,"context_line":"            column \u003d \u0027name\u0027"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        try:"},{"line_number":218,"context_line":"            port \u003d idlutils.row_by_value(self.api.idl,"},{"line_number":219,"context_line":"                                         \u0027Logical_Switch_Port\u0027,"},{"line_number":220,"context_line":"                                         column, port_id_or_name)"},{"line_number":221,"context_line":"        except idlutils.RowNotFound:"},{"line_number":222,"context_line":"            if self.if_exists:"},{"line_number":223,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":9,"id":"d704e733_321a3a95","line":220,"range":{"start_line":210,"start_character":0,"end_line":220,"end_character":65},"updated":"2022-08-18 12:31:53.000000000","message":"lookup() should handle both when a Command object is passed and looking up by name or uuid [1] so we shouldn\u0027t need to recreate that functionality here.\n\nI think it\u0027ll just work since there is a single index column in Logical_Switch_Port, but if it doesn\u0027t and for some reason needs it\u0027s own entry in the lookup table to handle it [2], we could add it to our subclass [3].\n[1] https://github.com/openstack/ovsdbapp/blob/96cf8d6288587423e65d5149016e07fb51430724/ovsdbapp/backend/ovs_idl/__init__.py#L192\n[2] https://github.com/openstack/ovsdbapp/blob/96cf8d6288587423e65d5149016e07fb51430724/ovsdbapp/schema/ovn_northbound/impl_idl.py#L23\n[3] https://github.com/openstack/neutron/blob/09207ba731bfc859c2f6d175588a8bfe09be01db/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py#L231","commit_id":"cc136f73150f0f5a3e9e9c17d8c1125025a97adb"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"9b3cee5a5be47cd3c9ad6e3444dd98be38eb2868","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        self.qos \u003d qos"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    def run_idl(self, txn):"},{"line_number":210,"context_line":"        if isinstance(self.lport, command.BaseCommand):"},{"line_number":211,"context_line":"            port_id_or_name \u003d self.lport.result"},{"line_number":212,"context_line":"            column \u003d \u0027uuid\u0027"},{"line_number":213,"context_line":"        else:"},{"line_number":214,"context_line":"            port_id_or_name \u003d self.lport.name"},{"line_number":215,"context_line":"            column \u003d \u0027name\u0027"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        try:"},{"line_number":218,"context_line":"            port \u003d idlutils.row_by_value(self.api.idl,"},{"line_number":219,"context_line":"                                         \u0027Logical_Switch_Port\u0027,"},{"line_number":220,"context_line":"                                         column, port_id_or_name)"},{"line_number":221,"context_line":"        except idlutils.RowNotFound:"},{"line_number":222,"context_line":"            if self.if_exists:"},{"line_number":223,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":9,"id":"b3faa046_66605c99","line":220,"range":{"start_line":210,"start_character":0,"end_line":220,"end_character":65},"in_reply_to":"8383af9e_30ff648d","updated":"2022-08-19 16:53:32.000000000","message":"This is what I tested. The \"AddLSwitchPortCommand.run_idl\" returns \"self.result \u003d port.uuid\". This is what I\u0027m getting in the lookup call. I can\u0027t use lookup here, or if I use it, I need to retrieve the row with another DB call.","commit_id":"cc136f73150f0f5a3e9e9c17d8c1125025a97adb"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"8dbf6c47bdda50ba75ba4e9997d86c5b0bc18403","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        self.qos \u003d qos"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    def run_idl(self, txn):"},{"line_number":210,"context_line":"        if isinstance(self.lport, command.BaseCommand):"},{"line_number":211,"context_line":"            port_id_or_name \u003d self.lport.result"},{"line_number":212,"context_line":"            column \u003d \u0027uuid\u0027"},{"line_number":213,"context_line":"        else:"},{"line_number":214,"context_line":"            port_id_or_name \u003d self.lport.name"},{"line_number":215,"context_line":"            column \u003d \u0027name\u0027"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        try:"},{"line_number":218,"context_line":"            port \u003d idlutils.row_by_value(self.api.idl,"},{"line_number":219,"context_line":"                                         \u0027Logical_Switch_Port\u0027,"},{"line_number":220,"context_line":"                                         column, port_id_or_name)"},{"line_number":221,"context_line":"        except idlutils.RowNotFound:"},{"line_number":222,"context_line":"            if self.if_exists:"},{"line_number":223,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa82617e_7dc20624","line":220,"range":{"start_line":210,"start_character":0,"end_line":220,"end_character":65},"in_reply_to":"d704e733_321a3a95","updated":"2022-08-19 10:08:38.000000000","message":"Good to read that lookup() accepts cmd.BaseCommand too. Something new learned today.\n\nI\u0027ve tried this but the problem is that lookup(), in the case of using a cmd.BaseCommand, returns the UUID only, not the row (record.result is the UUID). I need then to retrieve the row again using this UUID. I need the row to set/unset the \"options\" values.\n\nThis is why I need to use \"row_by_value\" and retrieve the row in one shoot.","commit_id":"cc136f73150f0f5a3e9e9c17d8c1125025a97adb"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"b8ef5a5a0d4d68632e8cee53e7dac8f68d7efa32","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        self.qos \u003d qos"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    def run_idl(self, txn):"},{"line_number":210,"context_line":"        if isinstance(self.lport, command.BaseCommand):"},{"line_number":211,"context_line":"            port_id_or_name \u003d self.lport.result"},{"line_number":212,"context_line":"            column \u003d \u0027uuid\u0027"},{"line_number":213,"context_line":"        else:"},{"line_number":214,"context_line":"            port_id_or_name \u003d self.lport.name"},{"line_number":215,"context_line":"            column \u003d \u0027name\u0027"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        try:"},{"line_number":218,"context_line":"            port \u003d idlutils.row_by_value(self.api.idl,"},{"line_number":219,"context_line":"                                         \u0027Logical_Switch_Port\u0027,"},{"line_number":220,"context_line":"                                         column, port_id_or_name)"},{"line_number":221,"context_line":"        except idlutils.RowNotFound:"},{"line_number":222,"context_line":"            if self.if_exists:"},{"line_number":223,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":9,"id":"8383af9e_30ff648d","line":220,"range":{"start_line":210,"start_character":0,"end_line":220,"end_character":65},"in_reply_to":"fa82617e_7dc20624","updated":"2022-08-19 15:57:00.000000000","message":"In the BaseCommand case, it returns whatever the result of the Command is. In this case it wasn\u0027t actually a UUID, it was a Row object because what is passed to it if lsp\u003dNone is the output of lsp_get().execute() which returns a Row object. lookup() can\u0027t search by Row (though perhaps we could add that in the future). All that you need to do is to set lsp\u003dlsp_get.execute().uuid and pass that in _update_lsp_qos_options. Then:\n\n      def run_idl(self, txn):\n -        if isinstance(self.lport, command.BaseCommand):\n -            port_id_or_name \u003d self.lport.result\n -            column \u003d \u0027uuid\u0027\n -        else:\n -            port_id_or_name \u003d self.lport.name\n -            column \u003d \u0027name\u0027\n -\n          try:\n -            port \u003d idlutils.row_by_value(self.api.idl,\n -                                         \u0027Logical_Switch_Port\u0027,\n -                                         column, port_id_or_name)\n +            port \u003d self.api.lookup(\u0027Logical_Switch_Port\u0027, self.lport)\n\nshould work.","commit_id":"cc136f73150f0f5a3e9e9c17d8c1125025a97adb"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/qos.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"530de0e31617fa27fe0abe7699da6a3f32997c65","unresolved":true,"context_lines":[{"line_number":253,"context_line":"            if rules:"},{"line_number":254,"context_line":"                if ovn_lsp_rule:"},{"line_number":255,"context_line":"                    txn.add(self._nb_idl.set_lswitch_port(lport_name\u003dport_id,  **ovn_lsp_rule))"},{"line_number":256,"context_line":"                else:"},{"line_number":257,"context_line":"                    # NOTE(ralonsoh): with \"may_exist\u003dTrue\", the \"qos_add\" will"},{"line_number":258,"context_line":"                    # create the QoS OVN rule or update the existing one."},{"line_number":259,"context_line":"                    txn.add(self.nb_idl.qos_add(**ovn_rule, may_exist\u003dTrue))"}],"source_content_type":"text/x-python","patch_set":3,"id":"94cb4f28_b8dd8690","line":256,"updated":"2022-07-19 00:57:00.000000000","message":"shouldn\u0027t we set both min rate and other types of qos rules in ovndb?","commit_id":"ce5763d4827e630d9c85b8ae114d627b3e073f37"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"7c0eec7d101fa57e31516a1a25d3c0add117e9e0","unresolved":false,"context_lines":[{"line_number":253,"context_line":"            if rules:"},{"line_number":254,"context_line":"                if ovn_lsp_rule:"},{"line_number":255,"context_line":"                    txn.add(self._nb_idl.set_lswitch_port(lport_name\u003dport_id,  **ovn_lsp_rule))"},{"line_number":256,"context_line":"                else:"},{"line_number":257,"context_line":"                    # NOTE(ralonsoh): with \"may_exist\u003dTrue\", the \"qos_add\" will"},{"line_number":258,"context_line":"                    # create the QoS OVN rule or update the existing one."},{"line_number":259,"context_line":"                    txn.add(self.nb_idl.qos_add(**ovn_rule, may_exist\u003dTrue))"}],"source_content_type":"text/x-python","patch_set":3,"id":"965209e4_2217cf86","line":256,"in_reply_to":"94cb4f28_b8dd8690","updated":"2022-07-27 14:01:35.000000000","message":"Done","commit_id":"ce5763d4827e630d9c85b8ae114d627b3e073f37"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"08b6b5f58e7cd42715e68c383da2e981a536bf3d","unresolved":true,"context_lines":[{"line_number":93,"context_line":"                r \u003d {rule.rule_type: {\u0027dscp_mark\u0027: rule.dscp_mark}}"},{"line_number":94,"context_line":"                qos_rules[constants.EGRESS_DIRECTION].update(r)"},{"line_number":95,"context_line":"            elif isinstance(rule, qos_rule.QosMinimumBandwidthRule):"},{"line_number":96,"context_line":"                # Rule supported for Placement scheduling but not enforced in"},{"line_number":97,"context_line":"                # the driver."},{"line_number":98,"context_line":"                r \u003d {rule.rule_type: {\u0027min_kbps\u0027: rule.min_kbps}}"},{"line_number":99,"context_line":"                qos_rules[constants.EGRESS_DIRECTION].update(r)"},{"line_number":100,"context_line":"                qos_rules[constants.INGRESS_DIRECTION].update(r)"}],"source_content_type":"text/x-python","patch_set":4,"id":"0cebd2c5_072507df","line":97,"range":{"start_line":96,"start_character":16,"end_line":97,"end_character":29},"updated":"2022-07-19 14:55:30.000000000","message":"Not true anymore","commit_id":"f7255710dce76365d9bc1f7c714ff1c1dfc92ae9"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"7c0eec7d101fa57e31516a1a25d3c0add117e9e0","unresolved":false,"context_lines":[{"line_number":93,"context_line":"                r \u003d {rule.rule_type: {\u0027dscp_mark\u0027: rule.dscp_mark}}"},{"line_number":94,"context_line":"                qos_rules[constants.EGRESS_DIRECTION].update(r)"},{"line_number":95,"context_line":"            elif isinstance(rule, qos_rule.QosMinimumBandwidthRule):"},{"line_number":96,"context_line":"                # Rule supported for Placement scheduling but not enforced in"},{"line_number":97,"context_line":"                # the driver."},{"line_number":98,"context_line":"                r \u003d {rule.rule_type: {\u0027min_kbps\u0027: rule.min_kbps}}"},{"line_number":99,"context_line":"                qos_rules[constants.EGRESS_DIRECTION].update(r)"},{"line_number":100,"context_line":"                qos_rules[constants.INGRESS_DIRECTION].update(r)"}],"source_content_type":"text/x-python","patch_set":4,"id":"fda342bb_2a064e0e","line":97,"range":{"start_line":96,"start_character":16,"end_line":97,"end_character":29},"in_reply_to":"0cebd2c5_072507df","updated":"2022-07-27 14:01:35.000000000","message":"Done","commit_id":"f7255710dce76365d9bc1f7c714ff1c1dfc92ae9"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"08b6b5f58e7cd42715e68c383da2e981a536bf3d","unresolved":true,"context_lines":[{"line_number":97,"context_line":"                # the driver."},{"line_number":98,"context_line":"                r \u003d {rule.rule_type: {\u0027min_kbps\u0027: rule.min_kbps}}"},{"line_number":99,"context_line":"                qos_rules[constants.EGRESS_DIRECTION].update(r)"},{"line_number":100,"context_line":"                qos_rules[constants.INGRESS_DIRECTION].update(r)"},{"line_number":101,"context_line":"            else:"},{"line_number":102,"context_line":"                LOG.warning(\u0027Rule type %(rule_type)s from QoS policy \u0027"},{"line_number":103,"context_line":"                            \u0027%(policy_id)s is not supported in OVN\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"67913013_f2657a8c","line":100,"range":{"start_line":100,"start_character":15,"end_line":100,"end_character":64},"updated":"2022-07-19 14:55:30.000000000","message":"We can\u0027t guarantee ingress min-bw, only egress. I would like Ihar to confirm that.","commit_id":"f7255710dce76365d9bc1f7c714ff1c1dfc92ae9"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"7c0eec7d101fa57e31516a1a25d3c0add117e9e0","unresolved":false,"context_lines":[{"line_number":97,"context_line":"                # the driver."},{"line_number":98,"context_line":"                r \u003d {rule.rule_type: {\u0027min_kbps\u0027: rule.min_kbps}}"},{"line_number":99,"context_line":"                qos_rules[constants.EGRESS_DIRECTION].update(r)"},{"line_number":100,"context_line":"                qos_rules[constants.INGRESS_DIRECTION].update(r)"},{"line_number":101,"context_line":"            else:"},{"line_number":102,"context_line":"                LOG.warning(\u0027Rule type %(rule_type)s from QoS policy \u0027"},{"line_number":103,"context_line":"                            \u0027%(policy_id)s is not supported in OVN\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"6a11b150_37e451f6","line":100,"range":{"start_line":100,"start_character":15,"end_line":100,"end_character":64},"in_reply_to":"67913013_f2657a8c","updated":"2022-07-27 14:01:35.000000000","message":"Done","commit_id":"f7255710dce76365d9bc1f7c714ff1c1dfc92ae9"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"08b6b5f58e7cd42715e68c383da2e981a536bf3d","unresolved":true,"context_lines":[{"line_number":122,"context_line":"        return match"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    @staticmethod"},{"line_number":125,"context_line":"    def _set_generic_qos_rule(qos_regular, qos_lsp, rule_type):"},{"line_number":126,"context_line":"        \"\"\"create a combined qos rule"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"        :param qos_regular: (dict) qos rules to be used with qos_add/ qos_del"},{"line_number":129,"context_line":"        :param qos_lsp: (dict) qos rules to be set using Logical_Switch_port"},{"line_number":130,"context_line":"        :param rule_type: (int) bitmask for selecting rules"},{"line_number":131,"context_line":"        \"\"\""},{"line_number":132,"context_line":"        generic_qos_rule \u003d {\u0027type\u0027: rule_type,"},{"line_number":133,"context_line":"                \u0027regular\u0027: qos_regular,"},{"line_number":134,"context_line":"                \u0027lsp\u0027: qos_lsp}"},{"line_number":135,"context_line":"        return generic_qos_rule"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    def _ovn_qos_rule(self, rules_direction, rules, port_id, network_id,"},{"line_number":138,"context_line":"                      fip_id\u003dNone, ip_address\u003dNone, resident_port\u003dNone,"}],"source_content_type":"text/x-python","patch_set":4,"id":"0ca76d50_44e8d978","line":135,"range":{"start_line":125,"start_character":1,"end_line":135,"end_character":31},"updated":"2022-07-19 14:55:30.000000000","message":"I don\u0027t like this idea. Instead of this, just return the qos rules and create methods for adding and deleting rules that differenciates the type. That is less intrusive in the QoS extension code.","commit_id":"f7255710dce76365d9bc1f7c714ff1c1dfc92ae9"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"7c0eec7d101fa57e31516a1a25d3c0add117e9e0","unresolved":false,"context_lines":[{"line_number":122,"context_line":"        return match"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    @staticmethod"},{"line_number":125,"context_line":"    def _set_generic_qos_rule(qos_regular, qos_lsp, rule_type):"},{"line_number":126,"context_line":"        \"\"\"create a combined qos rule"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"        :param qos_regular: (dict) qos rules to be used with qos_add/ qos_del"},{"line_number":129,"context_line":"        :param qos_lsp: (dict) qos rules to be set using Logical_Switch_port"},{"line_number":130,"context_line":"        :param rule_type: (int) bitmask for selecting rules"},{"line_number":131,"context_line":"        \"\"\""},{"line_number":132,"context_line":"        generic_qos_rule \u003d {\u0027type\u0027: rule_type,"},{"line_number":133,"context_line":"                \u0027regular\u0027: qos_regular,"},{"line_number":134,"context_line":"                \u0027lsp\u0027: qos_lsp}"},{"line_number":135,"context_line":"        return generic_qos_rule"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    def _ovn_qos_rule(self, rules_direction, rules, port_id, network_id,"},{"line_number":138,"context_line":"                      fip_id\u003dNone, ip_address\u003dNone, resident_port\u003dNone,"}],"source_content_type":"text/x-python","patch_set":4,"id":"f7de06b0_0595df80","line":135,"range":{"start_line":125,"start_character":1,"end_line":135,"end_character":31},"in_reply_to":"0ca76d50_44e8d978","updated":"2022-07-27 14:01:35.000000000","message":"Done","commit_id":"f7255710dce76365d9bc1f7c714ff1c1dfc92ae9"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"08b6b5f58e7cd42715e68c383da2e981a536bf3d","unresolved":true,"context_lines":[{"line_number":182,"context_line":"        ovn_qos_rule \u003d {\u0027switch\u0027: lswitch_name, \u0027direction\u0027: direction,"},{"line_number":183,"context_line":"                        \u0027priority\u0027: OVN_QOS_DEFAULT_RULE_PRIORITY,"},{"line_number":184,"context_line":"                        \u0027match\u0027: match}"},{"line_number":185,"context_line":"        ovn_qos_rule \u003d {\u0027switch\u0027: lswitch_name, \u0027direction\u0027: direction,"},{"line_number":186,"context_line":"                        \u0027priority\u0027: OVN_QOS_DEFAULT_RULE_PRIORITY,"},{"line_number":187,"context_line":"                        \u0027match\u0027: match}"},{"line_number":188,"context_line":"        ovn_qos_lsp_rule \u003d {\u0027qos_min_rate\u0027: None}"},{"line_number":189,"context_line":"        qos_rule_type \u003d 0"},{"line_number":190,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"0199de73_9ed3735b","line":187,"range":{"start_line":185,"start_character":7,"end_line":187,"end_character":39},"updated":"2022-07-19 14:55:30.000000000","message":"Why are you duplicating this variable?","commit_id":"f7255710dce76365d9bc1f7c714ff1c1dfc92ae9"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"7c0eec7d101fa57e31516a1a25d3c0add117e9e0","unresolved":false,"context_lines":[{"line_number":182,"context_line":"        ovn_qos_rule \u003d {\u0027switch\u0027: lswitch_name, \u0027direction\u0027: direction,"},{"line_number":183,"context_line":"                        \u0027priority\u0027: OVN_QOS_DEFAULT_RULE_PRIORITY,"},{"line_number":184,"context_line":"                        \u0027match\u0027: match}"},{"line_number":185,"context_line":"        ovn_qos_rule \u003d {\u0027switch\u0027: lswitch_name, \u0027direction\u0027: direction,"},{"line_number":186,"context_line":"                        \u0027priority\u0027: OVN_QOS_DEFAULT_RULE_PRIORITY,"},{"line_number":187,"context_line":"                        \u0027match\u0027: match}"},{"line_number":188,"context_line":"        ovn_qos_lsp_rule \u003d {\u0027qos_min_rate\u0027: None}"},{"line_number":189,"context_line":"        qos_rule_type \u003d 0"},{"line_number":190,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"a50205bb_7782b7a4","line":187,"range":{"start_line":185,"start_character":7,"end_line":187,"end_character":39},"in_reply_to":"0199de73_9ed3735b","updated":"2022-07-27 14:01:35.000000000","message":"Done","commit_id":"f7255710dce76365d9bc1f7c714ff1c1dfc92ae9"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"ea46eec5ab011b524a8a550b1e774dbd4afd1ac2","unresolved":true,"context_lines":[{"line_number":141,"context_line":"                         can contain bandwidth limit rule type fields, DSCP"},{"line_number":142,"context_line":"                         rule type fields and minimum bandwidth type fields."},{"line_number":143,"context_line":"        :param port_id: (str) Neutron port ID that matches the LSP.name."},{"line_number":144,"context_line":"                        If the port ID is None, the OVN QoS rule does apply to"},{"line_number":145,"context_line":"                        a LSP but to a router gateway port or a floating IP."},{"line_number":146,"context_line":"        :param may_exist: (bool) the LS QoS entry may exist; if enabled, the"},{"line_number":147,"context_line":"                          NB \"qos_add\" call won\u0027t fail."}],"source_content_type":"text/x-python","patch_set":5,"id":"1836f34c_02d90040","line":144,"range":{"start_line":144,"start_character":64,"end_line":144,"end_character":75},"updated":"2022-07-28 00:11:18.000000000","message":"does not?","commit_id":"5f3c330440f6dcb9716ab5b42c68f23ebe1c8c4c"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"83866df1482963a17bd6a6d7fc47e1ef7c12c0a2","unresolved":true,"context_lines":[{"line_number":141,"context_line":"                         can contain bandwidth limit rule type fields, DSCP"},{"line_number":142,"context_line":"                         rule type fields and minimum bandwidth type fields."},{"line_number":143,"context_line":"        :param port_id: (str) Neutron port ID that matches the LSP.name."},{"line_number":144,"context_line":"                        If the port ID is None, the OVN QoS rule does apply to"},{"line_number":145,"context_line":"                        a LSP but to a router gateway port or a floating IP."},{"line_number":146,"context_line":"        :param may_exist: (bool) the LS QoS entry may exist; if enabled, the"},{"line_number":147,"context_line":"                          NB \"qos_add\" call won\u0027t fail."}],"source_content_type":"text/x-python","patch_set":5,"id":"c4b966e0_bbb56fe5","line":144,"range":{"start_line":144,"start_character":64,"end_line":144,"end_character":75},"in_reply_to":"1836f34c_02d90040","updated":"2022-07-29 16:30:09.000000000","message":"Right!","commit_id":"5f3c330440f6dcb9716ab5b42c68f23ebe1c8c4c"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a801bb461549f1b6fbaa463a373c8cd2d42a856e","unresolved":false,"context_lines":[{"line_number":141,"context_line":"                         can contain bandwidth limit rule type fields, DSCP"},{"line_number":142,"context_line":"                         rule type fields and minimum bandwidth type fields."},{"line_number":143,"context_line":"        :param port_id: (str) Neutron port ID that matches the LSP.name."},{"line_number":144,"context_line":"                        If the port ID is None, the OVN QoS rule does apply to"},{"line_number":145,"context_line":"                        a LSP but to a router gateway port or a floating IP."},{"line_number":146,"context_line":"        :param may_exist: (bool) the LS QoS entry may exist; if enabled, the"},{"line_number":147,"context_line":"                          NB \"qos_add\" call won\u0027t fail."}],"source_content_type":"text/x-python","patch_set":5,"id":"a15b81a6_c7f355a7","line":144,"range":{"start_line":144,"start_character":64,"end_line":144,"end_character":75},"in_reply_to":"c4b966e0_bbb56fe5","updated":"2022-08-04 10:40:50.000000000","message":"Done","commit_id":"5f3c330440f6dcb9716ab5b42c68f23ebe1c8c4c"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a801bb461549f1b6fbaa463a373c8cd2d42a856e","unresolved":false,"context_lines":[{"line_number":12,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":13,"context_line":"#    under the License."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"import copy"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from neutron.objects.qos import binding as qos_binding"},{"line_number":18,"context_line":"from neutron.objects.qos import policy as qos_policy"}],"source_content_type":"text/x-python","patch_set":7,"id":"257a8d41_9d9b44fd","line":15,"range":{"start_line":15,"start_character":0,"end_line":15,"end_character":3},"updated":"2022-08-04 10:40:50.000000000","message":"To be removed.","commit_id":"b87f9880364e85e252501eeb20b01af294179c6b"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"04ab6c9b76755940da09577ba84ec102b02853de","unresolved":true,"context_lines":[{"line_number":94,"context_line":"                qos_rules[constants.EGRESS_DIRECTION].update(r)"},{"line_number":95,"context_line":"            elif isinstance(rule, qos_rule.QosMinimumBandwidthRule):"},{"line_number":96,"context_line":"                if rule.direction \u003d\u003d constants.INGRESS_DIRECTION:"},{"line_number":97,"context_line":"                    LOG.info(\u0027ML2/OVN QoS driver does not support minimum \u0027"},{"line_number":98,"context_line":"                             \u0027bandwidth rules enforcement with ingress \u0027"},{"line_number":99,"context_line":"                             \u0027direction\u0027)"},{"line_number":100,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"064d5f5c_fe126b7c","line":97,"range":{"start_line":97,"start_character":24,"end_line":97,"end_character":28},"updated":"2022-08-10 14:40:35.000000000","message":"Perhaps this should be a warning given that it has no effect but user might expect to have ingress minimum bandwidth?","commit_id":"1071de73b31599933cf89fd53eb85c474d9d8dc7"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"8667c83b42b99e059cf9f4896a2708bde0324549","unresolved":false,"context_lines":[{"line_number":94,"context_line":"                qos_rules[constants.EGRESS_DIRECTION].update(r)"},{"line_number":95,"context_line":"            elif isinstance(rule, qos_rule.QosMinimumBandwidthRule):"},{"line_number":96,"context_line":"                if rule.direction \u003d\u003d constants.INGRESS_DIRECTION:"},{"line_number":97,"context_line":"                    LOG.info(\u0027ML2/OVN QoS driver does not support minimum \u0027"},{"line_number":98,"context_line":"                             \u0027bandwidth rules enforcement with ingress \u0027"},{"line_number":99,"context_line":"                             \u0027direction\u0027)"},{"line_number":100,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"426e3722_4bbedfec","line":97,"range":{"start_line":97,"start_character":24,"end_line":97,"end_character":28},"in_reply_to":"064d5f5c_fe126b7c","updated":"2022-08-10 15:59:23.000000000","message":"Right","commit_id":"1071de73b31599933cf89fd53eb85c474d9d8dc7"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"04ab6c9b76755940da09577ba84ec102b02853de","unresolved":true,"context_lines":[{"line_number":123,"context_line":""},{"line_number":124,"context_line":"        return match"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    def _apply_qos_rule(self, txn, rules, ovn_rule, port_id\u003dNone, lsp\u003dNone,"},{"line_number":127,"context_line":"                        may_exist\u003dTrue, if_exists\u003dTrue, port_deleted\u003dFalse):"},{"line_number":128,"context_line":"        \"\"\"Add or remove an OVN rule set from the OVN database."},{"line_number":129,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"fcf6f622_632633a8","line":126,"range":{"start_line":126,"start_character":8,"end_line":126,"end_character":23},"updated":"2022-08-10 14:40:35.000000000","message":"(no action required): I really really, really hate having methods that \"create or delete\" objects since the caller is always very clear what operation we want to do. I know this behavior is all around the OVN mech driver codebase but IMHO it just makes code more complex, less readable and error prone. I\u0027d split this method into three methods: add_qos_rule, update_qos_rule and delete_qos rule even if some code snippets look alike. It will make the code more modular and will avoid \"if clauses\" that decide what action we actually meant to call (L160, L168) when we already knew what action we actually want (L279, L296).\n\nWhat do you think?","commit_id":"1071de73b31599933cf89fd53eb85c474d9d8dc7"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"8667c83b42b99e059cf9f4896a2708bde0324549","unresolved":false,"context_lines":[{"line_number":123,"context_line":""},{"line_number":124,"context_line":"        return match"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    def _apply_qos_rule(self, txn, rules, ovn_rule, port_id\u003dNone, lsp\u003dNone,"},{"line_number":127,"context_line":"                        may_exist\u003dTrue, if_exists\u003dTrue, port_deleted\u003dFalse):"},{"line_number":128,"context_line":"        \"\"\"Add or remove an OVN rule set from the OVN database."},{"line_number":129,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9101f2e8_229c1a4e","line":126,"range":{"start_line":126,"start_character":8,"end_line":126,"end_character":23},"in_reply_to":"fcf6f622_632633a8","updated":"2022-08-10 15:59:23.000000000","message":"I agree with this: be always explicit on what a method does. This method is \"hiding\" somehow what we are doing here.\n\nThis is how base PS was implemented. But the rationale of this method is to be able to create/delete QoS registers (for port max-BW and DSCP rules, and FIP and GW max-BW rules) and update the LSP.options to the the min-BW values. In other words, because we have two kind of rules this is why I\u0027m creating this method.\n\nBecause this applies only to ports (not GW ports nor FIPs), the change on those methods could be smaller. Let me try to refactor this again.","commit_id":"1071de73b31599933cf89fd53eb85c474d9d8dc7"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"04ab6c9b76755940da09577ba84ec102b02853de","unresolved":true,"context_lines":[{"line_number":305,"context_line":"            self._add_port_qos_rules(txn, port_id, network_id, qos_policy_id,"},{"line_number":306,"context_line":"                                     qos_rules, lsp\u003dlsp)"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"    def create_port(self, txn, port, lsp):"},{"line_number":309,"context_line":"        self.update_port(txn, port, None, reset\u003dTrue, lsp\u003dlsp)"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"    def delete_port(self, txn, port):"}],"source_content_type":"text/x-python","patch_set":8,"id":"211dae53_2784499f","line":308,"range":{"start_line":308,"start_character":37,"end_line":308,"end_character":40},"updated":"2022-08-10 14:40:35.000000000","message":"Should this default to None since this is \"public\" method?","commit_id":"1071de73b31599933cf89fd53eb85c474d9d8dc7"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"8667c83b42b99e059cf9f4896a2708bde0324549","unresolved":false,"context_lines":[{"line_number":305,"context_line":"            self._add_port_qos_rules(txn, port_id, network_id, qos_policy_id,"},{"line_number":306,"context_line":"                                     qos_rules, lsp\u003dlsp)"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"    def create_port(self, txn, port, lsp):"},{"line_number":309,"context_line":"        self.update_port(txn, port, None, reset\u003dTrue, lsp\u003dlsp)"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"    def delete_port(self, txn, port):"}],"source_content_type":"text/x-python","patch_set":8,"id":"f1b05924_c5928f32","line":308,"range":{"start_line":308,"start_character":37,"end_line":308,"end_character":40},"in_reply_to":"211dae53_2784499f","updated":"2022-08-10 15:59:23.000000000","message":"We always pass this value during the port creation. Actually this method is called once only (as it should be).","commit_id":"1071de73b31599933cf89fd53eb85c474d9d8dc7"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"6efeba910669942d746d13aff09a2d082100b98e","unresolved":true,"context_lines":[{"line_number":96,"context_line":"                if rule.direction \u003d\u003d constants.INGRESS_DIRECTION:"},{"line_number":97,"context_line":"                    LOG.warning(\u0027ML2/OVN QoS driver does not support minimum \u0027"},{"line_number":98,"context_line":"                                \u0027bandwidth rules enforcement with ingress \u0027"},{"line_number":99,"context_line":"                                \u0027direction\u0027)"},{"line_number":100,"context_line":"                else:"},{"line_number":101,"context_line":"                    r \u003d {rule.rule_type: {\u0027min_kbps\u0027: rule.min_kbps}}"},{"line_number":102,"context_line":"                    qos_rules[constants.EGRESS_DIRECTION].update(r)"}],"source_content_type":"text/x-python","patch_set":9,"id":"649b6ddc_ec462dd0","line":99,"updated":"2022-08-16 20:17:21.000000000","message":"nit: missing dot at the end. Also maybe it would be good to add info about policy id and/or rule id in that warning message.","commit_id":"cc136f73150f0f5a3e9e9c17d8c1125025a97adb"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"b793e5d92622095a1ecae4be90561f97c623f23f","unresolved":true,"context_lines":[{"line_number":96,"context_line":"                if rule.direction \u003d\u003d constants.INGRESS_DIRECTION:"},{"line_number":97,"context_line":"                    LOG.warning(\u0027ML2/OVN QoS driver does not support minimum \u0027"},{"line_number":98,"context_line":"                                \u0027bandwidth rules enforcement with ingress \u0027"},{"line_number":99,"context_line":"                                \u0027direction\u0027)"},{"line_number":100,"context_line":"                else:"},{"line_number":101,"context_line":"                    r \u003d {rule.rule_type: {\u0027min_kbps\u0027: rule.min_kbps}}"},{"line_number":102,"context_line":"                    qos_rules[constants.EGRESS_DIRECTION].update(r)"}],"source_content_type":"text/x-python","patch_set":9,"id":"2cf672ce_32848aa9","line":99,"in_reply_to":"54297cf9_b4deb496","updated":"2022-08-22 08:16:14.000000000","message":"And You forgot about it :D","commit_id":"cc136f73150f0f5a3e9e9c17d8c1125025a97adb"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"23d5aa5302f43d624cb79b2003877635ae3b89e0","unresolved":true,"context_lines":[{"line_number":96,"context_line":"                if rule.direction \u003d\u003d constants.INGRESS_DIRECTION:"},{"line_number":97,"context_line":"                    LOG.warning(\u0027ML2/OVN QoS driver does not support minimum \u0027"},{"line_number":98,"context_line":"                                \u0027bandwidth rules enforcement with ingress \u0027"},{"line_number":99,"context_line":"                                \u0027direction\u0027)"},{"line_number":100,"context_line":"                else:"},{"line_number":101,"context_line":"                    r \u003d {rule.rule_type: {\u0027min_kbps\u0027: rule.min_kbps}}"},{"line_number":102,"context_line":"                    qos_rules[constants.EGRESS_DIRECTION].update(r)"}],"source_content_type":"text/x-python","patch_set":9,"id":"54297cf9_b4deb496","line":99,"in_reply_to":"649b6ddc_ec462dd0","updated":"2022-08-17 11:43:29.000000000","message":"Right... if I need to respin, I\u0027ll add it for sure (if not, I\u0027ll push a follow-up patch indeed).","commit_id":"cc136f73150f0f5a3e9e9c17d8c1125025a97adb"}]}