[Feature/multi_tenancy] SdkClient interface with local and remote implementations for Connector #2459
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| /* | ||
| * Copyright OpenSearch Contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| * | ||
| * The OpenSearch Contributors require contributions made to | ||
| * this file be licensed under the Apache-2.0 license or a | ||
| * compatible open source license. | ||
| */ | ||
| package org.opensearch.sdk; | ||
|
|
||
| public class DeleteDataObjectRequest { | ||
|
|
||
| private final String index; | ||
| private final String id; | ||
|
|
||
| /** | ||
| * Instantiate this request with an index and id. | ||
| * <p> | ||
| * For data storage implementations other than OpenSearch, an index may be referred to as a table and the id may be referred to as a primary key. | ||
| * @param index the index location to delete the object | ||
| * @param id the document id | ||
| */ | ||
| public DeleteDataObjectRequest(String index, String id) { | ||
| this.index = index; | ||
| this.id = id; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the index | ||
| * @return the index | ||
| */ | ||
| public String index() { | ||
| return this.index; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the document id | ||
| * @return the id | ||
| */ | ||
| public String id() { | ||
| return this.id; | ||
| } | ||
|
|
||
| /** | ||
| * Class for constructing a Builder for this Request Object | ||
| */ | ||
| public static class Builder { | ||
| private String index = null; | ||
| private String id = null; | ||
|
|
||
| /** | ||
| * Empty Constructor for the Builder object | ||
| */ | ||
| public Builder() {} | ||
|
|
||
| /** | ||
| * Add an index to this builder | ||
| * @param index the index to put the object | ||
| * @return the updated builder | ||
| */ | ||
| public Builder index(String index) { | ||
| this.index = index; | ||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Add an id to this builder | ||
| * @param id the document id | ||
| * @return the updated builder | ||
| */ | ||
| public Builder id(String id) { | ||
| this.id = id; | ||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Builds the object | ||
| * @return A {@link DeleteDataObjectRequest} | ||
| */ | ||
| public DeleteDataObjectRequest build() { | ||
| return new DeleteDataObjectRequest(this.index, this.id); | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,141 @@ | ||
| /* | ||
| * Copyright OpenSearch Contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| * | ||
| * The OpenSearch Contributors require contributions made to | ||
| * this file be licensed under the Apache-2.0 license or a | ||
| * compatible open source license. | ||
| */ | ||
| package org.opensearch.sdk; | ||
|
|
||
| import org.opensearch.action.support.replication.ReplicationResponse.ShardInfo; | ||
| import org.opensearch.core.common.Strings; | ||
| import org.opensearch.core.index.shard.ShardId; | ||
|
|
||
| public class DeleteDataObjectResponse { | ||
| private final String id; | ||
| private final ShardId shardId; | ||
| private final ShardInfo shardInfo; | ||
| private final boolean deleted; | ||
|
|
||
| /** | ||
| * Instantiate this request. | ||
| * <p> | ||
| * For data storage implementations other than OpenSearch, an index may be referred to as a table and the id may be referred to as a primary key. | ||
| * @param id the document id | ||
| * @param shardId the shard id | ||
| * @param shardInfo the shard info | ||
| * @param deleted Whether the object was deleted. Use {@code false} if the object was not found. | ||
| */ | ||
| public DeleteDataObjectResponse(String id, ShardId shardId, ShardInfo shardInfo, boolean deleted) { | ||
| this.id = id; | ||
| this.shardId = shardId; | ||
| this.shardInfo = shardInfo; | ||
| this.deleted = deleted; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the document id | ||
| * @return the id | ||
| */ | ||
| public String id() { | ||
| return id; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the shard id. | ||
| * @return the shard id, or a generated id if shards are not applicable | ||
| */ | ||
| public ShardId shardId() { | ||
| return shardId; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the shard info. | ||
| * @return the shard info, or generated info if shards are not applicable | ||
| */ | ||
| public ShardInfo shardInfo() { | ||
| return shardInfo; | ||
| } | ||
|
|
||
| /** | ||
| * Returns whether deletion was successful | ||
| * @return true if deletion was successful, false if the object was not found | ||
| */ | ||
| public boolean deleted() { | ||
| return deleted; | ||
| } | ||
|
|
||
| /** | ||
| * Class for constructing a Builder for this Response Object | ||
| */ | ||
| public static class Builder { | ||
| private String id = null; | ||
| private ShardId shardId = null; | ||
| private ShardInfo shardInfo = null; | ||
| private boolean deleted = false; | ||
|
|
||
| /** | ||
| * Empty Constructor for the Builder object | ||
| */ | ||
| public Builder() {} | ||
|
|
||
| /** | ||
| * Add an id to this builder | ||
| * @param id the id to add | ||
| * @return the updated builder | ||
| */ | ||
| public Builder id(String id) { | ||
| this.id = id; | ||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Adds a shard id to this builder | ||
| * @param shardId the shard id to add | ||
| * @return the updated builder | ||
| */ | ||
| public Builder shardId(ShardId shardId) { | ||
| this.shardId = shardId; | ||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Adds a generated shard id to this builder | ||
| * @param indexName the index name to generate a shard id | ||
| * @return the updated builder | ||
| */ | ||
| public Builder shardId(String indexName) { | ||
| this.shardId = new ShardId(indexName, Strings.UNKNOWN_UUID_VALUE, 0); | ||
|
There was a problem hiding this comment. I'm trying to understand this part more. What's the significance of generating a random shard ID here? How this can help end user? There was a problem hiding this comment. Answered above and also here, but I"ll repeat: The this.shardId = Objects.requireNonNull(shardId);When there isn't one provided (such as DynamoDb) this code creates a simulated one. There was a problem hiding this comment. Yes, I saw your answer earlier too. I think for DDB / other non OS cases, if we just share a random shardID, that could be confusing for customers. I would prefer sharing any static String, maybe like There was a problem hiding this comment.
The |
||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Adds shard information (statistics) to this builder | ||
| * @param shardInfo the shard info to add | ||
| * @return the updated builder | ||
| */ | ||
| public Builder shardInfo(ShardInfo shardInfo) { | ||
| this.shardInfo = shardInfo; | ||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Add a deleted status to this builder | ||
| * @param deleted the deleted status to add | ||
| * @return the updated builder | ||
| */ | ||
| public Builder deleted(boolean deleted) { | ||
| this.deleted = deleted; | ||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Builds the object | ||
| * @return A {@link DeleteDataObjectResponse} | ||
| */ | ||
| public DeleteDataObjectResponse build() { | ||
| return new DeleteDataObjectResponse(this.id, this.shardId, this.shardInfo, this.deleted); | ||
| } | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| /* | ||
| * Copyright OpenSearch Contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| * | ||
| * The OpenSearch Contributors require contributions made to | ||
| * this file be licensed under the Apache-2.0 license or a | ||
| * compatible open source license. | ||
| */ | ||
| package org.opensearch.sdk; | ||
|
|
||
| import org.opensearch.search.fetch.subphase.FetchSourceContext; | ||
|
|
||
| public class GetDataObjectRequest { | ||
|
|
||
| private final String index; | ||
| private final String id; | ||
| private final FetchSourceContext fetchSourceContext; | ||
|
|
||
| /** | ||
| * Instantiate this request with an index and id | ||
| * <p> | ||
| * For data storage implementations other than OpenSearch, an index may be referred to as a table and the id may be referred to as a primary key. | ||
| * @param index the index location to get the object | ||
| * @param id the document id | ||
| * @param fetchSourceContext the context to use when fetching _source | ||
| */ | ||
| public GetDataObjectRequest(String index, String id, FetchSourceContext fetchSourceContext) { | ||
| this.index = index; | ||
| this.id = id; | ||
| this.fetchSourceContext = fetchSourceContext; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the index | ||
| * @return the index | ||
| */ | ||
| public String index() { | ||
| return this.index; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the document id | ||
| * @return the id | ||
| */ | ||
| public String id() { | ||
| return this.id; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the context for fetching _source | ||
| * @return the fetchSourceContext | ||
| */ | ||
| public FetchSourceContext fetchSourceContext() { | ||
| return this.fetchSourceContext; | ||
| } | ||
|
|
||
| /** | ||
| * Class for constructing a Builder for this Request Object | ||
| */ | ||
| public static class Builder { | ||
| private String index = null; | ||
| private String id = null; | ||
| private FetchSourceContext fetchSourceContext; | ||
|
|
||
| /** | ||
| * Empty Constructor for the Builder object | ||
| */ | ||
| public Builder() {} | ||
|
|
||
| /** | ||
| * Add an index to this builder | ||
| * @param index the index to put the object | ||
| * @return the updated builder | ||
| */ | ||
| public Builder index(String index) { | ||
| this.index = index; | ||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Add an id to this builder | ||
| * @param id the document id | ||
| * @return the updated builder | ||
| */ | ||
| public Builder id(String id) { | ||
| this.id = id; | ||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Add a fetchSourceContext to this builder | ||
| * @param fetchSourceContext the fetchSourceContext | ||
| * @return the updated builder | ||
| */ | ||
| public Builder fetchSourceContext(FetchSourceContext fetchSourceContext) { | ||
| this.fetchSourceContext = fetchSourceContext; | ||
|
There was a problem hiding this comment. what happens if the There was a problem hiding this comment. That's the default value for the |
||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Builds the request | ||
| * @return A {@link GetDataObjectRequest} | ||
| */ | ||
| public GetDataObjectRequest build() { | ||
| return new GetDataObjectRequest(this.index, this.id, this.fetchSourceContext); | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why are these fields required? Shard details are specific to opensearch which will make it difficult to extend the interface to non-opensearch data stores.
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.
I previously answered that question when you asked it here.
The return value to the end user is created from a
DeleteResponseobject, which requires a non-null ShardId:So I have to have a Shard ID. I can create one, or I can pass along the actual one that is returned from cluster-based OpenSearch. Which do you prefer? I see no good reason to delete information that exists. It can be ignored for other implementations.
Similarly, for
ShardInfo, the return value from the Delete Connector API looks like this:{ "_index": ".plugins-ml-connector", "_id": "qU00aowBH9rhI2IwnrH3", "_version": 2, "result": "deleted", "_shards": { "total": 1, "successful": 1, "failed": 0 }, "_seq_no": 2, "_primary_term": 1 }I can either pass along those actual statistics or make them up. Which is easier to explain to a customer, that a statistic might be irrelevant for their storage option, or that we're taking away information they used to have?