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

Recursive #3198

Merged
merged 14 commits into from
Sep 18, 2023
Merged

Recursive #3198

merged 14 commits into from
Sep 18, 2023

Conversation

shenkw1
Copy link
Contributor

@shenkw1 shenkw1 commented Aug 18, 2023

Description

A boolean specifying whether to drill down into values and recursively get more key-value pairs from it. The extra key-value pairs will be stored as subkeys of the root key.

Issues Resolved

Resolves #888

Check List

  • New functionality includes testing.
  • New functionality has a documentation issue. Please link to it in this PR.
    • New functionality has javadoc added
  • Commits are signed with a real name per the DCO

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.

@shenkw1
readme and config
db50860 
Signed-off-by: Kat Shen <[email protected]>
@shenkw1
clarify readme
822570d 
Signed-off-by: Kat Shen <[email protected]>
@shenkw1
Merge https://github.com/shenkw1/data-prepper into recursive
51cbd0b
@shenkw1
working on recursive implementation, resolving issues
47c9aa3 
Signed-off-by: Kat Shen <[email protected]>
@shenkw1
resolve errors
d003e46 
Signed-off-by: Kat Shen <[email protected]>
@shenkw1
inner string parse logic done, working on splitter logic
3a90c69 
Signed-off-by: Kat Shen <[email protected]>
@shenkw1
write recursive implementation and reorganize code for clarity, fixin…
c5bbaba 
…g bugs with recursing

Signed-off-by: Kat Shen <[email protected]>
@shenkw1
basic implementation done and working, cleaning code and testing edge…
2f0ccba 
… cases

Signed-off-by: Kat Shen <[email protected]>
@shenkw1
resolve duplicate value test failures and add basic recursive test
df0a26d 
Signed-off-by: Kat Shen <[email protected]>
@shenkw1
write tests and specify configs in regards to recursive
941a7e8 
Signed-off-by: Kat Shen <[email protected]>
chenqi0805
chenqi0805 reviewed Aug 21, 2023
View reviewed changes
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Show resolved Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
@shenkw1
switch transform_key config functionality, specify that splitters hav…
7d4aec7 
…e to have length = 1, switch bracket check logic to pattern matching

Signed-off-by: Kat Shen <[email protected]>
chenqi0805
chenqi0805 reviewed Aug 24, 2023
View reviewed changes
Copy link
Collaborator

@chenqi0805 chenqi0805 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 better. Some comments on regex match and coding style.

...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
@chenqi0805
Copy link
Collaborator

Also please fix the build failure due to checkStyle:

Error: eckstyle] [ERROR] /home/runner/work/data-prepper/data-prepper/data-prepper-plugins/key-value-processor/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java:14:8: Unused import - org.opensearch.dataprepper.model.event.JacksonEvent. [UnusedImports]

@shenkw1
clean code
4d8f218 
Signed-off-by: Kat Shen <[email protected]>
@shenkw1
fix errors
c895037 
Signed-off-by: Kat Shen <[email protected]>
chenqi0805
chenqi0805 reviewed Aug 25, 2023
View reviewed changes
Copy link
Collaborator

@chenqi0805 chenqi0805 left a comment

Choose a reason for hiding this comment

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

Just minor nits. Otherwise LGTM

...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
@shenkw1
fix nits
b4f62f9 
Signed-off-by: Kat Shen <[email protected]>
@shenkw1 shenkw1 mentioned this pull request Aug 25, 2023
Closed
4 tasks
@oeyh oeyh merged commit 2bdab0c into opensearch-project:main Sep 18, 2023
24 checks passed
asifsmohammed pushed a commit to asifsmohammed/data-prepper that referenced this pull request Sep 27, 2023
@shenkw1 @asifsmohammed
Recursive (opensearch-project#3198)
4ab6c34 
* readme and config

Signed-off-by: Kat Shen <[email protected]>

* clarify readme

Signed-off-by: Kat Shen <[email protected]>

* working on recursive implementation, resolving issues

Signed-off-by: Kat Shen <[email protected]>

* resolve errors

Signed-off-by: Kat Shen <[email protected]>

* inner string parse logic done, working on splitter logic

Signed-off-by: Kat Shen <[email protected]>

* write recursive implementation and reorganize code for clarity, fixing bugs with recursing

Signed-off-by: Kat Shen <[email protected]>

* basic implementation done and working, cleaning code and testing edge cases

Signed-off-by: Kat Shen <[email protected]>

* resolve duplicate value test failures and add basic recursive test

Signed-off-by: Kat Shen <[email protected]>

* write tests and specify configs in regards to recursive

Signed-off-by: Kat Shen <[email protected]>

* switch transform_key config functionality, specify that splitters have to have length = 1, switch bracket check logic to pattern matching

Signed-off-by: Kat Shen <[email protected]>

* clean code

Signed-off-by: Kat Shen <[email protected]>

* fix errors

Signed-off-by: Kat Shen <[email protected]>

* fix nits

Signed-off-by: Kat Shen <[email protected]>

---------

Signed-off-by: Kat Shen <[email protected]>
Co-authored-by: Kat Shen <[email protected]>
@shenkw1 shenkw1 deleted the recursive branch October 15, 2024 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chenqi0805 chenqi0805 chenqi0805 approved these changes

@asifsmohammed asifsmohammed asifsmohammed approved these changes

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

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

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

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

@cmanning09 cmanning09 Awaiting requested review from cmanning09

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

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

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Potential KeyValueProcessor feature: recursive
4 participants
@shenkw1 @chenqi0805 @asifsmohammed @oeyh