)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"9afb736676d8658f94d14b72aad12b7a8de4e230","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Andrey Volkov \u003cavolkov@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-06-22 15:45:55 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix host mapping saving"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"HostMapping.save was broken due incorrect param for"},{"line_number":10,"context_line":"HostMapping._save_in_db."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"3aaa91ec_f01ebf61","line":7,"updated":"2016-06-23 13:50:18.000000000","message":"If this is fixing a defect, we really should have a bug referenced. Would you mind please creating a Launchpad bug and updating the commit message below with:\n\nCloses-bug: XXXXXX\n\nThanks!","commit_id":"cf9900506dd99f99f2510338e6207c61430f8ece"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"b61091cfc1c4a0c767d4d1042fa34ba9a307b158","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Andrey Volkov \u003cavolkov@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-06-22 15:45:55 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix host mapping saving"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"HostMapping.save was broken due incorrect param for"},{"line_number":10,"context_line":"HostMapping._save_in_db."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"3aaa91ec_10da6417","line":7,"in_reply_to":"3aaa91ec_f01ebf61","updated":"2016-06-23 15:35:10.000000000","message":"Done","commit_id":"cf9900506dd99f99f2510338e6207c61430f8ece"}],"nova/objects/host_mapping.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1599010ce54c9242920515da4bc30e5cdd391015","unresolved":false,"context_lines":[{"line_number":57,"context_line":"    @staticmethod"},{"line_number":58,"context_line":"    def _from_db_object(context, host_mapping, db_host_mapping):"},{"line_number":59,"context_line":"        for key in host_mapping.fields:"},{"line_number":60,"context_line":"            db_value \u003d db_host_mapping.get(key)"},{"line_number":61,"context_line":"            if key \u003d\u003d \"cell_mapping\":"},{"line_number":62,"context_line":"                # NOTE(dheeraj): If cell_mapping is stashed in db object"},{"line_number":63,"context_line":"                # we load it here. Otherwise, lazy loading will happen"},{"line_number":64,"context_line":"                # when .cell_mapping is accessd later"}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_eea32d41","line":61,"range":{"start_line":60,"start_character":12,"end_line":61,"end_character":37},"updated":"2016-07-13 01:57:37.000000000","message":"This seems busted. I removed the addition of \".options(joinedload(\u0027cell_mapping\u0027))\" from _save_in_db in this patch and the functional test fails:\n\nhttp://paste.openstack.org/show/531083/\n\nBut it fails because we try to lazy-load the cell_mapping when we call db_host_mapping.get(key) above.\n\nSeems we should check to see if key \u003d\u003d \u0027cell_mapping\u0027 and key in db_host_mapping.","commit_id":"3cab543487f2a078caf710f9b9e13cf18432ec90"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"c9c6b1d978c560fdcc2c3ad9a7a7a6435ff49cda","unresolved":false,"context_lines":[{"line_number":57,"context_line":"    @staticmethod"},{"line_number":58,"context_line":"    def _from_db_object(context, host_mapping, db_host_mapping):"},{"line_number":59,"context_line":"        for key in host_mapping.fields:"},{"line_number":60,"context_line":"            db_value \u003d db_host_mapping.get(key)"},{"line_number":61,"context_line":"            if key \u003d\u003d \"cell_mapping\":"},{"line_number":62,"context_line":"                # NOTE(dheeraj): If cell_mapping is stashed in db object"},{"line_number":63,"context_line":"                # we load it here. Otherwise, lazy loading will happen"},{"line_number":64,"context_line":"                # when .cell_mapping is accessd later"}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_c6d1d158","line":61,"range":{"start_line":60,"start_character":12,"end_line":61,"end_character":37},"in_reply_to":"1aa78d24_5a7aed77","updated":"2016-07-13 16:13:49.000000000","message":"had a quick chat with Matt about this and his concern is why this(the joinedload) was being done as it doesn\u0027t seem to be related to fixing save.\n\nThe joinedload is added because the updated functional test exposes this secondary bug. The commit message should be updated to explain this.","commit_id":"3cab543487f2a078caf710f9b9e13cf18432ec90"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"37a76f748ccdd2baa3eafa18eeec9a86d5014222","unresolved":false,"context_lines":[{"line_number":57,"context_line":"    @staticmethod"},{"line_number":58,"context_line":"    def _from_db_object(context, host_mapping, db_host_mapping):"},{"line_number":59,"context_line":"        for key in host_mapping.fields:"},{"line_number":60,"context_line":"            db_value \u003d db_host_mapping.get(key)"},{"line_number":61,"context_line":"            if key \u003d\u003d \"cell_mapping\":"},{"line_number":62,"context_line":"                # NOTE(dheeraj): If cell_mapping is stashed in db object"},{"line_number":63,"context_line":"                # we load it here. Otherwise, lazy loading will happen"},{"line_number":64,"context_line":"                # when .cell_mapping is accessd later"}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_5a7aed77","line":61,"range":{"start_line":60,"start_character":12,"end_line":61,"end_character":37},"in_reply_to":"1aa78d24_eea32d41","updated":"2016-07-13 08:54:52.000000000","message":"Done","commit_id":"3cab543487f2a078caf710f9b9e13cf18432ec90"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6ee630531c4aeb18060a37804b5753c5d0f32703","unresolved":false,"context_lines":[{"line_number":112,"context_line":"    @db_api.api_context_manager.writer"},{"line_number":113,"context_line":"    def _save_in_db(context, obj, updates):"},{"line_number":114,"context_line":"        db_mapping \u003d context.session.query(api_models.HostMapping).filter_by("},{"line_number":115,"context_line":"            id\u003dobj.id).options(joinedload(\u0027cell_mapping\u0027)).first()"},{"line_number":116,"context_line":"        if not db_mapping:"},{"line_number":117,"context_line":"            raise exception.HostMappingNotFound(name\u003dobj.host)"},{"line_number":118,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_0ef15995","line":115,"range":{"start_line":115,"start_character":22,"end_line":115,"end_character":58},"updated":"2016-07-13 01:52:26.000000000","message":"Why do we need this? Because we lose any cell_mapping updates without it?","commit_id":"3cab543487f2a078caf710f9b9e13cf18432ec90"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"37a76f748ccdd2baa3eafa18eeec9a86d5014222","unresolved":false,"context_lines":[{"line_number":112,"context_line":"    @db_api.api_context_manager.writer"},{"line_number":113,"context_line":"    def _save_in_db(context, obj, updates):"},{"line_number":114,"context_line":"        db_mapping \u003d context.session.query(api_models.HostMapping).filter_by("},{"line_number":115,"context_line":"            id\u003dobj.id).options(joinedload(\u0027cell_mapping\u0027)).first()"},{"line_number":116,"context_line":"        if not db_mapping:"},{"line_number":117,"context_line":"            raise exception.HostMappingNotFound(name\u003dobj.host)"},{"line_number":118,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_3aa6010f","line":115,"range":{"start_line":115,"start_character":22,"end_line":115,"end_character":58},"in_reply_to":"1aa78d24_0ef15995","updated":"2016-07-13 08:54:52.000000000","message":"Done","commit_id":"3cab543487f2a078caf710f9b9e13cf18432ec90"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"e7707f027c024c41d9dd7c1690f780850cc8c3f0","unresolved":false,"context_lines":[{"line_number":66,"context_line":"                    is_loaded \u003d key not in inspect(db_host_mapping).unloaded"},{"line_number":67,"context_line":"                else:"},{"line_number":68,"context_line":"                    is_loaded \u003d key in db_host_mapping"},{"line_number":69,"context_line":"                if not is_loaded:"},{"line_number":70,"context_line":"                    continue"},{"line_number":71,"context_line":"                db_value \u003d cell_mapping.CellMapping._from_db_object("},{"line_number":72,"context_line":"                    host_mapping._context, cell_mapping.CellMapping(),"}],"source_content_type":"text/x-python","patch_set":12,"id":"1aa78d24_06dbd94f","line":69,"updated":"2016-07-13 16:16:53.000000000","message":"This works if we don\u0027t expect \u0027cell_mapping\u0027 to be in db_host_mapping but I think we should expect that it is. So I think that this should be back to what\u0027s in revision 9 and just update the commit message to explain the joinedload like I commented there.\n\nMatt was okay with that solution as well.","commit_id":"1a5e46b39fc6299eec6c3b92a8ea784c5ca2552c"}],"nova/tests/functional/db/test_host_mapping.py":[{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"c0dd8ef720fa38ea875d90d83154f193875cf131","unresolved":false,"context_lines":[{"line_number":101,"context_line":"            model_field \u003d getattr(hm, key)"},{"line_number":102,"context_line":"            if key \u003d\u003d \u0027created_at\u0027:"},{"line_number":103,"context_line":"                model_field \u003d model_field.replace(tzinfo\u003dNone)"},{"line_number":104,"context_line":"            self.assertEqual(model_field, db_hm[key], \u0027field %s\u0027 % key)"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"    def test_destroy_in_db(self):"},{"line_number":107,"context_line":"        mapping \u003d create_mapping()"}],"source_content_type":"text/x-python","patch_set":8,"id":"1aa78d24_eecb7251","line":104,"updated":"2016-07-11 23:03:07.000000000","message":"The order of the arguments here should be switched. When using any of the assert* methods the expected argument should go first and then the argument being tested. This is done because the error message if there\u0027s a failure is more clear when they\u0027re ordered that way.","commit_id":"e611ebccd5e33b5e435f8a9cac489f8a636fecfd"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"b9497475143cba31ac575506fa3c526250dae9de","unresolved":false,"context_lines":[{"line_number":101,"context_line":"            model_field \u003d getattr(hm, key)"},{"line_number":102,"context_line":"            if key \u003d\u003d \u0027created_at\u0027:"},{"line_number":103,"context_line":"                model_field \u003d model_field.replace(tzinfo\u003dNone)"},{"line_number":104,"context_line":"            self.assertEqual(model_field, db_hm[key], \u0027field %s\u0027 % key)"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"    def test_destroy_in_db(self):"},{"line_number":107,"context_line":"        mapping \u003d create_mapping()"}],"source_content_type":"text/x-python","patch_set":8,"id":"1aa78d24_394138aa","line":104,"in_reply_to":"1aa78d24_eecb7251","updated":"2016-07-12 05:37:18.000000000","message":"Done","commit_id":"e611ebccd5e33b5e435f8a9cac489f8a636fecfd"}],"nova/tests/unit/objects/test_host_mapping.py":[{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"64223f639cf84e190d1862162bcc84f13b194e6a","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        # This isn\u0027t needed here"},{"line_number":109,"context_line":"        db_mapping.pop(\"cell_mapping\")"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"        # hack to compare HostMapping objects in assert"},{"line_number":112,"context_line":"        def hm_equal(a, b):"},{"line_number":113,"context_line":"            return six.text_type(a) \u003d\u003d six.text_type(b)"},{"line_number":114,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5a9d85d2_522ff1f5","line":111,"updated":"2016-06-21 15:54:22.000000000","message":"Rather than doing this there\u0027s a MatchType helper in test.py that can be used here. You can use test.MatchType(objects.HostMapping) on line 134 below.","commit_id":"d65b94c8f42371de5aa2a6b4acab7cbf35657b68"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"64223f639cf84e190d1862162bcc84f13b194e6a","unresolved":false,"context_lines":[{"line_number":112,"context_line":"        def hm_equal(a, b):"},{"line_number":113,"context_line":"            return six.text_type(a) \u003d\u003d six.text_type(b)"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"        def fake_save_in_db(ctxt, obj, updates):"},{"line_number":116,"context_line":"            obj.id"},{"line_number":117,"context_line":"            obj.host"},{"line_number":118,"context_line":"            return db_mapping"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a9d85d2_523b719c","line":115,"updated":"2016-06-21 15:54:22.000000000","message":"This looks like it\u0027s not necessary.\n\nAlthough if you really want to use it you could remove the assert_called_once_with on line 133 below and just assert that the right things are passed in here.","commit_id":"d65b94c8f42371de5aa2a6b4acab7cbf35657b68"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"9a1d70d895b0360c1da4addc052951a60291dfae","unresolved":false,"context_lines":[{"line_number":112,"context_line":"        def hm_equal(a, b):"},{"line_number":113,"context_line":"            return six.text_type(a) \u003d\u003d six.text_type(b)"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"        def fake_save_in_db(ctxt, obj, updates):"},{"line_number":116,"context_line":"            obj.id"},{"line_number":117,"context_line":"            obj.host"},{"line_number":118,"context_line":"            return db_mapping"}],"source_content_type":"text/x-python","patch_set":3,"id":"3aaa91ec_840e7a32","line":115,"in_reply_to":"5a9d85d2_523b719c","updated":"2016-06-22 06:33:24.000000000","message":"Idea was to show why real saving fails while tests are passed.\nBut you are right this intermediate step is not necessary, thanks.","commit_id":"d65b94c8f42371de5aa2a6b4acab7cbf35657b68"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"9afb736676d8658f94d14b72aad12b7a8de4e230","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"3aaa91ec_d019037b","updated":"2016-06-23 13:50:18.000000000","message":"Would be good to have a functional test of this object in nova/tests/functional/objects/ ... perhaps consider adding one in a future patch?","commit_id":"cf9900506dd99f99f2510338e6207c61430f8ece"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"b61091cfc1c4a0c767d4d1042fa34ba9a307b158","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"3aaa91ec_eb152de5","in_reply_to":"3aaa91ec_d019037b","updated":"2016-06-23 15:35:10.000000000","message":"I this test I lack testing sqlalchemy stuff, it\u0027s mocked too much. I agree it can be useful to test the whole process of saving or conversion between model and db.","commit_id":"cf9900506dd99f99f2510338e6207c61430f8ece"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"9c0bd66c4a19281a8375ecbf389796f4805b8118","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"3aaa91ec_bc97f146","in_reply_to":"3aaa91ec_d019037b","updated":"2016-06-23 14:35:21.000000000","message":"This object is tested there, however what\u0027s being tested are the db methods directly not the object methods. If the test was changed to using the object methods it would have caught this.","commit_id":"cf9900506dd99f99f2510338e6207c61430f8ece"}]}