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

enhance: add resource group declarative api #755

Merged

Conversation

chyezh
Copy link
Contributor

@chyezh chyezh commented Feb 6, 2024

issue: milvus-io/milvus#32282

  • Add UpdateResourceGroups and modify AddResourceGroup api.

  • Add example for add resource group declarative api.

@chyezh
Copy link
Contributor Author

chyezh commented Feb 6, 2024

Some code should not be merged into sdk.
reopen it after ready.

@chyezh chyezh reopened this Feb 6, 2024
@chyezh chyezh force-pushed the feat_milvus_elastic_serverless_v1 branch from e0ef263 to e8f98a6 Compare February 20, 2024 02:42
@mergify mergify bot added the ci-passed label Feb 20, 2024
@chyezh chyezh force-pushed the feat_milvus_elastic_serverless_v1 branch from e8f98a6 to 2227ad0 Compare March 28, 2024 09:10
@chyezh chyezh force-pushed the feat_milvus_elastic_serverless_v1 branch from 2227ad0 to d927743 Compare April 9, 2024 03:27
@mergify mergify bot removed the ci-passed label Apr 9, 2024
- Add UpdateResourceGroups and modify AddResourceGroup api.

Signed-off-by: chyezh <[email protected]>
@chyezh chyezh force-pushed the feat_milvus_elastic_serverless_v1 branch from d927743 to e836ded Compare April 15, 2024 02:37
@mergify mergify bot added the ci-passed label Apr 15, 2024
@chyezh chyezh changed the title enhance: add updates resource group for java sdk enhance: add resource group declarative api Apr 15, 2024
@mergify mergify bot removed the ci-passed label Apr 15, 2024
@chyezh chyezh force-pushed the feat_milvus_elastic_serverless_v1 branch from e333af1 to 0016e15 Compare April 15, 2024 07:21
@mergify mergify bot added the ci-passed label Apr 15, 2024
ResourceGroupInfo.Builder builder = ResourceGroupInfo.newBuilder()
.withResourceGroupName(resourceGroupName)
.withRequestsNodeNum(resourceGroupInfo.getResourceGroup().getConfig().getRequests().getNodeNum())
.withConfig(new ResourceGroupConfig(resourceGroupInfo.getResourceGroup().getConfig()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's weird to pass rg config and requestNodeNum at same time

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redundant code here, I will remove it.

.withCollectionName(collection)
.withSourceGroupName(currentResourceGroupName)
.withTargetGroupName(resourceGroupName)
.withReplicaNumber(1L)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why just we transfer only one replica to target resource group?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an example. For easy coding, add a single replica limits here.
I will add a comment to explain it.

if (replicas.getReplicasCount() > 1) {
// Multi replica is now supported now.
throw new RuntimeException(String.format("Replica number {} is greater than 1, did not support now",
replicas.getReplicasCount()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

multi replica is supported now

return null;
}

return replicas.getReplicasList().get(0).getResourceGroupName();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@weiliu1031
Copy link
Contributor

/lgtm

Copy link
Collaborator

@yelusion2 yelusion2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@sre-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chyezh, yelusion2

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sre-ci-robot sre-ci-robot merged commit 491fe0d into milvus-io:master Apr 16, 2024
5 checks passed
@chyezh chyezh deleted the feat_milvus_elastic_serverless_v1 branch April 16, 2024 03:50
chyezh added a commit to chyezh/milvus-sdk-java that referenced this pull request Apr 18, 2024
…)"

This reverts commit 491fe0d.
Declarative Resource Group API should not released in 2.4.0.

Signed-off-by: chyezh <[email protected]>
sre-ci-robot pushed a commit that referenced this pull request Apr 18, 2024
This reverts commit 491fe0d.
Declarative Resource Group API should not released in 2.4.0.

Signed-off-by: chyezh <[email protected]>
chyezh added a commit to chyezh/milvus-sdk-java that referenced this pull request Apr 26, 2024
* enhance: add updates resource group for java sdk

- Add UpdateResourceGroups and modify AddResourceGroup api.

Signed-off-by: chyezh <[email protected]>

* fix: resource group example

Signed-off-by: chyezh <[email protected]>

* fix: remove redundant code

Signed-off-by: chyezh <[email protected]>

---------

Signed-off-by: chyezh <[email protected]>
sre-ci-robot pushed a commit that referenced this pull request Apr 26, 2024
* enhance: add resource group declarative api (#755)

* enhance: add updates resource group for java sdk

- Add UpdateResourceGroups and modify AddResourceGroup api.

Signed-off-by: chyezh <[email protected]>

* fix: resource group example

Signed-off-by: chyezh <[email protected]>

* fix: remove redundant code

Signed-off-by: chyezh <[email protected]>

---------

Signed-off-by: chyezh <[email protected]>

* fix: resource group: dependency lost in example, and support java8

Signed-off-by: chyezh <[email protected]>

---------

Signed-off-by: chyezh <[email protected]>
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