)]}'
{"nova/db/sqlalchemy/api_models.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"7f89203d37050e3c2734b758018590deb5200333","unresolved":false,"context_lines":[{"line_number":77,"context_line":"    name \u003d Column(String(255))"},{"line_number":78,"context_line":"    _hosts \u003d orm.relationship(AggregateHost,"},{"line_number":79,"context_line":"                    primaryjoin\u003d\u0027Aggregate.id \u003d\u003d AggregateHost.aggregate_id\u0027)"},{"line_number":80,"context_line":"    # The \u0027metadata\u0027 property is reserved for base class metadata"},{"line_number":81,"context_line":"    _metadata \u003d orm.relationship(AggregateMetadata,"},{"line_number":82,"context_line":"                primaryjoin\u003d\u0027Aggregate.id \u003d\u003d AggregateMetadata.aggregate_id\u0027)"},{"line_number":83,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"bab6814e_b3f77b8b","side":"PARENT","line":80,"updated":"2016-05-24 13:01:22.000000000","message":"nit : that comment removal looks unrelated to the change, right?","commit_id":"6a1755710b845f34df5d06793fbc40cd419c7ac2"},{"author":{"_account_id":18247,"name":"Mark Doffman","email":"mdoffman@linux.vnet.ibm.com","username":"doffm"},"change_message_id":"cc8e9c33b6ae12e68159536957e9c310d30008a8","unresolved":false,"context_lines":[{"line_number":77,"context_line":"    name \u003d Column(String(255))"},{"line_number":78,"context_line":"    _hosts \u003d orm.relationship(AggregateHost,"},{"line_number":79,"context_line":"                    primaryjoin\u003d\u0027Aggregate.id \u003d\u003d AggregateHost.aggregate_id\u0027)"},{"line_number":80,"context_line":"    # The \u0027metadata\u0027 property is reserved for base class metadata"},{"line_number":81,"context_line":"    _metadata \u003d orm.relationship(AggregateMetadata,"},{"line_number":82,"context_line":"                primaryjoin\u003d\u0027Aggregate.id \u003d\u003d AggregateMetadata.aggregate_id\u0027)"},{"line_number":83,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"bab6814e_10aeee84","side":"PARENT","line":80,"in_reply_to":"bab6814e_b3f77b8b","updated":"2016-05-25 16:04:51.000000000","message":"I think you are right. I changed this a number of times going back and forth on the \u0027_metadata\u0027 issue.","commit_id":"6a1755710b845f34df5d06793fbc40cd419c7ac2"}],"nova/objects/aggregate.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2fb8cc1ef6d872313898bee6f7df69d6f0f45f2c","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    @db_api.api_context_manager.reader"},{"line_number":52,"context_line":"    def _aggregate_get_from_db(context, aggregate_id):"},{"line_number":53,"context_line":"        query \u003d context.session.query(api_models.Aggregate).\\"},{"line_number":54,"context_line":"                options(joinedload(\u0027_hosts\u0027)).\\"},{"line_number":55,"context_line":"                options(joinedload(\u0027_metadata\u0027))"},{"line_number":56,"context_line":"        query \u003d query.filter(api_models.Aggregate.id \u003d\u003d aggregate_id)"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"fa0719c6_46ed13b6","line":54,"updated":"2016-03-23 17:36:04.000000000","message":"This is enormous right? Should we make it optional to load those? I guess we\u0027re doing it now, but.. since we\u0027re moving, it might be worth a look to see if this is too heavy-handed (maybe you already have?)","commit_id":"d8e8f0e68c688619c92f76dc1ceb7e451905620d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"718ebeb58fae4ee4764bacda52bec105b4bc6f78","unresolved":false,"context_lines":[{"line_number":49,"context_line":"    obj_extra_fields \u003d [\u0027availability_zone\u0027]"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    @staticmethod"},{"line_number":52,"context_line":"    @db_api.api_context_manager.reader"},{"line_number":53,"context_line":"    def _aggregate_get_from_db(context, aggregate_id):"},{"line_number":54,"context_line":"        query \u003d context.session.query(api_models.Aggregate).\\"},{"line_number":55,"context_line":"                options(joinedload(\u0027_hosts\u0027)).\\"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a1b2928_4b2d9ba6","line":52,"updated":"2016-04-11 18:17:46.000000000","message":"I\u0027m pretty sure you\u0027re going to hit the same issue we have elsewhere that introduces instability once you actually call this on py3. I think you will have to pull these out to module-level for the time being.","commit_id":"6d6102c92d9f418681fac5ad01b278dd5f82fffb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"718ebeb58fae4ee4764bacda52bec105b4bc6f78","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        if not aggregate:"},{"line_number":62,"context_line":"            raise exception.AggregateNotFound(aggregate_id\u003daggregate_id)"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"        return aggregate"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    @staticmethod"},{"line_number":67,"context_line":"    def _from_db_object(context, aggregate, db_aggregate):"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a1b2928_6b59775a","line":64,"updated":"2016-04-11 18:17:46.000000000","message":"This method is untested in this patch right? Can you add a functional test or a unit test at least to make sure it works?","commit_id":"6d6102c92d9f418681fac5ad01b278dd5f82fffb"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"7fe2ca047e2aa1c7b644c6cbc5f2a5391fce4e10","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"DEPRECATED_FIELDS \u003d [\u0027deleted\u0027, \u0027deleted_at\u0027]"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"@db_api.api_context_manager.reader"}],"source_content_type":"text/x-python","patch_set":6,"id":"dab17558_ee26fa14","line":31,"updated":"2016-05-13 21:24:25.000000000","message":"And hopefully at least updated_at :)","commit_id":"34442f48b1bc15072f118619f40aed3728e27cfa"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"7fe2ca047e2aa1c7b644c6cbc5f2a5391fce4e10","unresolved":false,"context_lines":[{"line_number":32,"context_line":""},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"@db_api.api_context_manager.reader"},{"line_number":35,"context_line":"def _aggregate_get_from_db(context, aggregate_id):"},{"line_number":36,"context_line":"    query \u003d context.session.query(api_models.Aggregate).\\"},{"line_number":37,"context_line":"            options(joinedload(\u0027_hosts\u0027)).\\"},{"line_number":38,"context_line":"            options(joinedload(\u0027_metadata\u0027))"}],"source_content_type":"text/x-python","patch_set":6,"id":"dab17558_ce2176fe","line":35,"updated":"2016-05-13 21:24:25.000000000","message":"Might be nice to name this _aggregate_get_from_api_db() just so when you call it below, it\u0027s easier for the reader to understand what\u0027s going on....","commit_id":"34442f48b1bc15072f118619f40aed3728e27cfa"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"7b115933270a18303ee76f3365262be5d72b0cd2","unresolved":false,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":"@db_api.api_context_manager.reader"},{"line_number":35,"context_line":"def _aggregate_get_from_db(context, aggregate_id):"},{"line_number":36,"context_line":"    query \u003d context.session.query(api_models.Aggregate).\\"},{"line_number":37,"context_line":"            options(joinedload(\u0027_hosts\u0027)).\\"},{"line_number":38,"context_line":"            options(joinedload(\u0027_metadata\u0027))"},{"line_number":39,"context_line":"    query \u003d query.filter(api_models.Aggregate.id \u003d\u003d aggregate_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bab6814e_95902997","line":36,"updated":"2016-05-25 09:48:48.000000000","message":"Not relevant to the review but why is there a pattern in nova of still using \\ for line breaks instead of wrapping complex statements in ()? Isn\u0027t that culturally deprecated?","commit_id":"6ad37b81f8bd80ef5888df40d5a690bbc4c40fc9"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"792b3cd51d198b9738072a026fd40e8f2cfa2403","unresolved":false,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":"@db_api.api_context_manager.reader"},{"line_number":35,"context_line":"def _aggregate_get_from_db(context, aggregate_id):"},{"line_number":36,"context_line":"    query \u003d context.session.query(api_models.Aggregate).\\"},{"line_number":37,"context_line":"            options(joinedload(\u0027_hosts\u0027)).\\"},{"line_number":38,"context_line":"            options(joinedload(\u0027_metadata\u0027))"},{"line_number":39,"context_line":"    query \u003d query.filter(api_models.Aggregate.id \u003d\u003d aggregate_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bab6814e_3dbaeae1","line":36,"in_reply_to":"bab6814e_2b92a74b","updated":"2016-05-25 16:27:20.000000000","message":"Yeah, the reason for \"Not relevant to the review\" was because it\u0027s, uh, not relevant to the review. I was just trying to satisfy my curiosity. So yeah, neither contentious nor worth arguing about.\n\nJust inquiring.\n\nhttp://www.metalsucks.net/wp-content/uploads/2014/04/the-more-you-know.jpg","commit_id":"6ad37b81f8bd80ef5888df40d5a690bbc4c40fc9"},{"author":{"_account_id":18247,"name":"Mark Doffman","email":"mdoffman@linux.vnet.ibm.com","username":"doffm"},"change_message_id":"cc8e9c33b6ae12e68159536957e9c310d30008a8","unresolved":false,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":"@db_api.api_context_manager.reader"},{"line_number":35,"context_line":"def _aggregate_get_from_db(context, aggregate_id):"},{"line_number":36,"context_line":"    query \u003d context.session.query(api_models.Aggregate).\\"},{"line_number":37,"context_line":"            options(joinedload(\u0027_hosts\u0027)).\\"},{"line_number":38,"context_line":"            options(joinedload(\u0027_metadata\u0027))"},{"line_number":39,"context_line":"    query \u003d query.filter(api_models.Aggregate.id \u003d\u003d aggregate_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bab6814e_d0d106fd","line":36,"in_reply_to":"bab6814e_95902997","updated":"2016-05-25 16:04:51.000000000","message":"This is my pattern. Not sure if it is followed by the rest of nova devs. I didn\u0027t know that is was contentious.","commit_id":"6ad37b81f8bd80ef5888df40d5a690bbc4c40fc9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d117c2b9f1c58204ef396d920a8a5a1eb9b2c220","unresolved":false,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":"@db_api.api_context_manager.reader"},{"line_number":35,"context_line":"def _aggregate_get_from_db(context, aggregate_id):"},{"line_number":36,"context_line":"    query \u003d context.session.query(api_models.Aggregate).\\"},{"line_number":37,"context_line":"            options(joinedload(\u0027_hosts\u0027)).\\"},{"line_number":38,"context_line":"            options(joinedload(\u0027_metadata\u0027))"},{"line_number":39,"context_line":"    query \u003d query.filter(api_models.Aggregate.id \u003d\u003d aggregate_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bab6814e_2b92a74b","line":36,"in_reply_to":"bab6814e_d0d106fd","updated":"2016-05-25 16:16:54.000000000","message":"For the most part we do use parens to split lines. The exception is in the db query-building code for whatever reason, which this is.\n\nPersonally I\u0027d rather us use either technique when it makes nicer code, favoring the parens if it\u0027s a wash. It doesn\u0027t seem like a useful thing to argue over though.","commit_id":"6ad37b81f8bd80ef5888df40d5a690bbc4c40fc9"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"7f89203d37050e3c2734b758018590deb5200333","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        if db_aggregate.get(\u0027uuid\u0027):"},{"line_number":83,"context_line":"            aggregate.uuid \u003d db_aggregate[\u0027uuid\u0027]"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"        # NOTE: This can be removed when we remove compatibility with"},{"line_number":86,"context_line":"        # the old aggregate model."},{"line_number":87,"context_line":"        if any(f not in db_aggregate for f in DEPRECATED_FIELDS):"},{"line_number":88,"context_line":"            aggregate.deleted_at \u003d None"}],"source_content_type":"text/x-python","patch_set":8,"id":"bab6814e_b36c9b37","line":85,"range":{"start_line":85,"start_character":10,"end_line":85,"end_character":14},"updated":"2016-05-24 13:01:22.000000000","message":"nit: in general, PEP8 likes having a name for a NOTE comment like # NOTE(owner)","commit_id":"6ad37b81f8bd80ef5888df40d5a690bbc4c40fc9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d117c2b9f1c58204ef396d920a8a5a1eb9b2c220","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        if db_aggregate.get(\u0027uuid\u0027):"},{"line_number":83,"context_line":"            aggregate.uuid \u003d db_aggregate[\u0027uuid\u0027]"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"        # NOTE: This can be removed when we remove compatibility with"},{"line_number":86,"context_line":"        # the old aggregate model."},{"line_number":87,"context_line":"        if any(f not in db_aggregate for f in DEPRECATED_FIELDS):"},{"line_number":88,"context_line":"            aggregate.deleted_at \u003d None"}],"source_content_type":"text/x-python","patch_set":8,"id":"bab6814e_cbcafb4e","line":85,"range":{"start_line":85,"start_character":10,"end_line":85,"end_character":14},"in_reply_to":"bab6814e_b36c9b37","updated":"2016-05-25 16:16:54.000000000","message":"I\u0027m not sure it\u0027s PEP8, but either way, agree :)","commit_id":"6ad37b81f8bd80ef5888df40d5a690bbc4c40fc9"}],"nova/tests/unit/objects/test_aggregate.py":[{"author":{"_account_id":18603,"name":"Béla Vancsics","email":"vancsics@inf.u-szeged.hu","username":"vancsics"},"change_message_id":"d4b72fb6829e7f6c525f612a0f04fcdf2b2aa157","unresolved":false,"context_lines":[{"line_number":94,"context_line":"    def test_aggregate_get_from_db(self):"},{"line_number":95,"context_line":"        result \u003d _create_aggregate_with_hosts(self.context)"},{"line_number":96,"context_line":"        expected \u003d aggregate._aggregate_get_from_db(self.context, result[\u0027id\u0027])"},{"line_number":97,"context_line":"        self.assertEqual(fake_db_aggregate_hosts, expected[\u0027hosts\u0027])"},{"line_number":98,"context_line":"        self.assertEqual(fake_db_aggregate_metadata, expected[\u0027metadetails\u0027])"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def test_aggregate_get_from_db_raise_not_found(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a18252c_18f0bcc8","line":97,"range":{"start_line":97,"start_character":8,"end_line":97,"end_character":68},"updated":"2016-04-14 09:21:16.000000000","message":"self.assertEqual(fake_db_aggregate_hosts, expected[\u0027hosts\u0027])\n-\u003e\nhosts \u003d [h[\u0027host\u0027] for h in expected[\u0027hosts\u0027]]\nself.assertEqual(fake_db_aggregate_hosts, hosts)","commit_id":"b54f66fd9e15f5a1e2d47ca0ce77b2178a3a19da"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"7fe2ca047e2aa1c7b644c6cbc5f2a5391fce4e10","unresolved":false,"context_lines":[{"line_number":92,"context_line":""},{"line_number":93,"context_line":"class _TestAggregateObject(object):"},{"line_number":94,"context_line":"    def test_aggregate_get_from_db(self):"},{"line_number":95,"context_line":"        result \u003d _create_aggregate_with_hosts(self.context)"},{"line_number":96,"context_line":"        expected \u003d aggregate._aggregate_get_from_db(self.context, result[\u0027id\u0027])"},{"line_number":97,"context_line":"        self.assertEqual(fake_db_aggregate_hosts, expected[\u0027hosts\u0027])"},{"line_number":98,"context_line":"        self.assertEqual(fake_db_aggregate_metadata, expected[\u0027metadetails\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"dab17558_8e0f6e87","line":95,"updated":"2016-05-13 21:24:25.000000000","message":"These unit tests should not be creating anything in the database. This test, and the _create_aggregate_with_hosts() above should really move into the nova/tests/functional/ modules which don\u0027t derive from nova.test.NoDBTestCase.","commit_id":"34442f48b1bc15072f118619f40aed3728e27cfa"},{"author":{"_account_id":18247,"name":"Mark Doffman","email":"mdoffman@linux.vnet.ibm.com","username":"doffm"},"change_message_id":"c7c729551030ebd2a8f992fd86e8a2fc82ebef85","unresolved":false,"context_lines":[{"line_number":92,"context_line":""},{"line_number":93,"context_line":"class _TestAggregateObject(object):"},{"line_number":94,"context_line":"    def test_aggregate_get_from_db(self):"},{"line_number":95,"context_line":"        result \u003d _create_aggregate_with_hosts(self.context)"},{"line_number":96,"context_line":"        expected \u003d aggregate._aggregate_get_from_db(self.context, result[\u0027id\u0027])"},{"line_number":97,"context_line":"        self.assertEqual(fake_db_aggregate_hosts, expected[\u0027hosts\u0027])"},{"line_number":98,"context_line":"        self.assertEqual(fake_db_aggregate_metadata, expected[\u0027metadetails\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"dab17558_e2b4bdf1","line":95,"in_reply_to":"dab17558_8e0f6e87","updated":"2016-05-17 21:57:48.000000000","message":"The object test cases have the cell DB and api DB fixtures. The reason these tests use database access is that the unit tests in unit/db/test_db_api.py also access the database. These tests are essentially copies of tests in that file, but for the equivalent function accessing the api db.","commit_id":"34442f48b1bc15072f118619f40aed3728e27cfa"}]}