Skip to content

Conversation

@Sbragul26
Copy link

What this PR does,

  • Adds validation to ensure --refresh-interval is greater than 0
  • Prevents invalid values (like 0 or negative durations) that could cause tight loops

Related issue

Signed-off-by: Sbragul26 <sbragul26@gmail.com>
@Sbragul26 Sbragul26 force-pushed the fix-refresh-interval-validation branch from 3a83b7e to e59dace Compare February 4, 2026 09:39
@rshdhere
Copy link

rshdhere commented Feb 4, 2026

Nice, this addresses the issue cleanly 👍 validating --refresh-interval upfront avoids accidental tight loops and makes the behavior safer.

Small UX thought (non-blocking): since Harbor CLI is Cobra-based, would it be worth returning this as a flag validation error (e.g. via PreRunE / returning an error) instead of log.Fatalf, so the CLI exits with a consistent usage/error message like other commands? what are your thoughts @NucleoFusion

Not required for this PR — overall this looks straightforward and well-scoped.

@Sbragul26
Copy link
Author

Thanks for the kind words and the suggestion,

Agreed — using Cobra’s error handling (e.g. PreRunE / returning an error) would be more idiomatic and provide a more consistent CLI UX. For this PR, I aimed to keep the change minimal and focused on validating the interval, but I’d be happy to follow up or adjust this if maintainers feel that’s preferred.

@NucleoFusion
Copy link
Contributor

I think the current check is enough, but we may need the duration to be more than 1s of 0.5sec or something.
We wouldnt want the user to put something so small

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Validation for audit-logs refresh-interval to be greater than 0

3 participants