)]}'
{"ovsdbapp/backend/ovs_idl/connection.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"1d2b46df5eb2f78d739b6e8af7a5f0136d8564d8","unresolved":true,"context_lines":[{"line_number":178,"context_line":"        else:"},{"line_number":179,"context_line":"            for table in helper_tables:"},{"line_number":180,"context_line":"                helper.register_table(table)"},{"line_number":181,"context_line":"        return cls(connection_string, helper, **kwargs)"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"    def post_connect(self):"},{"line_number":184,"context_line":"        \"\"\"Operations to execute after the Idl has connected to the server"}],"source_content_type":"text/x-python","patch_set":2,"id":"7b9fb1aa_8c784167","line":181,"range":{"start_line":181,"start_character":45,"end_line":181,"end_character":46},"updated":"2021-05-10 07:50:47.000000000","message":"Did you mean to also pass *args here?","commit_id":"35d0772a0028bb8baba4c5955b33e5274725c740"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"4649d248ffc923e985a05ffe1736a56e896f8a1b","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        else:"},{"line_number":179,"context_line":"            for table in helper_tables:"},{"line_number":180,"context_line":"                helper.register_table(table)"},{"line_number":181,"context_line":"        return cls(connection_string, helper, **kwargs)"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"    def post_connect(self):"},{"line_number":184,"context_line":"        \"\"\"Operations to execute after the Idl has connected to the server"}],"source_content_type":"text/x-python","patch_set":2,"id":"ddf0b219_25ebebee","line":181,"range":{"start_line":181,"start_character":45,"end_line":181,"end_character":46},"in_reply_to":"4fe811c0_5fe10e99","updated":"2021-05-11 19:04:52.000000000","message":"One of the reasons I didn\u0027t do this initially was that there were so many configuration options that could be passed and I could tell they were just adding more so I didn\u0027t want to keep updating to mirror the python-ovs API: register_column() and then cond_change() happened, etc.\n\nCapturing everything that you might want to configure ends up being a lot of arguments (tables, table w/ columns, table w/ conditions, etc.), so I just decided to go with \"each Idl can override this classmethod and do whatever they want and no super() calls\". It just seemed like there wasn\u0027t a lot of benefit to converting lines of code into lines of arguments. Best I came up with to handle all (existing) cases would be to pass something like:\n\n restrictions\u003d[\n     SchemaTable(\u0027Logical_Switch\u0027),\n     SchemaTable(\u0027Port_Group\u0027, columns\u003d[\u0027name\u0027, \u0027acls\u0027]),\n     SchemaTable(\u0027Chassis\u0027, conditions\u003d[[\u0027uuid\u0027, \u0027\u003d\u003d\u0027, \u0027$uuid\u0027]],\n ]\n\nwhich just looks a lot like calling register_table()/register_column()/cond_change() which is kind of why the SchemaHelper exists, to translate those calls into the underlying structures on the TableSchema.\n\nAnyway, that\u0027s enough of a history lesson. If it simplifies things, I can also see the argument for at least handling helper/tables which are fairly simple args here and leaving the more complex columns/conditions args for outside ovsdbapp subclasses.","commit_id":"35d0772a0028bb8baba4c5955b33e5274725c740"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"dad8752fc03c2471eca9a5a888b6e087d2589237","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        else:"},{"line_number":179,"context_line":"            for table in helper_tables:"},{"line_number":180,"context_line":"                helper.register_table(table)"},{"line_number":181,"context_line":"        return cls(connection_string, helper, **kwargs)"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"    def post_connect(self):"},{"line_number":184,"context_line":"        \"\"\"Operations to execute after the Idl has connected to the server"}],"source_content_type":"text/x-python","patch_set":2,"id":"ae0e5d44_18a1e372","line":181,"range":{"start_line":181,"start_character":45,"end_line":181,"end_character":46},"in_reply_to":"62f8cb7c_49ed985b","updated":"2021-06-11 19:35:26.000000000","message":"On the *args thing, I wouldn\u0027t worry about subclasses changing the signature. There\u0027s no case where we\u0027d ever call from_server(), a classmethod, and not know which one we were getting. Not that it hurts leaving *args in, it\u0027s just a nit/personal preference thing though. Doesn\u0027t really bother me leaving it in.","commit_id":"35d0772a0028bb8baba4c5955b33e5274725c740"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"d20a35b8cd08fcd614cebaeb24aee9c1637c3228","unresolved":true,"context_lines":[{"line_number":178,"context_line":"        else:"},{"line_number":179,"context_line":"            for table in helper_tables:"},{"line_number":180,"context_line":"                helper.register_table(table)"},{"line_number":181,"context_line":"        return cls(connection_string, helper, **kwargs)"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"    def post_connect(self):"},{"line_number":184,"context_line":"        \"\"\"Operations to execute after the Idl has connected to the server"}],"source_content_type":"text/x-python","patch_set":2,"id":"f2718ae2_4b7e74fd","line":181,"range":{"start_line":181,"start_character":45,"end_line":181,"end_character":46},"in_reply_to":"7b9fb1aa_8c784167","updated":"2021-05-10 16:04:31.000000000","message":"idl.Idl [1] accepts only two positional args, \"remote\" and \"schema_helper\". If that changes, we\u0027ll need to change this call.\n\n[1]https://github.com/openvswitch/ovs/blob/721488d4a5f497f5693e286cf6f1916804bce2eb/python/ovs/db/idl.py#L105-L106","commit_id":"35d0772a0028bb8baba4c5955b33e5274725c740"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"5954751fc24f5bb4f959eac4bad9846007c25649","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        else:"},{"line_number":179,"context_line":"            for table in helper_tables:"},{"line_number":180,"context_line":"                helper.register_table(table)"},{"line_number":181,"context_line":"        return cls(connection_string, helper, **kwargs)"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"    def post_connect(self):"},{"line_number":184,"context_line":"        \"\"\"Operations to execute after the Idl has connected to the server"}],"source_content_type":"text/x-python","patch_set":2,"id":"4fe811c0_5fe10e99","line":181,"range":{"start_line":181,"start_character":45,"end_line":181,"end_character":46},"in_reply_to":"bbfabadb_4b8f83c5","updated":"2021-05-11 16:24:46.000000000","message":"OK, now I remember why I did it: to make the method signature flexible enough for some particular implementations. For example: https://opendev.org/openstack/neutron/src/branch/master/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py#L638\n\n--\u003e this from_server implementation is passing 3 positional arguments instead of 2. With this proposal, this specific method won\u0027t override the signature.","commit_id":"35d0772a0028bb8baba4c5955b33e5274725c740"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"e57d3cecf9bc868998da55be1f17838ec4be9a60","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        else:"},{"line_number":179,"context_line":"            for table in helper_tables:"},{"line_number":180,"context_line":"                helper.register_table(table)"},{"line_number":181,"context_line":"        return cls(connection_string, helper, **kwargs)"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"    def post_connect(self):"},{"line_number":184,"context_line":"        \"\"\"Operations to execute after the Idl has connected to the server"}],"source_content_type":"text/x-python","patch_set":2,"id":"bbfabadb_4b8f83c5","line":181,"range":{"start_line":181,"start_character":45,"end_line":181,"end_character":46},"in_reply_to":"c0c914c1_d12dd63c","updated":"2021-05-11 16:18:19.000000000","message":"Done","commit_id":"35d0772a0028bb8baba4c5955b33e5274725c740"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"0d4e1c7f79732b8fceb9602794f1f77a50fd9d36","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        else:"},{"line_number":179,"context_line":"            for table in helper_tables:"},{"line_number":180,"context_line":"                helper.register_table(table)"},{"line_number":181,"context_line":"        return cls(connection_string, helper, **kwargs)"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"    def post_connect(self):"},{"line_number":184,"context_line":"        \"\"\"Operations to execute after the Idl has connected to the server"}],"source_content_type":"text/x-python","patch_set":2,"id":"62f8cb7c_49ed985b","line":181,"range":{"start_line":181,"start_character":45,"end_line":181,"end_character":46},"in_reply_to":"ddf0b219_25ebebee","updated":"2021-05-11 19:20:00.000000000","message":"note: the above restrictions example actually leaves out support for the \u0027readonly\u0027 argument for register_columns(). So yeah, that\u0027s why I just left all register_* up to non-ovsdbapp stuff.","commit_id":"35d0772a0028bb8baba4c5955b33e5274725c740"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"4b4972a34f6c9427ac646441f93e7639458361de","unresolved":true,"context_lines":[{"line_number":178,"context_line":"        else:"},{"line_number":179,"context_line":"            for table in helper_tables:"},{"line_number":180,"context_line":"                helper.register_table(table)"},{"line_number":181,"context_line":"        return cls(connection_string, helper, **kwargs)"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"    def post_connect(self):"},{"line_number":184,"context_line":"        \"\"\"Operations to execute after the Idl has connected to the server"}],"source_content_type":"text/x-python","patch_set":2,"id":"c0c914c1_d12dd63c","line":181,"range":{"start_line":181,"start_character":45,"end_line":181,"end_character":46},"in_reply_to":"f2718ae2_4b7e74fd","updated":"2021-05-11 07:52:01.000000000","message":"The perhaps let\u0027s not implement *args in from_server()? This way we could pass anything to from_server but it will never be used anywhere.","commit_id":"35d0772a0028bb8baba4c5955b33e5274725c740"}]}