)]}'
{"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":"e4b4668268776abbf383cd2743f3f60cc62ea3c6","unresolved":false,"context_lines":[{"line_number":125,"context_line":""},{"line_number":126,"context_line":"        port \u003d txn.insert(self.api._tables[\u0027Logical_Switch_Port\u0027])"},{"line_number":127,"context_line":"        port.name \u003d self.lport"},{"line_number":128,"context_line":"        port.tag \u003d self.columns.pop(\u0027tag\u0027, []) or []"},{"line_number":129,"context_line":"        dhcpv4_options \u003d self.columns.pop(\u0027dhcpv4_options\u0027, [])"},{"line_number":130,"context_line":"        if isinstance(dhcpv4_options, list):"},{"line_number":131,"context_line":"            port.dhcpv4_options \u003d dhcpv4_options"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_6e70a209","line":128,"updated":"2020-07-24 09:33:39.000000000","message":"Good catch.\n\nA couple of things.\n\n1) https://github.com/ovn-org/ovn/blob/1e07781310d8155997672bdce01a2ff4f5a93e83/ovn-nb.ovsschema#L85-L89\n\nThis parameter is an integer, not a list\n\n\n2) IMO, the command should not change the object values. That should be done properly in the methods calling this one. When creating the port object, getting the port options: https://github.com/openstack/neutron/blob/ccbebfeaccf657caf8bcf8d635e3032e4c51c658/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py#L233\n\n\nWhen a port can have a tag\u003dNone?","commit_id":"e2594453fd84be37d6ad8c24345f9a2eb8462cec"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"15f7b2167acf4097c7e942b699935dabee6d235e","unresolved":false,"context_lines":[{"line_number":125,"context_line":""},{"line_number":126,"context_line":"        port \u003d txn.insert(self.api._tables[\u0027Logical_Switch_Port\u0027])"},{"line_number":127,"context_line":"        port.name \u003d self.lport"},{"line_number":128,"context_line":"        port.tag \u003d self.columns.pop(\u0027tag\u0027, []) or []"},{"line_number":129,"context_line":"        dhcpv4_options \u003d self.columns.pop(\u0027dhcpv4_options\u0027, [])"},{"line_number":130,"context_line":"        if isinstance(dhcpv4_options, list):"},{"line_number":131,"context_line":"            port.dhcpv4_options \u003d dhcpv4_options"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_f7e5203a","line":128,"in_reply_to":"9f560f44_177a5487","updated":"2020-07-24 15:36:51.000000000","message":"TY Terry! I do not think I can add to what you said. \u003c3","commit_id":"e2594453fd84be37d6ad8c24345f9a2eb8462cec"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"31b37fbb8b28218c82aa8acb509abfc826036760","unresolved":false,"context_lines":[{"line_number":125,"context_line":""},{"line_number":126,"context_line":"        port \u003d txn.insert(self.api._tables[\u0027Logical_Switch_Port\u0027])"},{"line_number":127,"context_line":"        port.name \u003d self.lport"},{"line_number":128,"context_line":"        port.tag \u003d self.columns.pop(\u0027tag\u0027, []) or []"},{"line_number":129,"context_line":"        dhcpv4_options \u003d self.columns.pop(\u0027dhcpv4_options\u0027, [])"},{"line_number":130,"context_line":"        if isinstance(dhcpv4_options, list):"},{"line_number":131,"context_line":"            port.dhcpv4_options \u003d dhcpv4_options"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_57884c1e","line":128,"in_reply_to":"9f560f44_f7e5203a","updated":"2020-07-24 15:47:59.000000000","message":"Well, in this case of course this change makes sense. We don\u0027t need to take care of \"tag\" outside this method, we\u0027ll handle the None/unset status here.\n\nThanks for the clarification.\n\nAbout the ovs code, I didn\u0027t know the unspecified value was defined as an empty list. But as you said, this is in python-ovs, we won\u0027t change that here.\n\nGood to learn!","commit_id":"e2594453fd84be37d6ad8c24345f9a2eb8462cec"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"6d59a8ffa94df11efa65492e9ad582b28c700b00","unresolved":false,"context_lines":[{"line_number":125,"context_line":""},{"line_number":126,"context_line":"        port \u003d txn.insert(self.api._tables[\u0027Logical_Switch_Port\u0027])"},{"line_number":127,"context_line":"        port.name \u003d self.lport"},{"line_number":128,"context_line":"        port.tag \u003d self.columns.pop(\u0027tag\u0027, []) or []"},{"line_number":129,"context_line":"        dhcpv4_options \u003d self.columns.pop(\u0027dhcpv4_options\u0027, [])"},{"line_number":130,"context_line":"        if isinstance(dhcpv4_options, list):"},{"line_number":131,"context_line":"            port.dhcpv4_options \u003d dhcpv4_options"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_177a5487","line":128,"in_reply_to":"bf51134e_6e70a209","updated":"2020-07-24 15:32:59.000000000","message":"1) It is an *optional* integer (min:0, max:1), which is implemented in ovsdb as a set containing 0 or 1 items. It\u0027s weird. I hate it. :p\n\n2) This is basically doing what the underlying python-ovs code does: set an \"unspecified\" valued to the default value, which for this type is an empty set (list, because of other things I hate about python-ovs). We could do something like:\n\n if \u0027tag\u0027 in self.columns and self.colums.get(\u0027tag\u0027) is None:\n     del self.columns[\u0027tag\u0027]\n\nbut this will end up doing the same thing. As an example, the version of this command that is ovsdbapp (which we never got around to using) handles the case where tag\u003dNone: https://github.com/openstack/ovsdbapp/blob/master/ovsdbapp/schema/ovn_northbound/commands.py#L378 and this was pretty much a direct translation of how ovn-nbctl handles things: https://github.com/ovn-org/ovn/blob/master/utilities/ovn-nbctl.c#L1391 (with tag\u003d-1 similar to tag\u003dNone/unspecified).\n\n3) This I\u0027m not sure of, but Flavio mentioned in our chat that this was happening with a localnet port, so it might be in that case.","commit_id":"e2594453fd84be37d6ad8c24345f9a2eb8462cec"}]}
