fix: add ShutdownTimeout to SatelliteOptions struct - #605
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe satellite now stores shutdown timeout in ChangesSatellite shutdown timeout refactor
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Complexity | 2 medium |
🟢 Metrics 0 complexity · 0 duplication
Metric Results Complexity 0 Duplication 0
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Signed-off-by: Dishank-Sen <dishanksen05@gmail.com>
c91acd8 to
adf7ce7
Compare
|
@vg006 There is some issue with Codacy, can you help me out. |
|
Not a big issue, it usually complaints on code standards like line of codes, cyclomatic complexity. But I reviewed your PR and LGTM. |
Description
Moves
shutdownTimeoutinto theSatelliteOptionsstruct for consistency with all other CLI flags and environment variables.Changes:
ShutdownTimeoutfield toSatelliteOptionsstruct (line 61)envCfg.ShutdownTimeoutin main() (line 87)opts.ShutdownTimeout(line 104)shutdownTimeoutvariablerun()function signature from 3 to 2 parameters (line 253, 163)opts.ShutdownTimeouttogracefulShutdown()(line 435)Impact:
This is a pure refactoring with no functional changes. All 16 configuration values now follow the same pattern, making the codebase more consistent and maintainable.
Testing:
--shutdown-timeoutworks correctly with default value of 30sAdditional context
This inconsistency was introduced during the env package refactor (commit a3a6145, PR #514) where
ShutdownTimeoutwas added toenv.HarborSatellitebut not migrated toSatelliteOptionslike other fields.The change actually reduces complexity by removing a parameter from
run()while maintaining identical behavior.Summary by CodeRabbit
New Features
--shutdown-timeoutcommand-line option.Improvements