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

feat: add MilvusInterceptor #269

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open

feat: add MilvusInterceptor #269

wants to merge 21 commits into from

Conversation

brcarry
Copy link

@brcarry brcarry commented Jan 3, 2025

  • Add MilvusInterceptor
  • Save gRPC request headers in MilvusConnection
  • Support login using uri and token
  • Add ListIndexes, ListCollections, ListPartitions, Upsert, Get, ListUsers, DescribeUser, CreateUser, DropUser and UpdatePassword; rename GetCollectionStats, GetPartitionStats
  • Update milvus proto version to v2.5.0

@sre-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: brcarry
To complete the pull request process, please assign xiaofan-luan after the PR has been reviewed.
You can assign the PR to them by writing /assign @xiaofan-luan in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sre-ci-robot
Copy link

Welcome @brcarry! It looks like this is your first PR to milvus-io/milvus-sdk-cpp 🎉

@yhmo
Copy link
Collaborator

yhmo commented Jan 15, 2025

I just submit a pr to fix the ci failure: #272
Rebase the source code and update your pr to avoid CI failure.

Copy link

mergify bot commented Jan 17, 2025

@brcarry Thanks for your contribution. Please submit with DCO, see the contributing guide https://github.com/milvus-io/milvus/blob/master/CONTRIBUTING.md#developer-certificate-of-origin-dco.

@mergify mergify bot added the needs-dco label Jan 17, 2025
@xiaofan-luan
Copy link
Contributor

any reason of adding this interceptor?

@brcarry
Copy link
Author

brcarry commented Jan 20, 2025

any reason of adding this interceptor?

For authentication and configuration.
For example, when you login using username and password, you need to add the corresponding authentication key-value pair to gRPC request header through an interceptor. Like pymilvus, an interceptor can also be used to add key-value pairs to gRPC request header for configuring database to use, log_level, client_request_id.

Signed-off-by: Ruichen Bao <[email protected]>
Signed-off-by: Ruichen Bao <[email protected]>
Signed-off-by: Ruichen Bao <[email protected]>
- Add MilvusInterceptor
- Save gRPC request headers in MilvusConnection

Signed-off-by: Ruichen Bao <[email protected]>
…abase, AlterDatabaseProperties and DropDatabaseProperties

Signed-off-by: Ruichen Bao <[email protected]>
…s, AddPrivilegesToGroup, RemovePrivilegesFromGroup, GrantPrivilegeV2 and RevokePrivilegeV2

Signed-off-by: Ruichen Bao <[email protected]>
…eResourceGroup, ListResourceGroups and UpdateResourceGroup

Signed-off-by: Ruichen Bao <[email protected]>
Signed-off-by: Ruichen Bao <[email protected]>
Copy link

codecov bot commented Jan 26, 2025

Codecov Report

Attention: Patch coverage is 3.44444% with 1738 lines in your changes missing coverage. Please review.

Project coverage is 59.17%. Comparing base (e90aa2f) to head (086e1ac).
Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
src/impl/MilvusClientImplV2.cpp 0.00% 1326 Missing ⚠️
src/impl/MilvusConnection.cpp 31.25% 77 Missing ⚠️
src/impl/MilvusClientImplV2.h 0.00% 51 Missing ⚠️
src/impl/BulkImport.cpp 0.00% 42 Missing ⚠️
src/impl/types/ResourceGroupConfig.cpp 0.00% 33 Missing ⚠️
src/impl/types/LoadState.cpp 0.00% 31 Missing ⚠️
src/impl/types/DatabaseDesc.cpp 0.00% 28 Missing ⚠️
src/impl/types/GetArguments.cpp 0.00% 28 Missing ⚠️
src/impl/types/ResourceGroupDesc.cpp 0.00% 27 Missing ⚠️
src/impl/types/ListAliasesResult.cpp 0.00% 20 Missing ⚠️
... and 7 more

❌ Your patch check has failed because the patch coverage (3.44%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #269       +/-   ##
===========================================
- Coverage   97.33%   59.17%   -38.17%     
===========================================
  Files          34       50       +16     
  Lines        2628     4426     +1798     
===========================================
+ Hits         2558     2619       +61     
- Misses         70     1807     +1737     
Files with missing lines Coverage Δ
src/impl/MilvusConnection.h 100.00% <ø> (ø)
src/impl/MilvusInterceptor.cpp 100.00% <100.00%> (ø)
src/include/milvus/types/ResourceGroupConfig.h 0.00% <0.00%> (ø)
src/impl/types/NodeInfo.cpp 0.00% <0.00%> (ø)
src/impl/types/RoleDesc.cpp 0.00% <0.00%> (ø)
src/impl/types/ConnectParam.cpp 74.35% <38.88%> (-10.89%) ⬇️
src/impl/types/UserResult.cpp 0.00% <0.00%> (ø)
src/impl/types/PrivilegeGroupInfo.cpp 0.00% <0.00%> (ø)
src/impl/types/AliasDesc.cpp 0.00% <0.00%> (ø)
src/impl/types/ListAliasesResult.cpp 0.00% <0.00%> (ø)
... and 9 more

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

Successfully merging this pull request may close these issues.

4 participants