ENH: Plugin errors consolidator #4863
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: George Chen <qchea@amazon.com>
chenqi0805
requested review from
engechas,
graytaylor0,
dinujoh,
kkondaka,
KarstenSchnitter,
dlvenable and
oeyh
as code owners
Signed-off-by: George Chen <qchea@amazon.com>
oeyh
reviewed
Aug 22, 2024
Comment on lines
+66
to
+70
| final List<PluginError> extensionPluginErrors = pluginErrorCollector.getPluginErrors() | ||
| .stream().filter(pluginError -> PipelinesDataFlowModel.EXTENSION_PLUGIN_TYPE | ||
| .equals(pluginError.getComponentType())) | ||
| .collect(Collectors.toList()); | ||
| if (!extensionPluginErrors.isEmpty()) { |
There was a problem hiding this comment.
Can we have one of the unit tests cover this filtering? to verify that if there are pluginErrors with type other than "extension", they will be filtered out.
dlvenable
requested changes
Aug 23, 2024
| import java.util.stream.IntStream; | ||
|
|
||
| @Named | ||
| public class PluginErrorsConsolidator { |
There was a problem hiding this comment.
I think rather than make this a consolidator that creates a string, a more flexible approach would be to make a PluginErrorsHandler. Let it decide how to handle multiple errors. Maybe it handles it by creating a structured object. Maybe it handles it by logging. Maybe by stdout.
public interface PluginErrorsHandler {
public void handleErrors(final Collection<PluginError> pluginErrors);
}
And then we can have a default which logs:
@Named
public class LoggingPluginErrorsHandler implements PluginErrorsHandler {
public void handleErrors(final Collection<PluginError> pluginErrors) {
// build string
// log to Slf4j
}
}
Signed-off-by: George Chen <qchea@amazon.com>
dlvenable
previously approved these changes
Aug 27, 2024
oeyh
previously approved these changes
Aug 27, 2024
oeyh
reviewed
Aug 27, 2024
Signed-off-by: George Chen <qchea@amazon.com>
dlvenable
previously approved these changes
Aug 27, 2024
oeyh
previously approved these changes
Aug 27, 2024
Signed-off-by: George Chen <qchea@amazon.com>
oeyh
approved these changes
Aug 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This is an enhancement on consolidated plugin error messages to make sure error messages are filtered.
Issues Resolved
Resolves #4854
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.