-
Notifications
You must be signed in to change notification settings - Fork 117
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
Update cron-utils to 9.2.1 #864
Conversation
I'm not sure why we haven't upgrade cron-utils to the latest. There's no reason not to upgrade it. cron-utils: 5.0.5 -> 9.2.1
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #864 +/- ##
============================================
+ Coverage 65.50% 65.53% +0.02%
- Complexity 3328 3331 +3
============================================
Files 356 356
Lines 13962 13970 +8
Branches 1499 1500 +1
============================================
+ Hits 9146 9155 +9
+ Misses 3964 3960 -4
- Partials 852 855 +3
☔ View full report in Codecov by Sentry. |
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!
return next.get().plus(jitterMillis, ChronoUnit.MILLIS); | ||
} | ||
throw new IllegalArgumentException( | ||
"no next execution time for " + CRON_DESCRIPTOR.describe(schedule) + ", lastExecutionTime: " + |
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.
nit: Should we log localRepo
as well to inform the errors to users.
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 was also thinking about logging the information but it's logged here so I didn't include it. 😉
Line 236 in 0fd267d
logger.warn("Unexpected exception while mirroring: {}", m, e); |
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.
Makes sense.
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!
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! 👍 👍 👍
I think this would be a good release to add monitoring dashboards for mirroring as well. I'll add this before this release
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 keeping it up-to-date!
I'm not sure why we haven't upgrade cron-utils to the latest. There's no reason not to upgrade it.
cron-utils: 5.0.5 -> 9.2.1