)]}'
{"cinder/group/api.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"cbd0181cca3a33e78949ff079fbccd864453a0fb","unresolved":false,"context_lines":[{"line_number":546,"context_line":"                raise exception.InvalidGroup("},{"line_number":547,"context_line":"                    reason\u003d_(\"Group has existing snapshots.\"))"},{"line_number":548,"context_line":""},{"line_number":549,"context_line":"        # TODO(smcginnis): Add conditional update handling for volumes"},{"line_number":550,"context_line":"        volumes \u003d self.db.volume_get_all_by_generic_group(context.elevated(),"},{"line_number":551,"context_line":"                                                          group.id)"},{"line_number":552,"context_line":"        if volumes and not delete_volumes:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_c1771ed4","line":549,"updated":"2017-09-07 12:27:13.000000000","message":"?: It doesn\u0027t seem too hard...  Removing L549-578, then getting and updating the volumes after we change the group\u0027s status.  Should we fix this in this patch as well?","commit_id":"2970092d0442f727d57fb3779603f68f8b39ac51"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"610b5bddb538b95da07e281e175461f2570ff8d1","unresolved":false,"context_lines":[{"line_number":546,"context_line":"                raise exception.InvalidGroup("},{"line_number":547,"context_line":"                    reason\u003d_(\"Group has existing snapshots.\"))"},{"line_number":548,"context_line":""},{"line_number":549,"context_line":"        # TODO(smcginnis): Add conditional update handling for volumes"},{"line_number":550,"context_line":"        volumes \u003d self.db.volume_get_all_by_generic_group(context.elevated(),"},{"line_number":551,"context_line":"                                                          group.id)"},{"line_number":552,"context_line":"        if volumes and not delete_volumes:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_bfe09c98","line":549,"in_reply_to":"7f287b81_c1771ed4","updated":"2017-09-07 20:24:19.000000000","message":"I\u0027m going to skip this for now. There are some races in doing it this way too, so I think this needs a little more thought.","commit_id":"2970092d0442f727d57fb3779603f68f8b39ac51"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"be4c029ab070d0ba34c45adcc1bb57e84ccf5280","unresolved":false,"context_lines":[{"line_number":577,"context_line":""},{"line_number":578,"context_line":"        self.db.volumes_update(context, volumes_model_update)"},{"line_number":579,"context_line":""},{"line_number":580,"context_line":"        if cg_force:"},{"line_number":581,"context_line":"            expected \u003d {}"},{"line_number":582,"context_line":"        else:"},{"line_number":583,"context_line":"            expected \u003d {\u0027status\u0027: (c_fields.GroupStatus.AVAILABLE,"},{"line_number":584,"context_line":"                                   c_fields.GroupStatus.ERROR)}"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_bb625315","line":581,"range":{"start_line":580,"start_character":8,"end_line":581,"end_character":25},"updated":"2017-09-04 20:47:57.000000000","message":"The original CG code has conditional update handling, why not use the same code path for groups in the else block?","commit_id":"2970092d0442f727d57fb3779603f68f8b39ac51"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"b7d4b75c6537e831793a0500e362dc7395daa3b6","unresolved":false,"context_lines":[{"line_number":577,"context_line":""},{"line_number":578,"context_line":"        self.db.volumes_update(context, volumes_model_update)"},{"line_number":579,"context_line":""},{"line_number":580,"context_line":"        if cg_force:"},{"line_number":581,"context_line":"            expected \u003d {}"},{"line_number":582,"context_line":"        else:"},{"line_number":583,"context_line":"            expected \u003d {\u0027status\u0027: (c_fields.GroupStatus.AVAILABLE,"},{"line_number":584,"context_line":"                                   c_fields.GroupStatus.ERROR)}"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_cf986fc1","line":581,"range":{"start_line":580,"start_character":8,"end_line":581,"end_character":25},"in_reply_to":"7f287b81_498ff02d","updated":"2017-09-07 04:09:33.000000000","message":"They did mean the same.  \"force\" is a word that has been \"abused\", so the suggestion was to use \"delete_volumes\" instead.\n\n\"force\" means the user wants to delete the CG along with all the volumes in it.\n\"delete_volumes\" means the user wants to delete the Group along with all the volumes in it.\n\"force\" does not have any extra meaning other than that.  So they are absolutely the same.  If you look at the original design spec, I had \"force\" there for groups at first, but renamed to \"delete_volumes\" following a review suggestion.  So I can guarantee that they mean exactly the same!","commit_id":"2970092d0442f727d57fb3779603f68f8b39ac51"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"02fc5f32ec804a5540fc4388deac736a156acad6","unresolved":false,"context_lines":[{"line_number":577,"context_line":""},{"line_number":578,"context_line":"        self.db.volumes_update(context, volumes_model_update)"},{"line_number":579,"context_line":""},{"line_number":580,"context_line":"        if cg_force:"},{"line_number":581,"context_line":"            expected \u003d {}"},{"line_number":582,"context_line":"        else:"},{"line_number":583,"context_line":"            expected \u003d {\u0027status\u0027: (c_fields.GroupStatus.AVAILABLE,"},{"line_number":584,"context_line":"                                   c_fields.GroupStatus.ERROR)}"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_c5972045","line":581,"range":{"start_line":580,"start_character":8,"end_line":581,"end_character":25},"in_reply_to":"7f287b81_582c3c01","updated":"2017-09-07 14:59:48.000000000","message":"The bottom line is, no matter how we name this \"flag\" for delete_group, both flags were designed to do the SAME thing.","commit_id":"2970092d0442f727d57fb3779603f68f8b39ac51"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"0db9fae1a06df2e85f9edc07cb17530eb23fecec","unresolved":false,"context_lines":[{"line_number":577,"context_line":""},{"line_number":578,"context_line":"        self.db.volumes_update(context, volumes_model_update)"},{"line_number":579,"context_line":""},{"line_number":580,"context_line":"        if cg_force:"},{"line_number":581,"context_line":"            expected \u003d {}"},{"line_number":582,"context_line":"        else:"},{"line_number":583,"context_line":"            expected \u003d {\u0027status\u0027: (c_fields.GroupStatus.AVAILABLE,"},{"line_number":584,"context_line":"                                   c_fields.GroupStatus.ERROR)}"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_498ff02d","line":581,"range":{"start_line":580,"start_character":8,"end_line":581,"end_character":25},"in_reply_to":"7f287b81_6ea2b29c","updated":"2017-09-06 16:08:53.000000000","message":"Hmm, I don\u0027t really think they are the same. If we wanted delete_volumes to mean force we should have probably kept it force.\n\nI\u0027ll update this though if that\u0027s what we want to (ab)use.","commit_id":"2970092d0442f727d57fb3779603f68f8b39ac51"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"7d3b63b50798c8024e0c686b81a45d7bbb112418","unresolved":false,"context_lines":[{"line_number":577,"context_line":""},{"line_number":578,"context_line":"        self.db.volumes_update(context, volumes_model_update)"},{"line_number":579,"context_line":""},{"line_number":580,"context_line":"        if cg_force:"},{"line_number":581,"context_line":"            expected \u003d {}"},{"line_number":582,"context_line":"        else:"},{"line_number":583,"context_line":"            expected \u003d {\u0027status\u0027: (c_fields.GroupStatus.AVAILABLE,"},{"line_number":584,"context_line":"                                   c_fields.GroupStatus.ERROR)}"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_a7d77fee","line":581,"range":{"start_line":580,"start_character":8,"end_line":581,"end_character":25},"in_reply_to":"7f287b81_814596b6","updated":"2017-09-07 12:37:41.000000000","message":"Well, they aren\u0027t quite the same. delete_volumes means delete any volumes that exist. Force means I don\u0027t care what state the group is in, just get rid of it. So we will have a semantic mismatch in terms.\n\nI\u0027ll just add comments explaining that so there is not confusion down the road.","commit_id":"2970092d0442f727d57fb3779603f68f8b39ac51"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"183e1f7c6c7354a3c9dd5da46b1e550396ce25c5","unresolved":false,"context_lines":[{"line_number":577,"context_line":""},{"line_number":578,"context_line":"        self.db.volumes_update(context, volumes_model_update)"},{"line_number":579,"context_line":""},{"line_number":580,"context_line":"        if cg_force:"},{"line_number":581,"context_line":"            expected \u003d {}"},{"line_number":582,"context_line":"        else:"},{"line_number":583,"context_line":"            expected \u003d {\u0027status\u0027: (c_fields.GroupStatus.AVAILABLE,"},{"line_number":584,"context_line":"                                   c_fields.GroupStatus.ERROR)}"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_582c3c01","line":581,"range":{"start_line":580,"start_character":8,"end_line":581,"end_character":25},"in_reply_to":"7f287b81_82c35157","updated":"2017-09-07 14:05:43.000000000","message":"How about force? :D\n\nKidding! (mostly)","commit_id":"2970092d0442f727d57fb3779603f68f8b39ac51"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"7d068c57d35dff104e574841a1913f3dbfe736dd","unresolved":false,"context_lines":[{"line_number":577,"context_line":""},{"line_number":578,"context_line":"        self.db.volumes_update(context, volumes_model_update)"},{"line_number":579,"context_line":""},{"line_number":580,"context_line":"        if cg_force:"},{"line_number":581,"context_line":"            expected \u003d {}"},{"line_number":582,"context_line":"        else:"},{"line_number":583,"context_line":"            expected \u003d {\u0027status\u0027: (c_fields.GroupStatus.AVAILABLE,"},{"line_number":584,"context_line":"                                   c_fields.GroupStatus.ERROR)}"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_6ea2b29c","line":581,"range":{"start_line":580,"start_character":8,"end_line":581,"end_character":25},"in_reply_to":"7f287b81_9b9ae6c2","updated":"2017-09-06 15:43:01.000000000","message":"What I meant is the delete_volumes flag in groups API means exactly the same thing as the force flag in CG API.  It\u0027s renamed from \"force\" to \"delete_volumes\" during code review because \"delete_volumes\" has a more clear meaning and it is exactly what we want \"force\" to do.\n\nSo what I\u0027m trying to say is there\u0027s no need to add a cg_force flag because \"force\" means \"delete_volumes\".","commit_id":"2970092d0442f727d57fb3779603f68f8b39ac51"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b39b1ba591e50d09aafdd9a3084a2c6a1d17e2e9","unresolved":false,"context_lines":[{"line_number":577,"context_line":""},{"line_number":578,"context_line":"        self.db.volumes_update(context, volumes_model_update)"},{"line_number":579,"context_line":""},{"line_number":580,"context_line":"        if cg_force:"},{"line_number":581,"context_line":"            expected \u003d {}"},{"line_number":582,"context_line":"        else:"},{"line_number":583,"context_line":"            expected \u003d {\u0027status\u0027: (c_fields.GroupStatus.AVAILABLE,"},{"line_number":584,"context_line":"                                   c_fields.GroupStatus.ERROR)}"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_82c35157","line":581,"range":{"start_line":580,"start_character":8,"end_line":581,"end_character":25},"in_reply_to":"7f287b81_a7d77fee","updated":"2017-09-07 13:11:08.000000000","message":"True, I think the current name may not be the most appropriate, because like you say, in this case it\u0027s meaning both things, to delete volumes and to ignore the status.  But I don\u0027t really know a better name, except maybe \"i_just_want_it_gone_now\" ;-)","commit_id":"2970092d0442f727d57fb3779603f68f8b39ac51"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"3de67882d1eebbc546516d9c8d5f828a48ef04dc","unresolved":false,"context_lines":[{"line_number":577,"context_line":""},{"line_number":578,"context_line":"        self.db.volumes_update(context, volumes_model_update)"},{"line_number":579,"context_line":""},{"line_number":580,"context_line":"        if cg_force:"},{"line_number":581,"context_line":"            expected \u003d {}"},{"line_number":582,"context_line":"        else:"},{"line_number":583,"context_line":"            expected \u003d {\u0027status\u0027: (c_fields.GroupStatus.AVAILABLE,"},{"line_number":584,"context_line":"                                   c_fields.GroupStatus.ERROR)}"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_9b9ae6c2","line":581,"range":{"start_line":580,"start_character":8,"end_line":581,"end_character":25},"in_reply_to":"7f287b81_bb625315","updated":"2017-09-05 13:53:49.000000000","message":"Not sure what you mean. There was a call in from the cg code to the group code, but that is what I am fixing here with the cg_force parameter. It was overloading the delete_volumes parameter to have a different meaning that didn\u0027t align right. That is changed with this patch in the cg code.\n\nMaybe I\u0027m not following what you are asking though. If you are referring to something different, maybe link to the code you are referring to?","commit_id":"2970092d0442f727d57fb3779603f68f8b39ac51"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"21a712743cedf4d42657e8277bce76a042f79936","unresolved":false,"context_lines":[{"line_number":577,"context_line":""},{"line_number":578,"context_line":"        self.db.volumes_update(context, volumes_model_update)"},{"line_number":579,"context_line":""},{"line_number":580,"context_line":"        if cg_force:"},{"line_number":581,"context_line":"            expected \u003d {}"},{"line_number":582,"context_line":"        else:"},{"line_number":583,"context_line":"            expected \u003d {\u0027status\u0027: (c_fields.GroupStatus.AVAILABLE,"},{"line_number":584,"context_line":"                                   c_fields.GroupStatus.ERROR)}"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_e5e0dc99","line":581,"range":{"start_line":580,"start_character":8,"end_line":581,"end_character":25},"in_reply_to":"7f287b81_c5972045","updated":"2017-09-07 15:07:55.000000000","message":"Yep, I will change it. Just not happy with the naming since it\u0027s implemented meaning is different than it\u0027s literal meaning. Which is a case that has caused many bugs in our code over time. But hopefully comments will make it clear though.\n\n\"Inconceivable!\"\n\"You keep using that word. I do not think it means what you think it means.\"","commit_id":"2970092d0442f727d57fb3779603f68f8b39ac51"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"cbd0181cca3a33e78949ff079fbccd864453a0fb","unresolved":false,"context_lines":[{"line_number":577,"context_line":""},{"line_number":578,"context_line":"        self.db.volumes_update(context, volumes_model_update)"},{"line_number":579,"context_line":""},{"line_number":580,"context_line":"        if cg_force:"},{"line_number":581,"context_line":"            expected \u003d {}"},{"line_number":582,"context_line":"        else:"},{"line_number":583,"context_line":"            expected \u003d {\u0027status\u0027: (c_fields.GroupStatus.AVAILABLE,"},{"line_number":584,"context_line":"                                   c_fields.GroupStatus.ERROR)}"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_814596b6","line":581,"range":{"start_line":580,"start_character":8,"end_line":581,"end_character":25},"in_reply_to":"7f287b81_cf986fc1","updated":"2017-09-07 12:27:13.000000000","message":"-1: I agree, I think we should just use the delete_volumes parameter.","commit_id":"2970092d0442f727d57fb3779603f68f8b39ac51"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"55cd04a15cf0f9537d1578cda1c5b3e5d2a09296","unresolved":false,"context_lines":[{"line_number":577,"context_line":""},{"line_number":578,"context_line":"        self.db.volumes_update(context, volumes_model_update)"},{"line_number":579,"context_line":""},{"line_number":580,"context_line":"        if cg_force:"},{"line_number":581,"context_line":"            expected \u003d {}"},{"line_number":582,"context_line":"        else:"},{"line_number":583,"context_line":"            expected \u003d {\u0027status\u0027: (c_fields.GroupStatus.AVAILABLE,"},{"line_number":584,"context_line":"                                   c_fields.GroupStatus.ERROR)}"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_cc5f440d","line":581,"range":{"start_line":580,"start_character":8,"end_line":581,"end_character":25},"in_reply_to":"7f287b81_e5e0dc99","updated":"2017-09-07 19:23:36.000000000","message":"Agree that the naming is confusing.","commit_id":"2970092d0442f727d57fb3779603f68f8b39ac51"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"cbd0181cca3a33e78949ff079fbccd864453a0fb","unresolved":false,"context_lines":[{"line_number":584,"context_line":"                                   c_fields.GroupStatus.ERROR)}"},{"line_number":585,"context_line":"        filters \u003d [~db.group_has_group_snapshot_filter(),"},{"line_number":586,"context_line":"                   ~db.group_has_volumes_filter("},{"line_number":587,"context_line":"                       attached_or_with_snapshots\u003d(delete_volumes)),"},{"line_number":588,"context_line":"                   ~db.group_creating_from_src(group_id\u003dgroup.id)]"},{"line_number":589,"context_line":"        values \u003d {\u0027status\u0027: c_fields.GroupStatus.DELETING}"},{"line_number":590,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_e1845ad5","line":587,"range":{"start_line":587,"start_character":50,"end_line":587,"end_character":51},"updated":"2017-09-07 12:27:13.000000000","message":"nit: unnecessary parenthesis","commit_id":"2970092d0442f727d57fb3779603f68f8b39ac51"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"cbd0181cca3a33e78949ff079fbccd864453a0fb","unresolved":false,"context_lines":[{"line_number":671,"context_line":""},{"line_number":672,"context_line":"        if not group.conditional_update(fields, expected):"},{"line_number":673,"context_line":"            msg \u003d _(\"Group status must be available, \""},{"line_number":674,"context_line":"                    \"but current status is: %s.\") % group.status"},{"line_number":675,"context_line":"            raise exception.InvalidGroup(reason\u003dmsg)"},{"line_number":676,"context_line":""},{"line_number":677,"context_line":"        # Do an RPC call only if the update request includes"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_61718a44","line":674,"updated":"2017-09-07 12:27:13.000000000","message":"-1: We refrain from saying what the current status is because a race condition between retrieving the data and actually doing the update could end up saying: \"must be available but current status is: available\"","commit_id":"2970092d0442f727d57fb3779603f68f8b39ac51"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"be4c029ab070d0ba34c45adcc1bb57e84ccf5280","unresolved":false,"context_lines":[{"line_number":682,"context_line":"            self.volume_rpcapi.update_group("},{"line_number":683,"context_line":"                context, group,"},{"line_number":684,"context_line":"                add_volumes\u003dadd_volumes_new,"},{"line_number":685,"context_line":"                remove_volumes\u003dremove_volumes_new)"},{"line_number":686,"context_line":""},{"line_number":687,"context_line":"    def _validate_remove_volumes(self, volumes, remove_volumes_list, group):"},{"line_number":688,"context_line":"        # Validate volumes in remove_volumes."}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_5b480f86","line":685,"updated":"2017-09-04 20:47:57.000000000","message":"We should only do a RPC call if update group involves add volumes and delete volumes.  If it is just name/description change, we should just save to db.  I don\u0027t see a group.save() in this function.  Am I missing something?","commit_id":"2970092d0442f727d57fb3779603f68f8b39ac51"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"3de67882d1eebbc546516d9c8d5f828a48ef04dc","unresolved":false,"context_lines":[{"line_number":682,"context_line":"            self.volume_rpcapi.update_group("},{"line_number":683,"context_line":"                context, group,"},{"line_number":684,"context_line":"                add_volumes\u003dadd_volumes_new,"},{"line_number":685,"context_line":"                remove_volumes\u003dremove_volumes_new)"},{"line_number":686,"context_line":""},{"line_number":687,"context_line":"    def _validate_remove_volumes(self, volumes, remove_volumes_list, group):"},{"line_number":688,"context_line":"        # Validate volumes in remove_volumes."}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_bb4ec235","line":685,"in_reply_to":"7f287b81_5b480f86","updated":"2017-09-05 13:53:49.000000000","message":"Right, this only does the RPC call if there are volume changes.\n\nThe conditional_update is what now does the save.","commit_id":"2970092d0442f727d57fb3779603f68f8b39ac51"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"d8bf230bdb3ec53cfaaabcc2b926f155b09c83c4","unresolved":false,"context_lines":[{"line_number":516,"context_line":""},{"line_number":517,"context_line":"    @wrap_check_policy"},{"line_number":518,"context_line":"    def delete(self, context, group, delete_volumes\u003dFalse):"},{"line_number":519,"context_line":""},{"line_number":520,"context_line":"        if not group.host:"},{"line_number":521,"context_line":"            self.update_quota(context, group, -1, group.project_id)"},{"line_number":522,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5f2577a7_a1c88a21","line":519,"updated":"2017-09-15 15:38:10.000000000","message":"nit: unnecessary new line","commit_id":"fdfb2d51a4f362091ab5e94981d18d7741f11cf6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"d8bf230bdb3ec53cfaaabcc2b926f155b09c83c4","unresolved":false,"context_lines":[{"line_number":542,"context_line":"                    reason\u003d_(\"Group has existing snapshots.\"))"},{"line_number":543,"context_line":""},{"line_number":544,"context_line":"        # TODO(smcginnis): Add conditional update handling for volumes"},{"line_number":545,"context_line":"        # Should probably utilize the volume_api.delete code to handle"},{"line_number":546,"context_line":"        # cascade snapshot deletion and force delete."},{"line_number":547,"context_line":"        volumes \u003d self.db.volume_get_all_by_generic_group(context.elevated(),"},{"line_number":548,"context_line":"                                                          group.id)"},{"line_number":549,"context_line":"        if volumes and not delete_volumes:"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f2577a7_41a1e645","line":546,"range":{"start_line":545,"start_character":7,"end_line":546,"end_character":53},"updated":"2017-09-15 15:38:10.000000000","message":"nit: I don\u0027t necessarily agree with this comment.","commit_id":"fdfb2d51a4f362091ab5e94981d18d7741f11cf6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"d8bf230bdb3ec53cfaaabcc2b926f155b09c83c4","unresolved":false,"context_lines":[{"line_number":581,"context_line":"        else:"},{"line_number":582,"context_line":"            expected \u003d {\u0027status\u0027: (c_fields.GroupStatus.AVAILABLE,"},{"line_number":583,"context_line":"                                   c_fields.GroupStatus.ERROR)}"},{"line_number":584,"context_line":"        filters \u003d [~db.group_has_group_snapshot_filter(),"},{"line_number":585,"context_line":"                   ~db.group_has_volumes_filter("},{"line_number":586,"context_line":"                       attached_or_with_snapshots\u003ddelete_volumes),"},{"line_number":587,"context_line":"                   ~db.group_creating_from_src(group_id\u003dgroup.id)]"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f2577a7_c1a5d648","line":584,"range":{"start_line":584,"start_character":18,"end_line":584,"end_character":19},"updated":"2017-09-15 15:38:10.000000000","message":"nit: A tuple would do since we don\u0027t add additional filters later on.","commit_id":"fdfb2d51a4f362091ab5e94981d18d7741f11cf6"}],"cinder/tests/unit/api/v3/test_groups.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"cbd0181cca3a33e78949ff079fbccd864453a0fb","unresolved":false,"context_lines":[{"line_number":855,"context_line":""},{"line_number":856,"context_line":"        res_dict \u003d self.controller.update("},{"line_number":857,"context_line":"            req, self.group1.id, body)"},{"line_number":858,"context_line":"        self.assertEqual(http_client.ACCEPTED, res_dict.status_int)"},{"line_number":859,"context_line":""},{"line_number":860,"context_line":"    def test_update_group_add_volume_not_found(self):"},{"line_number":861,"context_line":"        self.group1.status \u003d fields.GroupStatus.AVAILABLE"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_e798e7c0","line":858,"updated":"2017-09-07 12:27:13.000000000","message":"-1: We should retrieve the group from the DB and confirm it was actually changed.","commit_id":"2970092d0442f727d57fb3779603f68f8b39ac51"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"cbd0181cca3a33e78949ff079fbccd864453a0fb","unresolved":false,"context_lines":[{"line_number":972,"context_line":"        self.group1.status \u003d status"},{"line_number":973,"context_line":"        add_volume \u003d utils.create_volume("},{"line_number":974,"context_line":"            self.ctxt,"},{"line_number":975,"context_line":"            volume_type_id\u003dfake.VOLUME_TYPE_ID,"},{"line_number":976,"context_line":"            host\u003dself.group1.host)"},{"line_number":977,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027/v3/%s/groups/%s/update\u0027 %"},{"line_number":978,"context_line":"                                      (fake.PROJECT_ID, self.group1.id),"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_47efbb1d","line":975,"range":{"start_line":975,"start_character":27,"end_line":975,"end_character":46},"updated":"2017-09-07 12:27:13.000000000","message":"nit: self.volume_type1.id","commit_id":"2970092d0442f727d57fb3779603f68f8b39ac51"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"cbd0181cca3a33e78949ff079fbccd864453a0fb","unresolved":false,"context_lines":[{"line_number":977,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027/v3/%s/groups/%s/update\u0027 %"},{"line_number":978,"context_line":"                                      (fake.PROJECT_ID, self.group1.id),"},{"line_number":979,"context_line":"                                      version\u003dGROUP_MICRO_VERSION)"},{"line_number":980,"context_line":"        add_volumes \u003d add_volume.id"},{"line_number":981,"context_line":"        body \u003d {\"group\": {\"name\": \"new name\","},{"line_number":982,"context_line":"                          \"description\": None,"},{"line_number":983,"context_line":"                          \"add_volumes\": add_volumes,"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_a7bc1f06","line":980,"updated":"2017-09-07 12:27:13.000000000","message":"nit: Don\u0027t see much point in creating this variable, we could use add_volume.id directly on L983","commit_id":"2970092d0442f727d57fb3779603f68f8b39ac51"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"cbd0181cca3a33e78949ff079fbccd864453a0fb","unresolved":false,"context_lines":[{"line_number":986,"context_line":"        self.assertRaises(webob.exc.HTTPBadRequest,"},{"line_number":987,"context_line":"                          self.controller.update,"},{"line_number":988,"context_line":"                          req, self.group1.id, body)"},{"line_number":989,"context_line":"        add_volume.destroy()"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"    @ddt.data((\u00273.11\u0027, \u0027fake_group_001\u0027,"},{"line_number":992,"context_line":"               fields.GroupStatus.AVAILABLE,"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_0743f314","line":989,"updated":"2017-09-07 12:27:13.000000000","message":"nit: Confirm volume in DB has not change","commit_id":"2970092d0442f727d57fb3779603f68f8b39ac51"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"d8bf230bdb3ec53cfaaabcc2b926f155b09c83c4","unresolved":false,"context_lines":[{"line_number":848,"context_line":""},{"line_number":849,"context_line":"    @ddt.data(fields.GroupStatus.CREATING, fields.GroupStatus.UPDATING)"},{"line_number":850,"context_line":"    def test_update_group_any_state(self, status):"},{"line_number":851,"context_line":"        self.group1.status \u003d status"},{"line_number":852,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027/v3/%s/groups/%s/update\u0027 %"},{"line_number":853,"context_line":"                                      (fake.PROJECT_ID, self.group1.id),"},{"line_number":854,"context_line":"                                      version\u003dGROUP_MICRO_VERSION)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f2577a7_6454f0fe","line":851,"updated":"2017-09-15 15:38:10.000000000","message":"nit: We would probably want to save this to the DB to make it clear that it\u0027s independent.","commit_id":"fdfb2d51a4f362091ab5e94981d18d7741f11cf6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"d8bf230bdb3ec53cfaaabcc2b926f155b09c83c4","unresolved":false,"context_lines":[{"line_number":997,"context_line":""},{"line_number":998,"context_line":"        vol \u003d objects.Volume.get_by_id(self.ctxt, add_volume.id)"},{"line_number":999,"context_line":"        self.assertEqual(add_volume.status, vol.status)"},{"line_number":1000,"context_line":"        add_volume.destroy()"},{"line_number":1001,"context_line":""},{"line_number":1002,"context_line":"    @ddt.data((\u00273.11\u0027, \u0027fake_group_001\u0027,"},{"line_number":1003,"context_line":"               fields.GroupStatus.AVAILABLE,"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f2577a7_646d10cb","line":1000,"updated":"2017-09-15 15:38:10.000000000","message":"nit: Unnecessary, the DB will be cleared anyway.","commit_id":"fdfb2d51a4f362091ab5e94981d18d7741f11cf6"}],"releasenotes/notes/group-update-d423eaa18dbcecc1.yaml":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"cbd0181cca3a33e78949ff079fbccd864453a0fb","unresolved":false,"context_lines":[{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Volume group updates of any kind had previously required the group to be"},{"line_number":5,"context_line":"    in ``Available`` status. Non-operational updates to the group name or"},{"line_number":6,"context_line":"    description will now work regardless of the volume group status."}],"source_content_type":"text/x-yaml","patch_set":2,"id":"7f287b81_a1ce5257","line":5,"range":{"start_line":5,"start_character":29,"end_line":5,"end_character":52},"updated":"2017-09-07 12:27:13.000000000","message":"nit: I think this could be confusing for some people, wouldn\u0027t it be easier to say which operations are allowed?","commit_id":"2970092d0442f727d57fb3779603f68f8b39ac51"}]}