)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"8062a82dd8d138189c7e26c31085d24a6aeb849f","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This patch adds \u0027unique_keys\u0027 to NeutronDbObject and allows get_object"},{"line_number":10,"context_line":"to lookup objects by unique keys in addition to primary keys."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Subsequent patch will make \u0027unique_keys\u0027 to be automatically generated"},{"line_number":13,"context_line":"from object\u0027s db_model constraints/column declarations by default."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: I31f5603c116892390d1f8025d3c1893355ad0bac"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7aa08908_7e03dd0d","line":12,"range":{"start_line":12,"start_character":0,"end_line":12,"end_character":16},"updated":"2016-06-07 08:41:11.000000000","message":"I think it\u0027s more appropriate to squash it into this patch.","commit_id":"04a26d547dcc4aa8e7cc718ba31fd778b36149f3"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"3c96030d141f2dc7d9b2118fe7fc9992345c5e29","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This patch adds \u0027unique_keys\u0027 to NeutronDbObject and allows get_object"},{"line_number":10,"context_line":"to lookup objects by unique keys in addition to primary keys."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Subsequent patch will make \u0027unique_keys\u0027 to be automatically generated"},{"line_number":13,"context_line":"from object\u0027s db_model constraints/column declarations by default."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: I31f5603c116892390d1f8025d3c1893355ad0bac"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7aa08908_2708212c","line":12,"range":{"start_line":12,"start_character":0,"end_line":12,"end_character":16},"in_reply_to":"7aa08908_7e03dd0d","updated":"2016-06-07 10:10:48.000000000","message":"Ok","commit_id":"04a26d547dcc4aa8e7cc718ba31fd778b36149f3"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"8062a82dd8d138189c7e26c31085d24a6aeb849f","unresolved":false,"context_lines":[{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Subsequent patch will make \u0027unique_keys\u0027 to be automatically generated"},{"line_number":13,"context_line":"from object\u0027s db_model constraints/column declarations by default."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: I31f5603c116892390d1f8025d3c1893355ad0bac"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7aa08908_3e1af56e","line":14,"updated":"2016-06-07 08:41:11.000000000","message":"Partially-implements: bp/adopt-oslo-versioned-objects-for-db","commit_id":"04a26d547dcc4aa8e7cc718ba31fd778b36149f3"}],"neutron/db/model_base.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"d22dc0e5a4409f67c2f503d65fdb92ec0f1bd433","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        constraints \u003d model.__table__.constraints"},{"line_number":185,"context_line":"    except AttributeError:"},{"line_number":186,"context_line":"        constraints \u003d []"},{"line_number":187,"context_line":"    return [[c.name for c in constraint.columns]"},{"line_number":188,"context_line":"            for constraint in constraints"},{"line_number":189,"context_line":"            if isinstance(constraint, sa.UniqueConstraint)]"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_6f1e000d","line":189,"range":{"start_line":187,"start_character":0,"end_line":189,"end_character":59},"updated":"2016-07-07 15:22:45.000000000","message":"This appears to return list of lists of column names while the usage seems to be returning column names only. Can you write unittest to verify its behavior?","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"07b99adf567cd9c95f9b53837ac4a8b8d8446bb7","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        constraints \u003d model.__table__.constraints"},{"line_number":185,"context_line":"    except AttributeError:"},{"line_number":186,"context_line":"        constraints \u003d []"},{"line_number":187,"context_line":"    return [[c.name for c in constraint.columns]"},{"line_number":188,"context_line":"            for constraint in constraints"},{"line_number":189,"context_line":"            if isinstance(constraint, sa.UniqueConstraint)]"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_f76c75d1","line":189,"range":{"start_line":187,"start_character":0,"end_line":189,"end_character":59},"in_reply_to":"1aa78d24_436b181e","updated":"2016-07-07 20:48:14.000000000","message":"Can you elaborate what do you want me to test here? SQLAlchemy API?","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"6da7926841ab7331978d5256426f4e3df27f87b4","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        constraints \u003d model.__table__.constraints"},{"line_number":185,"context_line":"    except AttributeError:"},{"line_number":186,"context_line":"        constraints \u003d []"},{"line_number":187,"context_line":"    return [[c.name for c in constraint.columns]"},{"line_number":188,"context_line":"            for constraint in constraints"},{"line_number":189,"context_line":"            if isinstance(constraint, sa.UniqueConstraint)]"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_cfc3b436","line":189,"range":{"start_line":187,"start_character":0,"end_line":189,"end_character":59},"in_reply_to":"1aa78d24_6f1e000d","updated":"2016-07-07 15:25:28.000000000","message":"I don\u0027t see where in base class we treat it as a list of strings and not a list of lists of strings. Can you elaborate?","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"1159f5f073274cd8568bd035e16553b7b7de5736","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        constraints \u003d model.__table__.constraints"},{"line_number":185,"context_line":"    except AttributeError:"},{"line_number":186,"context_line":"        constraints \u003d []"},{"line_number":187,"context_line":"    return [[c.name for c in constraint.columns]"},{"line_number":188,"context_line":"            for constraint in constraints"},{"line_number":189,"context_line":"            if isinstance(constraint, sa.UniqueConstraint)]"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_7496103f","line":189,"range":{"start_line":187,"start_character":0,"end_line":189,"end_character":59},"in_reply_to":"1aa78d24_77406e36","updated":"2016-07-12 13:28:50.000000000","message":"fixed","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"2c02e3487163770d2142f451f965038f503cf14f","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        constraints \u003d model.__table__.constraints"},{"line_number":185,"context_line":"    except AttributeError:"},{"line_number":186,"context_line":"        constraints \u003d []"},{"line_number":187,"context_line":"    return [[c.name for c in constraint.columns]"},{"line_number":188,"context_line":"            for constraint in constraints"},{"line_number":189,"context_line":"            if isinstance(constraint, sa.UniqueConstraint)]"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_436b181e","line":189,"range":{"start_line":187,"start_character":0,"end_line":189,"end_character":59},"in_reply_to":"1aa78d24_cfc3b436","updated":"2016-07-07 18:10:14.000000000","message":"My bad, sorry. Anyway, this function should be tested by UT.","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"5d5c6f947c9cc2d74bebe758a920a3cbb9b0641a","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        constraints \u003d model.__table__.constraints"},{"line_number":185,"context_line":"    except AttributeError:"},{"line_number":186,"context_line":"        constraints \u003d []"},{"line_number":187,"context_line":"    return [[c.name for c in constraint.columns]"},{"line_number":188,"context_line":"            for constraint in constraints"},{"line_number":189,"context_line":"            if isinstance(constraint, sa.UniqueConstraint)]"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_77406e36","line":189,"range":{"start_line":187,"start_character":0,"end_line":189,"end_character":59},"in_reply_to":"1aa78d24_f76c75d1","updated":"2016-07-08 09:37:26.000000000","message":"No, just the used algorithm so all cases are covered:\n1) model has no constraints -\u003e empty list is returned\n2) model has constraints -\u003e only UniqueConstraints are returned","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"}],"neutron/objects/base.py":[{"author":{"_account_id":15518,"name":"Artur Korzeniewski","email":"artur.korzen@wp.pl","username":"akorzeni"},"change_message_id":"15a96fd8b63e536a6c1b7f6dadcd8e9122e8597c","unresolved":false,"context_lines":[{"line_number":162,"context_line":"    primary_keys \u003d [\u0027id\u0027]"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    # unique_keys is a list of unique keys that can be used with get_object"},{"line_number":165,"context_line":"    # instead of primary_keys (e.g. [[\u0027key1\u0027], [\u0027key2a\u0027, \u0027key2b\u0027]])"},{"line_number":166,"context_line":"    unique_keys \u003d []"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    # this is a dict to store the association between the foreign key and the"}],"source_content_type":"text/x-python","patch_set":1,"id":"9abb7d3a_381cd612","line":165,"range":{"start_line":165,"start_character":37,"end_line":165,"end_character":65},"updated":"2016-05-30 13:46:59.000000000","message":"I can agree that list of lists can be used here, but just a simple list of keys can be enough.","commit_id":"14c37a55f537f6287e1ad46c69ab5c36e2865d84"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"8f5df13f366412618d116651925b463b22f749c5","unresolved":false,"context_lines":[{"line_number":162,"context_line":"    primary_keys \u003d [\u0027id\u0027]"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    # unique_keys is a list of unique keys that can be used with get_object"},{"line_number":165,"context_line":"    # instead of primary_keys (e.g. [[\u0027key1\u0027], [\u0027key2a\u0027, \u0027key2b\u0027]])"},{"line_number":166,"context_line":"    unique_keys \u003d []"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    # this is a dict to store the association between the foreign key and the"}],"source_content_type":"text/x-python","patch_set":1,"id":"9abb7d3a_0f17fced","line":165,"range":{"start_line":165,"start_character":37,"end_line":165,"end_character":65},"in_reply_to":"9abb7d3a_381cd612","updated":"2016-05-30 19:27:29.000000000","message":"A simple list would either cover multiple single-field unique keys or a single composite unique key. How do you suggest we cover multiple composite unique keys with a simple list? I don\u0027t think a pair of brackets is worth limiting the flexibility.","commit_id":"14c37a55f537f6287e1ad46c69ab5c36e2865d84"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"e3fe1e98a95edbc5c55bde05a5e7afe12dfae382","unresolved":false,"context_lines":[{"line_number":266,"context_line":"        return obj"},{"line_number":267,"context_line":""},{"line_number":268,"context_line":"    @classmethod"},{"line_number":269,"context_line":"    def get_object(cls, context, **kwargs):"},{"line_number":270,"context_line":"        \"\"\""},{"line_number":271,"context_line":"        Fetch object from DB and convert it to a versioned object."},{"line_number":272,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9abb7d3a_41d96756","line":269,"updated":"2016-06-01 16:24:13.000000000","message":"I think adding a new list of key is an overkill. What you really want is that the method doesn\u0027t raise if a primary key is not passed. How about adding a boolean check_primary_keys that is True by default?","commit_id":"420aca8562dc1d1e2045ca5bcd546fbcb381af0a"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"842396a87eb8349af07eece4e2f4d7bad84b71a9","unresolved":false,"context_lines":[{"line_number":266,"context_line":"        return obj"},{"line_number":267,"context_line":""},{"line_number":268,"context_line":"    @classmethod"},{"line_number":269,"context_line":"    def get_object(cls, context, **kwargs):"},{"line_number":270,"context_line":"        \"\"\""},{"line_number":271,"context_line":"        Fetch object from DB and convert it to a versioned object."},{"line_number":272,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9abb7d3a_76c8aa55","line":269,"in_reply_to":"9abb7d3a_41d96756","updated":"2016-06-01 17:32:33.000000000","message":"I\u0027d like it to be explicit. I think simply removing the safety checks would be prone to errors - \u0027unique_keys\u0027 is a part of the object\u0027s contract and is controlled by the object declaration, but the \u0027check_primary_keys\u0027 parameter would be controlled by the object\u0027s user and who knows what an unaware user could do with that.","commit_id":"420aca8562dc1d1e2045ca5bcd546fbcb381af0a"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"9088ae93f6a5c620564202846c353f72d4839360","unresolved":false,"context_lines":[{"line_number":266,"context_line":"        return obj"},{"line_number":267,"context_line":""},{"line_number":268,"context_line":"    @classmethod"},{"line_number":269,"context_line":"    def get_object(cls, context, **kwargs):"},{"line_number":270,"context_line":"        \"\"\""},{"line_number":271,"context_line":"        Fetch object from DB and convert it to a versioned object."},{"line_number":272,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7aa08908_6c04137e","line":269,"in_reply_to":"9abb7d3a_76c8aa55","updated":"2016-06-06 09:56:24.000000000","message":"@Rossella, \u0027unique_keys\u0027 value would be inherited from the \u0027db_model\u0027 by default (commit message updated). That is to be addressed by separate patch.","commit_id":"420aca8562dc1d1e2045ca5bcd546fbcb381af0a"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"8de2208c837e9f86265f7fbd26f34148eedfbafe","unresolved":false,"context_lines":[{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    # unique_keys is a list of unique keys that can be used with get_object"},{"line_number":188,"context_line":"    # instead of primary_keys (e.g. [[\u0027key1\u0027], [\u0027key2a\u0027, \u0027key2b\u0027]])"},{"line_number":189,"context_line":"    unique_keys \u003d []"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"    # this is a dict to store the association between the foreign key and the"},{"line_number":192,"context_line":"    # corresponding key in the main table, e.g. port extension have \u0027port_id\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_f8ade8e9","line":189,"updated":"2016-06-13 14:22:36.000000000","message":"While I appreciate the effort, what\u0027s the use case? The main reason for get_object() to exist is to serve API GET needs. And afaik API does not support multi-key GETs. Or do you think it\u0027s useful in some other places?","commit_id":"04a26d547dcc4aa8e7cc718ba31fd778b36149f3"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"c000107f971e6828350a32ee36ee8fe4d7c5e225","unresolved":false,"context_lines":[{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    # unique_keys is a list of unique keys that can be used with get_object"},{"line_number":188,"context_line":"    # instead of primary_keys (e.g. [[\u0027key1\u0027], [\u0027key2a\u0027, \u0027key2b\u0027]])"},{"line_number":189,"context_line":"    unique_keys \u003d []"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"    # this is a dict to store the association between the foreign key and the"},{"line_number":192,"context_line":"    # corresponding key in the main table, e.g. port extension have \u0027port_id\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_bd3ce493","line":189,"in_reply_to":"7aa08908_f8ade8e9","updated":"2016-06-13 18:56:52.000000000","message":"It comes from Trunk/SubPort data model and specifically [1] (but that part was removed from the final patch). At some point it seemed that we\u0027d need to lookup Trunk/SubPort just by \u0027port_id\u0027, but we could not do that with get_object as it is limited to only accepting the primary keys.\n\nMy understanding was that NeutronDbObject API is supposed to become the main Data-Access framework (i.e. fully replacing the SQLAlchemy/OsloDB queries/etc.). At least it seems to be heading in that direction considering the current state of this class and the blueprint statement [2]. So this patch was supposed to add more power to NeutronDbObject to allow  access by unique-keys without using that ugly \u0027get_objects()[0]\u0027 hack.\n\n[1] https://review.openstack.org/#/c/310410/13/neutron/objects/trunk.py@42\n[2] https://blueprints.launchpad.net/neutron/+spec/adopt-oslo-versioned-objects-for-db","commit_id":"04a26d547dcc4aa8e7cc718ba31fd778b36149f3"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"8062a82dd8d138189c7e26c31085d24a6aeb849f","unresolved":false,"context_lines":[{"line_number":275,"context_line":"        :return: single object of NeutronDbObject class"},{"line_number":276,"context_line":"        \"\"\""},{"line_number":277,"context_line":"        lookup_keys \u003d set(kwargs.keys())"},{"line_number":278,"context_line":"        for required_keys in [cls.primary_keys] + cls.unique_keys:"},{"line_number":279,"context_line":"            if lookup_keys.issuperset(required_keys):"},{"line_number":280,"context_line":"                break"},{"line_number":281,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_13dd72e0","line":278,"range":{"start_line":278,"start_character":29,"end_line":278,"end_character":65},"updated":"2016-06-07 08:41:11.000000000","message":"You can use itertools.chain() here","commit_id":"04a26d547dcc4aa8e7cc718ba31fd778b36149f3"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"fd9d995cab1d0217791ad82548b0a5d466db7451","unresolved":false,"context_lines":[{"line_number":275,"context_line":"        :return: single object of NeutronDbObject class"},{"line_number":276,"context_line":"        \"\"\""},{"line_number":277,"context_line":"        lookup_keys \u003d set(kwargs.keys())"},{"line_number":278,"context_line":"        for required_keys in [cls.primary_keys] + cls.unique_keys:"},{"line_number":279,"context_line":"            if lookup_keys.issuperset(required_keys):"},{"line_number":280,"context_line":"                break"},{"line_number":281,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_1f745366","line":278,"range":{"start_line":278,"start_character":29,"end_line":278,"end_character":65},"in_reply_to":"7aa08908_13dd72e0","updated":"2016-06-07 09:25:00.000000000","message":"I think something like that would be more readable:\n\n\nif lookup_keys.issuperset(cls.primary_keys) or lookup_keys.issuperset(cls.unique_keys):\n\n[lines 286-289]\n\nelse:\n    missing_keys \u003d set(cls.primary_keys).difference(lookup_keys)\n    raise NeutronPrimaryKeyMissing(object_class\u003dcls.__class__,\n                                           missing_keys\u003dmissing_keys)","commit_id":"04a26d547dcc4aa8e7cc718ba31fd778b36149f3"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0a5f6bfac68b1293bb8c509a4e1b1355b13f3f01","unresolved":false,"context_lines":[{"line_number":275,"context_line":"        :return: single object of NeutronDbObject class"},{"line_number":276,"context_line":"        \"\"\""},{"line_number":277,"context_line":"        lookup_keys \u003d set(kwargs.keys())"},{"line_number":278,"context_line":"        for required_keys in [cls.primary_keys] + cls.unique_keys:"},{"line_number":279,"context_line":"            if lookup_keys.issuperset(required_keys):"},{"line_number":280,"context_line":"                break"},{"line_number":281,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_d49f4d4e","line":278,"range":{"start_line":278,"start_character":29,"end_line":278,"end_character":65},"in_reply_to":"7aa08908_1f745366","updated":"2016-06-07 09:46:12.000000000","message":"Yep :) or:\n\n if not \u003cthe condition above\u003e:\n     missing_keys \u003d ...\n     raise ...","commit_id":"04a26d547dcc4aa8e7cc718ba31fd778b36149f3"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"3c96030d141f2dc7d9b2118fe7fc9992345c5e29","unresolved":false,"context_lines":[{"line_number":275,"context_line":"        :return: single object of NeutronDbObject class"},{"line_number":276,"context_line":"        \"\"\""},{"line_number":277,"context_line":"        lookup_keys \u003d set(kwargs.keys())"},{"line_number":278,"context_line":"        for required_keys in [cls.primary_keys] + cls.unique_keys:"},{"line_number":279,"context_line":"            if lookup_keys.issuperset(required_keys):"},{"line_number":280,"context_line":"                break"},{"line_number":281,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_67906944","line":278,"range":{"start_line":278,"start_character":29,"end_line":278,"end_character":65},"in_reply_to":"7aa08908_d49f4d4e","updated":"2016-06-07 10:10:48.000000000","message":"@Rossella, @Jakub, issuperset(cls.unique_keys) won\u0027t work because unique_keys is a list of lists, hence the \u0027for ... in\u0027. But I think I could fix it another way.","commit_id":"04a26d547dcc4aa8e7cc718ba31fd778b36149f3"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"fd9d995cab1d0217791ad82548b0a5d466db7451","unresolved":false,"context_lines":[{"line_number":280,"context_line":"                break"},{"line_number":281,"context_line":"        else:"},{"line_number":282,"context_line":"            missing_keys \u003d set(cls.primary_keys).difference(lookup_keys)"},{"line_number":283,"context_line":"            raise NeutronPrimaryKeyMissing(object_class\u003dcls.__class__,"},{"line_number":284,"context_line":"                                           missing_keys\u003dmissing_keys)"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"        with db_api.autonested_transaction(context.session):"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_5f225b01","line":283,"range":{"start_line":283,"start_character":18,"end_line":283,"end_character":42},"updated":"2016-06-07 09:25:00.000000000","message":"if we consider unique keys too we should probably rename this exception","commit_id":"04a26d547dcc4aa8e7cc718ba31fd778b36149f3"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"3c96030d141f2dc7d9b2118fe7fc9992345c5e29","unresolved":false,"context_lines":[{"line_number":280,"context_line":"                break"},{"line_number":281,"context_line":"        else:"},{"line_number":282,"context_line":"            missing_keys \u003d set(cls.primary_keys).difference(lookup_keys)"},{"line_number":283,"context_line":"            raise NeutronPrimaryKeyMissing(object_class\u003dcls.__class__,"},{"line_number":284,"context_line":"                                           missing_keys\u003dmissing_keys)"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"        with db_api.autonested_transaction(context.session):"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_4799ed1e","line":283,"range":{"start_line":283,"start_character":18,"end_line":283,"end_character":42},"in_reply_to":"7aa08908_5f225b01","updated":"2016-06-07 10:10:48.000000000","message":"If we do rename that exception, what do you suggest we put into it\u0027s message? What \u0027missing_keys\u0027 should it contain (from primary_key or from some unique key)?","commit_id":"04a26d547dcc4aa8e7cc718ba31fd778b36149f3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"67e2bc0b84c53954579f9e6f38e8e056b1e29502","unresolved":false,"context_lines":[{"line_number":219,"context_line":"            cls.unique_keys \u003d []"},{"line_number":220,"context_line":"            obj_field_names \u003d set(cls.fields.keys())"},{"line_number":221,"context_line":"            model_to_obj_translation \u003d {"},{"line_number":222,"context_line":"                v: k for (k, v) in six.iteritems(cls.fields_need_translation)}"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"            for model_unique_key in model_base.get_unique_keys(model):"},{"line_number":225,"context_line":"                obj_unique_key \u003d [model_to_obj_translation.get(key, key)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_b41661a5","line":222,"range":{"start_line":222,"start_character":35,"end_line":222,"end_character":48},"updated":"2016-07-02 08:19:46.000000000","message":"they say using six for that is never a good idea since it\u0027s a premature optimization, and we should just use .items() instead.","commit_id":"40aa1d5a8c16d0b2086b7be8013bfb344f64eef0"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"499078aadfb714d964dac28b3e81472671c930b3","unresolved":false,"context_lines":[{"line_number":219,"context_line":"            cls.unique_keys \u003d []"},{"line_number":220,"context_line":"            obj_field_names \u003d set(cls.fields.keys())"},{"line_number":221,"context_line":"            model_to_obj_translation \u003d {"},{"line_number":222,"context_line":"                v: k for (k, v) in six.iteritems(cls.fields_need_translation)}"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"            for model_unique_key in model_base.get_unique_keys(model):"},{"line_number":225,"context_line":"                obj_unique_key \u003d [model_to_obj_translation.get(key, key)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_4ae3eda3","line":222,"range":{"start_line":222,"start_character":35,"end_line":222,"end_character":48},"in_reply_to":"1aa78d24_b41661a5","updated":"2016-07-04 13:26:48.000000000","message":"fixed","commit_id":"40aa1d5a8c16d0b2086b7be8013bfb344f64eef0"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"2c02e3487163770d2142f451f965038f503cf14f","unresolved":false,"context_lines":[{"line_number":214,"context_line":"        cls.fields_no_update \u003d list(set(cls.fields_no_update))"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"        # generate unique_keys from the model"},{"line_number":217,"context_line":"        model \u003d getattr(cls, \u0027db_model\u0027, None)"},{"line_number":218,"context_line":"        if model and not getattr(cls, \u0027unique_keys\u0027, None):"},{"line_number":219,"context_line":"            cls.unique_keys \u003d []"},{"line_number":220,"context_line":"            obj_field_names \u003d set(cls.fields.keys())"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_63573458","line":217,"range":{"start_line":217,"start_character":16,"end_line":217,"end_character":23},"updated":"2016-07-07 18:10:14.000000000","message":"hasattr","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"07b99adf567cd9c95f9b53837ac4a8b8d8446bb7","unresolved":false,"context_lines":[{"line_number":214,"context_line":"        cls.fields_no_update \u003d list(set(cls.fields_no_update))"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"        # generate unique_keys from the model"},{"line_number":217,"context_line":"        model \u003d getattr(cls, \u0027db_model\u0027, None)"},{"line_number":218,"context_line":"        if model and not getattr(cls, \u0027unique_keys\u0027, None):"},{"line_number":219,"context_line":"            cls.unique_keys \u003d []"},{"line_number":220,"context_line":"            obj_field_names \u003d set(cls.fields.keys())"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_7c76aa06","line":217,"range":{"start_line":217,"start_character":16,"end_line":217,"end_character":23},"in_reply_to":"1aa78d24_63573458","updated":"2016-07-07 20:48:14.000000000","message":"Can you elaborate what do you mean? Do you want it to be\n\nif hasattr(cls, \u0027db_model\u0027) and getattr(cls, \u0027unique_keys\u0027, None):\n  model \u003d cls.db_model\n  ...\n\nI don\u0027t see how it is different from what it is now. What problem is it supposed to solve? Please note how \u0027getattr\u0027 is used with \u0027default\u003dNone\u0027 here.","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"d6ca5059be55971d0bb3458b0606c2ca8a722400","unresolved":false,"context_lines":[{"line_number":214,"context_line":"        cls.fields_no_update \u003d list(set(cls.fields_no_update))"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"        # generate unique_keys from the model"},{"line_number":217,"context_line":"        model \u003d getattr(cls, \u0027db_model\u0027, None)"},{"line_number":218,"context_line":"        if model and not getattr(cls, \u0027unique_keys\u0027, None):"},{"line_number":219,"context_line":"            cls.unique_keys \u003d []"},{"line_number":220,"context_line":"            obj_field_names \u003d set(cls.fields.keys())"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_7a0fae2e","line":217,"range":{"start_line":217,"start_character":16,"end_line":217,"end_character":23},"in_reply_to":"1aa78d24_77c34e8b","updated":"2016-07-08 11:19:50.000000000","message":"Please, see [1] ...\n\n[1] https://review.openstack.org/#/c/322024/7/neutron/objects/base.py@224","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"18322f18c457e873018f764cfc0a0d8b4f14124a","unresolved":false,"context_lines":[{"line_number":214,"context_line":"        cls.fields_no_update \u003d list(set(cls.fields_no_update))"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"        # generate unique_keys from the model"},{"line_number":217,"context_line":"        model \u003d getattr(cls, \u0027db_model\u0027, None)"},{"line_number":218,"context_line":"        if model and not getattr(cls, \u0027unique_keys\u0027, None):"},{"line_number":219,"context_line":"            cls.unique_keys \u003d []"},{"line_number":220,"context_line":"            obj_field_names \u003d set(cls.fields.keys())"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_daaad0d1","line":217,"range":{"start_line":217,"start_character":16,"end_line":217,"end_character":23},"in_reply_to":"1aa78d24_7a0fae2e","updated":"2016-07-11 12:19:44.000000000","message":"Whops, my search highligh must have missed that, sorry.","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"5d5c6f947c9cc2d74bebe758a920a3cbb9b0641a","unresolved":false,"context_lines":[{"line_number":214,"context_line":"        cls.fields_no_update \u003d list(set(cls.fields_no_update))"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"        # generate unique_keys from the model"},{"line_number":217,"context_line":"        model \u003d getattr(cls, \u0027db_model\u0027, None)"},{"line_number":218,"context_line":"        if model and not getattr(cls, \u0027unique_keys\u0027, None):"},{"line_number":219,"context_line":"            cls.unique_keys \u003d []"},{"line_number":220,"context_line":"            obj_field_names \u003d set(cls.fields.keys())"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_77c34e8b","line":217,"range":{"start_line":217,"start_character":16,"end_line":217,"end_character":23},"in_reply_to":"1aa78d24_7c76aa06","updated":"2016-07-08 09:37:26.000000000","message":"You don\u0027t use model object anywhere. If I understand the intention correctly, you want to apply the code below only on objects implementing db_model. Thus hasattr() makes imho more sense here.","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"2c02e3487163770d2142f451f965038f503cf14f","unresolved":false,"context_lines":[{"line_number":215,"context_line":""},{"line_number":216,"context_line":"        # generate unique_keys from the model"},{"line_number":217,"context_line":"        model \u003d getattr(cls, \u0027db_model\u0027, None)"},{"line_number":218,"context_line":"        if model and not getattr(cls, \u0027unique_keys\u0027, None):"},{"line_number":219,"context_line":"            cls.unique_keys \u003d []"},{"line_number":220,"context_line":"            obj_field_names \u003d set(cls.fields.keys())"},{"line_number":221,"context_line":"            model_to_obj_translation \u003d {"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_834860b4","line":218,"range":{"start_line":218,"start_character":25,"end_line":218,"end_character":32},"updated":"2016-07-07 18:10:14.000000000","message":"hasattr","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"5d5c6f947c9cc2d74bebe758a920a3cbb9b0641a","unresolved":false,"context_lines":[{"line_number":215,"context_line":""},{"line_number":216,"context_line":"        # generate unique_keys from the model"},{"line_number":217,"context_line":"        model \u003d getattr(cls, \u0027db_model\u0027, None)"},{"line_number":218,"context_line":"        if model and not getattr(cls, \u0027unique_keys\u0027, None):"},{"line_number":219,"context_line":"            cls.unique_keys \u003d []"},{"line_number":220,"context_line":"            obj_field_names \u003d set(cls.fields.keys())"},{"line_number":221,"context_line":"            model_to_obj_translation \u003d {"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_579292a0","line":218,"range":{"start_line":218,"start_character":25,"end_line":218,"end_character":32},"in_reply_to":"1aa78d24_3ccd4266","updated":"2016-07-08 09:37:26.000000000","message":"Aha, makes sense :)","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"07b99adf567cd9c95f9b53837ac4a8b8d8446bb7","unresolved":false,"context_lines":[{"line_number":215,"context_line":""},{"line_number":216,"context_line":"        # generate unique_keys from the model"},{"line_number":217,"context_line":"        model \u003d getattr(cls, \u0027db_model\u0027, None)"},{"line_number":218,"context_line":"        if model and not getattr(cls, \u0027unique_keys\u0027, None):"},{"line_number":219,"context_line":"            cls.unique_keys \u003d []"},{"line_number":220,"context_line":"            obj_field_names \u003d set(cls.fields.keys())"},{"line_number":221,"context_line":"            model_to_obj_translation \u003d {"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_3ccd4266","line":218,"range":{"start_line":218,"start_character":25,"end_line":218,"end_character":32},"in_reply_to":"1aa78d24_834860b4","updated":"2016-07-07 20:48:14.000000000","message":"No. This also checks for a default \u0027unique_keys\u0027 that is an empty list.","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"2c02e3487163770d2142f451f965038f503cf14f","unresolved":false,"context_lines":[{"line_number":219,"context_line":"            cls.unique_keys \u003d []"},{"line_number":220,"context_line":"            obj_field_names \u003d set(cls.fields.keys())"},{"line_number":221,"context_line":"            model_to_obj_translation \u003d {"},{"line_number":222,"context_line":"                v: k for (k, v) in cls.fields_need_translation.items()}"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"            for model_unique_key in model_base.get_unique_keys(model):"},{"line_number":225,"context_line":"                obj_unique_key \u003d [model_to_obj_translation.get(key, key)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_a3e47cab","line":222,"range":{"start_line":222,"start_character":25,"end_line":222,"end_character":26},"updated":"2016-07-07 18:10:14.000000000","message":"Parenthesis are not needed","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"07b99adf567cd9c95f9b53837ac4a8b8d8446bb7","unresolved":false,"context_lines":[{"line_number":219,"context_line":"            cls.unique_keys \u003d []"},{"line_number":220,"context_line":"            obj_field_names \u003d set(cls.fields.keys())"},{"line_number":221,"context_line":"            model_to_obj_translation \u003d {"},{"line_number":222,"context_line":"                v: k for (k, v) in cls.fields_need_translation.items()}"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"            for model_unique_key in model_base.get_unique_keys(model):"},{"line_number":225,"context_line":"                obj_unique_key \u003d [model_to_obj_translation.get(key, key)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_f73915f3","line":222,"range":{"start_line":222,"start_character":25,"end_line":222,"end_character":26},"in_reply_to":"1aa78d24_a3e47cab","updated":"2016-07-07 20:48:14.000000000","message":"Isn\u0027t it a matter of taste?","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"5d5c6f947c9cc2d74bebe758a920a3cbb9b0641a","unresolved":false,"context_lines":[{"line_number":219,"context_line":"            cls.unique_keys \u003d []"},{"line_number":220,"context_line":"            obj_field_names \u003d set(cls.fields.keys())"},{"line_number":221,"context_line":"            model_to_obj_translation \u003d {"},{"line_number":222,"context_line":"                v: k for (k, v) in cls.fields_need_translation.items()}"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"            for model_unique_key in model_base.get_unique_keys(model):"},{"line_number":225,"context_line":"                obj_unique_key \u003d [model_to_obj_translation.get(key, key)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_5747321d","line":222,"range":{"start_line":222,"start_character":25,"end_line":222,"end_character":26},"in_reply_to":"1aa78d24_f73915f3","updated":"2016-07-08 09:37:26.000000000","message":"It\u0027s a nit :) Generally, we should use parenthesis only when they are needed.","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"}],"neutron/tests/unit/objects/test_base.py":[{"author":{"_account_id":15518,"name":"Artur Korzeniewski","email":"artur.korzen@wp.pl","username":"akorzeni"},"change_message_id":"15a96fd8b63e536a6c1b7f6dadcd8e9122e8597c","unresolved":false,"context_lines":[{"line_number":160,"context_line":"                                            nullable\u003dTrue)"},{"line_number":161,"context_line":"    }"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    primary_keys \u003d [\u0027unique_key\u0027, \u0027id\u0027]"},{"line_number":164,"context_line":"    unique_keys \u003d [[\u0027unique_key\u0027], [\u0027id\u0027]]"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"    synthetic_fields \u003d [\u0027obj_field\u0027]"},{"line_number":167,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9abb7d3a_9b4fbcbc","line":164,"range":{"start_line":163,"start_character":4,"end_line":164,"end_character":42},"updated":"2016-05-30 13:46:59.000000000","message":"does it make any sense to test it that way?\nI would use:\nprimary_keys \u003d [\u0027id\u0027]\nunique_keys \u003d [[\u0027unique_key\u0027]]","commit_id":"14c37a55f537f6287e1ad46c69ab5c36e2865d84"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"2d1bd9589ee5ed06e885b9d03e64e904a618c711","unresolved":false,"context_lines":[{"line_number":160,"context_line":"                                            nullable\u003dTrue)"},{"line_number":161,"context_line":"    }"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    primary_keys \u003d [\u0027unique_key\u0027, \u0027id\u0027]"},{"line_number":164,"context_line":"    unique_keys \u003d [[\u0027unique_key\u0027], [\u0027id\u0027]]"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"    synthetic_fields \u003d [\u0027obj_field\u0027]"},{"line_number":167,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9abb7d3a_801a727b","line":164,"range":{"start_line":163,"start_character":4,"end_line":164,"end_character":42},"in_reply_to":"9abb7d3a_23000701","updated":"2016-05-31 14:24:22.000000000","message":"fixed","commit_id":"14c37a55f537f6287e1ad46c69ab5c36e2865d84"},{"author":{"_account_id":15518,"name":"Artur Korzeniewski","email":"artur.korzen@wp.pl","username":"akorzeni"},"change_message_id":"9058bf50e2e6121dc8797fc5726a62b657dd43f4","unresolved":false,"context_lines":[{"line_number":160,"context_line":"                                            nullable\u003dTrue)"},{"line_number":161,"context_line":"    }"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    primary_keys \u003d [\u0027unique_key\u0027, \u0027id\u0027]"},{"line_number":164,"context_line":"    unique_keys \u003d [[\u0027unique_key\u0027], [\u0027id\u0027]]"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"    synthetic_fields \u003d [\u0027obj_field\u0027]"},{"line_number":167,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9abb7d3a_23000701","line":164,"range":{"start_line":163,"start_character":4,"end_line":164,"end_character":42},"in_reply_to":"9abb7d3a_6f64c821","updated":"2016-05-31 09:01:50.000000000","message":"Yes, it looks better.","commit_id":"14c37a55f537f6287e1ad46c69ab5c36e2865d84"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"8f5df13f366412618d116651925b463b22f749c5","unresolved":false,"context_lines":[{"line_number":160,"context_line":"                                            nullable\u003dTrue)"},{"line_number":161,"context_line":"    }"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    primary_keys \u003d [\u0027unique_key\u0027, \u0027id\u0027]"},{"line_number":164,"context_line":"    unique_keys \u003d [[\u0027unique_key\u0027], [\u0027id\u0027]]"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"    synthetic_fields \u003d [\u0027obj_field\u0027]"},{"line_number":167,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9abb7d3a_6f64c821","line":164,"range":{"start_line":163,"start_character":4,"end_line":164,"end_character":42},"in_reply_to":"9abb7d3a_9b4fbcbc","updated":"2016-05-30 19:27:29.000000000","message":"I\u0027d like to keep \u0027primary_key\u0027 as composite key as it covers more cases (e.g. the effect this change has on \u0027test_get_object_missing_primary_keys\u0027 would only show up with the composite primary key). I could substitute it with the following, if you think that would make more sense:\n\n    primary_keys \u003d [\u0027id1\u0027, \u0027id2\u0027]\n    unique_keys \u003d [[\u0027unique_key\u0027], [\u0027id2\u0027]]","commit_id":"14c37a55f537f6287e1ad46c69ab5c36e2865d84"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"67e2bc0b84c53954579f9e6f38e8e056b1e29502","unresolved":false,"context_lines":[{"line_number":363,"context_line":"            self.assertIsNone(obj)"},{"line_number":364,"context_line":""},{"line_number":365,"context_line":"    def test_get_object_missing_primary_key(self):"},{"line_number":366,"context_line":"        obj_keys \u003d self.generate_object_keys(self._test_class)"},{"line_number":367,"context_line":"        obj_keys.popitem()"},{"line_number":368,"context_line":"        self.assertRaises(base.NeutronPrimaryKeyMissing,"},{"line_number":369,"context_line":"                          self._test_class.get_object,"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_74f9994d","side":"PARENT","line":366,"updated":"2016-07-02 08:19:46.000000000","message":"Why do you need to remove it from here? Seems like you effectively reduce test coverage with that, because we now don\u0027t validate that passing SOME of keys still ends with an error.","commit_id":"78dc79146f0c9714a3313c9d340b3cf5102c538a"},{"author":{"_account_id":15518,"name":"Artur Korzeniewski","email":"artur.korzen@wp.pl","username":"akorzeni"},"change_message_id":"48d93de410ea48b4e952a00777b0b0f0226aa070","unresolved":false,"context_lines":[{"line_number":363,"context_line":"            self.assertIsNone(obj)"},{"line_number":364,"context_line":""},{"line_number":365,"context_line":"    def test_get_object_missing_primary_key(self):"},{"line_number":366,"context_line":"        obj_keys \u003d self.generate_object_keys(self._test_class)"},{"line_number":367,"context_line":"        obj_keys.popitem()"},{"line_number":368,"context_line":"        self.assertRaises(base.NeutronPrimaryKeyMissing,"},{"line_number":369,"context_line":"                          self._test_class.get_object,"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_3ed51143","side":"PARENT","line":366,"in_reply_to":"1aa78d24_74f9994d","updated":"2016-07-04 09:05:06.000000000","message":"Agree with Ihar, this reduce the code coverage.","commit_id":"78dc79146f0c9714a3313c9d340b3cf5102c538a"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"11df42901ef4913d86ed6b605a4dc1e91e71bd9d","unresolved":false,"context_lines":[{"line_number":363,"context_line":"            self.assertIsNone(obj)"},{"line_number":364,"context_line":""},{"line_number":365,"context_line":"    def test_get_object_missing_primary_key(self):"},{"line_number":366,"context_line":"        obj_keys \u003d self.generate_object_keys(self._test_class)"},{"line_number":367,"context_line":"        obj_keys.popitem()"},{"line_number":368,"context_line":"        self.assertRaises(base.NeutronPrimaryKeyMissing,"},{"line_number":369,"context_line":"                          self._test_class.get_object,"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_c9cb7971","side":"PARENT","line":366,"in_reply_to":"1aa78d24_74f9994d","updated":"2016-07-04 09:31:53.000000000","message":"In terms of coverage there\u0027s no difference if we pass some keys or none at all (as long as we do not have special casing in \u0027get_object\u0027 just for that).\n\nIn my first attempt to fix this test I\u0027ve made \u0027obj_keys.popitem\u0027 aware of the unique keys, but it ended up being a bit messy. And for some cases it led to an empty \u0027obj_keys\u0027 being passed to \u0027get_object\u0027. So if we pass an empty \u0027obj_keys\u0027 for some test classes, why not do it for all of them as it has the same test coverage and leads to a cleaner test code?","commit_id":"78dc79146f0c9714a3313c9d340b3cf5102c538a"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"499078aadfb714d964dac28b3e81472671c930b3","unresolved":false,"context_lines":[{"line_number":363,"context_line":"            self.assertIsNone(obj)"},{"line_number":364,"context_line":""},{"line_number":365,"context_line":"    def test_get_object_missing_primary_key(self):"},{"line_number":366,"context_line":"        obj_keys \u003d self.generate_object_keys(self._test_class)"},{"line_number":367,"context_line":"        obj_keys.popitem()"},{"line_number":368,"context_line":"        self.assertRaises(base.NeutronPrimaryKeyMissing,"},{"line_number":369,"context_line":"                          self._test_class.get_object,"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_353242f2","side":"PARENT","line":366,"in_reply_to":"1aa78d24_c49360a2","updated":"2016-07-04 13:26:48.000000000","message":"In fact, it actually has better test coverage as with empty \u0027kwargs\u0027 it is guaranteed to iterate through all keys in [1].\n\n[1] https://review.openstack.org/#/c/322024/7/neutron/objects/base.py@352","commit_id":"78dc79146f0c9714a3313c9d340b3cf5102c538a"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"ca8ff493526b4bfdb1c59c924e09f339a1c8fdfd","unresolved":false,"context_lines":[{"line_number":363,"context_line":"            self.assertIsNone(obj)"},{"line_number":364,"context_line":""},{"line_number":365,"context_line":"    def test_get_object_missing_primary_key(self):"},{"line_number":366,"context_line":"        obj_keys \u003d self.generate_object_keys(self._test_class)"},{"line_number":367,"context_line":"        obj_keys.popitem()"},{"line_number":368,"context_line":"        self.assertRaises(base.NeutronPrimaryKeyMissing,"},{"line_number":369,"context_line":"                          self._test_class.get_object,"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_c49360a2","side":"PARENT","line":366,"in_reply_to":"1aa78d24_c9cb7971","updated":"2016-07-04 09:33:30.000000000","message":"@Artur, it does not reduce the code coverage, at least not with the current \u0027get_object\u0027 implementation.","commit_id":"78dc79146f0c9714a3313c9d340b3cf5102c538a"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"67e2bc0b84c53954579f9e6f38e8e056b1e29502","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        pass"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"class UniqueKeysModel(model_base.BASEV2, model_base.HasId):"},{"line_number":60,"context_line":"    field1 \u003d sa.Column(sa.String(), unique\u003dTrue)"},{"line_number":61,"context_line":"    field2 \u003d sa.Column(sa.String())"},{"line_number":62,"context_line":"    field3 \u003d sa.Column(sa.String())"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_34d8b1ef","line":59,"range":{"start_line":59,"start_character":22,"end_line":59,"end_character":39},"updated":"2016-07-02 08:19:46.000000000","message":"afaik that will effectively modify database metadata used in all other tests executed by the thread. Are we ok with that?","commit_id":"40aa1d5a8c16d0b2086b7be8013bfb344f64eef0"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"11df42901ef4913d86ed6b605a4dc1e91e71bd9d","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        pass"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"class UniqueKeysModel(model_base.BASEV2, model_base.HasId):"},{"line_number":60,"context_line":"    field1 \u003d sa.Column(sa.String(), unique\u003dTrue)"},{"line_number":61,"context_line":"    field2 \u003d sa.Column(sa.String())"},{"line_number":62,"context_line":"    field3 \u003d sa.Column(sa.String())"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_64ff0c02","line":59,"range":{"start_line":59,"start_character":22,"end_line":59,"end_character":39},"in_reply_to":"1aa78d24_34d8b1ef","updated":"2016-07-04 09:31:53.000000000","message":"I don\u0027t like it either. And I think I\u0027ve got an idea how to fix that.","commit_id":"40aa1d5a8c16d0b2086b7be8013bfb344f64eef0"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"499078aadfb714d964dac28b3e81472671c930b3","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        pass"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"class UniqueKeysModel(model_base.BASEV2, model_base.HasId):"},{"line_number":60,"context_line":"    field1 \u003d sa.Column(sa.String(), unique\u003dTrue)"},{"line_number":61,"context_line":"    field2 \u003d sa.Column(sa.String())"},{"line_number":62,"context_line":"    field3 \u003d sa.Column(sa.String())"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_0a00c533","line":59,"range":{"start_line":59,"start_character":22,"end_line":59,"end_character":39},"in_reply_to":"1aa78d24_64ff0c02","updated":"2016-07-04 13:26:48.000000000","message":"fixed [1]\n\n[1] https://review.openstack.org/#/c/322024/7/neutron/tests/unit/objects/test_base.py@701","commit_id":"40aa1d5a8c16d0b2086b7be8013bfb344f64eef0"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"67e2bc0b84c53954579f9e6f38e8e056b1e29502","unresolved":false,"context_lines":[{"line_number":168,"context_line":""},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"@obj_base.VersionedObjectRegistry.register_if(False)"},{"line_number":171,"context_line":"class FakeNeutronObjectUniqueKeys(base.NeutronDbObject):"},{"line_number":172,"context_line":"    # Version 1.0: Initial version"},{"line_number":173,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":174,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_94e525aa","line":171,"updated":"2016-07-02 08:19:46.000000000","message":"Maybe also add a test class to validate that renamed fields also work fine.","commit_id":"40aa1d5a8c16d0b2086b7be8013bfb344f64eef0"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"499078aadfb714d964dac28b3e81472671c930b3","unresolved":false,"context_lines":[{"line_number":168,"context_line":""},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"@obj_base.VersionedObjectRegistry.register_if(False)"},{"line_number":171,"context_line":"class FakeNeutronObjectUniqueKeys(base.NeutronDbObject):"},{"line_number":172,"context_line":"    # Version 1.0: Initial version"},{"line_number":173,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":174,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_4a7f6daa","line":171,"in_reply_to":"1aa78d24_691245b6","updated":"2016-07-04 13:26:48.000000000","message":"fixed [1]\n\n[1] https://review.openstack.org/#/c/322024/7/neutron/tests/unit/objects/test_base.py@719","commit_id":"40aa1d5a8c16d0b2086b7be8013bfb344f64eef0"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"11df42901ef4913d86ed6b605a4dc1e91e71bd9d","unresolved":false,"context_lines":[{"line_number":168,"context_line":""},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"@obj_base.VersionedObjectRegistry.register_if(False)"},{"line_number":171,"context_line":"class FakeNeutronObjectUniqueKeys(base.NeutronDbObject):"},{"line_number":172,"context_line":"    # Version 1.0: Initial version"},{"line_number":173,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":174,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_691245b6","line":171,"in_reply_to":"1aa78d24_94e525aa","updated":"2016-07-04 09:31:53.000000000","message":"Good point.\n\nThough I don\u0027t think it is worth creating a separate test class/case, we can do it in \u0027FakeNeutronObjectUniqueKeys\u0027.","commit_id":"40aa1d5a8c16d0b2086b7be8013bfb344f64eef0"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"2c02e3487163770d2142f451f965038f503cf14f","unresolved":false,"context_lines":[{"line_number":318,"context_line":""},{"line_number":319,"context_line":"    @classmethod"},{"line_number":320,"context_line":"    def generate_object_keys(cls, obj_cls, field_names\u003dNone):"},{"line_number":321,"context_line":"        if field_names is None:"},{"line_number":322,"context_line":"            field_names \u003d obj_cls.primary_keys"},{"line_number":323,"context_line":"        keys \u003d {}"},{"line_number":324,"context_line":"        for field in field_names:"},{"line_number":325,"context_line":"            field_obj \u003d obj_cls.fields[field]"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_a392bcf3","line":322,"range":{"start_line":321,"start_character":0,"end_line":322,"end_character":46},"updated":"2016-07-07 18:10:14.000000000","message":"field_names \u003d field_names or obj_cls.primary_keys","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"07b99adf567cd9c95f9b53837ac4a8b8d8446bb7","unresolved":false,"context_lines":[{"line_number":318,"context_line":""},{"line_number":319,"context_line":"    @classmethod"},{"line_number":320,"context_line":"    def generate_object_keys(cls, obj_cls, field_names\u003dNone):"},{"line_number":321,"context_line":"        if field_names is None:"},{"line_number":322,"context_line":"            field_names \u003d obj_cls.primary_keys"},{"line_number":323,"context_line":"        keys \u003d {}"},{"line_number":324,"context_line":"        for field in field_names:"},{"line_number":325,"context_line":"            field_obj \u003d obj_cls.fields[field]"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_77bd2501","line":322,"range":{"start_line":321,"start_character":0,"end_line":322,"end_character":46},"in_reply_to":"1aa78d24_a392bcf3","updated":"2016-07-07 20:48:14.000000000","message":"That\u0027s not equivalent. And anyway I\u0027d prefer \u0027if\u0027 over \u0027a or b\u0027.","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"2c02e3487163770d2142f451f965038f503cf14f","unresolved":false,"context_lines":[{"line_number":388,"context_line":"                    self.assertTrue(self._is_test_class(obj))"},{"line_number":389,"context_line":"                    self.assertEqual(self.obj_fields[0],"},{"line_number":390,"context_line":"                                     get_obj_db_fields(obj))"},{"line_number":391,"context_line":"                    get_object_mock.assert_called_once_with("},{"line_number":392,"context_line":"                        self.context, self._test_class.db_model,"},{"line_number":393,"context_line":"                        **self._test_class.modify_fields_to_db(obj_keys))"},{"line_number":394,"context_line":""},{"line_number":395,"context_line":"    def _get_synthetic_fields_get_objects_calls(self, db_objs):"},{"line_number":396,"context_line":"        mock_calls \u003d []"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_2311ac5d","line":393,"range":{"start_line":391,"start_character":0,"end_line":393,"end_character":73},"updated":"2016-07-07 18:10:14.000000000","message":"I think this test case aims on testing state and not interactions [1].\n\n[1] http://googletesting.blogspot.cz/2013/03/testing-on-toilet-testing-state-vs.html","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"07b99adf567cd9c95f9b53837ac4a8b8d8446bb7","unresolved":false,"context_lines":[{"line_number":388,"context_line":"                    self.assertTrue(self._is_test_class(obj))"},{"line_number":389,"context_line":"                    self.assertEqual(self.obj_fields[0],"},{"line_number":390,"context_line":"                                     get_obj_db_fields(obj))"},{"line_number":391,"context_line":"                    get_object_mock.assert_called_once_with("},{"line_number":392,"context_line":"                        self.context, self._test_class.db_model,"},{"line_number":393,"context_line":"                        **self._test_class.modify_fields_to_db(obj_keys))"},{"line_number":394,"context_line":""},{"line_number":395,"context_line":"    def _get_synthetic_fields_get_objects_calls(self, db_objs):"},{"line_number":396,"context_line":"        mock_calls \u003d []"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_97e251d3","line":393,"range":{"start_line":391,"start_character":0,"end_line":393,"end_character":73},"in_reply_to":"1aa78d24_2311ac5d","updated":"2016-07-07 20:48:14.000000000","message":"This test is copied from \u0027test_get_object\u0027 as-is (except for added loop). And I\u0027d like to keep this test consistent with \u0027test_get_object\u0027. If you think the \u0027test_get_object\u0027 test should be changed, we can certainly update it, but that\u0027s outside the scope of this patch.","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"5d5c6f947c9cc2d74bebe758a920a3cbb9b0641a","unresolved":false,"context_lines":[{"line_number":388,"context_line":"                    self.assertTrue(self._is_test_class(obj))"},{"line_number":389,"context_line":"                    self.assertEqual(self.obj_fields[0],"},{"line_number":390,"context_line":"                                     get_obj_db_fields(obj))"},{"line_number":391,"context_line":"                    get_object_mock.assert_called_once_with("},{"line_number":392,"context_line":"                        self.context, self._test_class.db_model,"},{"line_number":393,"context_line":"                        **self._test_class.modify_fields_to_db(obj_keys))"},{"line_number":394,"context_line":""},{"line_number":395,"context_line":"    def _get_synthetic_fields_get_objects_calls(self, db_objs):"},{"line_number":396,"context_line":"        mock_calls \u003d []"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_b7ac968d","line":393,"range":{"start_line":391,"start_character":0,"end_line":393,"end_character":73},"in_reply_to":"1aa78d24_97e251d3","updated":"2016-07-08 09:37:26.000000000","message":"I don\u0027t think it\u0027s outside of the scope. Part of this patch is implementing test. There is no real reason to keep same structure with other test cases. This assert doesn\u0027t validate anything, it only adds burden when you modify get_object(), e.g. if you add parameter, you need to change UT.","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"d6ca5059be55971d0bb3458b0606c2ca8a722400","unresolved":false,"context_lines":[{"line_number":388,"context_line":"                    self.assertTrue(self._is_test_class(obj))"},{"line_number":389,"context_line":"                    self.assertEqual(self.obj_fields[0],"},{"line_number":390,"context_line":"                                     get_obj_db_fields(obj))"},{"line_number":391,"context_line":"                    get_object_mock.assert_called_once_with("},{"line_number":392,"context_line":"                        self.context, self._test_class.db_model,"},{"line_number":393,"context_line":"                        **self._test_class.modify_fields_to_db(obj_keys))"},{"line_number":394,"context_line":""},{"line_number":395,"context_line":"    def _get_synthetic_fields_get_objects_calls(self, db_objs):"},{"line_number":396,"context_line":"        mock_calls \u003d []"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_fa1b9e7e","line":393,"range":{"start_line":391,"start_character":0,"end_line":393,"end_character":73},"in_reply_to":"1aa78d24_b7ac968d","updated":"2016-07-08 11:19:50.000000000","message":"The way it is tested makes perfect sense to me, considering how we mock the DB API\u0027s \u0027get_object\u0027 (\u0027get_object_mock\u0027).\n\nIf you think that \u0027test_get_object\u0027/\u0027test_get_object_unique_key\u0027 should be implemented in a different way, please create a new change and we can discuss it there. But as I said, that is outside the scope of this patch.","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"2c02e3487163770d2142f451f965038f503cf14f","unresolved":false,"context_lines":[{"line_number":691,"context_line":""},{"line_number":692,"context_line":"class BaseDbObjectUniqueKeysTestCase(BaseObjectIfaceTestCase):"},{"line_number":693,"context_line":""},{"line_number":694,"context_line":"    @classmethod"},{"line_number":695,"context_line":"    def setUpClass(cls):"},{"line_number":696,"context_line":"        m_get_unique_keys \u003d mock.patch.object(model_base, \u0027get_unique_keys\u0027)"},{"line_number":697,"context_line":"        with m_get_unique_keys as get_unique_keys:"},{"line_number":698,"context_line":"            get_unique_keys.return_value \u003d [[\u0027field1\u0027],"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_5ee00b29","line":695,"range":{"start_line":694,"start_character":0,"end_line":695,"end_character":18},"updated":"2016-07-07 18:10:14.000000000","message":"We don\u0027t use setUpClass - for such testing where this is needed we use fixtures.\n\n def setUp(self):\n\nshould do just fine.","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"d6ca5059be55971d0bb3458b0606c2ca8a722400","unresolved":false,"context_lines":[{"line_number":691,"context_line":""},{"line_number":692,"context_line":"class BaseDbObjectUniqueKeysTestCase(BaseObjectIfaceTestCase):"},{"line_number":693,"context_line":""},{"line_number":694,"context_line":"    @classmethod"},{"line_number":695,"context_line":"    def setUpClass(cls):"},{"line_number":696,"context_line":"        m_get_unique_keys \u003d mock.patch.object(model_base, \u0027get_unique_keys\u0027)"},{"line_number":697,"context_line":"        with m_get_unique_keys as get_unique_keys:"},{"line_number":698,"context_line":"            get_unique_keys.return_value \u003d [[\u0027field1\u0027],"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_50a45307","line":695,"range":{"start_line":694,"start_character":0,"end_line":695,"end_character":18},"in_reply_to":"1aa78d24_3208b428","updated":"2016-07-08 11:19:50.000000000","message":"Yes, I mean \u0027_test_class\u0027. It is not that \"I insist\" on setting class attribute. That is required by \u0027_BaseObjectTestCase\u0027.\n\n\u0027setUpClass\u0027 is part of the Python unittest [1] and it makes perfect sense to use it here because we are setting up a class. Using \u0027self.__class__\u0027 and \u0027setUp\u0027 looks like a workaround for a problem that we do not have (should we have problems with \u0027setUpClass\u0027, that would result in test failures, but the gates are green).\n\nI\u0027d like to hear others\u0027 opinions on that matter though.\n\n[1] https://docs.python.org/2.7/library/unittest.html#unittest.TestCase.setUpClass","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"18322f18c457e873018f764cfc0a0d8b4f14124a","unresolved":false,"context_lines":[{"line_number":691,"context_line":""},{"line_number":692,"context_line":"class BaseDbObjectUniqueKeysTestCase(BaseObjectIfaceTestCase):"},{"line_number":693,"context_line":""},{"line_number":694,"context_line":"    @classmethod"},{"line_number":695,"context_line":"    def setUpClass(cls):"},{"line_number":696,"context_line":"        m_get_unique_keys \u003d mock.patch.object(model_base, \u0027get_unique_keys\u0027)"},{"line_number":697,"context_line":"        with m_get_unique_keys as get_unique_keys:"},{"line_number":698,"context_line":"            get_unique_keys.return_value \u003d [[\u0027field1\u0027],"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_7a9bdcc9","line":695,"range":{"start_line":694,"start_character":0,"end_line":695,"end_character":18},"in_reply_to":"1aa78d24_50a45307","updated":"2016-07-11 12:19:44.000000000","message":"unittest module from Python is not used anywhere here, so whether it\u0027s implemented there or not is irrelevant. setUpClass is not officially supported by testtools, which is the library and runner we use for testing. (see https://github.com/openstack/neutron/blob/master/HACKING.rst#creating-unit-tests)","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"07b99adf567cd9c95f9b53837ac4a8b8d8446bb7","unresolved":false,"context_lines":[{"line_number":691,"context_line":""},{"line_number":692,"context_line":"class BaseDbObjectUniqueKeysTestCase(BaseObjectIfaceTestCase):"},{"line_number":693,"context_line":""},{"line_number":694,"context_line":"    @classmethod"},{"line_number":695,"context_line":"    def setUpClass(cls):"},{"line_number":696,"context_line":"        m_get_unique_keys \u003d mock.patch.object(model_base, \u0027get_unique_keys\u0027)"},{"line_number":697,"context_line":"        with m_get_unique_keys as get_unique_keys:"},{"line_number":698,"context_line":"            get_unique_keys.return_value \u003d [[\u0027field1\u0027],"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_d727b960","line":695,"range":{"start_line":694,"start_character":0,"end_line":695,"end_character":18},"in_reply_to":"1aa78d24_5ee00b29","updated":"2016-07-07 20:48:14.000000000","message":"I tried it with \u0027setUp\u0027, but \u0027db_model\u0027 has to be a class-level property and setting a class-level property in setUp just does not sit well with me (isn\u0027t \u0027setUpClass\u0027 supposed to be used exactly for that?).","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"1159f5f073274cd8568bd035e16553b7b7de5736","unresolved":false,"context_lines":[{"line_number":691,"context_line":""},{"line_number":692,"context_line":"class BaseDbObjectUniqueKeysTestCase(BaseObjectIfaceTestCase):"},{"line_number":693,"context_line":""},{"line_number":694,"context_line":"    @classmethod"},{"line_number":695,"context_line":"    def setUpClass(cls):"},{"line_number":696,"context_line":"        m_get_unique_keys \u003d mock.patch.object(model_base, \u0027get_unique_keys\u0027)"},{"line_number":697,"context_line":"        with m_get_unique_keys as get_unique_keys:"},{"line_number":698,"context_line":"            get_unique_keys.return_value \u003d [[\u0027field1\u0027],"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_d4c26452","line":695,"range":{"start_line":694,"start_character":0,"end_line":695,"end_character":18},"in_reply_to":"1aa78d24_7a9bdcc9","updated":"2016-07-12 13:28:50.000000000","message":"fixed","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"5d5c6f947c9cc2d74bebe758a920a3cbb9b0641a","unresolved":false,"context_lines":[{"line_number":691,"context_line":""},{"line_number":692,"context_line":"class BaseDbObjectUniqueKeysTestCase(BaseObjectIfaceTestCase):"},{"line_number":693,"context_line":""},{"line_number":694,"context_line":"    @classmethod"},{"line_number":695,"context_line":"    def setUpClass(cls):"},{"line_number":696,"context_line":"        m_get_unique_keys \u003d mock.patch.object(model_base, \u0027get_unique_keys\u0027)"},{"line_number":697,"context_line":"        with m_get_unique_keys as get_unique_keys:"},{"line_number":698,"context_line":"            get_unique_keys.return_value \u003d [[\u0027field1\u0027],"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_3208b428","line":695,"range":{"start_line":694,"start_character":0,"end_line":695,"end_character":18},"in_reply_to":"1aa78d24_d727b960","updated":"2016-07-08 09:37:26.000000000","message":"You mean _test_class, right? If you insist on setting class attribute, you can set it with\n\n self.__class__._test_class \u003d FakeNeutronObjectUniqueKeys\n\nsetUpClass is not documented (maybe not even truly supported) in testtools and there were issues with it in the past.","commit_id":"dfd373372de869b341f5d8c78caee0bfea2d217b"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"05aebe13bd0cd42d9fc9c685f202b1669c77e770","unresolved":false,"context_lines":[{"line_number":416,"context_line":"                    self.assertTrue(self._is_test_class(obj))"},{"line_number":417,"context_line":"                    self.assertEqual(self.obj_fields[0],"},{"line_number":418,"context_line":"                                     get_obj_db_fields(obj))"},{"line_number":419,"context_line":"                    get_object_mock.assert_called_once_with("},{"line_number":420,"context_line":"                        self.context, self._test_class.db_model,"},{"line_number":421,"context_line":"                        **self._test_class.modify_fields_to_db(obj_keys))"},{"line_number":422,"context_line":""},{"line_number":423,"context_line":"    def _get_synthetic_fields_get_objects_calls(self, db_objs):"},{"line_number":424,"context_line":"        mock_calls \u003d []"}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_7757325a","line":421,"range":{"start_line":419,"start_character":20,"end_line":421,"end_character":73},"updated":"2016-07-12 13:46:04.000000000","message":"I kinda agree with Jakub that this check is worthless. But I am ok-ish to proceed landing it.","commit_id":"581b854a8410d24b09381d7664c0ef2a993b3906"},{"author":{"_account_id":21273,"name":"Ilya Chukhnakov","email":"Ilya.chukhnakov@gmail.com","username":"ichukhnakov"},"change_message_id":"2f9ecd52b6b6eed908193bd45fd1fd4f24716df5","unresolved":false,"context_lines":[{"line_number":416,"context_line":"                    self.assertTrue(self._is_test_class(obj))"},{"line_number":417,"context_line":"                    self.assertEqual(self.obj_fields[0],"},{"line_number":418,"context_line":"                                     get_obj_db_fields(obj))"},{"line_number":419,"context_line":"                    get_object_mock.assert_called_once_with("},{"line_number":420,"context_line":"                        self.context, self._test_class.db_model,"},{"line_number":421,"context_line":"                        **self._test_class.modify_fields_to_db(obj_keys))"},{"line_number":422,"context_line":""},{"line_number":423,"context_line":"    def _get_synthetic_fields_get_objects_calls(self, db_objs):"},{"line_number":424,"context_line":"        mock_calls \u003d []"}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_45ad209a","line":421,"range":{"start_line":419,"start_character":20,"end_line":421,"end_character":73},"in_reply_to":"1aa78d24_7757325a","updated":"2016-07-12 14:14:25.000000000","message":"The point of this check as I see it is not to check interactions between OVO and db.api, but to validate the mock usage. The test uses the mock based on certain assumptions about the tested method implementation and this check validates those assumptions.","commit_id":"581b854a8410d24b09381d7664c0ef2a993b3906"}]}