-
Notifications
You must be signed in to change notification settings - Fork 114
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
enhance: Add Describe User methods #697
enhance: Add Describe User methods #697
Conversation
Welcome @punkerpunker! It looks like this is your first PR to milvus-io/milvus-sdk-go 🎉 |
a4730b9
to
dd101c1
Compare
Failing tests doesn't seem to be related to this specific change btw |
@punkerpunker thanks for the contribution. I've fixed failing cases. could you please rebase and fix the lint issue reported in |
@congqixia seems like fixed |
/kind enhancement |
Thanks @congqixia, once again the tests falling doesn't seem to be related to the change :) |
Hey @congqixia, are we concerned about something in particular so we didn't merge this one yet? Want to start working on tf-provider, and that piece is missing :) Let me know if I can help in any way |
@punkerpunker sorry for the late reply. Master branch e2e cases was unstable due to rapid change of milvus master behavior. We have just fixed the cases. You can rebase your PR now |
Signed-off-by: punkerpunker <[email protected]>
Signed-off-by: punkerpunker <[email protected]>
Signed-off-by: punkerpunker <[email protected]>
3cea4a8
to
891d019
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: congqixia, punkerpunker The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
resolves: #698
I've added an entity:
And two methods and tests for them:
DescribeUser(ctx context.Context, username string) (entity.UserDescription, error)
- which returns UserDescription for a specific user.DescribeUsers(ctx context.Context) ([]entity.UserDescription, error)
- which returns UserDescription for all users presented.The thing that I want to raise is that I am not sure if it is feasible to return empty
UserDescription
entity in case the user fromDescribeUser
method doesn't exist, I would highly appreciate suggestions here.Also, I am not much familiar with the
milvus-sdk-go
codebase and this is my first PR here, so any comments/feedback/suggestions would be highly appreciated, thanks!