fix: test channel frequency - #2119
Conversation
WalkthroughThe PR modifies AutoTestChannelMinutes to support fractional minute values by changing its type from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
setting/operation_setting/monitor_setting.go (1)
27-32: Environment variable parsing still uses integer conversion, preventing fractional values.The code parses
CHANNEL_TEST_FREQUENCYusingstrconv.Atoi, which only accepts integer values. This prevents users from specifying fractional minutes (e.g., "0.5" for 30 seconds) via the environment variable, undermining the purpose of changingAutoTestChannelMinutestofloat64.Apply this diff to support fractional values from the environment:
func GetMonitorSetting() *MonitorSetting { if os.Getenv("CHANNEL_TEST_FREQUENCY") != "" { - frequency, err := strconv.Atoi(os.Getenv("CHANNEL_TEST_FREQUENCY")) + frequency, err := strconv.ParseFloat(os.Getenv("CHANNEL_TEST_FREQUENCY"), 64) if err == nil && frequency > 0 { monitorSetting.AutoTestChannelEnabled = true - monitorSetting.AutoTestChannelMinutes = float64(frequency) + monitorSetting.AutoTestChannelMinutes = frequency } } return &monitorSetting }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
controller/channel-test.go(1 hunks)setting/operation_setting/monitor_setting.go(2 hunks)
| time.Sleep(time.Duration(int(math.Round(frequency))) * time.Minute) | ||
| common.SysLog(fmt.Sprintf("automatically test channels with interval %d minutes", frequency)) |
There was a problem hiding this comment.
Rounding negates fractional minute support; fix format specifier mismatch.
The implementation has two issues:
-
Line 628: Rounding
frequencyto an integer defeats the purpose of changingAutoTestChannelMinutestofloat64. If the intent is to support fractional minutes (e.g., 0.5 minutes = 30 seconds), the rounding should be removed. -
Line 629: The format specifier
%dexpects an integer butfrequencyisfloat64, causing incorrect output or potential issues.
Apply this diff to properly support fractional minutes:
- time.Sleep(time.Duration(int(math.Round(frequency))) * time.Minute)
- common.SysLog(fmt.Sprintf("automatically test channels with interval %d minutes", frequency))
+ time.Sleep(time.Duration(frequency * float64(time.Minute)))
+ common.SysLog(fmt.Sprintf("automatically test channels with interval %.2f minutes", frequency))Alternatively, if only whole minutes are needed, keep AutoTestChannelMinutes as int rather than converting to float64.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| time.Sleep(time.Duration(int(math.Round(frequency))) * time.Minute) | |
| common.SysLog(fmt.Sprintf("automatically test channels with interval %d minutes", frequency)) | |
| time.Sleep(time.Duration(frequency * float64(time.Minute))) | |
| common.SysLog(fmt.Sprintf("automatically test channels with interval %.2f minutes", frequency)) |
🤖 Prompt for AI Agents
controller/channel-test.go around lines 628-629: the code currently rounds the
float64 frequency and prints it with %d, which removes fractional-minute support
and mismatches the format specifier; remove the math.Round usage and compute a
time.Duration from the float64 minutes by multiplying frequency by
float64(time.Minute) then converting to time.Duration and call time.Sleep with
that duration, and update the log to use a float format (e.g. "%.2f" or "%f")
when printing frequency in minutes.
fix #2115
Summary by CodeRabbit