-
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
Adding guardrails to default use case params #658
Conversation
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #658 +/- ##
============================================
- Coverage 72.22% 72.20% -0.02%
- Complexity 680 682 +2
============================================
Files 82 82
Lines 3528 3562 +34
Branches 285 290 +5
============================================
+ Hits 2548 2572 +24
- Misses 854 859 +5
- Partials 126 131 +5 ☔ View full report in Codecov by Sentry. |
src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java
Show resolved
Hide resolved
… at all for default cases Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
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.
LG overall with few comments
src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java
Show resolved
Hide resolved
Signed-off-by: Joshua Palis <[email protected]>
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.
LGTM generally. Lots of nits for your consideration. I'm traveling and unlikely to review again promptly so please don't wait on me to review again.
src/main/java/org/opensearch/flowframework/common/DefaultUseCases.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/common/DefaultUseCases.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java
Outdated
Show resolved
Hide resolved
src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java
Outdated
Show resolved
Hide resolved
src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
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.
LGTM
* Adding guardrails to default use case params Signed-off-by: Joshua Palis <[email protected]> * Updating changelog and adding javadocs Signed-off-by: Joshua Palis <[email protected]> * Fixing test Signed-off-by: Joshua Palis <[email protected]> * Fixing integration tests, covering case in which no content is passed at all for default cases Signed-off-by: Joshua Palis <[email protected]> * Fixing tests Signed-off-by: Joshua Palis <[email protected]> * Fixing rest create workflow action Signed-off-by: Joshua Palis <[email protected]> * addressing PR comments Signed-off-by: Joshua Palis <[email protected]> * fixing test Signed-off-by: Joshua Palis <[email protected]> --------- Signed-off-by: Joshua Palis <[email protected]> (cherry picked from commit 9d28045) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Adding guardrails to default use case params (#658) * Adding guardrails to default use case params * Updating changelog and adding javadocs * Fixing test * Fixing integration tests, covering case in which no content is passed at all for default cases * Fixing tests * Fixing rest create workflow action * addressing PR comments * fixing test --------- (cherry picked from commit 9d28045) Signed-off-by: Joshua Palis <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Description
Adds a requiredParams field to the DefaultUseCases enum which is used validate the given request body of default use case request
Examples :
Issues Resolved
#655
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.