[Feature/extensions] Extensibility support for getNamedWriteables extension point #3925

joshpalis · 2022-07-15T22:08:29Z

Description

Adds support for registering named writeable entries from extensions and transporting parse requests to extensions to deserialize named writeables that they have registered.

Issues Resolved

#3495

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff

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.

…transport mechanisms to handle named writeable entry registration from extensions and parse requests. Signed-off-by: Joshua Palis <jpalis@amazon.com>

…testing purposes, exten response will now only include the parse action status Signed-off-by: Joshua Palis <jpalis@amazon.com>

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

github-actions · 2022-07-15T22:28:44Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/702/
CommitID: 1c6906f

owaiskazi19 · 2022-07-15T22:32:50Z

server/src/main/java/org/opensearch/extensions/ExtensionsOrchestrator.java

+                if (response.getRegistry().isEmpty() == false) {
+
+                    // Extension has sent over entries to register, initialize inner category map
+                    Map<Class<?>, Map<String, ExtensionReader>> categoryMap = new HashMap<>();


What's the need to have Class type as wildcard and not specific here and the for the rest?

owaiskazi19 · 2022-07-15T22:34:18Z

server/src/main/java/org/opensearch/extensions/ExtensionsOrchestrator.java

+                            // add name and callback method reference to inner reader map
+                            // Since appending the new reader entry returns the previous value associated with key, or null if there was no
+                            // mapping for key, validate that name has not yet been associated with a callback method to it's associated
+                            // extension


Let's use multi line comment and follow javadocs rules to write comments

owaiskazi19 · 2022-07-15T22:36:35Z

server/src/main/java/org/opensearch/extensions/ExtensionsOrchestrator.java

+                            // extension
+                            ExtensionReader newReader = (en, cc, context) -> parseNamedWriteable(en, cc, context);
+                            ExtensionReader oldReader = readers.put(name, newReader);
+                            if (oldReader != null) {


Better way to check

Suggested change

if (oldReader != null) {

if (oldReader != null && oldReader.getClass()) {

(I think the suggestion needs another != null there?)

owaiskazi19 · 2022-07-15T22:37:47Z

server/src/main/java/org/opensearch/extensions/ExtensionsOrchestrator.java

+                        }
+                    }
+
+                    // handle last category and reader entry


Suggested change

// handle last category and reader entry

// Handle last category and reader entry

owaiskazi19 · 2022-07-15T22:37:57Z

server/src/main/java/org/opensearch/extensions/ExtensionsOrchestrator.java

+                    // handle last category and reader entry
+                    categoryMap.put(currentCategory, readers);
+
+                    // attach extension node to categoryMap


Suggested change

// attach extension node to categoryMap

// Attach extension node to categoryMap

owaiskazi19 · 2022-07-15T22:38:38Z

server/src/main/java/org/opensearch/extensions/ExtensionsOrchestrator.java

+        };
+
+        try {
+            logger.info("Sending extension request type: " + REQUEST_EXTENSION_NAMED_WRITEABLE_REGISTRY);


Let's add this logger for all the request we are sending from OS to SDK.

owaiskazi19 · 2022-07-15T22:38:54Z

server/src/main/java/org/opensearch/extensions/ExtensionsOrchestrator.java

+                new NamedWriteableRegistryRequest(true),
+                namedWriteableRegistryResponseHandler
+            );
+            // wait for response from SDK


Suggested change

// wait for response from SDK

// Wait for response from SDK

owaiskazi19 · 2022-07-15T22:42:44Z

server/src/main/java/org/opensearch/extensions/ExtensionsOrchestrator.java

+            }
+
+            @Override
+            public void handleResponse(NamedWriteableRegistryResponse response) {


Let's create a separate class/file for handleResponse and not increase the size of this file

Good call, I'll separate out all the response handlers into different files and place them within the Extensions directory

owaiskazi19 · 2022-07-15T22:43:28Z

server/src/main/java/org/opensearch/common/io/stream/NamedWriteableRegistryParseRequest.java

+        this.context = in.readByteArray();
+    }
+
+    public Class<?> getCategoryClass() throws ClassNotFoundException {


Wildcard type? Can we have a specific type here for the Class?

Looking at opensearch-project/opensearch-sdk-java#54, it seems the type should be NamedWriteable?

Category Classes are expected to extend the NamedWriteable class, I'll change all wildcard type references to reflect this.

owaiskazi19 · 2022-07-15T22:46:25Z

server/src/main/java/org/opensearch/common/io/stream/NamedWriteableRegistryParseResponse.java

+ *
+ * @opensearch.internal
+ */
+public class NamedWriteableRegistryParseResponse extends TransportResponse {


Let's not create a separate class for every boolean response. We already have IndicesModuleNameResponse doing something similar. We can make it more generic and use it for IndicesModuleNameResponse and NamedWriteableRegistryParseResponse

BooleanResponse<NamedWritableRegistry> and BooleanResponse<IndicesModuleName>?

owaiskazi19 · 2022-07-15T22:47:15Z

server/src/main/java/org/opensearch/common/io/stream/NamedWriteableRegistryRequest.java

+ *
+ * @opensearch.internal
+ */
+public class NamedWriteableRegistryRequest extends TransportRequest {


Let's create a generic request class for Boolean Request

owaiskazi19 · 2022-07-15T22:51:46Z

server/src/main/java/org/opensearch/extensions/ExtensionsOrchestrator.java

+            }
+        }
+
+        // TODO : enable dynamic registration of named writeables by returning the extension registry map within Node.java


If this is not the part of the PR. Let's create an issue and link here to not miss the work

Great comment/advice, @owaiskazi19. Any temporary placeholder or TODO code should have an issue tracking it and put the link to the issue in a comment.

dbwiddis

Great start!

dbwiddis · 2022-07-16T01:22:56Z

server/src/main/java/org/opensearch/common/io/stream/NamedWriteableRegistryParseRequest.java

+/**
+ * Extensibility support for Named Writeable Registry: request to extensions to parse context
+ *
+ * @opensearch.internal


For my own education, when is this annotation used?

server/src/main/java/org/opensearch/common/io/stream/NamedWriteableRegistryParseRequest.java

dbwiddis · 2022-07-16T01:29:03Z

server/src/main/java/org/opensearch/common/io/stream/NamedWriteableRegistryParseRequest.java

+    }
+
+    public byte[] getContext() {
+        return this.context;


This returns a reference to this.context which would allow whoever received it to alter the values inside the array. You should use Arrays.copyOf(this.context, this.context.length);

dbwiddis · 2022-07-16T01:50:04Z

server/src/main/java/org/opensearch/common/io/stream/NamedWriteableRegistryParseRequest.java

+    }
+
+    public Class<?> getCategoryClass() throws ClassNotFoundException {
+        Class<?> categoryClass = Class.forName(this.categoryClassName);


Checking whether a valid class exists for categoryClassName may be better placed in the constructor where the String is passed (or read from a stream). You could then catch the exception and then throw an IllegalArgumentException (which you should document with @throws in the javadoc for the constructor!)

Although this opens the question of how to handle the class name when read from a stream. How do we make sure that nodes on both sides of the stream have the same classes? (Relevant to @owaiskazi19 comment above on restricting class type.)

And this opens an even bigger issue is dealing with same-named classes and getting name collisions. You probably want the fully qualified class name here. (EDIT: I see that the intention is to use Class.getName() for this which is fully qualified. But the string parameter should have that documented clearly.)

And given that the parsing signature in ExtensionsReader uses a Class rather than a String we might consider using that here as well. That would both assure a valid class as well as easily give access to the fully qualified name.

Category Class definitions reside within the OpenSearch JVM and are currently exposed to the SDK via the maven local repository, thus ensuring that the extension has the same class. I'll definitely add the check for valid classes within the constructor and also add more clarification with the javadocs.

dbwiddis · 2022-07-16T01:56:47Z

server/src/main/java/org/opensearch/common/io/stream/NamedWriteableRegistryParseRequest.java

+
+    @Override
+    public String toString() {
+        return "NamedWriteableRegistryParseRequest{"


Question for my own education: do we have a standard format for toStrings? Should we?

The pattern we have been following is to print out the request/response type along with the contents of the request/response

It's less the type(class name) and field listing it's more the delimiters. I've seen curly braces, square braces, newlines. Most IDEs will auto-generate a toString from fields. I'm wondering which particular format we are using, if any.

dbwiddis · 2022-07-16T02:27:21Z

server/src/main/java/org/opensearch/extensions/ExtensionReader.java

+    /**
+     * Transports category class, and byte array (context), to the extension identified by the Discovery Node
+     *
+     * @param extensionNode Discovery Node identifying the Extension


(Style) leave an extra space between parameter name and its description.

dbwiddis · 2022-07-16T02:30:12Z

server/src/main/java/org/opensearch/extensions/ExtensionReader.java

+     *
+     */
+
+    void parse(DiscoveryNode extensionNode, Class<?> categoryClass, byte[] context) throws UnknownHostException;


The exception should be added to the javadoc with a @throws. As this is an interface, this javadoc will be inherited by implementations.

dbwiddis · 2022-07-16T02:31:07Z

server/src/main/java/org/opensearch/extensions/ExtensionReader.java

+     * Transports category class, and byte array (context), to the extension identified by the Discovery Node
+     *
+     * @param extensionNode Discovery Node identifying the Extension
+     * @param categoryClass Super class that the reader extends


Should probably have an interface for the superclasses the readers extend, and use that for the type parameter. (And the type parameter needs its own javadoc.)

dbwiddis · 2022-07-16T02:37:40Z

server/src/main/java/org/opensearch/extensions/ExtensionsOrchestrator.java

+                            // extension
+                            ExtensionReader newReader = (en, cc, context) -> parseNamedWriteable(en, cc, context);
+                            ExtensionReader oldReader = readers.put(name, newReader);
+                            if (oldReader != null) {


(I think the suggestion needs another != null there?)

dbwiddis · 2022-07-16T02:39:18Z

server/src/main/java/org/opensearch/extensions/ExtensionsOrchestrator.java

+                                );
+                            }
+                        } catch (ClassNotFoundException e) {
+                            logger.error(e.toString());


May want an explanatory text string in addition to the error here.

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

github-actions · 2022-07-19T21:02:07Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/805/
CommitID: 62fe9f3

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

github-actions · 2022-07-20T23:39:51Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/850/
CommitID: d64f378

…oject/OpenSearch into namedwriteables rebasing fork with feature/extensions

github-actions · 2022-07-21T19:24:38Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/886/
CommitID: 07df663

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

github-actions · 2022-07-21T20:06:25Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/888/
CommitID: 1c99994

owaiskazi19 · 2022-07-21T21:48:08Z

server/src/main/java/org/opensearch/common/io/stream/NamedWriteableRegistryResponse.java

+     */
+    public NamedWriteableRegistryResponse(StreamInput in) throws IOException {
+        super(in);
+        // stream output for registry map begins with a variable integer that tells us the number of entries being sent across the wire


Suggested change

// stream output for registry map begins with a variable integer that tells us the number of entries being sent across the wire

// Stream output for registry map begins with a variable integer that tells us the number of entries being sent across the wire

Here and everywhere 🙂

owaiskazi19 · 2022-07-21T21:49:00Z

server/src/main/java/org/opensearch/extensions/BooleanResponse.java

+ *
+ * @opensearch.internal
+ */
+public class BooleanResponse extends TransportResponse {


We can use the same response for IndicesModuleNameResponse and get rid of that file?

Also, a better name here OpenSearchBooleanResponse?

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

github-actions · 2022-07-21T22:58:20Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/895/
CommitID: 34c2a5e

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

…nsionDir initialization within set up Signed-off-by: Joshua Palis <jpalis@amazon.com>

github-actions · 2022-07-29T22:33:26Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1123/
CommitID: b9f4a80

dbwiddis · 2022-07-31T05:55:31Z

server/src/main/java/org/opensearch/common/io/stream/NamedWriteableRegistryParseRequest.java

+ */
+public class NamedWriteableRegistryParseRequest extends TransportRequest {
+
+    private final Class categoryClass;


Over on your PR on the SDK side this class is restricted to one that extends NamedWriteable so you should add that restriction here (in the constructor, throwing IllegalArgumentException if it's not a compatible class)

I've removed the generic casting on the SDK end, as per your comment on the corresponding SDK PR. Since we already ensure that all category class entries sent over by the extension utilize the NamedWriteable framework, these restrictions are not necessary.

dbwiddis · 2022-07-31T05:56:40Z

server/src/main/java/org/opensearch/common/io/stream/NamedWriteableRegistryParseRequest.java

+    public NamedWriteableRegistryParseRequest(StreamInput in) throws IOException {
+        super(in);
+        try {
+            this.categoryClass = Class.forName(in.readString());


Similar comment to above, add some type safety here if you are going to cast to a superclass on the SDK side.

dbwiddis · 2022-07-31T06:00:28Z

server/src/main/java/org/opensearch/common/io/stream/NamedWriteableRegistryParseRequest.java

+
+    @Override
+    public String toString() {
+        return "NamedWriteableRegistryParseRequest{"


It's less the type(class name) and field listing it's more the delimiters. I've seen curly braces, square braces, newlines. Most IDEs will auto-generate a toString from fields. I'm wondering which particular format we are using, if any.

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

github-actions · 2022-08-01T14:58:23Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1187/
CommitID: 467182e

dbwiddis · 2022-08-01T15:49:39Z

server/src/main/java/org/opensearch/extensions/ExtensionsOrchestrator.java

+    public void getNamedWriteables() {
+
+        // Retrieve named writeable registry entries from each extension
+        for (DiscoveryNode extensionNode : extensionsList) {


You can iterate over extensionsInitializedList here, just in case extensions failed to initialize you won't waste time attempting to getNamedWriteables().

…hat we are only sending extension point requests to initialized nodes Signed-off-by: Joshua Palis <jpalis@amazon.com>

github-actions · 2022-08-01T16:55:10Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1191/
CommitID: bdc558c

github-actions · 2022-08-02T18:21:46Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1295/
CommitID: bdc558c

Rebasing with feature/extensions :

github-actions · 2022-08-02T18:58:44Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1302/
CommitID: d3d52a4

github-actions · 2022-08-02T19:30:57Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1306/
CommitID: d3d52a4

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

github-actions · 2022-08-02T20:21:40Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1309/
CommitID: 861daa4

saratvemulapalli

Thanks @joshpalis.
I did one pass, most the code looks good, really great work!!

I've dropped a couple of questions/suggestions.
I'll do another pass.

saratvemulapalli · 2022-07-26T17:14:10Z

server/src/main/java/org/opensearch/common/io/stream/NamedWriteableRegistryParseRequest.java

+            + "categoryClass="
+            + categoryClass.getName()
+            + ", context length="
+            + context.length


Is context printable?

the context is a byte array, not sure if printing out the entire array would provide any useful information, so I decided to just print the length of the array. I am open to suggestions on how to represent the context.

saratvemulapalli · 2022-08-03T17:45:14Z

server/src/main/java/org/opensearch/node/Node.java

@@ -1155,6 +1155,7 @@ public Node start() throws NodeValidationException {
            : "clusterService has a different local node than the factory provided";
        transportService.acceptIncomingRequests();
        extensionsOrchestrator.extensionsInitialize();
+        extensionsOrchestrator.getNamedWriteables();


How are these namedWritables registered with OpenSearch NamedWritableRegistry?

Currently this is a placeholder trigger. Registering extensions named writeables within the central named writeable registry is a larger problem that I have linked this issue to.

saratvemulapalli · 2022-08-03T17:47:02Z

server/src/main/java/org/opensearch/extensions/ExtensionsOrchestrator.java

+     * @throws UnknownHostException if connection to the extension node failed
+     * @return A map of category classes and their associated names and readers for this discovery node
+     */
+    public Map<DiscoveryNode, Map<Class, Map<String, ExtensionReader>>> getNamedWriteables(DiscoveryNode extensionNode)


Does this method have to be public?

Currently I target this method within my unit tests since extensionsInitializedList cannot be populated unless extensionsInitialize() occurs. The method which calls this iterates through initialized discovery nodes and sends a request to the extension associated with the node, therefore, in order to test this, I would have to call this method directly, which is only possible if it is public

saratvemulapalli · 2022-08-03T17:47:42Z

server/src/main/java/org/opensearch/extensions/ExtensionsOrchestrator.java

+    /**
+     * Iterates through all discovered extensions, sends transport requests for named writeables and consolidates all entires into a central named writeable registry for extensions.
+     */
+    public void getNamedWriteables() {


Similar comment as the SDK.
Can we fold all of this logic to Another Class under extensions package?

Merging feature/extensions

github-actions · 2022-08-03T18:43:53Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1371/
CommitID: 4bc44c2

github-actions · 2022-08-03T19:53:54Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/1384/
CommitID: 4bc44c2

…extension orchestrator to new class, updated tests to reflect change, fixed toString parse request override Signed-off-by: Joshua Palis <jpalis@amazon.com>

github-actions · 2022-08-03T22:37:10Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/1403/
CommitID: 39ab25c

codecov-commenter · 2022-08-03T22:38:55Z

Codecov Report

Merging #3925 (39ab25c) into feature/extensions (eba15fe) will decrease coverage by 0.09%.
The diff coverage is 46.35%.

@@                   Coverage Diff                    @@
##             feature/extensions    #3925      +/-   ##
========================================================
- Coverage                 70.66%   70.56%   -0.10%     
+ Complexity                57107    57079      -28     
========================================================
  Files                      4606     4612       +6     
  Lines                    274633   274810     +177     
  Branches                  40210    40228      +18     
========================================================
- Hits                     194059   193913     -146     
- Misses                    64331    64629     +298     
- Partials                  16243    16268      +25

Impacted Files	Coverage Δ
...pensearch/extensions/ExtensionBooleanResponse.java	`0.00% <0.00%> (ø)`
...mmon/io/stream/NamedWriteableRegistryResponse.java	`17.85% <17.85%> (ø)`
.../io/stream/NamedWriteableRegistryParseRequest.java	`20.68% <20.68%> (ø)`
...a/org/opensearch/extensions/OpenSearchRequest.java	`23.52% <23.52%> (ø)`
...ns/NamedWriteableRegistryParseResponseHandler.java	`50.00% <50.00%> (ø)`
...ch/extensions/ExtensionNamedWriteableRegistry.java	`65.90% <65.90%> (ø)`
...ensions/NamedWriteableRegistryResponseHandler.java	`79.48% <79.48%> (ø)`
.../opensearch/extensions/ExtensionsOrchestrator.java	`73.37% <81.81%> (+1.07%)`	⬆️
server/src/main/java/org/opensearch/node/Node.java	`86.31% <100.00%> (+0.02%)`	⬆️
...search/indices/recovery/RecoveryTargetHandler.java	`0.00% <0.00%> (-100.00%)`	⬇️
... and 478 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

…cronous since APIs will be utlizing the extensionsInitializedList to identify the correct node to send requests to. Since arraylists are not thread safe, this is achieved by adding countdown latches Signed-off-by: Joshua Palis <jpalis@amazon.com>

github-actions · 2022-08-08T19:33:05Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/1577/
CommitID: cd222aa

saratvemulapalli

Thanks @joshpalis for taking care of this.
This is great work!!

For next time, resolve the conversations you've taken care of.
It helps the reviewer to understand what has to be addressed.

joshpalis added 4 commits July 15, 2022 20:35

extensibility support for getNamedWriteables extension point. Adding …

9e31e52

…transport mechanisms to handle named writeable entry registration from extensions and parse requests. Signed-off-by: Joshua Palis <jpalis@amazon.com>

removing string response from parse request, this was added only for …

bbfc81d

…testing purposes, exten response will now only include the parse action status Signed-off-by: Joshua Palis <jpalis@amazon.com>

fixing syntax errors

3d3c113

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

modified logs for NamedWriteableRegistryRequest

1c6906f

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

saratvemulapalli changed the title ~~Extensibility support for getNamedWriteables extension point~~ [Feature/extensions] Extensibility support for getNamedWriteables extension point Jul 15, 2022

owaiskazi19 reviewed Jul 15, 2022

View reviewed changes

dbwiddis reviewed Jul 16, 2022

View reviewed changes

addressing PR comments

62fe9f3

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

addressing PR comments

d64f378

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

Merge branch 'feature/extensions' of https://github.com/opensearch-pr…

07df663

…oject/OpenSearch into namedwriteables rebasing fork with feature/extensions

fixing gradle precommit issues, fixed logger ussage errors

1c99994

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

owaiskazi19 reviewed Jul 21, 2022

View reviewed changes

Fixing all code comment format

34c2a5e

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

added unit tests, added more javadocs, fixed handleException message …

9800380

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

Updated ExtensionsOrchestratorTests, moved extensionYmlLines and exte…

b9f4a80

…nsionDir initialization within set up Signed-off-by: Joshua Palis <jpalis@amazon.com>

dbwiddis reviewed Jul 31, 2022

View reviewed changes

linking additional issue to TODO

467182e

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

dbwiddis reviewed Aug 1, 2022

View reviewed changes

using extensionsInitializedList instead of extensionsList to ensure t…

bdc558c

…hat we are only sending extension point requests to initialized nodes Signed-off-by: Joshua Palis <jpalis@amazon.com>

Merge branch 'feature/extensions' into namedwriteables

d3d52a4

Rebasing with feature/extensions :

updated tests to pass, after changing extension orchestrator to use t…

861daa4

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

saratvemulapalli reviewed Aug 3, 2022

View reviewed changes

Merge branch 'feature/extensions' into namedwriteables

4bc44c2

Merging feature/extensions

addressing PR comments, moving named writeable registry support from …

39ab25c

…extension orchestrator to new class, updated tests to reflect change, fixed toString parse request override Signed-off-by: Joshua Palis <jpalis@amazon.com>

joshpalis requested a review from ryanbogan August 4, 2022 20:45

ryanbogan approved these changes Aug 5, 2022

View reviewed changes

saratvemulapalli approved these changes Aug 8, 2022

View reviewed changes

saratvemulapalli merged commit 716756a into opensearch-project:feature/extensions Aug 8, 2022

joshpalis deleted the namedwriteables branch August 8, 2022 21:16

ryanbogan mentioned this pull request Nov 17, 2022

[Feature/extensions] Merge to main tracking - Extension Points, initial REST implementation and registering Transport Actions #5271

Closed

	if (oldReader != null) {
	if (oldReader != null && oldReader.getClass()) {

	// handle last category and reader entry
	// Handle last category and reader entry

	// attach extension node to categoryMap
	// Attach extension node to categoryMap

	// stream output for registry map begins with a variable integer that tells us the number of entries being sent across the wire
	// Stream output for registry map begins with a variable integer that tells us the number of entries being sent across the wire

[Feature/extensions] Extensibility support for getNamedWriteables extension point #3925

[Feature/extensions] Extensibility support for getNamedWriteables extension point #3925

Conversation

joshpalis commented Jul 15, 2022 • edited Loading

Description

Issues Resolved

Check List

github-actions bot commented Jul 15, 2022

Gradle Check (Jenkins) Run Completed with:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbwiddis Jul 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbwiddis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbwiddis Jul 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbwiddis Jul 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jul 19, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jul 20, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jul 21, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jul 21, 2022

Gradle Check (Jenkins) Run Completed with:

Choose a reason for hiding this comment

owaiskazi19 Jul 21, 2022 • edited Loading

Choose a reason for hiding this comment

owaiskazi19 Jul 21, 2022 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jul 21, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jul 29, 2022

Gradle Check (Jenkins) Run Completed with:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 1, 2022

Gradle Check (Jenkins) Run Completed with:

Choose a reason for hiding this comment

github-actions bot commented Aug 1, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Aug 2, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Aug 2, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Aug 2, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Aug 2, 2022

Gradle Check (Jenkins) Run Completed with:

saratvemulapalli left a comment

Choose a reason for hiding this comment

joshpalis commented Jul 15, 2022 •

edited

Loading

dbwiddis Jul 16, 2022 •

edited

Loading

dbwiddis Jul 16, 2022 •

edited

Loading

dbwiddis Jul 16, 2022 •

edited

Loading

owaiskazi19 Jul 21, 2022 •

edited

Loading

owaiskazi19 Jul 21, 2022 •

edited

Loading