)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"3a2fa5f1e28f9f1f928b27c9cb4dfd5000d19576","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Terry Wilson \u003ctwilson@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-10-19 16:06:22 -0500"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"[WIP] Make OVSDB native lib more customizable"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Moves non-Neutron-specific code out of impl_idl so outside"},{"line_number":10,"context_line":"projects don\u0027t have to import neutron-specific code that resides"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"fa6399be_562f973d","line":7,"updated":"2016-10-19 22:28:16.000000000","message":"This is just a proposal, since we\u0027ve been having trouble dealing with the ugliness of the API. Anyone who would need to do anything with schemas would just do it themselves in the idl factory function where they could create whatever subclass of the Idl they want with whatever columns they want. There might be prettier ways to do it, but this is what I could throw together in 10 minutes. :)","commit_id":"aa307582f0996c60ac0f16d059c8dcdec850eaaf"}],"neutron/agent/ovsdb/impl_idl.py":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"564e7b5166e4cd680cd44f7a78d8e5351d8ce9fb","unresolved":false,"context_lines":[{"line_number":37,"context_line":"    message \u003d _(\"Failed to add interfaces: %(ifaces)s\")"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"class Transaction(api.Transaction):"},{"line_number":41,"context_line":"    def __init__(self, api, ovsdb_connection, timeout,"},{"line_number":42,"context_line":"                 check_error\u003dFalse, log_errors\u003dTrue):"},{"line_number":43,"context_line":"        self.api \u003d api"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa6399be_b67c7321","side":"PARENT","line":40,"updated":"2016-10-19 22:34:41.000000000","message":"Moved to neutron/agent/ovsdb/native/transaction.py so that projects like OVN no longer have to import impl_idl which has tons of code they don\u0027t need.","commit_id":"b8e6803966005dbd8b0a097f430f687ded32b68b"}],"neutron/agent/ovsdb/native/connection.py":[{"author":{"_account_id":20229,"name":"Omer Anson","email":"omer.anson@toganetworks.com","username":"oanson"},"change_message_id":"f0a5b4756c95651164451df0bc548c6f30a33cfd","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        return self.alertin.fileno()"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"class Connection(object):"},{"line_number":53,"context_line":"    def __init__(self, timeout, idl_factory):"},{"line_number":54,"context_line":"        self.idl \u003d None"},{"line_number":55,"context_line":"        self.timeout \u003d timeout"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa6399be_b3f71ad2","line":52,"updated":"2016-10-20 07:42:34.000000000","message":"Maybe the API breakage can be avoided if a new connection class is created (e.g. ThinConnection) with the changes here, and mark Connection as deprecated? Then Connection could be removed later.","commit_id":"f25254084dec79a973e88b8b6102a10d42d149e2"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"0b5272ba637a0c6f68563e1a72b377696e98337f","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        return self.alertin.fileno()"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"class Connection(object):"},{"line_number":53,"context_line":"    def __init__(self, timeout, idl_factory):"},{"line_number":54,"context_line":"        self.idl \u003d None"},{"line_number":55,"context_line":"        self.timeout \u003d timeout"}],"source_content_type":"text/x-python","patch_set":2,"id":"da6895a0_c2ed1827","line":52,"in_reply_to":"fa6399be_6764cab8","updated":"2016-10-20 19:49:33.000000000","message":"Omer: an example of maintaining backward compatibility like you mentioned is here: https://review.openstack.org/#/c/389333/\n\nThoughts? Still not sure whether it is worth it. But it was easy enough to implement, I figured I\u0027d do it so we could look at it.","commit_id":"f25254084dec79a973e88b8b6102a10d42d149e2"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"71aaef64f2a02773980a7aace4e63a35442af351","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        return self.alertin.fileno()"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"class Connection(object):"},{"line_number":53,"context_line":"    def __init__(self, timeout, idl_factory):"},{"line_number":54,"context_line":"        self.idl \u003d None"},{"line_number":55,"context_line":"        self.timeout \u003d timeout"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa6399be_6764cab8","line":52,"in_reply_to":"fa6399be_b3f71ad2","updated":"2016-10-20 15:17:07.000000000","message":"The question is whether or not backward compatibility is needed or even desirable. If there are only 2 users of the library outside of neutron, collaborating and updating them all at the same time with Depends-On tags is pretty easy. And then we have less code and complexity to maintain in here.","commit_id":"f25254084dec79a973e88b8b6102a10d42d149e2"},{"author":{"_account_id":20229,"name":"Omer Anson","email":"omer.anson@toganetworks.com","username":"oanson"},"change_message_id":"eb80c47bbc0fd30561572b5d2c61e6ef29fe01a1","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        return self.alertin.fileno()"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"class Connection(object):"},{"line_number":53,"context_line":"    def __init__(self, timeout, idl_factory):"},{"line_number":54,"context_line":"        self.idl \u003d None"},{"line_number":55,"context_line":"        self.timeout \u003d timeout"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa6399be_195f07e6","line":52,"in_reply_to":"fa6399be_b3f71ad2","updated":"2016-10-20 08:48:29.000000000","message":"With a reference implementation for IDL factory, Connection could inherit from ThinConnection, the functionality won\u0027t change, and there\u0027ll be an in-code usage example.","commit_id":"f25254084dec79a973e88b8b6102a10d42d149e2"},{"author":{"_account_id":20229,"name":"Omer Anson","email":"omer.anson@toganetworks.com","username":"oanson"},"change_message_id":"f0a5b4756c95651164451df0bc548c6f30a33cfd","unresolved":false,"context_lines":[{"line_number":55,"context_line":"        self.timeout \u003d timeout"},{"line_number":56,"context_line":"        self.txns \u003d TransactionQueue(1)"},{"line_number":57,"context_line":"        self.lock \u003d threading.Lock()"},{"line_number":58,"context_line":"        self.idl_factory \u003d idl_factory"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    def start(self):"},{"line_number":61,"context_line":"        with self.lock:"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa6399be_d369ee8d","line":58,"range":{"start_line":58,"start_character":13,"end_line":58,"end_character":24},"updated":"2016-10-20 07:42:34.000000000","message":"I think a reference IDL factory implementation should also be provided, possibly containing the code removed from Connection.","commit_id":"f25254084dec79a973e88b8b6102a10d42d149e2"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"71aaef64f2a02773980a7aace4e63a35442af351","unresolved":false,"context_lines":[{"line_number":55,"context_line":"        self.timeout \u003d timeout"},{"line_number":56,"context_line":"        self.txns \u003d TransactionQueue(1)"},{"line_number":57,"context_line":"        self.lock \u003d threading.Lock()"},{"line_number":58,"context_line":"        self.idl_factory \u003d idl_factory"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    def start(self):"},{"line_number":61,"context_line":"        with self.lock:"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa6399be_4146d64f","line":58,"range":{"start_line":58,"start_character":13,"end_line":58,"end_character":24},"in_reply_to":"fa6399be_d369ee8d","updated":"2016-10-20 15:17:07.000000000","message":"It is in impl_idl.py. I moved it there since it is the \"neutron version\" (specifically because it makes reference to neutron config info now). I considered defining a factory class and breaking things up some, but with all of the dependencies (connection-\u003eidl-\u003e[schema filters]-\u003eschema) it\u0027s just hard to do anything that looks decent when trying to subclass. It\u0027s hard to beat a single user-defined function that just does:\n\n def idl_factory():\n     sh \u003d idlutils.get_schema_helper(conn, schema)\n     sh.register_table(\u0027table1\u0027)\n     sh.register_columns(\u0027table2\u0027, [\u0027col1\u0027, \u0027col2\u0027, \u0027col3\u0027])\n     return idl.Idl(conn, sh)\n\nor something more complex if you need. Any time I try to split it up, I get *something* I don\u0027t like. But I keep trying because I keep thinking \"What if someone just wants to override the class or just filter different columns?! It should be easy to separate!\". But especially if you want to support passing through whatever filtering options are actually available in the upstream lib, it just ends up being easier to call them directly.","commit_id":"f25254084dec79a973e88b8b6102a10d42d149e2"},{"author":{"_account_id":20229,"name":"Omer Anson","email":"omer.anson@toganetworks.com","username":"oanson"},"change_message_id":"9361c77946340a74d4c2039a615d919f23aeb87d","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        return self.alertin.fileno()"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"class Connection(object):"},{"line_number":53,"context_line":"    def __init__(self, timeout, idl_factory):"},{"line_number":54,"context_line":"        self.idl \u003d None"},{"line_number":55,"context_line":"        self.timeout \u003d timeout"}],"source_content_type":"text/x-python","patch_set":4,"id":"da6895a0_610c3843","line":52,"updated":"2016-10-21 09:03:59.000000000","message":"Continuing over from patch set 2.\n\nThat implementation looks good ( https://review.openstack.org/#/c/389333 ). I think this patch can stay as it is (up to class naming differences), but move that implementation of Connection here, so the API doesn\u0027t break, and clients have an in-code example (albeit temporary) of how IdlConnection can be used.\n\nWould there be a way to keep the old get_schema_helper implementation? I think the retry mechanism there would be of value to other clients too. Maybe it can be added to idlutils (e.g. get_schema_helper_with_retry(connection, schema_name)).\n\nI think keeping backwards compatibility (with deprecation, if necessary) is important. Having been on the side of a library client, the result is that your code breaks for no fault of your own, at the worst possible time (e.g. Friday, 17:00, and there\u0027s a demo in 10 minutes (: ). And this sort of breakage would require some understanding/investigation, e.g. realising that the code you were using was moved to a different file.","commit_id":"2372c7efb9d245264ea27263dba53d675f24c156"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"fa874d2b1276ef602c56fc1f1e762d9ce71f0a03","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        return self.alertin.fileno()"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"class Connection(object):"},{"line_number":53,"context_line":"    def __init__(self, timeout, idl_factory):"},{"line_number":54,"context_line":"        self.idl \u003d None"},{"line_number":55,"context_line":"        self.timeout \u003d timeout"}],"source_content_type":"text/x-python","patch_set":4,"id":"da6895a0_efdae177","line":52,"in_reply_to":"da6895a0_610c3843","updated":"2016-10-21 15:50:15.000000000","message":"idlutils seems like a good place for a retying get_schema_helper.\n\nre: backward compatibility, normally I\u0027d completely agree. It\u0027s just that we generally know the users of the library and all share a CI infrastructure and we can have cross-project dependencies on patches. So it\u0027s really easy to fix everything at the same time. The thing I don\u0027t like about the backward compatibility solution is that even if we deprecated the old Connection, we have renamed the Connection class to IdlConnection and can never go back to just having it be Connection (without yet another round of deprecation). It makes things cruft-y. If we could change everything at once to a new clean better thought out implementation, we\u0027d have nicer code to maintain.\n\nI was just looking at the dragonflow code and see that you actually subclass OvsdbIdl, so I guess having impl_idl.py as a place completely safe for neutron-specific changes isn\u0027t necessarily sound either. And I suppose OvsdbIdl is a bad name for the class. It\u0027s really just a collection of commands specifically for the Open_vSwitch schema. If this ever gets split out into a separate library, I\u0027m definitely moving things around based on what I\u0027ve learned since I wrote it. :)","commit_id":"2372c7efb9d245264ea27263dba53d675f24c156"},{"author":{"_account_id":20229,"name":"Omer Anson","email":"omer.anson@toganetworks.com","username":"oanson"},"change_message_id":"c37cb94b31b078959225b96b02d58d77932116e0","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        return self.alertin.fileno()"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"class Connection(object):"},{"line_number":53,"context_line":"    def __init__(self, timeout, idl_factory):"},{"line_number":54,"context_line":"        self.idl \u003d None"},{"line_number":55,"context_line":"        self.timeout \u003d timeout"}],"source_content_type":"text/x-python","patch_set":4,"id":"da6895a0_cd1d2011","line":52,"in_reply_to":"da6895a0_efdae177","updated":"2016-10-21 20:03:43.000000000","message":"re: backwards compatibility, if it\u0027s just Dragonflow and OVN, then we could do that. I can write a Dragonflow patch when we return from the summit to prepare for the merge of this patch.\n\nI see what you mean about OvsdbIdl. I just read the code :).\n\nWhere did you think to split the library out to? Neutron-lib? OVS? Standalone openstack or github project? If you do that, then Neutron could be changed just to use that code by providing the necessary parameters (e.g. connection, schema name). Dragonflow and OVN could do the same thing. (Is this what you meant by \u0027if this ever gets split out into a separate library\u0027?)\n\nThis will also mean that this code could be used without Neutron as a requirement. In Dragonflow, that\u0027s a plus, since the code runs in a controller on the compute node, which doesn\u0027t use any other Neutron features.","commit_id":"2372c7efb9d245264ea27263dba53d675f24c156"}],"neutron/agent/ovsdb/native/transaction.py":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"1f3a7012a2d59f9a244040a016910ace626dcf4e","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright (c) 2016 Red Hat, Inc."},{"line_number":2,"context_line":"#"},{"line_number":3,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":4,"context_line":"#    not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa6399be_76abbb89","line":1,"updated":"2016-10-19 22:33:07.000000000","message":"No logic changes here. Just moved.","commit_id":"aa307582f0996c60ac0f16d059c8dcdec850eaaf"}]}