-
Notifications
You must be signed in to change notification settings - Fork 2.9k
NIFI-15104 - Populate Bulletin's stackTrace and expose it to the bulletin board #10431
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.
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
Conversation
exceptionfactory
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for implementing the stack trace propagation for Bulletins @pvillard31. The implementation looks good in general, just a few minor notes, mostly related to tests.
...-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/events/BulletinFactory.java
Outdated
Show resolved
Hide resolved
| import static org.junit.jupiter.api.Assertions.assertNotNull; | ||
| import static org.junit.jupiter.api.Assertions.assertTrue; | ||
|
|
||
| public class BulletinFactoryStackTraceTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor formatting bit, the public modifiers are not needed on the class or method level in JUnit 5 tests.
...i-framework-core-api/src/test/java/org/apache/nifi/events/BulletinFactoryStackTraceTest.java
Outdated
Show resolved
Hide resolved
...rk/nifi-framework-core/src/test/java/org/apache/nifi/jaxb/BulletinAdapterStackTraceTest.java
Outdated
Show resolved
Hide resolved
...rk/nifi-framework-core/src/test/java/org/apache/nifi/jaxb/BulletinAdapterStackTraceTest.java
Outdated
Show resolved
Hide resolved
| * @param bulletin bulletin | ||
| * @return dto | ||
| */ | ||
| public BulletinDTO createBulletinDto(final Bulletin bulletin) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessary to retain this method? It might be better to remove this signature to make sure all references explicitly declare whether to include stack traces.
...nifi-web-api/src/test/java/org/apache/nifi/web/api/dto/DtoFactoryBulletinStackTraceTest.java
Outdated
Show resolved
Hide resolved
exceptionfactory
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making the adjustments, looks good! +1 merging
…e#10431) Signed-off-by: David Handermann <[email protected]>
Summary
NIFI-15104 - Populate Bulletin's stackTrace and expose it to the bulletin board
This change is required for #10343.
Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000NIFI-00000Pull Request Formatting
mainbranchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
./mvnw clean install -P contrib-checkLicensing
LICENSEandNOTICEfilesDocumentation