-
Notifications
You must be signed in to change notification settings - Fork 29
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
#126: Monitoring for ide-urls #160
Conversation
Pull Request Test Coverage Report for Build 10613389805Details
💛 - Coveralls |
@@ -441,6 +446,9 @@ private void doUpdateStatusJson(boolean success, int statusCode, UrlVersion urlV | |||
|
|||
logger.info("For tool {} and version {} the download verification succeeded with status code {} for URL {}.", tool, | |||
version, code, url); | |||
|
|||
urlErrorState.updateVerifications(true); |
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.
We need probably another place for this function. Currently, I have commented out updateExistingVersions
in the update
function. If not, the number of verification is doubled, because doUpdateStatusJson
is called twice.
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.
Looks good, I've added some small CR's.
|
||
private static final UrlErrorReport instance = new UrlErrorReport(); | ||
|
||
private static UrlErrorReport getInstance() { |
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.
why do we need to make all this static?
Couldn't the UpdateInitiator
create the errorReport and pass it to every UrlUpdater
via UpdateManager
?
In general static
without final
can be evil and if it can be avoided that is always good to do.
mvn -B -ntp -Dstyle.color=always exec:java -Dexec.mainClass="com.devonfw.tools.ide.url.UpdateInitiator" -Dexec.args="../ide-urls PT5H30M" | tee log.txt | ||
sed -n '/ERROR REPORT FROM/,/ERROR REPORT ENDS/p' log.txt > error_report.txt | ||
cat error_report.txt |
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.
If we have Java to collect all the errorReport
, then why can't this directly print the report at the endinstead of writing all logs to disc and then do sed
magic filtering the error report
out of the log that we have produced ourselves in Java in a structured way and print it via cat
?
Branch is outdated. The new PR is #584 |
Fixes: #126