New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rgw: add a new error code for non-existed subuser. #16095

Merged
merged 1 commit into from Jul 7, 2017

Conversation

Projects
None yet
3 participants
@zhaochao
Contributor

zhaochao commented Jul 4, 2017

When modify or delete non-existed subuser, a new error code
'NoSuchSubUser' is more specific than 'InvalidArgument'.

Signed-off-by: Zhao Chao zhaochao1984@gmail.com

@zhaochao

This comment has been minimized.

Contributor

zhaochao commented Jul 4, 2017

As suggested by @joscollin in #16033 .

@joscollin joscollin self-requested a review Jul 4, 2017

if (ret < 0)
if (ret < 0) {
if (ret == -ENOENT)
ret = -ERR_NO_SUCH_SUBUSER;

This comment has been minimized.

@joscollin

joscollin Jul 5, 2017

Member

With this logic, do you think that ENOENT will be returned at another place in the same originating function or by a different function in the future, so that the execution may reach this point ? Instead it's safe returning a specific error such as `ERR_NO_SUCH_SUBUSER' right ?. What do you say ?

This comment has been minimized.

@zhaochao

zhaochao Jul 5, 2017

Contributor

ERR_NO_SUCH_SUBUSER and the other similar error codes are returned to the rgw frontend and interpreted by the clients, so I think it's safe to use specific erros.

This comment has been minimized.

@joscollin

joscollin Jul 5, 2017

Member

@zhaochao You didn't get my point. I meant: why do you return ENOENT and then on the receiving end modify it to ERR_NO_SUCH_SUBUSER ? You could return the specific error ERR_NO_SUCH_SUBUSER, so that it won't conflict with other errors conditions that may return ENOENT in the future. Please read my previous comment once again.

This comment has been minimized.

@zhaochao

zhaochao Jul 5, 2017

Contributor

@joscollin
Yes, I missed your point in the first comment, sorry. I was hoping to follow the code path just like the ones of RGWUser, narrow the returning of these specific error codes only to the frontend part. After rechecking rgw_user.cc and rgw_admin.cc, I think you are right, it won't conflict and is simpler to return ERR_NO_SUCH_SUBUSR

rgw: add a new error code for non-existed subuser.
When modify or delete non-existed subuser, a new error code
'NoSuchSubUser' is more specific than 'InvalidArgument'.

Signed-off-by: Zhao Chao <zhaochao1984@gmail.com>
@joscollin

Requesting changes based on the previous communication.

@zhaochao

This comment has been minimized.

Contributor

zhaochao commented Jul 5, 2017

@joscollin
PR updated, now ERR_NO_SUCH_SUBUSER will be returned by RGWSubUserPool::execute_modify and RGWSubUserPool::execute_remove.

@joscollin

LGTM

@yuriw yuriw merged commit 457b443 into ceph:master Jul 7, 2017

4 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment