Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue with second-level cron jobs on OnOneServer by adjusting how the key suffix is computed.
- Updates key suffix computation by checking the cron string format
- Introduces conditional logic to use a more granular time format when necessary
| if event.IsOnOneServer() && event.GetName() != "" { | ||
| if app.cache.Lock(event.GetName()+carbon.Now().Format("Hi"), 1*time.Hour).Get() { | ||
| keySuffix := carbon.Now().Format("Hi") | ||
| if segments := strings.Split(event.GetCron(), " "); len(segments) == 6 { | ||
| keySuffix = carbon.Now().Format("His") |
There was a problem hiding this comment.
Consider caching the result of carbon.Now() in a local variable so that both calls (for "Hi" and "His") use the same timestamp, preventing potential inconsistencies if the time changes between calls.
| if event.IsOnOneServer() && event.GetName() != "" { | ||
| if app.cache.Lock(event.GetName()+carbon.Now().Format("Hi"), 1*time.Hour).Get() { | ||
| keySuffix := carbon.Now().Format("Hi") | ||
| if segments := strings.Split(event.GetCron(), " "); len(segments) == 6 { |
There was a problem hiding this comment.
[nitpick] It might be beneficial to trim the cron string before splitting to handle any extraneous whitespace that could lead to an unexpected segment count.
| if segments := strings.Split(event.GetCron(), " "); len(segments) == 6 { | |
| if segments := strings.Split(strings.TrimSpace(event.GetCron()), " "); len(segments) == 6 { |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## v1.15.x #1053 +/- ##
==========================================
Coverage ? 68.78%
==========================================
Files ? 218
Lines ? 18869
Branches ? 0
==========================================
Hits ? 12979
Misses ? 5229
Partials ? 661 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
📑 Description
Closes goravel/goravel#690
✅ Checks