Refactored the PipelineParser/PipelineConfiguration to use the PipelineDataFlowModel #1680
Conversation
…pelineDataFlowModel for deserializing the pipeline YAML. This makes the deserialization consistent with serialization. It is also necessary for the upcoming conditional routing on sinks work. Signed-off-by: David Venable <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #1680 +/- ##
============================================
+ Coverage 93.54% 93.58% +0.04%
+ Complexity 1313 1307 -6
============================================
Files 172 172
Lines 3826 3820 -6
Branches 303 301 -2
============================================
- Hits 3579 3575 -4
+ Misses 177 175 -2
Partials 70 70
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| final PipelineModel pipelineModel = mock(PipelineModel.class); | ||
| when(pipelineModel.getSource()).thenReturn(source); | ||
| when(pipelineModel.getProcessors()).thenReturn(processors); | ||
| when(pipelineModel.getPreppers()).thenReturn(null); |
There was a problem hiding this comment.
Do we still have preppers for this when statement?
There was a problem hiding this comment.
I might be able to remove this line. I originally had to set this to null for preppers to work. But, then some other issues came up, so I removed any support for prepper: in the PipelineConfiguration class.
| TestDataProvider.validMultipleConfigurationOfSizeOne(), | ||
| null, null); | ||
| void testOnlySourceAndSink() { | ||
| sinks = TestDataProvider.validMultipleConfigurationOfSizeOne(); |
There was a problem hiding this comment.
Why are we not continuing to use validMultipleConfiguration for sinks?
There was a problem hiding this comment.
I don't think it matters. But, the original test was using size of one. So, I kept it that way. I wanted to keep the tests as similar as possible to their original intent.
| } | ||
| @Test | ||
| void testNoSourceConfiguration() { | ||
| final PipelineModel pipelineModel = mock(PipelineModel.class); |
There was a problem hiding this comment.
Don't we want to pass null for source here?
There was a problem hiding this comment.
Mockito's default behavior is to provide null when not mocked (it is different for collections and primitives).
…pelineDataFlowModel for deserializing the pipeline YAML. This makes the deserialization consistent with serialization. It is also necessary for the upcoming conditional routing on sinks work. (opensearch-project#1680) Signed-off-by: David Venable <[email protected]>
Description
Prior to this PR, Data Prepper had split approaches for serializing configuration YAML from deserializing YAML. The
PipelineDataFlowModelis currently being used for serialization. And deserialization is handled by thePipelineParserandPipelineConfigurationclasses. This PR fixes this split-brain issue. Deserialization now usesPipelineDataFlowModelas well. This helps ensure that deserialization is consistent with serialization.This change also starts to remove the legacy
prepper:configuration in YAML. I wasn't intending to do this, but it would be harder to keep that behavior and we plan to remove it in #619.This change is important for supporting conditional routing of sinks.
Issues Resolved
None, but contributes toward #619 and #1337
Check List
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.