)]}'
{"nova/cells/messaging.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"0c15eee0393833c4fb74889d0d65c5a1b97089bb","unresolved":false,"context_lines":[{"line_number":822,"context_line":"        instance.obj_reset_changes([\u0027metadata\u0027, \u0027system_metadata\u0027])"},{"line_number":823,"context_line":"        with instance.skip_cells_sync():"},{"line_number":824,"context_line":"            instance.save(expected_vm_state\u003dexpected_vm_state,"},{"line_number":825,"context_line":"                          expected_task_state\u003dexpected_task_state)"},{"line_number":826,"context_line":""},{"line_number":827,"context_line":"    def _call_compute_api_with_obj(self, ctxt, instance, method, *args,"},{"line_number":828,"context_line":"                                   **kwargs):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abe1de1_11045c7c","line":825,"updated":"2015-04-27 10:51:05.000000000","message":"is_update_from_api\u003dTrue here seems possibly cleaner? or maybe just something in the context that disables the cell sync?","commit_id":"4dc3b8c2f79e103774ecd827a091ed5d8c36f822"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"87848479024c5101ec58f0ec281d4cc35296cb30","unresolved":false,"context_lines":[{"line_number":822,"context_line":"        instance.obj_reset_changes([\u0027metadata\u0027, \u0027system_metadata\u0027])"},{"line_number":823,"context_line":"        with instance.skip_cells_sync():"},{"line_number":824,"context_line":"            instance.save(expected_vm_state\u003dexpected_vm_state,"},{"line_number":825,"context_line":"                          expected_task_state\u003dexpected_task_state)"},{"line_number":826,"context_line":""},{"line_number":827,"context_line":"    def _call_compute_api_with_obj(self, ctxt, instance, method, *args,"},{"line_number":828,"context_line":"                                   **kwargs):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abe1de1_8f48402f","line":825,"in_reply_to":"9abe1de1_11045c7c","updated":"2015-04-27 13:30:15.000000000","message":"See my reply to Dan on the other file.  Those are both decent options with some drawbacks so I\u0027m going to go in a different direction.","commit_id":"4dc3b8c2f79e103774ecd827a091ed5d8c36f822"}],"nova/objects/instance.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"07927fd82be6baf880496628a4da3eca60eb1e24","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        \u0027shutdown_terminate\u0027: fields.BooleanField(default\u003dFalse),"},{"line_number":228,"context_line":"        \u0027disable_terminate\u0027: fields.BooleanField(default\u003dFalse),"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"        \u0027cell_name\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"        \u0027metadata\u0027: fields.DictOfStringsField(),"},{"line_number":233,"context_line":"        \u0027system_metadata\u0027: fields.DictOfNullableStringsField(),"}],"source_content_type":"text/x-python","patch_set":2,"id":"bab921fb_5d7b878c","line":230,"updated":"2015-04-24 23:58:09.000000000","message":"Where did we land on just using something here? Sure seems like your  context manager could prefix or suffix this value with an @ or something to signal the same thing. Adding a new property here just for this is less than desirable :(","commit_id":"4dc3b8c2f79e103774ecd827a091ed5d8c36f822"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"405c57693fcf4cae6570e8e7d6540728399b4d8a","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        \u0027shutdown_terminate\u0027: fields.BooleanField(default\u003dFalse),"},{"line_number":228,"context_line":"        \u0027disable_terminate\u0027: fields.BooleanField(default\u003dFalse),"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"        \u0027cell_name\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"        \u0027metadata\u0027: fields.DictOfStringsField(),"},{"line_number":233,"context_line":"        \u0027system_metadata\u0027: fields.DictOfNullableStringsField(),"}],"source_content_type":"text/x-python","patch_set":2,"id":"7ad32947_f6d43548","line":230,"in_reply_to":"7ad32947_7c72517e","updated":"2015-04-27 17:08:03.000000000","message":"-N, where N is big :)\n\nIt sounds handy, but if it\u0027s not in fields, it\u0027s not tracked by versions, which means it can\u0027t go over the wire at all, IMHO.","commit_id":"4dc3b8c2f79e103774ecd827a091ed5d8c36f822"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8282e39437c86b0026938856b3d0fab55faa7679","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        \u0027shutdown_terminate\u0027: fields.BooleanField(default\u003dFalse),"},{"line_number":228,"context_line":"        \u0027disable_terminate\u0027: fields.BooleanField(default\u003dFalse),"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"        \u0027cell_name\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"        \u0027metadata\u0027: fields.DictOfStringsField(),"},{"line_number":233,"context_line":"        \u0027system_metadata\u0027: fields.DictOfNullableStringsField(),"}],"source_content_type":"text/x-python","patch_set":2,"id":"7ad32947_2d049313","line":230,"in_reply_to":"7ad32947_f6d43548","updated":"2015-04-27 20:53:41.000000000","message":"Right, fair point.","commit_id":"4dc3b8c2f79e103774ecd827a091ed5d8c36f822"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"c4cc87d500c516c06aaeb90b6ff8a4acbd0f2817","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        \u0027shutdown_terminate\u0027: fields.BooleanField(default\u003dFalse),"},{"line_number":228,"context_line":"        \u0027disable_terminate\u0027: fields.BooleanField(default\u003dFalse),"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"        \u0027cell_name\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"        \u0027metadata\u0027: fields.DictOfStringsField(),"},{"line_number":233,"context_line":"        \u0027system_metadata\u0027: fields.DictOfNullableStringsField(),"}],"source_content_type":"text/x-python","patch_set":2,"id":"7ad32947_7c72517e","line":230,"in_reply_to":"9abe1de1_2f37f4b1","updated":"2015-04-27 14:13:39.000000000","message":"Yeah, the problem you mention is not exactly because it has to be DB-backed, but rather that object serialization only accepts typed fields. As it would really be ugly to just add an indirection field for that change purpose, yeah I think modifying cell_name sounds unfortunately the best option.\n\nThe last option you didn\u0027t investigate is the price to pay for letting NovaObject.obj_to_primitive() accept extra fields...\n\n@Dan, thoughts on that ?","commit_id":"4dc3b8c2f79e103774ecd827a091ed5d8c36f822"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"87848479024c5101ec58f0ec281d4cc35296cb30","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        \u0027shutdown_terminate\u0027: fields.BooleanField(default\u003dFalse),"},{"line_number":228,"context_line":"        \u0027disable_terminate\u0027: fields.BooleanField(default\u003dFalse),"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"        \u0027cell_name\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"        \u0027metadata\u0027: fields.DictOfStringsField(),"},{"line_number":233,"context_line":"        \u0027system_metadata\u0027: fields.DictOfNullableStringsField(),"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abe1de1_2f37f4b1","line":230,"in_reply_to":"bab921fb_5d7b878c","updated":"2015-04-27 13:30:15.000000000","message":"This is what I was going to look at next.  I have four possible fixes in mind:\n\nThis change, adding a property.  Probably the worst option.  But this started as a local variable that wasn\u0027t in fields which wasn\u0027t too bad.  But of course it didn\u0027t remote which means it didn\u0027t work for conductor.  So this patch became a learning tool for me to mess with adding a field that isn\u0027t backed by the db.\n\nAdd a parameter to save().  Best balance of hacky and invasive I think, but still requires adding a basically already deprecated parameter.\n\nAdd something to the context.  If we were going to grow this pattern in a number of places that might be best, but is too much for just this one place IMO.\n\nSignal using cell_name.  Most hacky, but nothing outside of cells really needs to know about it.  This is what I\u0027m going to update this patch to now.","commit_id":"4dc3b8c2f79e103774ecd827a091ed5d8c36f822"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"da91a49f920c9335a6d4ca868f8d8211992dcd30","unresolved":false,"context_lines":[{"line_number":773,"context_line":"                self.cell_name is not None and"},{"line_number":774,"context_line":"                self.cell_name.startswith(\u0027!!\u0027)):"},{"line_number":775,"context_line":"            self._sync_cells \u003d False"},{"line_number":776,"context_line":"            self._normalize_cell_name()"},{"line_number":777,"context_line":""},{"line_number":778,"context_line":"        context \u003d self._context"},{"line_number":779,"context_line":"        cell_type \u003d cells_opts.get_cell_type()"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ad32947_1688d121","line":776,"updated":"2015-04-27 17:22:55.000000000","message":"Instead of doing this, you could make sync_cells a property, which just:\n\n return self.cell_name.startswith(\u0027!!\u0027)\"","commit_id":"9535d2ca807fcac1e804fd91c962d72b1cf8ae4e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"da91a49f920c9335a6d4ca868f8d8211992dcd30","unresolved":false,"context_lines":[{"line_number":795,"context_line":"                    cells_api.instance_update_from_api(context, stale_instance,"},{"line_number":796,"context_line":"                            expected_vm_state,"},{"line_number":797,"context_line":"                            expected_task_state,"},{"line_number":798,"context_line":"                            admin_state_reset)"},{"line_number":799,"context_line":"        else:"},{"line_number":800,"context_line":"            stale_instance \u003d None"},{"line_number":801,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7ad32947_b69d9d5b","line":798,"updated":"2015-04-27 17:22:55.000000000","message":"Since stale_instance is a clone, you could just always do:\n\n stale_instance.cell_name.replace(\u0027!!\u0027, \u0027\u0027)\n\n(do the right thing, just using that for simplicity)","commit_id":"9535d2ca807fcac1e804fd91c962d72b1cf8ae4e"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"512557cdbe617e36d341daac0537ddb93e62c6ff","unresolved":false,"context_lines":[{"line_number":795,"context_line":"                    cells_api.instance_update_from_api(context, stale_instance,"},{"line_number":796,"context_line":"                            expected_vm_state,"},{"line_number":797,"context_line":"                            expected_task_state,"},{"line_number":798,"context_line":"                            admin_state_reset)"},{"line_number":799,"context_line":"        else:"},{"line_number":800,"context_line":"            stale_instance \u003d None"},{"line_number":801,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7ad32947_8c48502f","line":798,"in_reply_to":"7ad32947_b69d9d5b","updated":"2015-04-27 18:33:36.000000000","message":"Fortunately, we know that if we made it past _sync_cells then cell_name isn\u0027t prefixed with anything and doesn\u0027t need cleaning up.","commit_id":"9535d2ca807fcac1e804fd91c962d72b1cf8ae4e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4bc8c5e952628626415de0f9ed7d04e7ae0ed06b","unresolved":false,"context_lines":[{"line_number":812,"context_line":"                    getattr(self, \u0027_save_%s\u0027 % field)(context)"},{"line_number":813,"context_line":"                except AttributeError:"},{"line_number":814,"context_line":"                    LOG.exception(_LE(\u0027No save handler for %s\u0027), field,"},{"line_number":815,"context_line":"                                  instance\u003dself)"},{"line_number":816,"context_line":"            elif field in changes:"},{"line_number":817,"context_line":"                updates[field] \u003d self[field]"},{"line_number":818,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7ad32947_71db8f1c","line":815,"updated":"2015-04-27 17:39:28.000000000","message":"You\u0027d also need an elif (cell_name) here, and just sanitize it with the same helper you use in the other two places. This would avoid modifying the instance itself, and just modify what we\u0027re slapping into the db.","commit_id":"9535d2ca807fcac1e804fd91c962d72b1cf8ae4e"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"512557cdbe617e36d341daac0537ddb93e62c6ff","unresolved":false,"context_lines":[{"line_number":869,"context_line":"        if stale_instance:"},{"line_number":870,"context_line":"            _handle_cell_update_from_api()"},{"line_number":871,"context_line":"        elif cell_type \u003d\u003d \u0027compute\u0027:"},{"line_number":872,"context_line":"            if self._sync_cells:"},{"line_number":873,"context_line":"                cells_api \u003d cells_rpcapi.CellsAPI()"},{"line_number":874,"context_line":"                cells_api.instance_update_at_top(context,"},{"line_number":875,"context_line":"                        base.obj_to_primitive(new_ref))"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ad32947_67762d40","line":872,"updated":"2015-04-27 18:33:36.000000000","message":"Unless we store a _sync_cells data member on the class this will always fail because it will be updated by db.instance_update_and_get_original.  Or cell_name could be remodified, but then it\u0027ll need to get cleaned up in a bit anyways, so adding a _sync_cells seems easiest.","commit_id":"9535d2ca807fcac1e804fd91c962d72b1cf8ae4e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e4c3f156a27f502a6fe9db4c6210e9eedd04066f","unresolved":false,"context_lines":[{"line_number":869,"context_line":"        if stale_instance:"},{"line_number":870,"context_line":"            _handle_cell_update_from_api()"},{"line_number":871,"context_line":"        elif cell_type \u003d\u003d \u0027compute\u0027:"},{"line_number":872,"context_line":"            if self._sync_cells:"},{"line_number":873,"context_line":"                cells_api \u003d cells_rpcapi.CellsAPI()"},{"line_number":874,"context_line":"                cells_api.instance_update_at_top(context,"},{"line_number":875,"context_line":"                        base.obj_to_primitive(new_ref))"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ad32947_d30dc038","line":872,"in_reply_to":"7ad32947_67762d40","updated":"2015-04-27 22:16:55.000000000","message":"well, it doesn\u0027t have to be on the class or instance, this is all within the save function, right?","commit_id":"9535d2ca807fcac1e804fd91c962d72b1cf8ae4e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"da91a49f920c9335a6d4ca868f8d8211992dcd30","unresolved":false,"context_lines":[{"line_number":872,"context_line":"            if self._sync_cells:"},{"line_number":873,"context_line":"                cells_api \u003d cells_rpcapi.CellsAPI()"},{"line_number":874,"context_line":"                cells_api.instance_update_at_top(context,"},{"line_number":875,"context_line":"                        base.obj_to_primitive(new_ref))"},{"line_number":876,"context_line":""},{"line_number":877,"context_line":"        notifications.send_update(context, old_ref, new_ref)"},{"line_number":878,"context_line":"        self.obj_reset_changes()"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ad32947_f6baf5bf","line":875,"updated":"2015-04-27 17:22:55.000000000","message":"Since you\u0027re passing the primitive, you could again just clean the name here. I think the goal is to stop sending the primitive, but you could do the same clone/modify as above.","commit_id":"9535d2ca807fcac1e804fd91c962d72b1cf8ae4e"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"512557cdbe617e36d341daac0537ddb93e62c6ff","unresolved":false,"context_lines":[{"line_number":872,"context_line":"            if self._sync_cells:"},{"line_number":873,"context_line":"                cells_api \u003d cells_rpcapi.CellsAPI()"},{"line_number":874,"context_line":"                cells_api.instance_update_at_top(context,"},{"line_number":875,"context_line":"                        base.obj_to_primitive(new_ref))"},{"line_number":876,"context_line":""},{"line_number":877,"context_line":"        notifications.send_update(context, old_ref, new_ref)"},{"line_number":878,"context_line":"        self.obj_reset_changes()"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ad32947_87a02992","line":875,"in_reply_to":"7ad32947_f6baf5bf","updated":"2015-04-27 18:33:36.000000000","message":"Right, once this is in place we can use objects in instance_update_at_top without looping.\n\nAs above, if we\u0027re past _sync_cells we know that cell_name is unmodified.","commit_id":"9535d2ca807fcac1e804fd91c962d72b1cf8ae4e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e8608514bb2fb961f764b6b33c9661d025564320","unresolved":false,"context_lines":[{"line_number":769,"context_line":"        \"\"\""},{"line_number":770,"context_line":"        # Store this on the class because _cell_name_blocks_sync is useless"},{"line_number":771,"context_line":"        # after the db update call below."},{"line_number":772,"context_line":"        self._sync_cells \u003d not self._cell_name_blocks_sync()"},{"line_number":773,"context_line":""},{"line_number":774,"context_line":"        context \u003d self._context"},{"line_number":775,"context_line":"        cell_type \u003d cells_opts.get_cell_type()"}],"source_content_type":"text/x-python","patch_set":5,"id":"7ad32947_339bd4cb","line":772,"updated":"2015-04-27 22:24:09.000000000","message":"You always overwrite _sync_cells here, based on the name (which is good, IMHO). Thus, I don\u0027t think you need it set in __init__, nor on the object instance at all, right?","commit_id":"0d11b81074d41b60038e06255ba592f148210238"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e8608514bb2fb961f764b6b33c9661d025564320","unresolved":false,"context_lines":[{"line_number":1116,"context_line":"        try:"},{"line_number":1117,"context_line":"            yield"},{"line_number":1118,"context_line":"        finally:"},{"line_number":1119,"context_line":"            self._sync_cells \u003d orig_sync_cells"},{"line_number":1120,"context_line":"            self._normalize_cell_name()"},{"line_number":1121,"context_line":""},{"line_number":1122,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7ad32947_3382b453","line":1119,"updated":"2015-04-27 22:24:09.000000000","message":"Who are you setting/restoring this for? You\u0027re setting the cell name to include the !!, and _sync_cells is overwritten in save() immediately. Can we just remove it?","commit_id":"0d11b81074d41b60038e06255ba592f148210238"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"82446bb276397946e11b7679df536023d959ef73","unresolved":false,"context_lines":[{"line_number":809,"context_line":"                    LOG.exception(_LE(\u0027No save handler for %s\u0027), field,"},{"line_number":810,"context_line":"                                  instance\u003dself)"},{"line_number":811,"context_line":"            elif field in changes:"},{"line_number":812,"context_line":"                if (field \u003d\u003d \u0027cell_name\u0027 and self[field] is not None and"},{"line_number":813,"context_line":"                        self[field].startswith(\u0027!!\u0027)):"},{"line_number":814,"context_line":"                    updates[field] \u003d self[field].replace(\u0027!!\u0027, \u0027\u0027, 1)"},{"line_number":815,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":6,"id":"7ad32947_55186d84","line":812,"updated":"2015-04-28 08:37:28.000000000","message":"stupid question but why are you using the dict compat and not getattr ?","commit_id":"94b2c0d547b6e06542a986fee9338e63bab0127e"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"d8edbb8708e0d0aa0d8be723d4c29de2b06e88b8","unresolved":false,"context_lines":[{"line_number":809,"context_line":"                    LOG.exception(_LE(\u0027No save handler for %s\u0027), field,"},{"line_number":810,"context_line":"                                  instance\u003dself)"},{"line_number":811,"context_line":"            elif field in changes:"},{"line_number":812,"context_line":"                if (field \u003d\u003d \u0027cell_name\u0027 and self[field] is not None and"},{"line_number":813,"context_line":"                        self[field].startswith(\u0027!!\u0027)):"},{"line_number":814,"context_line":"                    updates[field] \u003d self[field].replace(\u0027!!\u0027, \u0027\u0027, 1)"},{"line_number":815,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":6,"id":"7ad32947_81747d9c","line":812,"in_reply_to":"7ad32947_55186d84","updated":"2015-04-28 12:05:43.000000000","message":"Because that\u0027s what was already here.  We\u0027ll need to switch over before removing the dict compatibility mixin, but using a consistent access here makes it more readable IMO.","commit_id":"94b2c0d547b6e06542a986fee9338e63bab0127e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"82446bb276397946e11b7679df536023d959ef73","unresolved":false,"context_lines":[{"line_number":810,"context_line":"                                  instance\u003dself)"},{"line_number":811,"context_line":"            elif field in changes:"},{"line_number":812,"context_line":"                if (field \u003d\u003d \u0027cell_name\u0027 and self[field] is not None and"},{"line_number":813,"context_line":"                        self[field].startswith(\u0027!!\u0027)):"},{"line_number":814,"context_line":"                    updates[field] \u003d self[field].replace(\u0027!!\u0027, \u0027\u0027, 1)"},{"line_number":815,"context_line":"                else:"},{"line_number":816,"context_line":"                    updates[field] \u003d self[field]"}],"source_content_type":"text/x-python","patch_set":6,"id":"7ad32947_f5165954","line":813,"updated":"2015-04-28 08:37:28.000000000","message":"nit: see my comment below, maybe an helper in cells.utils could be safer","commit_id":"94b2c0d547b6e06542a986fee9338e63bab0127e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"82446bb276397946e11b7679df536023d959ef73","unresolved":false,"context_lines":[{"line_number":1075,"context_line":"    def _cell_name_blocks_sync(self):"},{"line_number":1076,"context_line":"        if (self.obj_attr_is_set(\u0027cell_name\u0027) and"},{"line_number":1077,"context_line":"                self.cell_name is not None and"},{"line_number":1078,"context_line":"                self.cell_name.startswith(\u0027!!\u0027)):"},{"line_number":1079,"context_line":"            return True"},{"line_number":1080,"context_line":"        return False"},{"line_number":1081,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7ad32947_75036910","line":1078,"updated":"2015-04-28 08:37:28.000000000","message":"nit : maybe a constant in the cells.utils instead of \u0027!!\u0027 would just make sure that it would not be regressed if we would change how cell paths are written","commit_id":"94b2c0d547b6e06542a986fee9338e63bab0127e"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"d8edbb8708e0d0aa0d8be723d4c29de2b06e88b8","unresolved":false,"context_lines":[{"line_number":1075,"context_line":"    def _cell_name_blocks_sync(self):"},{"line_number":1076,"context_line":"        if (self.obj_attr_is_set(\u0027cell_name\u0027) and"},{"line_number":1077,"context_line":"                self.cell_name is not None and"},{"line_number":1078,"context_line":"                self.cell_name.startswith(\u0027!!\u0027)):"},{"line_number":1079,"context_line":"            return True"},{"line_number":1080,"context_line":"        return False"},{"line_number":1081,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7ad32947_61528115","line":1078,"in_reply_to":"7ad32947_75036910","updated":"2015-04-28 12:05:43.000000000","message":"Sure.","commit_id":"94b2c0d547b6e06542a986fee9338e63bab0127e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"82446bb276397946e11b7679df536023d959ef73","unresolved":false,"context_lines":[{"line_number":1086,"context_line":"            return"},{"line_number":1087,"context_line":"        cn_changed \u003d \u0027cell_name\u0027 in self.obj_what_changed()"},{"line_number":1088,"context_line":"        if self.cell_name.startswith(\u0027!!\u0027):"},{"line_number":1089,"context_line":"            self.cell_name \u003d self.cell_name.replace(\u0027!!\u0027, \u0027\u0027, 1)"},{"line_number":1090,"context_line":"            # cell_name is not normally an empty string, this means it was None"},{"line_number":1091,"context_line":"            # before !! was applied."},{"line_number":1092,"context_line":"            if len(self.cell_name) \u003d\u003d 0:"}],"source_content_type":"text/x-python","patch_set":6,"id":"7ad32947_1009e32c","line":1089,"updated":"2015-04-28 08:37:28.000000000","message":"same remark as above","commit_id":"94b2c0d547b6e06542a986fee9338e63bab0127e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"82446bb276397946e11b7679df536023d959ef73","unresolved":false,"context_lines":[{"line_number":1090,"context_line":"            # cell_name is not normally an empty string, this means it was None"},{"line_number":1091,"context_line":"            # before !! was applied."},{"line_number":1092,"context_line":"            if len(self.cell_name) \u003d\u003d 0:"},{"line_number":1093,"context_line":"                self.cell_name \u003d None"},{"line_number":1094,"context_line":"        if not cn_changed:"},{"line_number":1095,"context_line":"            self.obj_reset_changes([\u0027cell_name\u0027])"},{"line_number":1096,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7ad32947_b0da2f77","line":1093,"updated":"2015-04-28 08:37:28.000000000","message":"technically, if the cell_name attribute was formerly unset, it\u0027s now set to None but I think that\u0027s not a corner case","commit_id":"94b2c0d547b6e06542a986fee9338e63bab0127e"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"d8edbb8708e0d0aa0d8be723d4c29de2b06e88b8","unresolved":false,"context_lines":[{"line_number":1090,"context_line":"            # cell_name is not normally an empty string, this means it was None"},{"line_number":1091,"context_line":"            # before !! was applied."},{"line_number":1092,"context_line":"            if len(self.cell_name) \u003d\u003d 0:"},{"line_number":1093,"context_line":"                self.cell_name \u003d None"},{"line_number":1094,"context_line":"        if not cn_changed:"},{"line_number":1095,"context_line":"            self.obj_reset_changes([\u0027cell_name\u0027])"},{"line_number":1096,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7ad32947_c11dd567","line":1093,"in_reply_to":"7ad32947_b0da2f77","updated":"2015-04-28 12:05:43.000000000","message":"Yeah, but nothing should rely on there being a difference between cell_name being None or unset.  It\u0027s not a lazy loadable attribute.","commit_id":"94b2c0d547b6e06542a986fee9338e63bab0127e"}],"nova/tests/unit/cells/test_cells_messaging.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"52ef8e26453d0f2d06e6955caa931899705154e4","unresolved":false,"context_lines":[{"line_number":1246,"context_line":"            self.tgt_methods_cls.instance_update_from_api(message, instance,"},{"line_number":1247,"context_line":"                    expected_vm_state\u003d\u0027exp_vm\u0027, expected_task_state\u003d\u0027exp_task\u0027,"},{"line_number":1248,"context_line":"                    admin_state_reset\u003dFalse)"},{"line_number":1249,"context_line":"            self.assertEqual(0, inst_upd_at_top.call_count)"},{"line_number":1250,"context_line":""},{"line_number":1251,"context_line":"    def _test_instance_action_method(self, method, args, kwargs,"},{"line_number":1252,"context_line":"                                     expected_args, expected_kwargs,"}],"source_content_type":"text/x-python","patch_set":6,"id":"7ad32947_3e883b22","line":1249,"updated":"2015-04-27 23:23:23.000000000","message":"This is cool, but it doesn\u0027t really make sure that it\u0027s calling the context manager. Like, this could break later if we break something that makes instance erroneously not send updates for some reason.\n\nAnyway, it obviously serves the purpose here.","commit_id":"94b2c0d547b6e06542a986fee9338e63bab0127e"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"d8edbb8708e0d0aa0d8be723d4c29de2b06e88b8","unresolved":false,"context_lines":[{"line_number":1246,"context_line":"            self.tgt_methods_cls.instance_update_from_api(message, instance,"},{"line_number":1247,"context_line":"                    expected_vm_state\u003d\u0027exp_vm\u0027, expected_task_state\u003d\u0027exp_task\u0027,"},{"line_number":1248,"context_line":"                    admin_state_reset\u003dFalse)"},{"line_number":1249,"context_line":"            self.assertEqual(0, inst_upd_at_top.call_count)"},{"line_number":1250,"context_line":""},{"line_number":1251,"context_line":"    def _test_instance_action_method(self, method, args, kwargs,"},{"line_number":1252,"context_line":"                                     expected_args, expected_kwargs,"}],"source_content_type":"text/x-python","patch_set":6,"id":"7ad32947_e19331a2","line":1249,"in_reply_to":"7ad32947_30a61fc2","updated":"2015-04-28 12:05:43.000000000","message":"Fair points.  I can add another test for the contextmanager.","commit_id":"94b2c0d547b6e06542a986fee9338e63bab0127e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"82446bb276397946e11b7679df536023d959ef73","unresolved":false,"context_lines":[{"line_number":1246,"context_line":"            self.tgt_methods_cls.instance_update_from_api(message, instance,"},{"line_number":1247,"context_line":"                    expected_vm_state\u003d\u0027exp_vm\u0027, expected_task_state\u003d\u0027exp_task\u0027,"},{"line_number":1248,"context_line":"                    admin_state_reset\u003dFalse)"},{"line_number":1249,"context_line":"            self.assertEqual(0, inst_upd_at_top.call_count)"},{"line_number":1250,"context_line":""},{"line_number":1251,"context_line":"    def _test_instance_action_method(self, method, args, kwargs,"},{"line_number":1252,"context_line":"                                     expected_args, expected_kwargs,"}],"source_content_type":"text/x-python","patch_set":6,"id":"7ad32947_30a61fc2","line":1249,"in_reply_to":"7ad32947_3e883b22","updated":"2015-04-28 08:37:28.000000000","message":"Maybe a simplier approach would be to mock the contextmanager directly and only make sure it\u0027s called by checking the __exit__ method()\n\nThe whole testing of the contextmanager could then be happening on the Instance testcase.\n\nBut that\u0027s just something for correctly isolating tests, I don\u0027t see a point on arguing about your proposal because your test is nice for getting an higher level of check and increases readability about what is your goal","commit_id":"94b2c0d547b6e06542a986fee9338e63bab0127e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1a9b16cf04276cbe43e2a8f691bfcdd90f324ab3","unresolved":false,"context_lines":[{"line_number":1243,"context_line":"            self.assertRaises(self.tgt_methods_cls.instance_update_from_api,"},{"line_number":1244,"context_line":"                    message, instance, expected_vm_state\u003d\u0027exp_vm\u0027,"},{"line_number":1245,"context_line":"                    expected_task_state\u003d\u0027exp_task\u0027, admin_state_reset\u003dFalse)"},{"line_number":1246,"context_line":"            mock_sync.assert_has_calls([mock.call()])"},{"line_number":1247,"context_line":"        self.assertEqual(self.tgt_cell_name, instance.cell_name)"},{"line_number":1248,"context_line":""},{"line_number":1249,"context_line":"    @mock.patch.object(db, \u0027instance_update_and_get_original\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5ad0253b_3e2cc8aa","line":1246,"updated":"2015-04-28 19:14:56.000000000","message":"It looks like nothing is calling _ensure_skip_cells_sync_called to do this part of the test.","commit_id":"f23a73b57b2e76ead87d3e1cdda3e2e1c160c960"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"96327cfd5e86b4cfc73ac35a8c25727a6f45281c","unresolved":false,"context_lines":[{"line_number":1243,"context_line":"            self.assertRaises(self.tgt_methods_cls.instance_update_from_api,"},{"line_number":1244,"context_line":"                    message, instance, expected_vm_state\u003d\u0027exp_vm\u0027,"},{"line_number":1245,"context_line":"                    expected_task_state\u003d\u0027exp_task\u0027, admin_state_reset\u003dFalse)"},{"line_number":1246,"context_line":"            mock_sync.assert_has_calls([mock.call()])"},{"line_number":1247,"context_line":"        self.assertEqual(self.tgt_cell_name, instance.cell_name)"},{"line_number":1248,"context_line":""},{"line_number":1249,"context_line":"    @mock.patch.object(db, \u0027instance_update_and_get_original\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5ad0253b_9713bf5c","line":1246,"in_reply_to":"5ad0253b_142c4d49","updated":"2015-04-28 19:43:38.000000000","message":"Damn","commit_id":"f23a73b57b2e76ead87d3e1cdda3e2e1c160c960"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1f6b165add96f0869cf9cead3bbe178db6f572d9","unresolved":false,"context_lines":[{"line_number":1243,"context_line":"            self.assertRaises(self.tgt_methods_cls.instance_update_from_api,"},{"line_number":1244,"context_line":"                    message, instance, expected_vm_state\u003d\u0027exp_vm\u0027,"},{"line_number":1245,"context_line":"                    expected_task_state\u003d\u0027exp_task\u0027, admin_state_reset\u003dFalse)"},{"line_number":1246,"context_line":"            mock_sync.assert_has_calls([mock.call()])"},{"line_number":1247,"context_line":"        self.assertEqual(self.tgt_cell_name, instance.cell_name)"},{"line_number":1248,"context_line":""},{"line_number":1249,"context_line":"    @mock.patch.object(db, \u0027instance_update_and_get_original\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5ad0253b_411d65bc","line":1246,"in_reply_to":"5ad0253b_3e2cc8aa","updated":"2015-04-28 19:24:02.000000000","message":"Doh!","commit_id":"f23a73b57b2e76ead87d3e1cdda3e2e1c160c960"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"04a9df0130204dbdecbeb4248b1bd6fe1664e1b8","unresolved":false,"context_lines":[{"line_number":1243,"context_line":"            self.assertRaises(self.tgt_methods_cls.instance_update_from_api,"},{"line_number":1244,"context_line":"                    message, instance, expected_vm_state\u003d\u0027exp_vm\u0027,"},{"line_number":1245,"context_line":"                    expected_task_state\u003d\u0027exp_task\u0027, admin_state_reset\u003dFalse)"},{"line_number":1246,"context_line":"            mock_sync.assert_has_calls([mock.call()])"},{"line_number":1247,"context_line":"        self.assertEqual(self.tgt_cell_name, instance.cell_name)"},{"line_number":1248,"context_line":""},{"line_number":1249,"context_line":"    @mock.patch.object(db, \u0027instance_update_and_get_original\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5ad0253b_142c4d49","line":1246,"in_reply_to":"5ad0253b_411d65bc","updated":"2015-04-28 19:35:24.000000000","message":"Woops.","commit_id":"f23a73b57b2e76ead87d3e1cdda3e2e1c160c960"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"39e74bcd744804202ea64b33ec74f0c8949d62c8","unresolved":false,"context_lines":[{"line_number":1246,"context_line":"                    expected_task_state\u003d\u0027exp_task\u0027, admin_state_reset\u003dFalse)"},{"line_number":1247,"context_line":"            mock_sync.assert_has_calls([mock.call()])"},{"line_number":1248,"context_line":""},{"line_number":1249,"context_line":"        _ensure_skip_cells_sync_called()"},{"line_number":1250,"context_line":"        self.assertEqual(self.tgt_cell_name, instance.cell_name)"},{"line_number":1251,"context_line":""},{"line_number":1252,"context_line":"    @mock.patch.object(db, \u0027instance_update_and_get_original\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5ad0253b_29d80753","line":1249,"updated":"2015-04-28 22:36:02.000000000","message":"So you figure maybe we should run this test now? Meh *shrug* :)","commit_id":"6d6fdeeff880cc5fe8edcc24ef94d3bc4ca924e2"}],"nova/tests/unit/objects/test_instance.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"82446bb276397946e11b7679df536023d959ef73","unresolved":false,"context_lines":[{"line_number":496,"context_line":"    @mock.patch.object(cells_rpcapi.CellsAPI, \u0027instance_update_from_api\u0027)"},{"line_number":497,"context_line":"    @mock.patch.object(cells_rpcapi.CellsAPI, \u0027instance_update_at_top\u0027)"},{"line_number":498,"context_line":"    @mock.patch.object(db, \u0027instance_update_and_get_original\u0027)"},{"line_number":499,"context_line":"    def _test_skip_cells_sync_helper(self, mock_db_update, mock_update_at_top,"},{"line_number":500,"context_line":"            mock_update_from_api, cell_type):"},{"line_number":501,"context_line":"        self.flags(enable\u003dTrue, cell_type\u003dcell_type, group\u003d\u0027cells\u0027)"},{"line_number":502,"context_line":"        inst \u003d fake_instance.fake_instance_obj(self.context, cell_name\u003d\u0027fake\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"7ad32947_7055b71b","line":499,"updated":"2015-04-28 08:37:28.000000000","message":"here, you\u0027re writing the unittest I was describing...","commit_id":"94b2c0d547b6e06542a986fee9338e63bab0127e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"82446bb276397946e11b7679df536023d959ef73","unresolved":false,"context_lines":[{"line_number":513,"context_line":"        with inst.skip_cells_sync():"},{"line_number":514,"context_line":"            inst.save()"},{"line_number":515,"context_line":""},{"line_number":516,"context_line":"        self.assertEqual(0, mock_update_at_top.call_count)"},{"line_number":517,"context_line":"        self.assertEqual(0, mock_update_from_api.call_count)"},{"line_number":518,"context_line":""},{"line_number":519,"context_line":"        inst.vm_state \u003d \u0027bar\u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"7ad32947_7a047514","line":516,"updated":"2015-04-28 08:37:28.000000000","message":"nit, we prefer using :\n mock_update_at_top.assert_has_calls()","commit_id":"94b2c0d547b6e06542a986fee9338e63bab0127e"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"d8edbb8708e0d0aa0d8be723d4c29de2b06e88b8","unresolved":false,"context_lines":[{"line_number":513,"context_line":"        with inst.skip_cells_sync():"},{"line_number":514,"context_line":"            inst.save()"},{"line_number":515,"context_line":""},{"line_number":516,"context_line":"        self.assertEqual(0, mock_update_at_top.call_count)"},{"line_number":517,"context_line":"        self.assertEqual(0, mock_update_from_api.call_count)"},{"line_number":518,"context_line":""},{"line_number":519,"context_line":"        inst.vm_state \u003d \u0027bar\u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"7ad32947_c1e7153f","line":516,"in_reply_to":"7ad32947_7a047514","updated":"2015-04-28 12:05:43.000000000","message":"Okay.  I\u0027ve never seen that usage, but either works for me.","commit_id":"94b2c0d547b6e06542a986fee9338e63bab0127e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"82446bb276397946e11b7679df536023d959ef73","unresolved":false,"context_lines":[{"line_number":519,"context_line":"        inst.vm_state \u003d \u0027bar\u0027"},{"line_number":520,"context_line":"        inst.task_state \u003d \u0027foo\u0027"},{"line_number":521,"context_line":""},{"line_number":522,"context_line":"        update_from_api \u003d {\u0027call_count\u0027: 0}"},{"line_number":523,"context_line":""},{"line_number":524,"context_line":"        def fake_update_from_api(self2, context, instance, expected_vm_state,"},{"line_number":525,"context_line":"                expected_task_state, admin_state_reset, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":6,"id":"7ad32947_fa93e5a1","line":522,"updated":"2015-04-28 08:37:28.000000000","message":"nit: you don\u0027t need a dict, just an integer","commit_id":"94b2c0d547b6e06542a986fee9338e63bab0127e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b355db05d6ac1c9c8474787c5b8558c258ec85f9","unresolved":false,"context_lines":[{"line_number":514,"context_line":"            inst.save()"},{"line_number":515,"context_line":""},{"line_number":516,"context_line":"        mock_update_at_top.assert_has_calls([])"},{"line_number":517,"context_line":"        mock_update_from_api.assert_has_calls([])"},{"line_number":518,"context_line":""},{"line_number":519,"context_line":"        inst.vm_state \u003d \u0027bar\u0027"},{"line_number":520,"context_line":"        inst.task_state \u003d \u0027foo\u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"5ad0253b_55c76c45","line":517,"updated":"2015-04-28 15:46:00.000000000","message":"You can also do:\n\n self.assertFalse(mock_update_from_api.called)","commit_id":"f23a73b57b2e76ead87d3e1cdda3e2e1c160c960"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1a9b16cf04276cbe43e2a8f691bfcdd90f324ab3","unresolved":false,"context_lines":[{"line_number":514,"context_line":"            inst.save()"},{"line_number":515,"context_line":""},{"line_number":516,"context_line":"        mock_update_at_top.assert_has_calls([])"},{"line_number":517,"context_line":"        mock_update_from_api.assert_has_calls([])"},{"line_number":518,"context_line":""},{"line_number":519,"context_line":"        inst.vm_state \u003d \u0027bar\u0027"},{"line_number":520,"context_line":"        inst.task_state \u003d \u0027foo\u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"5ad0253b_ca562f06","line":517,"in_reply_to":"5ad0253b_55c76c45","updated":"2015-04-28 19:14:56.000000000","message":"assertFalse(called) is my preferred way","commit_id":"f23a73b57b2e76ead87d3e1cdda3e2e1c160c960"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1a9b16cf04276cbe43e2a8f691bfcdd90f324ab3","unresolved":false,"context_lines":[{"line_number":519,"context_line":"        inst.vm_state \u003d \u0027bar\u0027"},{"line_number":520,"context_line":"        inst.task_state \u003d \u0027foo\u0027"},{"line_number":521,"context_line":""},{"line_number":522,"context_line":"        update_from_api \u003d {\u0027call_count\u0027: 0}"},{"line_number":523,"context_line":""},{"line_number":524,"context_line":"        def fake_update_from_api(self2, context, instance, expected_vm_state,"},{"line_number":525,"context_line":"                expected_task_state, admin_state_reset, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":7,"id":"5ad0253b_619f4160","line":522,"updated":"2015-04-28 19:14:56.000000000","message":"Did you do this to avoid having for example \"as mock_update_from_api2\" when using the context manager and letting the mock keep the call_count?","commit_id":"f23a73b57b2e76ead87d3e1cdda3e2e1c160c960"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1f6b165add96f0869cf9cead3bbe178db6f572d9","unresolved":false,"context_lines":[{"line_number":519,"context_line":"        inst.vm_state \u003d \u0027bar\u0027"},{"line_number":520,"context_line":"        inst.task_state \u003d \u0027foo\u0027"},{"line_number":521,"context_line":""},{"line_number":522,"context_line":"        update_from_api \u003d {\u0027call_count\u0027: 0}"},{"line_number":523,"context_line":""},{"line_number":524,"context_line":"        def fake_update_from_api(self2, context, instance, expected_vm_state,"},{"line_number":525,"context_line":"                expected_task_state, admin_state_reset, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":7,"id":"5ad0253b_743ee1b9","line":522,"in_reply_to":"5ad0253b_619f4160","updated":"2015-04-28 19:24:02.000000000","message":"I figured it was because he didn\u0027t realize he could use the rest of the mock goo (like asserting calls) when he provided a side_effect.\n\nProbably better to use the mock machinery, but...this does work :)","commit_id":"f23a73b57b2e76ead87d3e1cdda3e2e1c160c960"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"04a9df0130204dbdecbeb4248b1bd6fe1664e1b8","unresolved":false,"context_lines":[{"line_number":519,"context_line":"        inst.vm_state \u003d \u0027bar\u0027"},{"line_number":520,"context_line":"        inst.task_state \u003d \u0027foo\u0027"},{"line_number":521,"context_line":""},{"line_number":522,"context_line":"        update_from_api \u003d {\u0027call_count\u0027: 0}"},{"line_number":523,"context_line":""},{"line_number":524,"context_line":"        def fake_update_from_api(self2, context, instance, expected_vm_state,"},{"line_number":525,"context_line":"                expected_task_state, admin_state_reset, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":7,"id":"5ad0253b_b4eb59b0","line":522,"in_reply_to":"5ad0253b_743ee1b9","updated":"2015-04-28 19:35:24.000000000","message":"I tried that and it didn\u0027t work in this case.  The mock docs indicate that this usage means instance_update_from_api is now this method rather than a MagicMock so the standard mock goo wouldn\u0027t work.  There\u0027s probably a convoluted way to do this in a more mock friendly way though.","commit_id":"f23a73b57b2e76ead87d3e1cdda3e2e1c160c960"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6432d2ec585c4c752905f8cba130f9b96b3ed895","unresolved":false,"context_lines":[{"line_number":519,"context_line":"        inst.vm_state \u003d \u0027bar\u0027"},{"line_number":520,"context_line":"        inst.task_state \u003d \u0027foo\u0027"},{"line_number":521,"context_line":""},{"line_number":522,"context_line":"        update_from_api \u003d {\u0027call_count\u0027: 0}"},{"line_number":523,"context_line":""},{"line_number":524,"context_line":"        def fake_update_from_api(self2, context, instance, expected_vm_state,"},{"line_number":525,"context_line":"                expected_task_state, admin_state_reset, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":7,"id":"5ad0253b_d756d705","line":522,"in_reply_to":"5ad0253b_b4eb59b0","updated":"2015-04-28 19:44:45.000000000","message":"Okay, that makes sense. For the sake of discussion, I think you could set the second mock side_effect \u003d fake_update_from_api instead of doing the mock.patch.object(..., ..., fake_update_from_api) and have the mock machinery work.","commit_id":"f23a73b57b2e76ead87d3e1cdda3e2e1c160c960"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"3b27e7b32bd693540ae1b453cbcaad6bd70a3253","unresolved":false,"context_lines":[{"line_number":519,"context_line":"        inst.vm_state \u003d \u0027bar\u0027"},{"line_number":520,"context_line":"        inst.task_state \u003d \u0027foo\u0027"},{"line_number":521,"context_line":""},{"line_number":522,"context_line":"        update_from_api \u003d {\u0027call_count\u0027: 0}"},{"line_number":523,"context_line":""},{"line_number":524,"context_line":"        def fake_update_from_api(self2, context, instance, expected_vm_state,"},{"line_number":525,"context_line":"                expected_task_state, admin_state_reset, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5ad0253b_923e2db7","line":522,"updated":"2015-04-28 20:09:54.000000000","message":"as I said in PS6, why not :\n\n  update_from_api \u003d 0\n\n?\nI can\u0027t find why you need a dict since you only have one key","commit_id":"f7aab91ff34553c15e14d63477019013f2759923"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f2a149ca1aa7167e3994113d004659ea84cc6be3","unresolved":false,"context_lines":[{"line_number":519,"context_line":"        inst.vm_state \u003d \u0027bar\u0027"},{"line_number":520,"context_line":"        inst.task_state \u003d \u0027foo\u0027"},{"line_number":521,"context_line":""},{"line_number":522,"context_line":"        update_from_api \u003d {\u0027call_count\u0027: 0}"},{"line_number":523,"context_line":""},{"line_number":524,"context_line":"        def fake_update_from_api(self2, context, instance, expected_vm_state,"},{"line_number":525,"context_line":"                expected_task_state, admin_state_reset, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5ad0253b_68771e03","line":522,"in_reply_to":"5ad0253b_88a03a92","updated":"2015-04-28 20:58:34.000000000","message":"Right, this has to be something that is defined that you\u0027re modifying, not an assignment, else scope nails you. There are examples of single-key dicts all over the tests for this reason :)","commit_id":"f7aab91ff34553c15e14d63477019013f2759923"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"455011e330e5f63b6094562f0a8a1c71a8b0ab5a","unresolved":false,"context_lines":[{"line_number":519,"context_line":"        inst.vm_state \u003d \u0027bar\u0027"},{"line_number":520,"context_line":"        inst.task_state \u003d \u0027foo\u0027"},{"line_number":521,"context_line":""},{"line_number":522,"context_line":"        update_from_api \u003d {\u0027call_count\u0027: 0}"},{"line_number":523,"context_line":""},{"line_number":524,"context_line":"        def fake_update_from_api(self2, context, instance, expected_vm_state,"},{"line_number":525,"context_line":"                expected_task_state, admin_state_reset, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5ad0253b_88a03a92","line":522,"in_reply_to":"5ad0253b_923e2db7","updated":"2015-04-28 20:56:53.000000000","message":"If not a dict, would have to use the global keyword on the variable. I\u0027m not sure if there might be any other problems with the scoping","commit_id":"f7aab91ff34553c15e14d63477019013f2759923"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"5126f21b8ab562c9478ad20daa0d9c72cd86a19c","unresolved":false,"context_lines":[{"line_number":519,"context_line":"        inst.vm_state \u003d \u0027bar\u0027"},{"line_number":520,"context_line":"        inst.task_state \u003d \u0027foo\u0027"},{"line_number":521,"context_line":""},{"line_number":522,"context_line":"        update_from_api \u003d {\u0027call_count\u0027: 0}"},{"line_number":523,"context_line":""},{"line_number":524,"context_line":"        def fake_update_from_api(self2, context, instance, expected_vm_state,"},{"line_number":525,"context_line":"                expected_task_state, admin_state_reset, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5ad0253b_d22a652c","line":522,"in_reply_to":"5ad0253b_923e2db7","updated":"2015-04-28 20:13:41.000000000","message":"My flake8 checker doesn\u0027t like updating a call_count variable on line 527 that is defined out here.  So this is mainly to appease it.  But following melwitts suggestion I have a better way to do this.","commit_id":"f7aab91ff34553c15e14d63477019013f2759923"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"3b27e7b32bd693540ae1b453cbcaad6bd70a3253","unresolved":false,"context_lines":[{"line_number":533,"context_line":"                \u0027instance_update_from_api\u0027, fake_update_from_api):"},{"line_number":534,"context_line":"            inst.save()"},{"line_number":535,"context_line":""},{"line_number":536,"context_line":"        self.assertEqual(\u0027foo!bar@baz\u0027, inst.cell_name)"},{"line_number":537,"context_line":"        if cell_type \u003d\u003d \u0027compute\u0027:"},{"line_number":538,"context_line":"            self.assertEqual(1, mock_update_at_top.call_count)"},{"line_number":539,"context_line":"            mock_update_at_top.assert_called_once_with(self.context,"}],"source_content_type":"text/x-python","patch_set":8,"id":"5ad0253b_5781a78e","line":536,"updated":"2015-04-28 20:09:54.000000000","message":"technically the same than L526 so not necessary right ?","commit_id":"f7aab91ff34553c15e14d63477019013f2759923"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"5126f21b8ab562c9478ad20daa0d9c72cd86a19c","unresolved":false,"context_lines":[{"line_number":533,"context_line":"                \u0027instance_update_from_api\u0027, fake_update_from_api):"},{"line_number":534,"context_line":"            inst.save()"},{"line_number":535,"context_line":""},{"line_number":536,"context_line":"        self.assertEqual(\u0027foo!bar@baz\u0027, inst.cell_name)"},{"line_number":537,"context_line":"        if cell_type \u003d\u003d \u0027compute\u0027:"},{"line_number":538,"context_line":"            self.assertEqual(1, mock_update_at_top.call_count)"},{"line_number":539,"context_line":"            mock_update_at_top.assert_called_once_with(self.context,"}],"source_content_type":"text/x-python","patch_set":8,"id":"5ad0253b_12bfbdd1","line":536,"in_reply_to":"5ad0253b_5781a78e","updated":"2015-04-28 20:13:41.000000000","message":"Line 526 is run when the context manager is active, and this is run after.  So they do check slightly different things.","commit_id":"f7aab91ff34553c15e14d63477019013f2759923"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"049000bed5f5eebdbc9431d66dc534d435944ec2","unresolved":false,"context_lines":[{"line_number":539,"context_line":"        elif cell_type \u003d\u003d \u0027api\u0027:"},{"line_number":540,"context_line":"            self.assertFalse(mock_update_at_top.called)"},{"line_number":541,"context_line":"            fake_update_from_api.assert_called_once_with(self.context,"},{"line_number":542,"context_line":"                    mock.ANY, None, None, False)"},{"line_number":543,"context_line":""},{"line_number":544,"context_line":"        expected_calls \u003d ["},{"line_number":545,"context_line":"                mock.call(self.context, inst.uuid,"}],"source_content_type":"text/x-python","patch_set":9,"id":"5ad0253b_43efa5b0","line":542,"updated":"2015-04-28 21:24:01.000000000","message":"I noticed this could be a mock.IsA(objects.Instance) instead of mock.ANY, but the instance.cell_name sort of does it already","commit_id":"6d6fdeeff880cc5fe8edcc24ef94d3bc4ca924e2"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"50632f5e0b98471e5c7f4f8c3a9b8a22d283ea79","unresolved":false,"context_lines":[{"line_number":539,"context_line":"        elif cell_type \u003d\u003d \u0027api\u0027:"},{"line_number":540,"context_line":"            self.assertFalse(mock_update_at_top.called)"},{"line_number":541,"context_line":"            fake_update_from_api.assert_called_once_with(self.context,"},{"line_number":542,"context_line":"                    mock.ANY, None, None, False)"},{"line_number":543,"context_line":""},{"line_number":544,"context_line":"        expected_calls \u003d ["},{"line_number":545,"context_line":"                mock.call(self.context, inst.uuid,"}],"source_content_type":"text/x-python","patch_set":9,"id":"5ad0253b_c3d5f5b9","line":542,"in_reply_to":"5ad0253b_43efa5b0","updated":"2015-04-28 21:28:13.000000000","message":"Sorry, I was thinking of mox.IsA(). Please ignore","commit_id":"6d6fdeeff880cc5fe8edcc24ef94d3bc4ca924e2"}]}