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.

Sign up for GitHub

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

Jump to bottom

Created SDK Client to interact with OpenSearch #67

Merged

owaiskazi19 merged 12 commits into opensearch-project:main from owaiskazi19:sdk-client

Aug 23, 2022

Member

owaiskazi19 commented Jul 29, 2022 •

edited

Loading

Description

Created SDK Client to interact with OpenSearch for request calls required by the extensions. Once #20 is done, the next step would be to integrate SDK with the feature/extensions branch of AD to convert the AD plugin to extension. This client will help us to make the API calls from AD extension to OpenSearch.

Build will fail as opensearch-project/OpenSearch#4197 should be merged first

Issues Resolved

Closes #66

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

owaiskazi19 requested a review from a team

July 29, 2022 20:40

owaiskazi19 commented

View reviewed changes

src/main/java/org/opensearch/sdk/SDKClient.java Outdated Show resolved Hide resolved

owaiskazi19 requested review from dbwiddis and saratvemulapalli

July 29, 2022 21:21

dbwiddis requested changes

View reviewed changes

Member

dbwiddis left a comment

Need to decide what connection and authentication protocols we want to use so we can build this securely from the start.

src/main/java/org/opensearch/sdk/SDKClient.java Outdated Show resolved Hide resolved

src/main/java/org/opensearch/sdk/SDKClient.java Outdated Show resolved Hide resolved

src/main/java/org/opensearch/sdk/SDKClient.java Outdated Show resolved Hide resolved

src/main/java/org/opensearch/sdk/SDKClient.java Outdated Show resolved Hide resolved

src/test/java/org/opensearch/sdk/TestSDKClient.java Outdated Show resolved Hide resolved

src/test/java/org/opensearch/sdk/FailingTransport.java Outdated Show resolved Hide resolved

saratvemulapalli reviewed

View reviewed changes

build.gradle Show resolved Hide resolved

src/main/java/org/opensearch/sdk/SDKClient.java Outdated Show resolved Hide resolved

src/main/java/org/opensearch/sdk/SDKClient.java Outdated Show resolved Hide resolved

src/main/java/org/opensearch/sdk/SDKClient.java Outdated Show resolved Hide resolved

src/test/java/org/opensearch/sdk/FailingTransport.java Outdated Show resolved Hide resolved

owaiskazi19 force-pushed the sdk-client branch from 2439fac to c16f6bf Compare

August 12, 2022 00:19

owaiskazi19 mentioned this pull request

[FEATURE] Add Security for SDKClient #90

Open

owaiskazi19 requested review from dbwiddis, peternied and saratvemulapalli

August 12, 2022 00:45

dbwiddis reviewed

View reviewed changes

src/main/java/org/opensearch/sdk/ExtensionsRunner.java Outdated Show resolved Hide resolved

src/main/java/org/opensearch/sdk/SDKClient.java Outdated Show resolved Hide resolved

saratvemulapalli reviewed

View reviewed changes

src/main/java/org/opensearch/sdk/SDKClient.java Outdated Show resolved Hide resolved

saratvemulapalli reviewed

View reviewed changes

src/main/java/org/opensearch/sdk/SDKClient.java Outdated Show resolved Hide resolved

src/main/java/org/opensearch/sdk/SDKClient.java

+                   */
+                  public OpenSearchClient createClient(String hostAddress, int port) throws IOException {
+                      RestClientBuilder builder = RestClient.builder(new HttpHost(hostAddress, port));
+                      builder.setStrictDeprecationMode(true);

Member

saratvemulapalli Aug 17, 2022

Curious, what does this do?
Side note: I could only find elasticsearch documentation[1].
@VachaShah

[1] https://javadoc.io/static/org.elasticsearch.client/elasticsearch-rest-client/6.8.3/org/elasticsearch/client/RestClientBuilder.html

src/test/java/org/opensearch/sdk/TestSDKClient.java Outdated Show resolved Hide resolved

src/main/java/org/opensearch/sdk/SDKClient.java Outdated Show resolved Hide resolved

owaiskazi19 requested review from dbwiddis and saratvemulapalli

August 19, 2022 19:31

owaiskazi19 added 9 commits

August 22, 2022 10:57


          Integrated SDK Client to make request to OpenSearch

a8d5d39

Signed-off-by: Owais Kazi <[email protected]>


          Added test and restructure SDK

f148523

Signed-off-by: Owais Kazi <[email protected]>


          Added license

a777644

Signed-off-by: Owais Kazi <[email protected]>


          Send config from initial request

009fbbe

Signed-off-by: Owais Kazi <[email protected]>


          Utilized the hostaddress and port from extension initialize request

Signed-off-by: Owais Kazi <[email protected]>


          Integrated SDK Client to make request to OpenSearch

bd0f00c

Signed-off-by: Owais Kazi <[email protected]>


          Added test and restructure SDK

d822c59

Signed-off-by: Owais Kazi <[email protected]>


          Removed security related attributes

b25bccf

Signed-off-by: Owais Kazi <[email protected]>


          Updated branch

e0a24cf

Signed-off-by: Owais Kazi <[email protected]>

owaiskazi19 force-pushed the sdk-client branch from 623836f to e0a24cf Compare

August 22, 2022 18:09

dbwiddis requested changes

View reviewed changes

src/main/java/org/opensearch/sdk/ExtensionsRunner.java Outdated Show resolved Hide resolved

src/main/java/org/opensearch/sdk/ExtensionsRunner.java Outdated Show resolved Hide resolved

src/main/java/org/opensearch/sdk/SDKClient.java Outdated Show resolved Hide resolved

src/main/java/org/opensearch/sdk/SDKClient.java Outdated Show resolved Hide resolved

src/main/java/org/opensearch/sdk/SDKClient.java Outdated Show resolved Hide resolved

src/main/java/org/opensearch/sdk/SDKClient.java Outdated Show resolved Hide resolved

src/main/java/org/opensearch/sdk/SDKClient.java Show resolved Hide resolved

src/main/java/org/opensearch/sdk/SDKClient.java Outdated Show resolved Hide resolved

src/test/java/org/opensearch/sdk/FailingTransport.java Outdated Show resolved Hide resolved

src/test/java/org/opensearch/sdk/TestSDKClient.java Outdated Show resolved Hide resolved

This was referenced Aug 22, 2022

[Feature/extensions] Added HostAddress and Port in initial extension request opensearch-project/OpenSearch#4197

Merged

[Feature/extensions] Revert "[Feature/extensions] Added HostAddress and Port in initial extension request" opensearch-project/OpenSearch#4277

Merged


          PR comments

29f07cf

Signed-off-by: Owais Kazi <[email protected]>


          Changed the method name

f258869

Signed-off-by: Owais Kazi <[email protected]>

owaiskazi19 requested a review from dbwiddis

August 22, 2022 20:11

dbwiddis previously approved these changes

View reviewed changes

Member

dbwiddis left a comment

LGTM. I'm still a bit confused on the client naming stuff, but that can be clarified in later PRs.

src/main/java/org/opensearch/sdk/SDKClient.java Outdated Show resolved Hide resolved

src/main/java/org/opensearch/sdk/SDKClient.java Outdated Show resolved Hide resolved


          Small changes

997ed6d

Signed-off-by: Owais Kazi <[email protected]>

owaiskazi19 dismissed dbwiddis’s stale review via

997ed6d

August 22, 2022 22:15

owaiskazi19 requested a review from dbwiddis

August 22, 2022 22:15

dbwiddis approved these changes

View reviewed changes

joshpalis self-requested a review

August 23, 2022 17:10

joshpalis approved these changes

View reviewed changes

owaiskazi19 merged commit 7af4f07 into opensearch-project:main

Member

dblock commented Oct 5, 2022

Is it strange that the SDK depends on the Java client? 🤔 Would the Java client itself ever want to use the SDK?

Member

saratvemulapalli commented Oct 5, 2022

@dblock thats a good question.
SDK uses RestAPIs to talk to OpenSearch instead of using traditional internal client built by OpenSearch and handed over to plugins[1].

Do you think the java client would ever need to use the SDK?

[1] https://github.com/opensearch-project/OpenSearch/blob/main/server/src/main/java/org/opensearch/plugins/Plugin.java#L141

Member

dblock commented Oct 6, 2022

I can't come up with an example, I came from an angle of wanting the SDK to have its dependencies to a minimum, but I suppose until I have a good reason things should remain as is ;)

kokibas pushed a commit to kokibas/opensearch-sdk-java that referenced this pull request


          Created SDK Client to interact with OpenSearch (opensearch-project#67)

22beb07

* Integrated SDK Client to make request to OpenSearch

Signed-off-by: Owais Kazi <[email protected]>

* Added test and restructure SDK

Signed-off-by: Owais Kazi <[email protected]>

* Added license

Signed-off-by: Owais Kazi <[email protected]>

* Send config from initial request

Signed-off-by: Owais Kazi <[email protected]>

* Utilized the hostaddress and port from extension initialize request

Signed-off-by: Owais Kazi <[email protected]>

* Integrated SDK Client to make request to OpenSearch

Signed-off-by: Owais Kazi <[email protected]>

* Added test and restructure SDK

Signed-off-by: Owais Kazi <[email protected]>

* Removed security related attributes

Signed-off-by: Owais Kazi <[email protected]>

* Updated branch

Signed-off-by: Owais Kazi <[email protected]>

* PR comments

Signed-off-by: Owais Kazi <[email protected]>

* Changed the method name

Signed-off-by: Owais Kazi <[email protected]>

* Small changes

Signed-off-by: Owais Kazi <[email protected]>

Signed-off-by: Owais Kazi <[email protected]>

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

Reviewers

joshpalis joshpalis approved these changes

dbwiddis dbwiddis approved these changes

peternied Awaiting requested review from peternied

saratvemulapalli Awaiting requested review from saratvemulapalli saratvemulapalli is a code owner

Labels

None yet