)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"3eb20df9e13c4add28aee4e701c34dcec8cc3277","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The instance service.py was functionally redundant with respect"},{"line_number":10,"context_line":"to \u0027configuration\u0027 functionality. This change modifed update()"},{"line_number":11,"context_line":"to updater its body with appropriate configuration for edit(),"},{"line_number":12,"context_line":"then delegate processing to edit()."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"No APIs were changed."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9a80dd14_0672ab10","line":11,"updated":"2015-03-07 19:02:42.000000000","message":"updater?","commit_id":"2e7f9d86435f4655aaec7e892e9ae989d1723199"},{"author":{"_account_id":14009,"name":"Dan Ritchie","email":"dan.ritchie@hp.com","username":"danr"},"change_message_id":"2ae7057705c07f0e135ed4ca6c0ae3cc4455c468","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The instance service.py was functionally redundant with respect"},{"line_number":10,"context_line":"to \u0027configuration\u0027 functionality. This change modifed update()"},{"line_number":11,"context_line":"to updater its body with appropriate configuration for edit(),"},{"line_number":12,"context_line":"then delegate processing to edit()."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"No APIs were changed."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9a80dd14_2dc12c54","line":11,"in_reply_to":"9a80dd14_0672ab10","updated":"2015-03-09 15:45:29.000000000","message":"Fixed.","commit_id":"2e7f9d86435f4655aaec7e892e9ae989d1723199"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"3eb20df9e13c4add28aee4e701c34dcec8cc3277","unresolved":false,"context_lines":[{"line_number":17,"context_line":"--configuration and --remove-configuration CLIs.  Existing API"},{"line_number":18,"context_line":"tests were deemed sufficient."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Modified the implementation to include a ne method,"},{"line_number":21,"context_line":"_modify_instance(), which is called by both update() and"},{"line_number":22,"context_line":"edit().  In the previous implementation, update() called"},{"line_number":23,"context_line":"edit() directly."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9a80dd14_4667434d","line":20,"updated":"2015-03-07 19:02:42.000000000","message":"I assume this should be \u0027new method\u0027 ?","commit_id":"2e7f9d86435f4655aaec7e892e9ae989d1723199"},{"author":{"_account_id":14009,"name":"Dan Ritchie","email":"dan.ritchie@hp.com","username":"danr"},"change_message_id":"2ae7057705c07f0e135ed4ca6c0ae3cc4455c468","unresolved":false,"context_lines":[{"line_number":17,"context_line":"--configuration and --remove-configuration CLIs.  Existing API"},{"line_number":18,"context_line":"tests were deemed sufficient."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Modified the implementation to include a ne method,"},{"line_number":21,"context_line":"_modify_instance(), which is called by both update() and"},{"line_number":22,"context_line":"edit().  In the previous implementation, update() called"},{"line_number":23,"context_line":"edit() directly."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9a80dd14_8dd13825","line":20,"in_reply_to":"9a80dd14_4667434d","updated":"2015-03-09 15:45:29.000000000","message":"Yes, fixed.","commit_id":"2e7f9d86435f4655aaec7e892e9ae989d1723199"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"3eb20df9e13c4add28aee4e701c34dcec8cc3277","unresolved":false,"context_lines":[{"line_number":23,"context_line":"edit() directly."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Added unit tests for _modify_instance()."},{"line_number":26,"context_line":"Modified _modify_instance() unit tests per Eddie\u0027s comment."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"This is a partial fix for bug 1364701."},{"line_number":29,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9a80dd14_e64bd7ae","line":26,"updated":"2015-03-07 19:02:42.000000000","message":"I\u0027m confused, why did you modify tests for a \u0027new\u0027 method? Aren\u0027t the tests new too?","commit_id":"2e7f9d86435f4655aaec7e892e9ae989d1723199"},{"author":{"_account_id":14009,"name":"Dan Ritchie","email":"dan.ritchie@hp.com","username":"danr"},"change_message_id":"2ae7057705c07f0e135ed4ca6c0ae3cc4455c468","unresolved":false,"context_lines":[{"line_number":23,"context_line":"edit() directly."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Added unit tests for _modify_instance()."},{"line_number":26,"context_line":"Modified _modify_instance() unit tests per Eddie\u0027s comment."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"This is a partial fix for bug 1364701."},{"line_number":29,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9a80dd14_cdbdf003","line":26,"in_reply_to":"9a80dd14_e64bd7ae","updated":"2015-03-09 15:45:29.000000000","message":"Peter, I added the tests in a previous patch set, modified them in this patch set per Eddie\u0027s comments.","commit_id":"2e7f9d86435f4655aaec7e892e9ae989d1723199"}],"trove/instance/service.py":[{"author":{"_account_id":13355,"name":"Edmond Kotowski","email":"ekotowski@gmail.com","username":"ekotowski"},"change_message_id":"8fc24527b9e6fbcb0db3b0437dbd4438cd49ebe1","unresolved":false,"context_lines":[{"line_number":255,"context_line":"        # Make sure instance in body contains a \u0027configuration\u0027 argument,"},{"line_number":256,"context_line":"        # this is needed by edit()"},{"line_number":257,"context_line":"        configuration_id \u003d self._configuration_parse(context, body)"},{"line_number":258,"context_line":"        body[\u0027instance\u0027][\u0027configuration\u0027] \u003d configuration_id"},{"line_number":259,"context_line":"        return self.edit(req, id, body, tenant_id)"},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"    def edit(self, req, id, body, tenant_id):"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba7be1f8_20494c31","line":258,"updated":"2015-03-03 17:53:16.000000000","message":"This change causes a change of behavior because update can now detach_replicas and update the name which may cause some untended side effects. You can simply extract method on lines 263 - 267 and use that in both update and edit.","commit_id":"ffe7c50a956269b35cfcddd322a24d2298fc64fa"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"dc00a25f7df1ad781d5a4156d14bdf4df66b64ac","unresolved":false,"context_lines":[{"line_number":255,"context_line":"        # Make sure instance in body contains a \u0027configuration\u0027 argument,"},{"line_number":256,"context_line":"        # this is needed by edit()"},{"line_number":257,"context_line":"        configuration_id \u003d self._configuration_parse(context, body)"},{"line_number":258,"context_line":"        body[\u0027instance\u0027][\u0027configuration\u0027] \u003d configuration_id"},{"line_number":259,"context_line":"        return self.edit(req, id, body, tenant_id)"},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"    def edit(self, req, id, body, tenant_id):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_b621b6b6","line":258,"in_reply_to":"ba7be1f8_20494c31","updated":"2015-03-04 08:43:53.000000000","message":"Agreed -- one approach to fix this would be to refactor just the logic that makes the actual changes out, and have both edit and update call this newly refactored logic.","commit_id":"ffe7c50a956269b35cfcddd322a24d2298fc64fa"},{"author":{"_account_id":13355,"name":"Edmond Kotowski","email":"ekotowski@gmail.com","username":"ekotowski"},"change_message_id":"5749e5427f3a0529767a94e137078f8b5456a087","unresolved":false,"context_lines":[{"line_number":268,"context_line":"        context \u003d req.environ[wsgi.CONTEXT_KEY]"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"        instance \u003d models.Instance.load(context, id)"},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"        if \u0027slave_of\u0027 in body[\u0027instance\u0027]:"},{"line_number":273,"context_line":"            LOG.debug(\"Detaching replica from source.\")"},{"line_number":274,"context_line":"            instance.detach_replica()"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba7be1f8_20cf8c57","line":271,"updated":"2015-03-03 17:58:46.000000000","message":"Unrelated to this change, but it seems like this could be a possible code smell. We are just checking if the key \u0027slave_of\u0027 exists and detaching replica even if that value itself is None.","commit_id":"ffe7c50a956269b35cfcddd322a24d2298fc64fa"},{"author":{"_account_id":13355,"name":"Edmond Kotowski","email":"ekotowski@gmail.com","username":"ekotowski"},"change_message_id":"8dfafa6a96459308ce1847e0a7dc82621ed67f78","unresolved":false,"context_lines":[{"line_number":268,"context_line":"        context \u003d req.environ[wsgi.CONTEXT_KEY]"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"        instance \u003d models.Instance.load(context, id)"},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"        if \u0027slave_of\u0027 in body[\u0027instance\u0027]:"},{"line_number":273,"context_line":"            LOG.debug(\"Detaching replica from source.\")"},{"line_number":274,"context_line":"            instance.detach_replica()"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba7be1f8_11856359","line":271,"in_reply_to":"ba7be1f8_20cf8c57","updated":"2015-03-04 03:49:09.000000000","message":"Looked further into it turns out we could use replica_of or slave_of here and right now we are always passing slave_of: NULL.","commit_id":"ffe7c50a956269b35cfcddd322a24d2298fc64fa"}],"trove/tests/unittests/instance/test_instance_controller.py":[{"author":{"_account_id":13355,"name":"Edmond Kotowski","email":"ekotowski@gmail.com","username":"ekotowski"},"change_message_id":"cf556ce14a667d1492fda6f725287a79974a0655","unresolved":false,"context_lines":[{"line_number":233,"context_line":"        instance.update_db \u003d Mock()"},{"line_number":234,"context_line":"        return instance"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"    def _verify_modify_instance_calls(self, instance, **kwargs):"},{"line_number":237,"context_line":"        if \u0027detach_replica\u0027 in kwargs:"},{"line_number":238,"context_line":"            if kwargs[\u0027detach_replica\u0027]:"},{"line_number":239,"context_line":"                self.assertEqual(instance.detach_replica.call_count, 1)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a80dd14_d6ab6187","line":236,"updated":"2015-03-05 00:24:58.000000000","message":"This method contains a bit of logic. We could instead have individual asserts on each test method that cover the necessary test cases.","commit_id":"d08b848717969e86333acf89de13c59df0d1d2f5"}]}