-
Notifications
You must be signed in to change notification settings - Fork 37
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
Add global context index and indices handler #43
Changes from all commits
1096170
14b9a95
2069fb2
a4d052e
21c0788
bce1b62
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -119,8 +119,12 @@ dependencies { | |
|
||
test { | ||
include '**/*Tests.class' | ||
systemProperty 'tests.security.manager', 'false' | ||
} | ||
|
||
def opensearch_tmp_dir = rootProject.file('build/private/opensearch_tmp').absoluteFile | ||
opensearch_tmp_dir.mkdirs() | ||
|
||
jacocoTestReport { | ||
dependsOn test | ||
reports { | ||
|
@@ -137,6 +141,29 @@ task integTest(type: RestIntegTestTask) { | |
tasks.named("check").configure { dependsOn(integTest) } | ||
|
||
integTest { | ||
dependsOn "bundlePlugin" | ||
systemProperty 'tests.security.manager', 'false' | ||
systemProperty 'java.io.tmpdir', opensearch_tmp_dir.absolutePath | ||
systemProperty "https", System.getProperty("https") | ||
systemProperty "user", System.getProperty("user") | ||
systemProperty "password", System.getProperty("password") | ||
|
||
// // Only rest case can run with remote cluster | ||
// if (System.getProperty("tests.rest.cluster") != null) { | ||
// filter { | ||
// includeTestsMatching "org.opensearch.flowframework.rest.*IT" | ||
// } | ||
// } | ||
// | ||
// if (System.getProperty("https") == null || System.getProperty("https") == "false") { | ||
// filter { | ||
// } | ||
// } | ||
|
||
filter { | ||
excludeTestsMatching "org.opensearch.flowframework.indices.*Tests" | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we remove the integ test code from this PR and add it in the next PR as this PR doesn't supports the framework? |
||
|
||
// The --debug-jvm command-line option makes the cluster debuggable; this makes the tests debuggable | ||
if (System.getProperty("test.debug") != null) { | ||
jvmArgs '-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=*:5005' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
/* | ||
* 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.flowframework.constant; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Curious about this choice of package name. Given the class names, seems |
||
|
||
/** | ||
* Representation of common names that used across the project | ||
*/ | ||
public class CommonName { | ||
public static final String GLOBAL_CONTEXT_INDEX_NAME = ".opensearch-flow-framework-global-context"; | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
/* | ||
* 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.flowframework.constant; | ||
|
||
/** | ||
* Representation of common values that are used across project | ||
*/ | ||
public class CommonValue { | ||
|
||
public static Integer NO_SCHEMA_VERSION = 0; | ||
public static final String META = "_meta"; | ||
public static final String SCHEMA_VERSION_FIELD = "schema_version"; | ||
public static final Integer GLOBAL_CONTEXT_INDEX_SCHEMA_VERSION = 1; | ||
public static final String GLOBAL_CONTEXT_INDEX_MAPPING = "{\n " | ||
+ " \"dynamic\": false,\n" | ||
+ " \"_meta\": {\n" | ||
+ " \"schema_version\": " | ||
+ GLOBAL_CONTEXT_INDEX_SCHEMA_VERSION | ||
+ "\n" | ||
+ " },\n" | ||
+ " \"properties\": {\n" | ||
+ " \"pipeline_id\": {\n" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let's change this |
||
+ " \"type\": \"keyword\"\n" | ||
+ " },\n" | ||
+ " \"name\": {\n" | ||
+ " \"type\": \"text\",\n" | ||
+ " \"fields\": {\n" | ||
+ " \"keyword\": {\n" | ||
+ " \"type\": \"keyword\",\n" | ||
+ " \"ignore_above\": 256\n" | ||
+ " }\n" | ||
+ " }\n" | ||
+ " },\n" | ||
+ " \"description\": {\n" | ||
+ " \"type\": \"text\"\n" | ||
+ " },\n" | ||
+ " \"use_case\": {\n" | ||
+ " \"type\": \"keyword\"\n" | ||
+ " },\n" | ||
+ " \"operations\": {\n" | ||
+ " \"type\": \"keyword\"\n" | ||
+ " },\n" | ||
+ " \"version\": {\n" | ||
+ " \"type\": \"nested\",\n" | ||
+ " \"properties\": {\n" | ||
+ " \"template\": {\n" | ||
+ " \"type\": \"integer\"\n" | ||
+ " },\n" | ||
+ " \"compatibility\": {\n" | ||
+ " \"type\": \"integer\"\n" | ||
+ " }\n" | ||
+ " }\n" | ||
+ " },\n" | ||
+ " \"user_inputs\": {\n" | ||
+ " \"type\": \"nested\",\n" | ||
+ " \"properties\": {\n" | ||
+ " \"model_id\": {\n" | ||
+ " \"type\": \"keyword\"\n" | ||
+ " },\n" | ||
+ " \"input_field\": {\n" | ||
+ " \"type\": \"keyword\"\n" | ||
+ " },\n" | ||
+ " \"output_field\": {\n" | ||
+ " \"type\": \"keyword\"\n" | ||
+ " },\n" | ||
+ " \"ingest_pipeline_name\": {\n" | ||
+ " \"type\": \"keyword\"\n" | ||
+ " },\n" | ||
+ " \"index_name\": {\n" | ||
+ " \"type\": \"keyword\"\n" | ||
+ " }\n" | ||
+ " }\n" | ||
+ " },\n" | ||
+ " \"workflows\": {\n" | ||
+ " \"type\": \"text\"\n" | ||
+ " },\n" | ||
+ " \"responses\": {\n" | ||
+ " \"type\": \"text\"\n" | ||
+ " },\n" | ||
+ " \"resources_created\": {\n" | ||
+ " \"type\": \"text\"\n" | ||
+ " }\n" | ||
+ " }\n" | ||
+ "}"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should have a JSON file for mapping and then parse it to create a String. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
/* | ||
* 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.flowframework.exception; | ||
|
||
/** | ||
* Representation of Flow Framework Exceptions | ||
*/ | ||
public class FlowFrameworkException extends RuntimeException { | ||
/** | ||
* Constructor with error message. | ||
* | ||
* @param message message of the exception | ||
*/ | ||
public FlowFrameworkException(String message) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like the idea of having our own custom exceptions. Given that these will eventually need to be processed in a RestResponse, can we consider including the appropriate return code as part of constructing this exception? For example, "index not found" should return |
||
super(message); | ||
} | ||
|
||
/** | ||
* Constructor with specified cause. | ||
* @param cause exception cause | ||
*/ | ||
public FlowFrameworkException(Throwable cause) { | ||
super(cause); | ||
} | ||
Check warning on line 30 in src/main/java/org/opensearch/flowframework/exception/FlowFrameworkException.java Codecov / codecov/patchsrc/main/java/org/opensearch/flowframework/exception/FlowFrameworkException.java#L29-L30
|
||
|
||
/** | ||
* Constructor with specified error message adn cause. | ||
* @param message error message | ||
* @param cause exception cause | ||
*/ | ||
public FlowFrameworkException(String message, Throwable cause) { | ||
super(message, cause); | ||
} | ||
Check warning on line 39 in src/main/java/org/opensearch/flowframework/exception/FlowFrameworkException.java Codecov / codecov/patchsrc/main/java/org/opensearch/flowframework/exception/FlowFrameworkException.java#L38-L39
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
/* | ||
* 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.flowframework.indices; | ||
|
||
import static org.opensearch.flowframework.constant.CommonName.GLOBAL_CONTEXT_INDEX_NAME; | ||
import static org.opensearch.flowframework.constant.CommonValue.GLOBAL_CONTEXT_INDEX_MAPPING; | ||
import static org.opensearch.flowframework.constant.CommonValue.GLOBAL_CONTEXT_INDEX_SCHEMA_VERSION; | ||
|
||
/** | ||
* An enumeration of Flow Framework indices | ||
*/ | ||
public enum FlowFrameworkIndex { | ||
GLOBAL_CONTEXT(GLOBAL_CONTEXT_INDEX_NAME, GLOBAL_CONTEXT_INDEX_MAPPING, GLOBAL_CONTEXT_INDEX_SCHEMA_VERSION); | ||
|
||
private final String indexName; | ||
private final String mapping; | ||
private final Integer version; | ||
|
||
FlowFrameworkIndex(String name, String mapping, Integer version) { | ||
this.indexName = name; | ||
this.mapping = mapping; | ||
this.version = version; | ||
} | ||
|
||
public String getIndexName() { | ||
return indexName; | ||
} | ||
|
||
public String getMapping() { | ||
return mapping; | ||
} | ||
|
||
public Integer getVersion() { | ||
return version; | ||
} | ||
} |
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.
Trying to understand these build.gradle settings better, does this disable security for tests?
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.
We should not disable security manager for the plugin. I know it's deprecated but OpenSearch still uses it and doesn't have our security manager yet.