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.

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

Change WorkflowData from interface to class #54

Merged
merged 3 commits into from
Sep 25, 2023
Merged

Change WorkflowData from interface to class #54

merged 3 commits into from
Sep 25, 2023

Conversation

dbwiddis
Copy link
Member

Description

Simplifies WorkflowData by making it a POJO.

Note: Impacts PRs #47, #38, #44, and #52. Happy to rebase this after those are merged if they are ready to go, or we can merge this and rebase those.

Issues Resolved

Fixes #53

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.

@codecov
Copy link

codecov bot commented Sep 25, 2023

Codecov Report

Merging #54 (a2b0bd4) into main (a97b7d0) will decrease coverage by 6.82%.
The diff coverage is 80.00%.

❗ Current head a2b0bd4 differs from pull request most recent head ee99427. Consider uploading reports for the commit ee99427 to get more accurate results

@@             Coverage Diff              @@
##               main      #54      +/-   ##
============================================
- Coverage     75.14%   68.33%   -6.82%     
- Complexity       47       48       +1     
============================================
  Files             7        7              
  Lines           173      180       +7     
  Branches         18       18              
============================================
- Hits            130      123       -7     
- Misses           32       48      +16     
+ Partials         11        9       -2     
Files Coverage Δ
...ramework/workflow/CreateIndex/CreateIndexStep.java 84.84% <100.00%> (-0.45%) ⬇️
...pensearch/flowframework/workflow/WorkflowData.java 100.00% <100.00%> (ø)
...nsearch/flowframework/template/TemplateParser.java 91.66% <0.00%> (ø)

... and 1 file with indirect coverage changes

Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
@dbwiddis
Copy link
Member Author

Note the changes required in the other PRs are minimal/trivial, so merging this first if there's still any iteration on your PRs will be a benefit.

- future.complete(new WorkflowData() {
-     @Override
-     public Map<String, Object> getContent() {
-         return Map.of("index-name", createIndexResponse.index());
-     }
- });
+ future.complete(new WorkflowData(Map.of("index-name", createIndexResponse.index())));

@owaiskazi19
Copy link
Member

Note the changes required in the other PRs are minimal/trivial, so merging this first if there's still any iteration on your PRs will be a benefit.

- future.complete(new WorkflowData() {
-     @Override
-     public Map<String, Object> getContent() {
-         return Map.of("index-name", createIndexResponse.index());
-     }
- });
+ future.complete(new WorkflowData(Map.of("index-name", createIndexResponse.index())));

Ah! Missed this. Let me take care of this in my followup PR.

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.

Thanks for this change!

@dbwiddis dbwiddis merged commit a530739 into opensearch-project:main Sep 25, 2023
9 of 10 checks passed
@dbwiddis dbwiddis deleted the workflow-data branch September 25, 2023 20:32
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 25, 2023
* Change WorkflowData from interface to class

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

* Disable flaky test

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

* Rebase with changes from #38

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

---------

Signed-off-by: Daniel Widdis <widdis@gmail.com>
(cherry picked from commit a530739)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
joshpalis pushed a commit that referenced this pull request Sep 25, 2023
Change WorkflowData from interface to class (#54)

* Change WorkflowData from interface to class



* Disable flaky test



* Rebase with changes from #38



---------


(cherry picked from commit a530739)

Signed-off-by: Daniel Widdis <widdis@gmail.com>
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>
dbwiddis added a commit that referenced this pull request Sep 28, 2023
#47)

* Add WorkflowStepFactory class

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

* Add XContent classes representing Template JSON

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

* Add parse methods for the Template XContent

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

* Cleanup parsing, javadocs, and demo output

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

* Refactor to use field name constants, get tests working again

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

* Separate WorkflowNode and ProcessNode functionality

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

* Fix demos to align with template field names

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

* Add workflow, node, and edge tests

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

* Add Template tests

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

* Refactor TemplateParser to WorkflowProcessSorter

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

* Test exceptional cases

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

* Finish up exceptional cases

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

* Fix a template field name bug in demo

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

* Rebase with #34

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

* Rebase changes from #54

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

* Integrate thread pool executor service

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

* Fix flaky ProcessNodeTests by removing orTimeout

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

* Rebase and refactor with #44

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

* Fix demos and remove DataDemo

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

* Use non-deprecated mapping method for CreateIndexStep

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

* Eliminate casting and deprecation warnings on test classes

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

* Remove unused/leftover demo class

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

* Typo

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

* Don't offer steps as an alternative to nodes

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

* Move Workflow into package with all the other parsing classes

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

* Move process sequencing classes into workflow package

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

* Add PipelineProcessor class and XContent parsing, rename package

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

---------

Signed-off-by: Daniel Widdis <widdis@gmail.com>
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 28, 2023
#47)

* Add WorkflowStepFactory class

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

* Add XContent classes representing Template JSON

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

* Add parse methods for the Template XContent

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

* Cleanup parsing, javadocs, and demo output

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

* Refactor to use field name constants, get tests working again

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

* Separate WorkflowNode and ProcessNode functionality

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

* Fix demos to align with template field names

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

* Add workflow, node, and edge tests

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

* Add Template tests

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

* Refactor TemplateParser to WorkflowProcessSorter

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

* Test exceptional cases

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

* Finish up exceptional cases

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

* Fix a template field name bug in demo

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

* Rebase with #34

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

* Rebase changes from #54

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

* Integrate thread pool executor service

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

* Fix flaky ProcessNodeTests by removing orTimeout

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

* Rebase and refactor with #44

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

* Fix demos and remove DataDemo

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

* Use non-deprecated mapping method for CreateIndexStep

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

* Eliminate casting and deprecation warnings on test classes

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

* Remove unused/leftover demo class

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

* Typo

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

* Don't offer steps as an alternative to nodes

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

* Move Workflow into package with all the other parsing classes

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

* Move process sequencing classes into workflow package

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

* Add PipelineProcessor class and XContent parsing, rename package

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

---------

Signed-off-by: Daniel Widdis <widdis@gmail.com>
(cherry picked from commit 734f9c2)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
dbwiddis pushed a commit that referenced this pull request Sep 28, 2023
…Template Parsing (#60)

Add WorkflowStep Factory and implement XContent-based Template Parsing (#47)

* Add WorkflowStepFactory class



* Add XContent classes representing Template JSON



* Add parse methods for the Template XContent



* Cleanup parsing, javadocs, and demo output



* Refactor to use field name constants, get tests working again



* Separate WorkflowNode and ProcessNode functionality



* Fix demos to align with template field names



* Add workflow, node, and edge tests



* Add Template tests



* Refactor TemplateParser to WorkflowProcessSorter



* Test exceptional cases



* Finish up exceptional cases



* Fix a template field name bug in demo



* Rebase with #34



* Rebase changes from #54



* Integrate thread pool executor service



* Fix flaky ProcessNodeTests by removing orTimeout



* Rebase and refactor with #44



* Fix demos and remove DataDemo



* Use non-deprecated mapping method for CreateIndexStep



* Eliminate casting and deprecation warnings on test classes



* Remove unused/leftover demo class



* Typo



* Don't offer steps as an alternative to nodes



* Move Workflow into package with all the other parsing classes



* Move process sequencing classes into workflow package



* Add PipelineProcessor class and XContent parsing, rename package



---------


(cherry picked from commit 734f9c2)

Signed-off-by: Daniel Widdis <widdis@gmail.com>
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x backport PRs to 2.x branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Simplify WorkflowData to a concrete class
3 participants