-
Notifications
You must be signed in to change notification settings - Fork 36
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 create ingest pipeline step #558
Conversation
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.
Overall looks good to me, just a few comments
src/main/java/org/opensearch/flowframework/workflow/CreateIngestPipelineStep.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/workflow/CreateIngestPipelineStep.java
Show resolved
Hide resolved
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.
Not sure how much of this is working around the bug fixed in #559, but can definitely improve the parsing...
src/main/java/org/opensearch/flowframework/workflow/CreateIngestPipelineStep.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/workflow/CreateIngestPipelineStep.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/workflow/CreateIngestPipelineStep.java
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/workflow/CreateIngestPipelineStep.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/workflow/CreateIngestPipelineStep.java
Outdated
Show resolved
Hide resolved
f751fb2
to
69aa46a
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #558 +/- ##
============================================
- Coverage 72.54% 72.24% -0.31%
+ Complexity 665 653 -12
============================================
Files 78 78
Lines 3416 3397 -19
Branches 271 269 -2
============================================
- Hits 2478 2454 -24
- Misses 822 825 +3
- Partials 116 118 +2 ☔ View full report in Codecov by Sentry. |
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! A few suggestions.
src/main/java/org/opensearch/flowframework/util/ParseUtils.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/workflow/CreateIngestPipelineStep.java
Outdated
Show resolved
Hide resolved
30bb904
to
baff494
Compare
Signed-off-by: Amit Galitzky <[email protected]>
Signed-off-by: Amit Galitzky <[email protected]>
Signed-off-by: Amit Galitzky <[email protected]>
The backport to
To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/flow-framework/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/flow-framework/backport-2.x
# Create a new branch
git switch --create backport/backport-558-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 b6d2092c35c5f94fd2ea644d375bfed345e0ede8
# Push it to GitHub
git push --set-upstream origin backport/backport-558-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/flow-framework/backport-2.x Then, create a pull request where the |
@amitgalitz can you raise a manual backport PR? |
* adding create ingest pipeline step Signed-off-by: Amit Galitzky <[email protected]> * adding IT and move configurations parsing to input parsing Signed-off-by: Amit Galitzky <[email protected]> * cleaning up comments Signed-off-by: Amit Galitzky <[email protected]> --------- Signed-off-by: Amit Galitzky <[email protected]>
* adding create ingest pipeline step Signed-off-by: Amit Galitzky <[email protected]> * adding IT and move configurations parsing to input parsing Signed-off-by: Amit Galitzky <[email protected]> * cleaning up comments Signed-off-by: Amit Galitzky <[email protected]> --------- Signed-off-by: Amit Galitzky <[email protected]>
* adding create ingest pipeline step Signed-off-by: Amit Galitzky <[email protected]> * adding IT and move configurations parsing to input parsing Signed-off-by: Amit Galitzky <[email protected]> * cleaning up comments Signed-off-by: Amit Galitzky <[email protected]> --------- Signed-off-by: Amit Galitzky <[email protected]>
Adding create ingest pipeline step (opensearch-project#558) * adding create ingest pipeline step * adding IT and move configurations parsing to input parsing * cleaning up comments --------- Signed-off-by: Amit Galitzky <[email protected]>
Description
Created ingest pipeline step so we can create a pipeline through a template. This supports any number of processors, and we support all processors here due to the limited validation. The pipeline steps also supports taking in a model ID currently from a previous step through substitution.
The two required fields for a pipeline are the pipelineId which is also referred to as pipeline name, and configurations which refer to the configurations of the pipeline, this includes the processors list, the description, tag or other optional lists. In order to support any ingest processor out there without ammending this step each time a new one is added, we convert the configurations given to a string and pass that string without extra validation on our end to the create pipeline API.
Examples:
Issues Resolved
resolves #24
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.