Skip to content
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

Inconsistent user name format between NewConnWithUser() and NewConnWithClusterAndUser() #244

Open
BenoitKnecht opened this issue May 8, 2020 · 2 comments

Comments

@BenoitKnecht
Copy link

rados.NewConnWithUser(user string) expects user to be a simple user ID (e.g. admin), while rados.NewConnWithClusterAndUser(clusterName string, userName string) expects userName in the form client.<ID> (e.g. client.admin). Omitting the client. prefix in rados.NewConnWithClusterAndUser() results in a rather obscure rados: ret=22, Invalid argument.

This expectation isn't made clear in the documentation, and would require users to know that rados.NewConnWithUser() uses C.rados_create() while rados.NewConnWithClusterAndUser() uses C.rados_create2(). And even then, the documentation for rados_create2() states:

Like rados_create, but don’t assume ‘client.’+id; allow full specification of name.

which sounds like both formats would be valid.

So I think either

  1. Documentation should be updated to make it clear which user name format is valid for each function;
  2. rados.NewConnWithUser() and rados.NewConnWithClusterAndUser() should be made to accept both user name formats, i.e. with or without the client. prefix;
  3. Both functions should use C.rados_create2() under the hood, meaning both would require user names to include the client. prefix (that would be a breaking change though).
@phlogistonjohn
Copy link
Collaborator

We can certainly look to improve the documentation here. I would hesitate to change the behavior of these functions as they've been based on their corresponding librados functions quite a while and as you point out that could be considered a breaking change.

@BenoitKnecht
Copy link
Author

What about option 2 then? It would be fairly easy to add the client. prefix (if it's missing) before passing the user name on to C.rados_create2(), and remove it (if present) before passing the user name on to C.rados_create().

This should be fairly backwards compatible, and it follows the principle of least astonishment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants