)]}'
{"cinder/tests/unit/test_volume.py":[{"author":{"_account_id":16708,"name":"Kendall Nelson","display_name":"Kendall (diablo_rojo)","email":"kennelson11@gmail.com","username":"kjnelson"},"change_message_id":"0cf913f1ee9a9bd8e338a5bcee776505defd006b","unresolved":false,"context_lines":[{"line_number":6275,"context_line":"        with mock.patch.object(volume_rpcapi.VolumeAPI,"},{"line_number":6276,"context_line":"                               \u0027disable_replication\u0027) as mock_disable_rep:"},{"line_number":6277,"context_line":"            volume_api.disable_replication(ctxt, volume)"},{"line_number":6278,"context_line":"            # We don\u0027t call disable recplication if it\u0027s already disabled"},{"line_number":6279,"context_line":"            self.assertFalse(mock_disable_rep.called)"},{"line_number":6280,"context_line":""},{"line_number":6281,"context_line":"            db.volume_update(self.context, volume.id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_8b7eb068","line":6278,"updated":"2015-09-10 14:44:37.000000000","message":"recplication -\u003e replication","commit_id":"d8782e227afeef37962eea31ac45f76f21f07d47"}],"cinder/volume/api.py":[{"author":{"_account_id":16708,"name":"Kendall Nelson","display_name":"Kendall (diablo_rojo)","email":"kennelson11@gmail.com","username":"kjnelson"},"change_message_id":"0cf913f1ee9a9bd8e338a5bcee776505defd006b","unresolved":false,"context_lines":[{"line_number":1756,"context_line":"        # this indicates an issue with the driver, but might be useful"},{"line_number":1757,"context_line":"        # cases to  consider modifying this for in the future."},{"line_number":1758,"context_line":""},{"line_number":1759,"context_line":"        # Loop until we make a decission on whether to go forward with the"},{"line_number":1760,"context_line":"        # enabling or raise and error"},{"line_number":1761,"context_line":"        # TODO(geguileo): Once we are using versioned objects we should change"},{"line_number":1762,"context_line":"        # this to use volume.conditional_update and remove the id from the"}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_4b917899","line":1759,"updated":"2015-09-10 14:44:37.000000000","message":"decission -\u003e decision","commit_id":"d8782e227afeef37962eea31ac45f76f21f07d47"},{"author":{"_account_id":16708,"name":"Kendall Nelson","display_name":"Kendall (diablo_rojo)","email":"kennelson11@gmail.com","username":"kjnelson"},"change_message_id":"0cf913f1ee9a9bd8e338a5bcee776505defd006b","unresolved":false,"context_lines":[{"line_number":1792,"context_line":""},{"line_number":1793,"context_line":"        disabled_status \u003d (\u0027disabled\u0027, None)"},{"line_number":1794,"context_line":""},{"line_number":1795,"context_line":"        # Loop until we make a decission on whether to go forward with the"},{"line_number":1796,"context_line":"        # disabling or raise and error"},{"line_number":1797,"context_line":"        # TODO(geguileo): Once we are using versioned objects we should change"},{"line_number":1798,"context_line":"        # this to use volume.conditional_update and remove the id from the"}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_eb8debd3","line":1795,"updated":"2015-09-10 14:44:37.000000000","message":"decission -\u003e decision","commit_id":"d8782e227afeef37962eea31ac45f76f21f07d47"},{"author":{"_account_id":16708,"name":"Kendall Nelson","display_name":"Kendall (diablo_rojo)","email":"kennelson11@gmail.com","username":"kjnelson"},"change_message_id":"0cf913f1ee9a9bd8e338a5bcee776505defd006b","unresolved":false,"context_lines":[{"line_number":1836,"context_line":"        # secondaries that includes an ID/Name that can be"},{"line_number":1837,"context_line":"        # used as a handle"},{"line_number":1838,"context_line":""},{"line_number":1839,"context_line":"        # Loop until we make a decission on whether to go forward with the"},{"line_number":1840,"context_line":"        # failover or raise and error"},{"line_number":1841,"context_line":"        # TODO(geguileo): Once we are using versioned objects we should change"},{"line_number":1842,"context_line":"        # this to use volume.conditional_update and remove the id from the"}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_6b6f9bfc","line":1839,"updated":"2015-09-10 14:44:37.000000000","message":"decission -\u003e decision","commit_id":"d8782e227afeef37962eea31ac45f76f21f07d47"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"d6ef233b88d578c776f17056d15c3c404364aa9b","unresolved":false,"context_lines":[{"line_number":1604,"context_line":"        expected \u003d {\u0027replication_status\u0027: (\u0027disabled\u0027, \u0027failed-over\u0027, \u0027error\u0027,"},{"line_number":1605,"context_line":"                                           None)}"},{"line_number":1606,"context_line":"        result \u003d volume.conditional_update({\u0027replication_status\u0027: \u0027enabling\u0027},"},{"line_number":1607,"context_line":"                                           expected)"},{"line_number":1608,"context_line":""},{"line_number":1609,"context_line":"        if not result:"},{"line_number":1610,"context_line":"            msg \u003d _(\u0027Replication status must not be set or set to disabled to \u0027"}],"source_content_type":"text/x-python","patch_set":17,"id":"dae33548_c110a2ae","line":1607,"updated":"2016-02-18 23:37:36.000000000","message":"so the only issue I see with this change is that it seems like we lose the ability to say that the state is invalid, like what happens on line 1618 prior to this patch.  \n\nI don\u0027t see where this happens in the CinderObject.conditional_update.  \n\nI understand that the underlying DB object builds a query with the expected values, but how can we notify the user that the state wasn\u0027t one of the expected states?","commit_id":"522eec013d177a5c563b6686808d90b8391299ca"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"55aa3fccc11a9bcb74868042ead62b7aa6235e3d","unresolved":false,"context_lines":[{"line_number":1604,"context_line":"        expected \u003d {\u0027replication_status\u0027: (\u0027disabled\u0027, \u0027failed-over\u0027, \u0027error\u0027,"},{"line_number":1605,"context_line":"                                           None)}"},{"line_number":1606,"context_line":"        result \u003d volume.conditional_update({\u0027replication_status\u0027: \u0027enabling\u0027},"},{"line_number":1607,"context_line":"                                           expected)"},{"line_number":1608,"context_line":""},{"line_number":1609,"context_line":"        if not result:"},{"line_number":1610,"context_line":"            msg \u003d _(\u0027Replication status must not be set or set to disabled to \u0027"}],"source_content_type":"text/x-python","patch_set":17,"id":"bae84128_df46374e","line":1607,"in_reply_to":"bae84128_293e95b8","updated":"2016-02-22 21:12:02.000000000","message":"ok I can see that we\u0027d have races in checking the existing state after the conditional_update fails.   \n\nSo, can we simply log a message saying that we couldn\u0027t update the status due to a state not being in the expected list?","commit_id":"522eec013d177a5c563b6686808d90b8391299ca"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e78ec7d8a7ab102db2a4d8695ce1f2d6a78f07e2","unresolved":false,"context_lines":[{"line_number":1604,"context_line":"        expected \u003d {\u0027replication_status\u0027: (\u0027disabled\u0027, \u0027failed-over\u0027, \u0027error\u0027,"},{"line_number":1605,"context_line":"                                           None)}"},{"line_number":1606,"context_line":"        result \u003d volume.conditional_update({\u0027replication_status\u0027: \u0027enabling\u0027},"},{"line_number":1607,"context_line":"                                           expected)"},{"line_number":1608,"context_line":""},{"line_number":1609,"context_line":"        if not result:"},{"line_number":1610,"context_line":"            msg \u003d _(\u0027Replication status must not be set or set to disabled to \u0027"}],"source_content_type":"text/x-python","patch_set":17,"id":"bae84128_069e1e07","line":1607,"in_reply_to":"bae84128_df46374e","updated":"2016-02-24 18:12:39.000000000","message":"You are totally right, I must have changed allowed replication_status in one of the rebases and didn\u0027t update the error message.","commit_id":"522eec013d177a5c563b6686808d90b8391299ca"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"51465fa0f307556b048878c7e29f57a5694a432e","unresolved":false,"context_lines":[{"line_number":1604,"context_line":"        expected \u003d {\u0027replication_status\u0027: (\u0027disabled\u0027, \u0027failed-over\u0027, \u0027error\u0027,"},{"line_number":1605,"context_line":"                                           None)}"},{"line_number":1606,"context_line":"        result \u003d volume.conditional_update({\u0027replication_status\u0027: \u0027enabling\u0027},"},{"line_number":1607,"context_line":"                                           expected)"},{"line_number":1608,"context_line":""},{"line_number":1609,"context_line":"        if not result:"},{"line_number":1610,"context_line":"            msg \u003d _(\u0027Replication status must not be set or set to disabled to \u0027"}],"source_content_type":"text/x-python","patch_set":17,"id":"bae84128_293e95b8","line":1607,"in_reply_to":"dae33548_c110a2ae","updated":"2016-02-19 09:37:19.000000000","message":"I have implemented the API race conditional removal patches in different ways since I begun this effort, and one method that mimicked current behavior was one of them.\n\nThe problem was that we needed to read back the contents from the DB once the update had failed to check the fields, but then we could have race conditions, since the fields could have changed since the moment we failed the conditional update, so we had to check the data we had read and not blindly return it as the reason.  It would be quite bad if we were to say \"valid states are disabled, error, but current status is available\".\n\nSo to remove this race condition a loop which checks was implemented, but it added quite a lot of code and the readability of the code dropped significantly.\n\nIt was decided (I think it was in a meeting, but I may be wrong since it has been quite a long time since that) that it was good enough to return a generic error and all patches were changed to do it that way.\n\nYou can see the old way of doing it on patch 7: https://review.openstack.org/#/c/221441/7/cinder/volume/api.py\n\nIt used a generic method called `_db_conditional_update` that would do the looping for all conditional updates of the API.","commit_id":"522eec013d177a5c563b6686808d90b8391299ca"}]}