)]}'
{"nova/conductor/manager.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ed382c420b0c1be0dcde5efed7d32c16b65e429f","unresolved":false,"context_lines":[{"line_number":891,"context_line":"                        LOG.warning(\"Server with unsupported policy \""},{"line_number":892,"context_line":"                                    \"cannot be rebuilt\", instance\u003dinstance)"},{"line_number":893,"context_line":""},{"line_number":894,"context_line":"            compute_utils.notify_about_instance_usage("},{"line_number":895,"context_line":"                self.notifier, context, instance, \"rebuild.scheduled\")"},{"line_number":896,"context_line":""},{"line_number":897,"context_line":"            instance.availability_zone \u003d ("},{"line_number":898,"context_line":"                availability_zones.get_host_availability_zone("}],"source_content_type":"text/x-python","patch_set":1,"id":"7f515b1d_9802e9b3","line":895,"range":{"start_line":894,"start_character":0,"end_line":895,"end_character":70},"updated":"2017-10-06 09:09:34.000000000","message":"I guess either here the context is already targeted somehow or nothing is lazy loaded from the instance. We have to be aware of a possible problem when this is transformed to versioned notification as that could trigger lazy-load.\nhttps://review.openstack.org/#/c/473929/","commit_id":"dfff5ceca6e46ee50d21c502fb840fcbbe4b785d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d3da238815fc236c321307c45a5f517791f8544e","unresolved":false,"context_lines":[{"line_number":1037,"context_line":""},{"line_number":1038,"context_line":"        for (build_request, request_spec, host) in six.moves.zip("},{"line_number":1039,"context_line":"                build_requests, request_specs, hosts):"},{"line_number":1040,"context_line":"            instance \u003d build_request.get_new_instance(context)"},{"line_number":1041,"context_line":"            # Convert host from the scheduler into a cell record"},{"line_number":1042,"context_line":"            if host[\u0027host\u0027] not in host_mapping_cache:"},{"line_number":1043,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f515b1d_2302990a","line":1040,"updated":"2017-10-06 15:25:26.000000000","message":"Here is where the instance object comes from, it\u0027s created with the context that was passed in.","commit_id":"dfff5ceca6e46ee50d21c502fb840fcbbe4b785d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ed382c420b0c1be0dcde5efed7d32c16b65e429f","unresolved":false,"context_lines":[{"line_number":1111,"context_line":"                                                       host)"},{"line_number":1112,"context_line":"            # send a state update notification for the initial create to"},{"line_number":1113,"context_line":"            # show it going from non-existent to BUILDING"},{"line_number":1114,"context_line":"            # TODO(melwitt): Maybe we should set_target_cell on the contexts"},{"line_number":1115,"context_line":"            # once we map to a cell, and remove these separate with statements."},{"line_number":1116,"context_line":"            with obj_target_cell(instance, cell):"},{"line_number":1117,"context_line":"                # This can lazy-load attributes on instance."},{"line_number":1118,"context_line":"                notifications.send_update_with_states(context, instance, None,"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f515b1d_7b1cc7c0","line":1115,"range":{"start_line":1114,"start_character":0,"end_line":1115,"end_character":79},"updated":"2017-10-06 09:09:34.000000000","message":"I like the idea to target the context as soon as the instance is mapped. That would simplify things.","commit_id":"dfff5ceca6e46ee50d21c502fb840fcbbe4b785d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ed382c420b0c1be0dcde5efed7d32c16b65e429f","unresolved":false,"context_lines":[{"line_number":1113,"context_line":"            # show it going from non-existent to BUILDING"},{"line_number":1114,"context_line":"            # TODO(melwitt): Maybe we should set_target_cell on the contexts"},{"line_number":1115,"context_line":"            # once we map to a cell, and remove these separate with statements."},{"line_number":1116,"context_line":"            with obj_target_cell(instance, cell):"},{"line_number":1117,"context_line":"                # This can lazy-load attributes on instance."},{"line_number":1118,"context_line":"                notifications.send_update_with_states(context, instance, None,"},{"line_number":1119,"context_line":"                        vm_states.BUILDING, None, None, service\u003d\"conductor\")"},{"line_number":1120,"context_line":""},{"line_number":1121,"context_line":"            with obj_target_cell(instance, cell) as cctxt:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f515b1d_fbd317d4","line":1118,"range":{"start_line":1116,"start_character":0,"end_line":1118,"end_character":78},"updated":"2017-10-06 09:09:34.000000000","message":"The context manager will change the instance._context but the notification call uses the context that was passed in to schedule_and_build_instances(). I think that two contexts are not the same and this causes that the db_connection is None in your test.\nSo I suggest the following:\n\nwith obj_target_cell(instance, cell) as ctxt:\n    notifications.send_update_with_states(ctxt, instance, None,","commit_id":"dfff5ceca6e46ee50d21c502fb840fcbbe4b785d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d3da238815fc236c321307c45a5f517791f8544e","unresolved":false,"context_lines":[{"line_number":1113,"context_line":"            # show it going from non-existent to BUILDING"},{"line_number":1114,"context_line":"            # TODO(melwitt): Maybe we should set_target_cell on the contexts"},{"line_number":1115,"context_line":"            # once we map to a cell, and remove these separate with statements."},{"line_number":1116,"context_line":"            with obj_target_cell(instance, cell):"},{"line_number":1117,"context_line":"                # This can lazy-load attributes on instance."},{"line_number":1118,"context_line":"                notifications.send_update_with_states(context, instance, None,"},{"line_number":1119,"context_line":"                        vm_states.BUILDING, None, None, service\u003d\"conductor\")"},{"line_number":1120,"context_line":""},{"line_number":1121,"context_line":"            with obj_target_cell(instance, cell) as cctxt:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f515b1d_633091e9","line":1118,"range":{"start_line":1116,"start_character":0,"end_line":1118,"end_character":78},"in_reply_to":"7f515b1d_889e0c61","updated":"2017-10-06 15:25:26.000000000","message":"Per my comment on L1040, the contexts are the same BUT I think you\u0027re right that they should both be targeted. I\u0027m going to push an update with both targeted soon (I\u0027m working on a problem on a patch at the top of this stack at the moment).","commit_id":"dfff5ceca6e46ee50d21c502fb840fcbbe4b785d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6ebe8cad215de13a4dec441c856ab16716b072b9","unresolved":false,"context_lines":[{"line_number":1113,"context_line":"            # show it going from non-existent to BUILDING"},{"line_number":1114,"context_line":"            # TODO(melwitt): Maybe we should set_target_cell on the contexts"},{"line_number":1115,"context_line":"            # once we map to a cell, and remove these separate with statements."},{"line_number":1116,"context_line":"            with obj_target_cell(instance, cell):"},{"line_number":1117,"context_line":"                # This can lazy-load attributes on instance."},{"line_number":1118,"context_line":"                notifications.send_update_with_states(context, instance, None,"},{"line_number":1119,"context_line":"                        vm_states.BUILDING, None, None, service\u003d\"conductor\")"},{"line_number":1120,"context_line":""},{"line_number":1121,"context_line":"            with obj_target_cell(instance, cell) as cctxt:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f515b1d_889e0c61","line":1118,"range":{"start_line":1116,"start_character":0,"end_line":1118,"end_character":78},"in_reply_to":"7f515b1d_b2c5b990","updated":"2017-10-06 15:17:36.000000000","message":"In a real nova-conductor service can the instance._context and the context passed in to schedule_and_build_instances be two references to two different context objects? If yes then I think we have to make sure that both context object is re-targeted to avoid confusion later in the send_update_with_states() call chain.","commit_id":"dfff5ceca6e46ee50d21c502fb840fcbbe4b785d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a4d2453fb10de4fa0f31f2b97a0c26c04370e76e","unresolved":false,"context_lines":[{"line_number":1113,"context_line":"            # show it going from non-existent to BUILDING"},{"line_number":1114,"context_line":"            # TODO(melwitt): Maybe we should set_target_cell on the contexts"},{"line_number":1115,"context_line":"            # once we map to a cell, and remove these separate with statements."},{"line_number":1116,"context_line":"            with obj_target_cell(instance, cell):"},{"line_number":1117,"context_line":"                # This can lazy-load attributes on instance."},{"line_number":1118,"context_line":"                notifications.send_update_with_states(context, instance, None,"},{"line_number":1119,"context_line":"                        vm_states.BUILDING, None, None, service\u003d\"conductor\")"},{"line_number":1120,"context_line":""},{"line_number":1121,"context_line":"            with obj_target_cell(instance, cell) as cctxt:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f515b1d_b2c5b990","line":1118,"range":{"start_line":1116,"start_character":0,"end_line":1118,"end_character":78},"in_reply_to":"7f515b1d_fbd317d4","updated":"2017-10-06 14:10:00.000000000","message":"No, what I meant in the test comment is that when I used mock_notify.call_args to check the Instance._context it had None for the db_connection but when I use mock_notify.side_effect to check the Instance._context it has the expected db_connection.\n\nBasically, I tested this with my CellDatabases fixture patch *first* and had to work backward to write a unit test. So I know this is what prevents a notification from being sent. When an Instance does a lazy-load, it uses its stored Instance._context to do it, not the context passed in to send_updates_with_states.\n\nBut, in general the contexts should probably match so I can do that.","commit_id":"dfff5ceca6e46ee50d21c502fb840fcbbe4b785d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ed382c420b0c1be0dcde5efed7d32c16b65e429f","unresolved":false,"context_lines":[{"line_number":1228,"context_line":"            # This indicates an instance deletion request has been"},{"line_number":1229,"context_line":"            # processed, and the build should halt here. Clean up the"},{"line_number":1230,"context_line":"            # bdm, tags and instance record."},{"line_number":1231,"context_line":"            with obj_target_cell(instance, cell) as cctxt:"},{"line_number":1232,"context_line":"                with compute_utils.notify_about_instance_delete("},{"line_number":1233,"context_line":"                        self.notifier, cctxt, instance):"},{"line_number":1234,"context_line":"                    try:"},{"line_number":1235,"context_line":"                        instance.destroy()"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f515b1d_3863f5b3","line":1232,"range":{"start_line":1231,"start_character":0,"end_line":1232,"end_character":64},"updated":"2017-10-06 09:09:34.000000000","message":"We are lucky as here the context is already targeted.","commit_id":"dfff5ceca6e46ee50d21c502fb840fcbbe4b785d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b0a9fd38090ffa633acf05ddc17280d6d6dc593a","unresolved":false,"context_lines":[{"line_number":1118,"context_line":"                notifications.send_update_with_states(cctxt, instance, None,"},{"line_number":1119,"context_line":"                        vm_states.BUILDING, None, None, service\u003d\"conductor\")"},{"line_number":1120,"context_line":""},{"line_number":1121,"context_line":"            with obj_target_cell(instance, cell) as cctxt:"},{"line_number":1122,"context_line":"                objects.InstanceAction.action_start("},{"line_number":1123,"context_line":"                    cctxt, instance.uuid, instance_actions.CREATE,"},{"line_number":1124,"context_line":"                    want_result\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f515b1d_30de213e","line":1121,"updated":"2017-10-09 21:12:54.000000000","message":"We do the same thing right here, why not just put the notification line underneath this context?","commit_id":"bbeb4c78e0fb2efc5dfaadeae50a4aaa6b881614"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3915a6718f80a6917e05b881f51fa027d57fae03","unresolved":false,"context_lines":[{"line_number":1118,"context_line":"                notifications.send_update_with_states(cctxt, instance, None,"},{"line_number":1119,"context_line":"                        vm_states.BUILDING, None, None, service\u003d\"conductor\")"},{"line_number":1120,"context_line":""},{"line_number":1121,"context_line":"            with obj_target_cell(instance, cell) as cctxt:"},{"line_number":1122,"context_line":"                objects.InstanceAction.action_start("},{"line_number":1123,"context_line":"                    cctxt, instance.uuid, instance_actions.CREATE,"},{"line_number":1124,"context_line":"                    want_result\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f515b1d_d0734d4d","line":1121,"in_reply_to":"7f515b1d_30de213e","updated":"2017-10-09 21:37:53.000000000","message":"Brain lapse probably. Will update.","commit_id":"bbeb4c78e0fb2efc5dfaadeae50a4aaa6b881614"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b5a48840fb5444dbcbafd5bd2bf8cca8ca7e3287","unresolved":false,"context_lines":[{"line_number":1110,"context_line":"            scheduler_utils.populate_filter_properties(filter_props,"},{"line_number":1111,"context_line":"                                                       host)"},{"line_number":1112,"context_line":"            # TODO(melwitt): Maybe we should set_target_cell on the contexts"},{"line_number":1113,"context_line":"            # once we map to a cell, and remove these separate with statements."},{"line_number":1114,"context_line":"            with obj_target_cell(instance, cell) as cctxt:"},{"line_number":1115,"context_line":"                # send a state update notification for the initial create to"},{"line_number":1116,"context_line":"                # show it going from non-existent to BUILDING"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f4e5783_fc9a3a6c","line":1113,"updated":"2017-10-10 15:42:30.000000000","message":"We\u0027d need to carefully copy the context and set_target_cell() on the copy to avoid racing with setting and re-setting the one we were given.\n\nAlong those lines, I wonder if we should make set_target_cell() raise an exception if you try to target an already-targeted context? That might help catch places where we do it twice in a loop and would be susceptible to racing like that.","commit_id":"f11d05e9013100339675e03e7b6df0957a65cbf5"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"520373fdfd3377bc18c3e76010ab919f57820288","unresolved":false,"context_lines":[{"line_number":1110,"context_line":"            scheduler_utils.populate_filter_properties(filter_props,"},{"line_number":1111,"context_line":"                                                       host)"},{"line_number":1112,"context_line":"            # TODO(melwitt): Maybe we should set_target_cell on the contexts"},{"line_number":1113,"context_line":"            # once we map to a cell, and remove these separate with statements."},{"line_number":1114,"context_line":"            with obj_target_cell(instance, cell) as cctxt:"},{"line_number":1115,"context_line":"                # send a state update notification for the initial create to"},{"line_number":1116,"context_line":"                # show it going from non-existent to BUILDING"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f4e5783_bc996220","line":1113,"in_reply_to":"5f4e5783_fc9a3a6c","updated":"2017-10-10 17:32:25.000000000","message":"Good points.","commit_id":"f11d05e9013100339675e03e7b6df0957a65cbf5"}],"nova/tests/unit/conductor/test_conductor.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ed382c420b0c1be0dcde5efed7d32c16b65e429f","unresolved":false,"context_lines":[{"line_number":1461,"context_line":"        def fake_notify(ctxt, instance, *args, **kwargs):"},{"line_number":1462,"context_line":"            # Assert the instance object is targeted to \u0027cell1\u0027 when going"},{"line_number":1463,"context_line":"            # through the notification code."},{"line_number":1464,"context_line":"            with context.target_cell(self.ctxt,"},{"line_number":1465,"context_line":"                                     self.cell_mappings[\u0027cell1\u0027]) as cctxt:"},{"line_number":1466,"context_line":"                self.assertEqual(cctxt.db_connection,"},{"line_number":1467,"context_line":"                                 instance._context.db_connection)"},{"line_number":1468,"context_line":""},{"line_number":1469,"context_line":"        mock_notify.side_effect \u003d fake_notify"},{"line_number":1470,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7f515b1d_7b8de7d1","line":1467,"range":{"start_line":1464,"start_character":12,"end_line":1467,"end_character":65},"updated":"2017-10-06 09:09:34.000000000","message":"This only tests that context.target_cell works and doesn\u0027t assert that the send_update_with_state is called with a targeted context as the ctxt param of the fake_notify is not checked.","commit_id":"dfff5ceca6e46ee50d21c502fb840fcbbe4b785d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6ebe8cad215de13a4dec441c856ab16716b072b9","unresolved":false,"context_lines":[{"line_number":1461,"context_line":"        def fake_notify(ctxt, instance, *args, **kwargs):"},{"line_number":1462,"context_line":"            # Assert the instance object is targeted to \u0027cell1\u0027 when going"},{"line_number":1463,"context_line":"            # through the notification code."},{"line_number":1464,"context_line":"            with context.target_cell(self.ctxt,"},{"line_number":1465,"context_line":"                                     self.cell_mappings[\u0027cell1\u0027]) as cctxt:"},{"line_number":1466,"context_line":"                self.assertEqual(cctxt.db_connection,"},{"line_number":1467,"context_line":"                                 instance._context.db_connection)"},{"line_number":1468,"context_line":""},{"line_number":1469,"context_line":"        mock_notify.side_effect \u003d fake_notify"},{"line_number":1470,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7f515b1d_c873e47f","line":1467,"range":{"start_line":1464,"start_character":12,"end_line":1467,"end_character":65},"in_reply_to":"7f515b1d_12e00dcc","updated":"2017-10-06 15:17:36.000000000","message":"Ohh I see now. So we only need that the instance._context is well targeted as lazy-load will use that.","commit_id":"dfff5ceca6e46ee50d21c502fb840fcbbe4b785d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a4d2453fb10de4fa0f31f2b97a0c26c04370e76e","unresolved":false,"context_lines":[{"line_number":1461,"context_line":"        def fake_notify(ctxt, instance, *args, **kwargs):"},{"line_number":1462,"context_line":"            # Assert the instance object is targeted to \u0027cell1\u0027 when going"},{"line_number":1463,"context_line":"            # through the notification code."},{"line_number":1464,"context_line":"            with context.target_cell(self.ctxt,"},{"line_number":1465,"context_line":"                                     self.cell_mappings[\u0027cell1\u0027]) as cctxt:"},{"line_number":1466,"context_line":"                self.assertEqual(cctxt.db_connection,"},{"line_number":1467,"context_line":"                                 instance._context.db_connection)"},{"line_number":1468,"context_line":""},{"line_number":1469,"context_line":"        mock_notify.side_effect \u003d fake_notify"},{"line_number":1470,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7f515b1d_12e00dcc","line":1467,"range":{"start_line":1464,"start_character":12,"end_line":1467,"end_character":65},"in_reply_to":"7f515b1d_7b8de7d1","updated":"2017-10-06 14:10:00.000000000","message":"Right, because I had only targeted the Instance._context. My comment above is saying that literally this exact same check works with mock_notify.side_effect but does not work with mock_notify.call_args. Initially I had tried to check it with mock_notify.call_args[0][1] as the Instance.","commit_id":"dfff5ceca6e46ee50d21c502fb840fcbbe4b785d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bac20bfd19d56ba6f9bb11d04888b5b6e3019193","unresolved":false,"context_lines":[{"line_number":1464,"context_line":"            # through the notification code."},{"line_number":1465,"context_line":"            with context.target_cell(self.ctxt,"},{"line_number":1466,"context_line":"                                     self.cell_mappings[\u0027cell1\u0027]) as cctxt:"},{"line_number":1467,"context_line":"                self.assertEqual(cctxt.db_connection, ctxt.db_connection)"},{"line_number":1468,"context_line":"                self.assertEqual(cctxt.db_connection,"},{"line_number":1469,"context_line":"                                 instance._context.db_connection)"},{"line_number":1470,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7f515b1d_f0cc098a","line":1467,"updated":"2017-10-09 21:16:30.000000000","message":"Aren\u0027t you just testing target_cell here?","commit_id":"bbeb4c78e0fb2efc5dfaadeae50a4aaa6b881614"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3915a6718f80a6917e05b881f51fa027d57fae03","unresolved":false,"context_lines":[{"line_number":1464,"context_line":"            # through the notification code."},{"line_number":1465,"context_line":"            with context.target_cell(self.ctxt,"},{"line_number":1466,"context_line":"                                     self.cell_mappings[\u0027cell1\u0027]) as cctxt:"},{"line_number":1467,"context_line":"                self.assertEqual(cctxt.db_connection, ctxt.db_connection)"},{"line_number":1468,"context_line":"                self.assertEqual(cctxt.db_connection,"},{"line_number":1469,"context_line":"                                 instance._context.db_connection)"},{"line_number":1470,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7f515b1d_106d6531","line":1467,"in_reply_to":"7f515b1d_f0cc098a","updated":"2017-10-09 21:37:53.000000000","message":"Here I\u0027m using target_cell to get the transaction context manager for cell1. Somehow we know the instance is supposed to be created in cell1 (L1484) so I was thinking I wanted to make sure the context passed to notify was targeted to cell1.","commit_id":"bbeb4c78e0fb2efc5dfaadeae50a4aaa6b881614"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bac20bfd19d56ba6f9bb11d04888b5b6e3019193","unresolved":false,"context_lines":[{"line_number":1466,"context_line":"                                     self.cell_mappings[\u0027cell1\u0027]) as cctxt:"},{"line_number":1467,"context_line":"                self.assertEqual(cctxt.db_connection, ctxt.db_connection)"},{"line_number":1468,"context_line":"                self.assertEqual(cctxt.db_connection,"},{"line_number":1469,"context_line":"                                 instance._context.db_connection)"},{"line_number":1470,"context_line":""},{"line_number":1471,"context_line":"        mock_notify.side_effect \u003d fake_notify"},{"line_number":1472,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7f515b1d_70a6d9c3","line":1469,"updated":"2017-10-09 21:16:30.000000000","message":"cctxt here is the thing you created on L1465. Don\u0027t we want to assert instance._context is targeted in fake_notify() by the caller?\n\nOr, hmm, are you doing this target because of the way the fixture works and always giving the same context for subsequent targets? That seems like not the best thing to rely on, in case we change that.\n\nWhy not just make sure that instance._context.db_connecton is not None in this handler, as set by our caller?","commit_id":"bbeb4c78e0fb2efc5dfaadeae50a4aaa6b881614"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3915a6718f80a6917e05b881f51fa027d57fae03","unresolved":false,"context_lines":[{"line_number":1466,"context_line":"                                     self.cell_mappings[\u0027cell1\u0027]) as cctxt:"},{"line_number":1467,"context_line":"                self.assertEqual(cctxt.db_connection, ctxt.db_connection)"},{"line_number":1468,"context_line":"                self.assertEqual(cctxt.db_connection,"},{"line_number":1469,"context_line":"                                 instance._context.db_connection)"},{"line_number":1470,"context_line":""},{"line_number":1471,"context_line":"        mock_notify.side_effect \u003d fake_notify"},{"line_number":1472,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7f515b1d_8b280686","line":1469,"in_reply_to":"7f515b1d_70a6d9c3","updated":"2017-10-09 21:37:53.000000000","message":"Hm, yeah. I guess really asserting connection is not None accomplishes the same thing. I was trying to assert that the context passed to notify matched the cell the instance is in (cell1 specifically) but we can probably trust if the context is targeted at all, it\u0027s targeted to the right cell.","commit_id":"bbeb4c78e0fb2efc5dfaadeae50a4aaa6b881614"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"048ef05eacb25a8218e390e84a7815945be88265","unresolved":false,"context_lines":[{"line_number":1466,"context_line":"                                     self.cell_mappings[\u0027cell1\u0027]) as cctxt:"},{"line_number":1467,"context_line":"                self.assertEqual(cctxt.db_connection, ctxt.db_connection)"},{"line_number":1468,"context_line":"                self.assertEqual(cctxt.db_connection,"},{"line_number":1469,"context_line":"                                 instance._context.db_connection)"},{"line_number":1470,"context_line":""},{"line_number":1471,"context_line":"        mock_notify.side_effect \u003d fake_notify"},{"line_number":1472,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7f515b1d_2b7112a9","line":1469,"in_reply_to":"7f515b1d_8b280686","updated":"2017-10-09 21:43:30.000000000","message":"Yeah, IsNotNone is good enough I think and follows a lot more logically for someone not well-versed in the details of the fixture.","commit_id":"bbeb4c78e0fb2efc5dfaadeae50a4aaa6b881614"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a92c94c93a13534e516a3b6caa25557a00c12cad","unresolved":false,"context_lines":[{"line_number":1456,"context_line":""},{"line_number":1457,"context_line":"    @mock.patch(\u0027nova.notifications.send_update_with_states\u0027)"},{"line_number":1458,"context_line":"    def test_schedule_and_build_instances(self, mock_notify):"},{"line_number":1459,"context_line":"        # NOTE(melwitt): For some reason, this won\u0027t work with call_args. The"},{"line_number":1460,"context_line":"        # mock.call arg had a db_connection of None for Instance._context. But"},{"line_number":1461,"context_line":"        # mock.side_effect has a db_connection set."},{"line_number":1462,"context_line":"        def fake_notify(ctxt, instance, *args, **kwargs):"},{"line_number":1463,"context_line":"            # Assert the instance object is targeted to \u0027cell1\u0027 when going"},{"line_number":1464,"context_line":"            # through the notification code."},{"line_number":1465,"context_line":"            self.assertIsNotNone(ctxt.db_connection)"},{"line_number":1466,"context_line":"            self.assertIsNotNone(instance._context.db_connection)"},{"line_number":1467,"context_line":""},{"line_number":1468,"context_line":"        mock_notify.side_effect \u003d fake_notify"},{"line_number":1469,"context_line":""},{"line_number":1470,"context_line":"        instance_uuid \u003d self._do_schedule_and_build_instances_test("},{"line_number":1471,"context_line":"            self.params)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f4e5783_f3c4b242","line":1468,"range":{"start_line":1459,"start_character":0,"end_line":1468,"end_character":45},"updated":"2017-10-10 11:44:39.000000000","message":"This is not needed any more as now the send_update_with_states() is called with a targeted context so you can simply assert at line 1485 that:\n\nself.assertIsNotNone(cctxt, mock_notify.call_args[0[0].db_connection)","commit_id":"f11d05e9013100339675e03e7b6df0957a65cbf5"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5056262361c02897b85b6b732759023eb17be67e","unresolved":false,"context_lines":[{"line_number":1456,"context_line":""},{"line_number":1457,"context_line":"    @mock.patch(\u0027nova.notifications.send_update_with_states\u0027)"},{"line_number":1458,"context_line":"    def test_schedule_and_build_instances(self, mock_notify):"},{"line_number":1459,"context_line":"        # NOTE(melwitt): For some reason, this won\u0027t work with call_args. The"},{"line_number":1460,"context_line":"        # mock.call arg had a db_connection of None for Instance._context. But"},{"line_number":1461,"context_line":"        # mock.side_effect has a db_connection set."},{"line_number":1462,"context_line":"        def fake_notify(ctxt, instance, *args, **kwargs):"},{"line_number":1463,"context_line":"            # Assert the instance object is targeted to \u0027cell1\u0027 when going"},{"line_number":1464,"context_line":"            # through the notification code."},{"line_number":1465,"context_line":"            self.assertIsNotNone(ctxt.db_connection)"},{"line_number":1466,"context_line":"            self.assertIsNotNone(instance._context.db_connection)"},{"line_number":1467,"context_line":""},{"line_number":1468,"context_line":"        mock_notify.side_effect \u003d fake_notify"},{"line_number":1469,"context_line":""},{"line_number":1470,"context_line":"        instance_uuid \u003d self._do_schedule_and_build_instances_test("},{"line_number":1471,"context_line":"            self.params)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f4e5783_28d08df7","line":1468,"range":{"start_line":1459,"start_character":0,"end_line":1468,"end_character":45},"in_reply_to":"5f4e5783_6db577ee","updated":"2017-10-10 15:19:05.000000000","message":"Thanks for figuring out what was going on there. I also wanted to know why it wouldn\u0027t work but my brain was burnt. Awesome find.\n\nYeah, I can update that comment. I also forgot to remove the word \u0027cell1\u0027 last time that I meant to remove.","commit_id":"f11d05e9013100339675e03e7b6df0957a65cbf5"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"468839b27045ba63cd9acb0284efa0ad2f5e64bd","unresolved":false,"context_lines":[{"line_number":1456,"context_line":""},{"line_number":1457,"context_line":"    @mock.patch(\u0027nova.notifications.send_update_with_states\u0027)"},{"line_number":1458,"context_line":"    def test_schedule_and_build_instances(self, mock_notify):"},{"line_number":1459,"context_line":"        # NOTE(melwitt): For some reason, this won\u0027t work with call_args. The"},{"line_number":1460,"context_line":"        # mock.call arg had a db_connection of None for Instance._context. But"},{"line_number":1461,"context_line":"        # mock.side_effect has a db_connection set."},{"line_number":1462,"context_line":"        def fake_notify(ctxt, instance, *args, **kwargs):"},{"line_number":1463,"context_line":"            # Assert the instance object is targeted to \u0027cell1\u0027 when going"},{"line_number":1464,"context_line":"            # through the notification code."},{"line_number":1465,"context_line":"            self.assertIsNotNone(ctxt.db_connection)"},{"line_number":1466,"context_line":"            self.assertIsNotNone(instance._context.db_connection)"},{"line_number":1467,"context_line":""},{"line_number":1468,"context_line":"        mock_notify.side_effect \u003d fake_notify"},{"line_number":1469,"context_line":""},{"line_number":1470,"context_line":"        instance_uuid \u003d self._do_schedule_and_build_instances_test("},{"line_number":1471,"context_line":"            self.params)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f4e5783_fc06bf20","line":1468,"range":{"start_line":1459,"start_character":0,"end_line":1468,"end_character":45},"in_reply_to":"5f4e5783_f3c4b242","updated":"2017-10-10 13:42:02.000000000","message":"That won\u0027t assert that the Instance._context is properly targeted though, and we need to assert that too. And for whatever reason:\n\n self.assertIsNotNone(mock_notify.call_args[0][1]._context.db_connection)\n\ndoes not work but what I have here does work.","commit_id":"f11d05e9013100339675e03e7b6df0957a65cbf5"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"939eee62409754618b41ef35b4f8c29992f5b84e","unresolved":false,"context_lines":[{"line_number":1456,"context_line":""},{"line_number":1457,"context_line":"    @mock.patch(\u0027nova.notifications.send_update_with_states\u0027)"},{"line_number":1458,"context_line":"    def test_schedule_and_build_instances(self, mock_notify):"},{"line_number":1459,"context_line":"        # NOTE(melwitt): For some reason, this won\u0027t work with call_args. The"},{"line_number":1460,"context_line":"        # mock.call arg had a db_connection of None for Instance._context. But"},{"line_number":1461,"context_line":"        # mock.side_effect has a db_connection set."},{"line_number":1462,"context_line":"        def fake_notify(ctxt, instance, *args, **kwargs):"},{"line_number":1463,"context_line":"            # Assert the instance object is targeted to \u0027cell1\u0027 when going"},{"line_number":1464,"context_line":"            # through the notification code."},{"line_number":1465,"context_line":"            self.assertIsNotNone(ctxt.db_connection)"},{"line_number":1466,"context_line":"            self.assertIsNotNone(instance._context.db_connection)"},{"line_number":1467,"context_line":""},{"line_number":1468,"context_line":"        mock_notify.side_effect \u003d fake_notify"},{"line_number":1469,"context_line":""},{"line_number":1470,"context_line":"        instance_uuid \u003d self._do_schedule_and_build_instances_test("},{"line_number":1471,"context_line":"            self.params)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f4e5783_6db577ee","line":1468,"range":{"start_line":1459,"start_character":0,"end_line":1468,"end_character":45},"in_reply_to":"5f4e5783_fc06bf20","updated":"2017-10-10 14:46:52.000000000","message":"You are right sorry. \nMeanwhile I also figured out why your asserts sees the instance._context as targeted and why the call_args way does not show it that way.\nThe mock records what call was made with what parameters but I guess it only records the parameters as references (not as a deep copy). Then let the call return. Then later in the test you assert the parameters. Now the instance._context was targeted when the send_update_with_states was called but was untargeted when the context send_update_with_states returned and the contextmanager exited. In the call_args case the test  asserts the instace._context _after_ the contextmanager exited and therefore the instance._context is already untargeted. \nin the current test you force the mock to call your fake_notify _when_ the send_update_with_states is called therefore the fake_notify sees the instance._context as targeted.\n\nSorry for being picky here I just wanted to understand first why the call_args method doesn\u0027t work to see if it is not a problem outside of the test env.\n\nMaybe you can mention the above reason in your NOTE for the future reader.","commit_id":"f11d05e9013100339675e03e7b6df0957a65cbf5"}]}