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.

Sign up for GitHub

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

Jump to bottom

Introduce global-context index and related operations #65

Merged
merged 20 commits into from
Oct 9, 2023
Merged

Introduce global-context index and related operations #65

merged 20 commits into from
Oct 9, 2023

Conversation

jackiehanyang
Copy link
Collaborator

@jackiehanyang jackiehanyang commented Sep 30, 2023 •
edited
Loading

Description

  • Add global-context index as an enum and its mapping
  • Add global-context index handler that contains methods like initIfAbsent, put, and update
  • Add custom exception class
  • Update Template parser to add responses and resources_created fields
  • Disable checkStyleTest

Issues Resolved

#51

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.

@github-actions github-actions bot added the backport 2.x backport PRs to 2.x branch label Sep 30, 2023
dbwiddis
dbwiddis reviewed Oct 1, 2023
View reviewed changes
src/main/java/org/opensearch/flowframework/exception/FlowFrameworkException.java Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/function/ThrowingSupplier.java Outdated Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/function/ThrowingSupplierWrapper.java Outdated Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/function/ThrowingSupplierWrapper.java Outdated Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/indices/FlowFrameworkIndex.java Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/model/Template.java Outdated Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/workflow/CreateIndexStep.java Outdated Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/workflow/CreateIndexStep.java Outdated Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/workflow/ProcessNode.java Outdated Show resolved Hide resolved
src/test/java/org/opensearch/flowframework/indices/GlobalContextHandlerTests.java Outdated Show resolved Hide resolved
@jackiehanyang jackiehanyang marked this pull request as ready for review October 2, 2023 16:59
@codecov
Copy link

codecov bot commented Oct 2, 2023 •
edited
Loading

Codecov Report

Merging #65 (5e1fe8e) into main (28326bd) will decrease coverage by 7.59%.
The diff coverage is 61.20%.

@@             Coverage Diff              @@
##               main      #65      +/-   ##
============================================
- Coverage     91.68%   84.09%   -7.59%     
- Complexity      167      197      +30     
============================================
  Files            13       18       +5     
  Lines           565      742     +177     
  Branches         78       90      +12     
============================================
+ Hits            518      624     +106     
- Misses           24       88      +64     
- Partials         23       30       +7
Files Coverage Δ
.../opensearch/flowframework/FlowFrameworkPlugin.java 100.00% <100.00%> (ø)
...arch/flowframework/indices/FlowFrameworkIndex.java 100.00% <100.00%> (ø)
...pensearch/flowframework/workflow/WorkflowData.java 100.00% <ø> (ø)
...ch/flowframework/workflow/WorkflowStepFactory.java 64.70% <100.00%> (ø)
...g/opensearch/flowframework/common/CommonValue.java 66.66% <66.66%> (ø)
.../flowframework/common/ThrowingSupplierWrapper.java 50.00% <50.00%> (ø)
...a/org/opensearch/flowframework/model/Template.java 90.41% <83.78%> (-1.95%) ⬇️
...lowframework/exception/FlowFrameworkException.java 0.00% <0.00%> (ø)
...ch/flowframework/indices/GlobalContextHandler.java 68.42% <68.42%> (ø)
...search/flowframework/workflow/CreateIndexStep.java 58.33% <47.36%> (-26.52%) ⬇️

dbwiddis
dbwiddis reviewed Oct 2, 2023
View reviewed changes
build.gradle Outdated Show resolved Hide resolved
owaiskazi19
owaiskazi19 reviewed Oct 6, 2023
View reviewed changes
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.

Overall looks with few comments

src/main/java/org/opensearch/flowframework/indices/GlobalContextHandler.java Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/indices/GlobalContextHandler.java Outdated Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/model/Template.java Outdated Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/workflow/CreateIndexStep.java Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/workflow/CreateIndexStep.java Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/workflow/CreateIndexStep.java Show resolved Hide resolved
src/main/java/org/opensearch/flowframework/workflow/ProcessNode.java Outdated Show resolved Hide resolved
src/main/resources/mappings/global-context.json Outdated Show resolved Hide resolved
src/test/java/org/opensearch/flowframework/indices/GlobalContextHandlerTests.java Outdated Show resolved Hide resolved
src/test/java/org/opensearch/flowframework/workflow/CreateIndexStepTests.java Show resolved Hide resolved
@joshpalis joshpalis mentioned this pull request Oct 6, 2023
Merged
@jackiehanyang
Add global context index and indices handler
a27ac80 
Signed-off-by: Jackie Han <[email protected]>
@jackiehanyang
Update global context index mapping
3565ad7 
Signed-off-by: Jackie Han <[email protected]>
@jackiehanyang
correct checkstyle errors
77d7664 
Signed-off-by: Jackie Han <[email protected]>
@jackiehanyang
skip index handler integ tests
cff1669 
Signed-off-by: Jackie Han <[email protected]>
@jackiehanyang
remove indices integration tests for now
a9b4802 
Signed-off-by: Jackie Han <[email protected]>
@jackiehanyang
rebase - add global-context index handler
69c500a 
Signed-off-by: Jackie Han <[email protected]>
@jackiehanyang
Add unit tests
a319753 
Signed-off-by: Jackie Han <[email protected]>
@jackiehanyang
remove duplicate index name file
2fac05b 
Signed-off-by: Jackie Han <[email protected]>
@jackiehanyang
refactor package and file names
cabd302 
Signed-off-by: Jackie Han <[email protected]>
@jackiehanyang
spotless apply
9bbffc0 
Signed-off-by: Jackie Han <[email protected]>
@jackiehanyang
add javax ws dependency
14dfeef 
Signed-off-by: Jackie Han <[email protected]>
@jackiehanyang
remove visible for testing
2b5b4a9 
Signed-off-by: Jackie Han <[email protected]>
@jackiehanyang
add final keyword to map in Template ToXContect parser
6adfa46 
Signed-off-by: Jackie Han <[email protected]>
@jackiehanyang
spotless apply
9bcf125 
Signed-off-by: Jackie Han <[email protected]>
dbwiddis
dbwiddis approved these changes Oct 9, 2023
View reviewed changes
Copy link
Member

@dbwiddis dbwiddis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for addressing all the comments.

joshpalis
joshpalis approved these changes Oct 9, 2023
View reviewed changes
Copy link
Member

@joshpalis joshpalis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jackiehanyang for addressing all the comments

@jackiehanyang jackiehanyang merged commit c4f1fc7 into opensearch-project:main Oct 9, 2023
8 of 10 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 9, 2023
@github-actions
Introduce global-context index and related operations (#65)
a218e7d 
* Add global context index and indices handler

Signed-off-by: Jackie Han <[email protected]>

* Update global context index mapping

Signed-off-by: Jackie Han <[email protected]>

* correct checkstyle errors

Signed-off-by: Jackie Han <[email protected]>

* skip index handler integ tests

Signed-off-by: Jackie Han <[email protected]>

* remove indices integration tests for now

Signed-off-by: Jackie Han <[email protected]>

* rebase - add global-context index handler

Signed-off-by: Jackie Han <[email protected]>

* Add unit tests

Signed-off-by: Jackie Han <[email protected]>

* remove duplicate index name file

Signed-off-by: Jackie Han <[email protected]>

* refactor package and file names

Signed-off-by: Jackie Han <[email protected]>

* spotless apply

Signed-off-by: Jackie Han <[email protected]>

* add javax ws dependency

Signed-off-by: Jackie Han <[email protected]>

* remove visible for testing

Signed-off-by: Jackie Han <[email protected]>

* add final keyword to map in Template ToXContect parser

Signed-off-by: Jackie Han <[email protected]>

* spotless apply

Signed-off-by: Jackie Han <[email protected]>

* disable checkStyleTest

Signed-off-by: Jackie Han <[email protected]>

* Add more unit tests

Signed-off-by: Jackie Han <[email protected]>

* use OpenSearch rest status code

Signed-off-by: Jackie Han <[email protected]>

* Addressing comments

Signed-off-by: Jackie Han <[email protected]>

* update resposnes field name to userOutputs

Signed-off-by: Jackie Han <[email protected]>

* spotlessApply

Signed-off-by: Jackie Han <[email protected]>

---------

Signed-off-by: Jackie Han <[email protected]>
(cherry picked from commit c4f1fc7)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Oct 9, 2023
Merged
dbwiddis pushed a commit that referenced this pull request Oct 9, 2023
@opensearch-trigger-bot @github-actions
[Backport 2.x] Introduce global-context index and related operations (#…
3aa4c9e 
…76)

Introduce global-context index and related operations (#65)

* Add global context index and indices handler



* Update global context index mapping



* correct checkstyle errors



* skip index handler integ tests



* remove indices integration tests for now



* rebase - add global-context index handler



* Add unit tests



* remove duplicate index name file



* refactor package and file names



* spotless apply



* add javax ws dependency



* remove visible for testing



* add final keyword to map in Template ToXContect parser



* spotless apply



* disable checkStyleTest



* Add more unit tests



* use OpenSearch rest status code



* Addressing comments



* update resposnes field name to userOutputs



* spotlessApply



---------


(cherry picked from commit c4f1fc7)

Signed-off-by: Jackie Han <[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>
@minalsha minalsha mentioned this pull request Oct 12, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amitgalitz amitgalitz amitgalitz left review comments

@owaiskazi19 owaiskazi19 owaiskazi19 left review comments

@joshpalis joshpalis joshpalis approved these changes

@dbwiddis dbwiddis dbwiddis approved these changes

@ohltyler ohltyler Awaiting requested review from ohltyler ohltyler is a code owner

Assignees
No one assigned
Labels
backport 2.x backport PRs to 2.x branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jackiehanyang @dbwiddis @owaiskazi19 @amitgalitz @joshpalis