Skip to content

Fix issue 5106#5501

Merged
dlvenable merged 4 commits intoopensearch-project:mainfrom
fidelity-contributions:fix-issue-5106
Apr 17, 2025
Merged

Fix issue 5106#5501
dlvenable merged 4 commits intoopensearch-project:mainfrom
fidelity-contributions:fix-issue-5106

Conversation

@saketh-pallempati
Copy link
Copy Markdown
Contributor

Description

  • Added validation for sink routes in PipelineConfigurationValidator
  • Added unit tests in PipelineConfigurationValidatorTest
  • Added integration tests in PipelineConfigurationValidatorIT
  • Routes now properly validate against defined conditional routes

Issues Resolved

Resolves #5106

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.

* Added validation for sink routes in PipelineConfigurationValidator
* Added unit tests in PipelineConfigurationValidatorTest
* Added integration tests in PipelineConfigurationValidatorIT
* Routes now properly validate against defined conditional routes

Signed-off-by: Pallempati Saketh <pallempati.saketh@fmr.com>
@graytaylor0
Copy link
Copy Markdown
Member

Thanks @saketh-pallempati! This change looks good. Re-ran the builds as those are failing

@saketh-pallempati
Copy link
Copy Markdown
Contributor Author

Thanks @saketh-pallempati! This change looks good. Re-ran the builds as those are failing

Hi @graytaylor0

I don't see any relation to build errors and my code changes. Can you please help me out a bit to get the PR merged.

@graytaylor0
Copy link
Copy Markdown
Member

Thanks @saketh-pallempati! This change looks good. Re-ran the builds as those are failing

Hi @graytaylor0

I don't see any relation to build errors and my code changes. Can you please help me out a bit to get the PR merged.

You should be able to run ./gradlew :data-prepper-core:build to see the tests that are failing

@saketh-pallempati
Copy link
Copy Markdown
Contributor Author

@graytaylor0 Thanks for the information will check on my end and get back

…e Configuration

Signed-off-by: Pallempati Saketh <pallempati.saketh@fmr.com>
@saketh-pallempati
Copy link
Copy Markdown
Contributor Author

Hi @graytaylor0,
I was able to run ./gradlew :data-prepper-core:build with BUILD SUCCESSFUL.
Pushed the changes please do review.

graytaylor0
graytaylor0 previously approved these changes Mar 27, 2025
Copy link
Copy Markdown
Member

@graytaylor0 graytaylor0 left a comment

Choose a reason for hiding this comment

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

Looks good!

@saketh-pallempati
Copy link
Copy Markdown
Contributor Author

Looks good!

@graytaylor0 Thank you

Copy link
Copy Markdown
Collaborator

@san81 san81 left a comment

Choose a reason for hiding this comment

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

Thank you for this nice fix. Just handling null route cases will make this fix ready to go

@saketh-pallempati
Copy link
Copy Markdown
Contributor Author

Will work on the changes and get back. Thanks for the suggestions @san81

…ases

Signed-off-by: Pallempati Saketh <pallempati.saketh@fmr.com>
@saketh-pallempati
Copy link
Copy Markdown
Contributor Author

Hi @san81

I made the requried changes can you check the new commit.

dlvenable
dlvenable previously approved these changes Apr 16, 2025
Copy link
Copy Markdown
Member

@dlvenable dlvenable left a comment

Choose a reason for hiding this comment

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

Thanks @saketh-pallempati for this change. This will be a nice improvement for users.

@san81 san81 self-requested a review April 16, 2025 21:14
san81
san81 previously approved these changes Apr 16, 2025
@san81
Copy link
Copy Markdown
Collaborator

san81 commented Apr 16, 2025

@saketh-pallempati build is failing with some format violations. Please fix them

Signed-off-by: Pallempati Saketh <pallempati.saketh@fmr.com>
@saketh-pallempati
Copy link
Copy Markdown
Contributor Author

@saketh-pallempati build is failing with some format violations. Please fix them

Sorry that's an unused import error. Will submit a fix ASAP.

@kayhern kayhern dismissed stale reviews from san81 and dlvenable via 0639b14 April 17, 2025 14:17
@dlvenable dlvenable merged commit 9d1ba04 into opensearch-project:main Apr 17, 2025
5 of 6 checks passed
@dlvenable
Copy link
Copy Markdown
Member

Thank you @saketh-pallempati !

Davidding4718 pushed a commit to Davidding4718/data-prepper that referenced this pull request Apr 25, 2025
Fix validation of pipeline's sink routes opensearch-project#5106

* Added validation for sink routes in PipelineConfigurationValidator
* Added unit tests in PipelineConfigurationValidatorTest
* Added integration tests in PipelineConfigurationValidatorIT
* Routes now properly validate against defined conditional routes

Signed-off-by: Pallempati Saketh <pallempati.saketh@fmr.com>
Davidding4718 pushed a commit to Davidding4718/data-prepper that referenced this pull request Apr 25, 2025
Fix validation of pipeline's sink routes opensearch-project#5106

* Added validation for sink routes in PipelineConfigurationValidator
* Added unit tests in PipelineConfigurationValidatorTest
* Added integration tests in PipelineConfigurationValidatorIT
* Routes now properly validate against defined conditional routes

Signed-off-by: Pallempati Saketh <pallempati.saketh@fmr.com>
@graytaylor0 graytaylor0 mentioned this pull request May 6, 2025
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Validate that routes configured in the sink exist on startup of Data Prepper

4 participants