-
Notifications
You must be signed in to change notification settings - Fork 64
TraceValidator #32
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
TraceValidator #32
Conversation
bhautikpip
commented
Oct 16, 2020
- Added trace validation logic
- Added http and aws pre stored json file
- tested with spring boot sample app
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.
just some small comments, thanks!
validator/src/main/java/com/amazon/aoc/validators/TraceValidator.java
Outdated
Show resolved
Hide resolved
validator/src/main/java/com/amazon/aoc/validators/TraceValidator.java
Outdated
Show resolved
Hide resolved
validator/src/main/java/com/amazon/aoc/validators/TraceValidator.java
Outdated
Show resolved
Hide resolved
validator/src/main/resources/expected-data-template/expectedAWSSDKTrace.json
Outdated
Show resolved
Hide resolved
validator/src/main/resources/expected-data-template/expectedAWSSDKTrace.json
Outdated
Show resolved
Hide resolved
validator/src/main/resources/expected-data-template/expectedHTTPTrace.json
Outdated
Show resolved
Hide resolved
validator/src/main/java/com/amazon/aoc/validators/TraceValidator.java
Outdated
Show resolved
Hide resolved
validator/src/main/resources/expected-data-template/expectedAWSSDKTrace.mustache
Outdated
Show resolved
Hide resolved
retryable.execute(); | ||
return; | ||
} catch (Exception ex) { | ||
log.error("exception during retry, you may ignore it", ex); | ||
log.info("retrying after 10 seconds"); |
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.
i remember the interval is a parameter, not a fixed 10 seconds
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.
sure will add param here.
log.error("exception during retry, you may ignore it", ex); | ||
log.info("retrying after 10 seconds"); | ||
|
||
if (retryCount == 0) { |
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.
can we move this out of the loop? since when the retries exhausted, it exit the loop
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.
the reason I kept that inside is because we can print the actual exception after retry is exhausted (why retry failed?).