)]}'
{"vif_plug_ovs/ovsdb/impl_vsctl.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4ee8ce7aa41938cc392409cee78ee7d131eaa029","unresolved":false,"context_lines":[{"line_number":111,"context_line":"                    ctxt.reraise \u003d False"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"class TransactionDBClient(Transaction):"},{"line_number":115,"context_line":"    def __init__(self, context, check_error\u003dFalse, log_errors\u003dTrue, opts\u003dNone):"},{"line_number":116,"context_line":"        super(TransactionDBClient, self).__init__("},{"line_number":117,"context_line":"            context, check_error\u003dcheck_error, log_errors\u003dlog_errors, opts\u003dopts)"},{"line_number":118,"context_line":"        self.opts \u003d [\u0027--format\u003djson\u0027]"},{"line_number":119,"context_line":"        if self.context.connection:"},{"line_number":120,"context_line":"            self.opts +\u003d [\u0027--db\u003d%s\u0027 % self.context.connection]"},{"line_number":121,"context_line":"        if opts:"},{"line_number":122,"context_line":"            self.opts +\u003d opts"},{"line_number":123,"context_line":"        self._shell_command \u003d \u0027ovsdb-client\u0027"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"class BaseCommand(ovsdb_api.Command):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_00825053","line":123,"range":{"start_line":114,"start_character":0,"end_line":123,"end_character":44},"updated":"2019-02-20 02:12:09.000000000","message":"do we really need to add another dependcy on ovsdb-client.\n\nlooking at how we used to do this before you intoduced the ovsdb api in https://github.com/openstack/os-vif/commit/7bfb01bcb151087d70df115ba01f789a8a7f161f\n\nwe did\n\n@privsep.vif_plug.entrypoint\ndef _ovs_supports_mtu_requests(timeout\u003dNone):\n    args \u003d [\u0027--columns\u003dmtu_request\u0027, \u0027list\u0027, \u0027interface\u0027]\n    _, error \u003d _ovs_vsctl(args, timeout\u003dtimeout)\n    if (error \u003d\u003d \u0027ovs-vsctl: Interface does not contain\u0027 +\n              \u0027 a column whose name matches \"mtu_request\"\u0027):\n            return False\nreturn True\n\nhttps://github.com/openstack/os-vif/blob/stable/queens/vif_plug_ovs/linux_net.py#L234-L241\n\nthat seams simpler and does not introduce an extra dependency.","commit_id":"95dde3780b76fab1b375abdc6eb4c2f00aa5513d"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"4e5af3c50ea09632a5d7c1976c5f598464885e18","unresolved":false,"context_lines":[{"line_number":111,"context_line":"                    ctxt.reraise \u003d False"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"class TransactionDBClient(Transaction):"},{"line_number":115,"context_line":"    def __init__(self, context, check_error\u003dFalse, log_errors\u003dTrue, opts\u003dNone):"},{"line_number":116,"context_line":"        super(TransactionDBClient, self).__init__("},{"line_number":117,"context_line":"            context, check_error\u003dcheck_error, log_errors\u003dlog_errors, opts\u003dopts)"},{"line_number":118,"context_line":"        self.opts \u003d [\u0027--format\u003djson\u0027]"},{"line_number":119,"context_line":"        if self.context.connection:"},{"line_number":120,"context_line":"            self.opts +\u003d [\u0027--db\u003d%s\u0027 % self.context.connection]"},{"line_number":121,"context_line":"        if opts:"},{"line_number":122,"context_line":"            self.opts +\u003d opts"},{"line_number":123,"context_line":"        self._shell_command \u003d \u0027ovsdb-client\u0027"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"class BaseCommand(ovsdb_api.Command):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_8822e200","line":123,"range":{"start_line":114,"start_character":0,"end_line":123,"end_character":44},"in_reply_to":"9fdfeff1_00825053","updated":"2019-02-20 08:51:27.000000000","message":"I understand your point here.\n\nMy reasons in favor to this are:\n- \"ovsdb-client\" CLI is always present in OVS distribution. We are not introducing a \"new\" dependency (yes, it\u0027s a new CLI but the dependency, OVS, is already installed)\n- With this implementation we are avoiding the problem of relying on an error message. This is prone to changes and future errors in our code.\n- \"get_table_columns\" can be used in future features if needed.\n- Native implementation is around! [1]\n\n[1] https://review.openstack.org/#/c/482226/","commit_id":"95dde3780b76fab1b375abdc6eb4c2f00aa5513d"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c54b36e4f9a3c59a37801857030383f366e302ba","unresolved":false,"context_lines":[{"line_number":111,"context_line":"                    ctxt.reraise \u003d False"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"class TransactionDBClient(Transaction):"},{"line_number":115,"context_line":"    def __init__(self, context, check_error\u003dFalse, log_errors\u003dTrue, opts\u003dNone):"},{"line_number":116,"context_line":"        super(TransactionDBClient, self).__init__("},{"line_number":117,"context_line":"            context, check_error\u003dcheck_error, log_errors\u003dlog_errors, opts\u003dopts)"},{"line_number":118,"context_line":"        self.opts \u003d [\u0027--format\u003djson\u0027]"},{"line_number":119,"context_line":"        if self.context.connection:"},{"line_number":120,"context_line":"            self.opts +\u003d [\u0027--db\u003d%s\u0027 % self.context.connection]"},{"line_number":121,"context_line":"        if opts:"},{"line_number":122,"context_line":"            self.opts +\u003d opts"},{"line_number":123,"context_line":"        self._shell_command \u003d \u0027ovsdb-client\u0027"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"class BaseCommand(ovsdb_api.Command):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_d92e7aaa","line":123,"range":{"start_line":114,"start_character":0,"end_line":123,"end_character":44},"in_reply_to":"9fdfeff1_8822e200","updated":"2019-02-20 11:46:01.000000000","message":"i +1\u0027d the code because i thought the code was fine but wanted to know why you did not handle it the way we had before.\n\nthe error message could change but it didnt for the 2-3 years we depended on it.\n\nmy hope would be that if we merge the native implementation in stien we can remove the cli backend in train or early U","commit_id":"95dde3780b76fab1b375abdc6eb4c2f00aa5513d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"35e3128dfd78591f928ec0e0def29583041c5930","unresolved":false,"context_lines":[{"line_number":243,"context_line":"                                                    log_errors\u003dFalse)"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"class OvsdbVsctl(ovsdb_api.API):"},{"line_number":247,"context_line":"    def __init__(self, context):"},{"line_number":248,"context_line":"        super(OvsdbVsctl, self).__init__()"},{"line_number":249,"context_line":"        self.context \u003d context"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_1d114dad","line":246,"range":{"start_line":246,"start_character":6,"end_line":246,"end_character":16},"updated":"2019-02-20 12:45:30.000000000","message":"So the name of the class is OvsdbVsctl, but you\u0027ve added a function that relies on a client that isn\u0027t ovs-vsctl... just seems to me that this is an indication of improper composition and naming.","commit_id":"95dde3780b76fab1b375abdc6eb4c2f00aa5513d"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"444f954e6d7743190fb6ff1192361faa2cb222b9","unresolved":false,"context_lines":[{"line_number":243,"context_line":"                                                    log_errors\u003dFalse)"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"class OvsdbVsctl(ovsdb_api.API):"},{"line_number":247,"context_line":"    def __init__(self, context):"},{"line_number":248,"context_line":"        super(OvsdbVsctl, self).__init__()"},{"line_number":249,"context_line":"        self.context \u003d context"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_d085d55a","line":246,"range":{"start_line":246,"start_character":6,"end_line":246,"end_character":16},"in_reply_to":"9fdfeff1_0326d069","updated":"2019-02-21 11:48:56.000000000","message":"Right. I\u0027ll create a new class for this new command.","commit_id":"95dde3780b76fab1b375abdc6eb4c2f00aa5513d"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ebaf51c861a13345af1ca596c74fa108265f2a30","unresolved":false,"context_lines":[{"line_number":243,"context_line":"                                                    log_errors\u003dFalse)"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"class OvsdbVsctl(ovsdb_api.API):"},{"line_number":247,"context_line":"    def __init__(self, context):"},{"line_number":248,"context_line":"        super(OvsdbVsctl, self).__init__()"},{"line_number":249,"context_line":"        self.context \u003d context"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_0326d069","line":246,"range":{"start_line":246,"start_character":6,"end_line":246,"end_character":16},"in_reply_to":"9fdfeff1_1d114dad","updated":"2019-02-20 14:07:43.000000000","message":"ah good point i did not see that.","commit_id":"95dde3780b76fab1b375abdc6eb4c2f00aa5513d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"43650d7ec9d626146b3d580c71224312461e6416","unresolved":false,"context_lines":[{"line_number":354,"context_line":"    def db_remove(self, table, record, column, *values, **keyvalues):"},{"line_number":355,"context_line":"        raise NotImplementedError()"},{"line_number":356,"context_line":""},{"line_number":357,"context_line":"    def has_table_columns(self, table, column):"},{"line_number":358,"context_line":"        try:"},{"line_number":359,"context_line":"            self.db_list(table, columns\u003d[column]).execute(check_error\u003dTrue)"},{"line_number":360,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fdfeff1_697c4472","line":357,"range":{"start_line":357,"start_character":24,"end_line":357,"end_character":25},"updated":"2019-02-21 14:00:13.000000000","message":"column or columns?\n\nYou could make the method accept a list of columns or you could change the method name to has_table_column(). Your choice :)","commit_id":"3c7589ff1b0cc899256b131acf41fc5312c054c1"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"405d098a575b348b563282ff9c60d714bbbea652","unresolved":false,"context_lines":[{"line_number":354,"context_line":"    def db_remove(self, table, record, column, *values, **keyvalues):"},{"line_number":355,"context_line":"        raise NotImplementedError()"},{"line_number":356,"context_line":""},{"line_number":357,"context_line":"    def has_table_columns(self, table, column):"},{"line_number":358,"context_line":"        try:"},{"line_number":359,"context_line":"            self.db_list(table, columns\u003d[column]).execute(check_error\u003dTrue)"},{"line_number":360,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fdfeff1_c9a210ae","line":357,"range":{"start_line":357,"start_character":24,"end_line":357,"end_character":25},"in_reply_to":"9fdfeff1_697c4472","updated":"2019-02-21 14:09:22.000000000","message":"Just one. I\u0027ll change this.","commit_id":"3c7589ff1b0cc899256b131acf41fc5312c054c1"}],"vif_plug_ovs/ovsdb/ovsdb_lib.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"35e3128dfd78591f928ec0e0def29583041c5930","unresolved":false,"context_lines":[{"line_number":31,"context_line":"        self.ovsdb \u003d ovsdb_api.get_instance(self)"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def _ovs_supports_mtu_requests(self):"},{"line_number":34,"context_line":"        return \u0027mtu_request\u0027 in self.ovsdb.get_table_columns(\u0027Interface\u0027)"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def _set_mtu_request(self, dev, mtu):"},{"line_number":37,"context_line":"        self.ovsdb.db_set(\u0027Interface\u0027, dev, (\u0027mtu_request\u0027, mtu)).execute()"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_3d77710b","line":34,"updated":"2019-02-20 12:45:30.000000000","message":"so, from what I gather, ovs-vsctl doesn\u0027t have the list-columns command but ovsdb-client does...\n\nI still don\u0027t see why we need to introduce the callout to ovsdb-client when we could just use the approach that previously was used, i.e.:\n\n try:\n     self.ovsdb.db_list(\u0027Interface\u0027, columns\u003d[\u0027mtu_request\u0027]).execute(\n         check_error\u003dTrue)\n     return True\n except Exception as e:\n     # Possibly log the exception...\n     return False","commit_id":"95dde3780b76fab1b375abdc6eb4c2f00aa5513d"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ebaf51c861a13345af1ca596c74fa108265f2a30","unresolved":false,"context_lines":[{"line_number":31,"context_line":"        self.ovsdb \u003d ovsdb_api.get_instance(self)"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def _ovs_supports_mtu_requests(self):"},{"line_number":34,"context_line":"        return \u0027mtu_request\u0027 in self.ovsdb.get_table_columns(\u0027Interface\u0027)"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def _set_mtu_request(self, dev, mtu):"},{"line_number":37,"context_line":"        self.ovsdb.db_set(\u0027Interface\u0027, dev, (\u0027mtu_request\u0027, mtu)).execute()"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_43ba98e1","line":34,"in_reply_to":"9fdfeff1_3d77710b","updated":"2019-02-20 14:07:43.000000000","message":"ya that is basically what we used to do \nbut we used to check the error message.\ni kind of prefer that approch too which is why i originally +1 instead of +2\n\nperhaps we shold have a has_table_column function that does that test.\n\nso this would just be\n return self.ovsdb.has_table_columns(\u0027Interface\u0027, \u0027mtu_request\u0027)","commit_id":"95dde3780b76fab1b375abdc6eb4c2f00aa5513d"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"444f954e6d7743190fb6ff1192361faa2cb222b9","unresolved":false,"context_lines":[{"line_number":31,"context_line":"        self.ovsdb \u003d ovsdb_api.get_instance(self)"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def _ovs_supports_mtu_requests(self):"},{"line_number":34,"context_line":"        return \u0027mtu_request\u0027 in self.ovsdb.get_table_columns(\u0027Interface\u0027)"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def _set_mtu_request(self, dev, mtu):"},{"line_number":37,"context_line":"        self.ovsdb.db_set(\u0027Interface\u0027, dev, (\u0027mtu_request\u0027, mtu)).execute()"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_b0d05155","line":34,"in_reply_to":"9fdfeff1_43ba98e1","updated":"2019-02-21 11:48:56.000000000","message":"OK, you both prefer the first approach. I\u0027ll change the implementation.","commit_id":"95dde3780b76fab1b375abdc6eb4c2f00aa5513d"}]}