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.

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

Provide Extension API to OpenSearch #74

Merged
merged 15 commits into from
Aug 20, 2022
Merged

Provide Extension API to OpenSearch #74

merged 15 commits into from
Aug 20, 2022

Conversation

dbwiddis
Copy link
Member

@dbwiddis dbwiddis commented Aug 2, 2022

Signed-off-by: Daniel Widdis widdis@gmail.com

Description

Companion PR: opensearch-project/OpenSearch#4100

Extensions will report their APIs to the ExtensionsOrchestrator upon initial registration and potentially later in the Extension's lifecycle. These APIs will be registered and mapped so that when Users submit API requests they can be forwarded to the appropriate extensions.

This PR handles the communication of the REST API from the Extension to OpenSearch. Future PRs will complete the process of registering the REST Handlers and redirecting inbound requests.

Issues Resolved

Fixes #68

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.

@dbwiddis
Provide Extension API to OpenSearch
98be7a8 
Signed-off-by: Daniel Widdis <widdis@gmail.com>
@dbwiddis dbwiddis requested a review from a team August 2, 2022 21:56
@dbwiddis dbwiddis mentioned this pull request Aug 2, 2022
Merged
5 tasks
@dbwiddis
Fix newline, add test
17a509b 
Signed-off-by: Daniel Widdis <widdis@gmail.com>
@dbwiddis dbwiddis requested a review from ryanbogan August 3, 2022 05:17
dbwiddis added 2 commits August 3, 2022 21:00
@dbwiddis
Clarify Valid API format
8de2af7 
Signed-off-by: Daniel Widdis <widdis@gmail.com>
@dbwiddis
Store uniqueID of extensions
2e17d96 
Signed-off-by: Daniel Widdis <widdis@gmail.com>
@dbwiddis
Copy link
Member Author

dbwiddis commented Aug 4, 2022

On extension orchestrator log:

[2022-08-04T16:20:43,929][INFO ][o.o.e.ExtensionsOrchestrator] [dev-dsk-widdisd-2a-8f5e7316.us-west-2.amazon.com] Initialized extension: sample-extension
[2022-08-04T16:20:43,930][INFO ][o.o.e.ExtensionsOrchestrator] [dev-dsk-widdisd-2a-8f5e7316.us-west-2.amazon.com] Registering: GET /_extensions/sample-extension/api_1
[2022-08-04T16:20:43,930][INFO ][o.o.e.ExtensionsOrchestrator] [dev-dsk-widdisd-2a-8f5e7316.us-west-2.amazon.com] Registering: PUT /_extensions/sample-extension/api_2
[2022-08-04T16:20:43,930][INFO ][o.o.e.ExtensionsOrchestrator] [dev-dsk-widdisd-2a-8f5e7316.us-west-2.amazon.com] Registering: POST /_extensions/sample-extension/api_3

On extension runner log:

MESSAGE RECEIVED:E-ǣ�pRegistered node opensearch-sdk-1, extension sample-extension to handle API [GET /api_1, PUT /api_2, POST /api_3]

@dbwiddis dbwiddis marked this pull request as draft August 4, 2022 17:29
@dbwiddis dbwiddis marked this pull request as ready for review August 4, 2022 19:50
@dbwiddis dbwiddis requested a review from joshpalis August 4, 2022 21:00
joshpalis
joshpalis previously approved these changes Aug 4, 2022
View reviewed changes
Copy link
Member

@joshpalis joshpalis left a comment

Choose a reason for hiding this comment

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

LGTM! Just a small suggestion regarding missing javadocs

src/main/java/org/opensearch/sdk/ExtensionsRunner.java Show resolved Hide resolved
@dbwiddis dbwiddis requested a review from joshpalis August 9, 2022 13:29
joshpalis
joshpalis previously approved these changes Aug 9, 2022
View reviewed changes
@dbwiddis dbwiddis requested a review from joshpalis August 13, 2022 18:57
@dbwiddis
Fix errors in merge commit
0e4a903 
Signed-off-by: Daniel Widdis <widdis@gmail.com>
@dbwiddis
Rename Api to RestApi
9a3feab 
Signed-off-by: Daniel Widdis <widdis@gmail.com>
@dbwiddis
Delay sending REST API until after responding to initialization
0dd6519 
Signed-off-by: Daniel Widdis <widdis@gmail.com>
This was referenced Aug 16, 2022
Closed
Closed
@dbwiddis
Move YAML file to classpath
821d27f 
Signed-off-by: Daniel Widdis <widdis@gmail.com>
@dbwiddis
Temporarily disable failing test for debug
4dd6306 
Signed-off-by: Daniel Widdis <widdis@gmail.com>
This was referenced Aug 16, 2022
Closed
Closed
joshpalis and others added 4 commits August 16, 2022 17:14
@joshpalis @dbwiddis
fixed testHandleExtensionInitRequest, added protected transport servi…
c6e2d0b 
…ce setter in ExtensionsRunner for testing purporses

Signed-off-by: Joshua Palis <jpalis@amazon.com>
@dbwiddis
Change transport service setter to package privage
2602177 
Signed-off-by: Daniel Widdis <widdis@gmail.com>
@dbwiddis
Merge pull request #1 from joshpalis/fix-test
8a2e556 
Fixed testHandleExtensionInitRequest
@dbwiddis
Rename RestApi to RestActions or RestPaths
1376493 
Signed-off-by: Daniel Widdis <widdis@gmail.com>
saratvemulapalli
saratvemulapalli reviewed Aug 17, 2022
View reviewed changes
Copy link
Member

@saratvemulapalli saratvemulapalli left a comment

Choose a reason for hiding this comment

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

Thanks @dbwiddis. This is a great start for RestAPIs.
Could you update the DESIGN.md.

@dbwiddis
Copy link
Member Author

Thanks @dbwiddis. This is a great start for RestAPIs. Could you update the DESIGN.md.

I can do that as part of the follow-on PR to address #69 (based a lot on the comment discussing my plan there) to avoid delaying this PR any further.

Any other changes needed before this and the companion PR is merged?

@dbwiddis dbwiddis mentioned this pull request Aug 17, 2022
Merged
peternied
peternied approved these changes Aug 18, 2022
View reviewed changes
Copy link
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

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

Thanks for the updates, looks good for my perspective

saratvemulapalli
saratvemulapalli approved these changes Aug 18, 2022
View reviewed changes
Copy link
Member

@saratvemulapalli saratvemulapalli left a comment

Choose a reason for hiding this comment

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

Thanks @dbwiddis.
Gradle check is failing probably because of the PR pending in OpenSearch.

@dbwiddis
Copy link
Member Author

Thanks @dbwiddis.

Gradle check is failing probably because of the PR pending in OpenSearch.

Indeed as it passes locally when they branch is published to maven local.

What will it take to get that companion PR approved? I'd like to continue work on both branches.

owaiskazi19
owaiskazi19 approved these changes Aug 18, 2022
View reviewed changes
Copy link
Member

@owaiskazi19 owaiskazi19 left a comment

Choose a reason for hiding this comment

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

One small ask :) Rest LGTM!

@dbwiddis dbwiddis merged commit 1fe933f into opensearch-project:main Aug 20, 2022
@dbwiddis dbwiddis deleted the registerApi branch August 20, 2022 00:03
This was referenced Aug 23, 2022
Merged
Merged
@owaiskazi19 owaiskazi19 mentioned this pull request Sep 7, 2022
Merged
kokibas pushed a commit to kokibas/opensearch-sdk-java that referenced this pull request Mar 17, 2023
@dbwiddis @joshpalis @kokibas
Provide Extension API to OpenSearch (opensearch-project#74)
a02464f 
* Provide Extension API to OpenSearch

Signed-off-by: Daniel Widdis <widdis@gmail.com>

* Fix newline, add test

Signed-off-by: Daniel Widdis <widdis@gmail.com>

* Clarify Valid API format

Signed-off-by: Daniel Widdis <widdis@gmail.com>

* Store uniqueID of extensions

Signed-off-by: Daniel Widdis <widdis@gmail.com>

* Fix errors in merge commit

Signed-off-by: Daniel Widdis <widdis@gmail.com>

* Rename Api to RestApi

Signed-off-by: Daniel Widdis <widdis@gmail.com>

* Delay sending REST API until after responding to initialization

Signed-off-by: Daniel Widdis <widdis@gmail.com>

* Move YAML file to classpath

Signed-off-by: Daniel Widdis <widdis@gmail.com>

* Temporarily disable failing test for debug

Signed-off-by: Daniel Widdis <widdis@gmail.com>

* fixed testHandleExtensionInitRequest, added protected transport service setter in ExtensionsRunner for testing purporses

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* Change transport service setter to package privage

Signed-off-by: Daniel Widdis <widdis@gmail.com>

* Rename RestApi to RestActions or RestPaths

Signed-off-by: Daniel Widdis <widdis@gmail.com>

Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Joshua Palis <jpalis@amazon.com>
Co-authored-by: Joshua Palis <jpalis@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@saratvemulapalli saratvemulapalli saratvemulapalli approved these changes

@owaiskazi19 owaiskazi19 owaiskazi19 approved these changes

@peternied peternied peternied approved these changes

@ryanbogan ryanbogan Awaiting requested review from ryanbogan ryanbogan is a code owner

@joshpalis joshpalis Awaiting requested review from joshpalis joshpalis is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEATURE] Send required REST APIs to OpenSearch
5 participants
@dbwiddis @peternied @owaiskazi19 @saratvemulapalli @joshpalis