Skip to content

docs: replace invalid duration "1w" with "168h" in time-based-fairshare examples#1361

Closed
JheSue wants to merge 2 commits intokai-scheduler:mainfrom
JheSue:fix/docs-windowsize-invalid-duration
Closed

docs: replace invalid duration "1w" with "168h" in time-based-fairshare examples#1361
JheSue wants to merge 2 commits intokai-scheduler:mainfrom
JheSue:fix/docs-windowsize-invalid-duration

Conversation

@JheSue
Copy link
Copy Markdown
Contributor

@JheSue JheSue commented Mar 31, 2026

Problem

The time-based-fairshare documentation uses windowSize: 1w as an example value.
However, Go's time.ParseDuration does not support the w (week) unit —
only ns, us, ms, s, m, and h are valid.

When a user follows the documentation and sets windowSize: 1w in their
SchedulingShard CR, the kai-operator fails to list SchedulingShard resources
and enters a crash loop:

failed to list *v1.SchedulingShard: time: unknown unit "w" in duration "1w"

Fix

Replace 1w with the equivalent 168h (7 × 24h) in all documentation examples.
The semantic meaning is preserved — one week is still the default.

Changes

  • docs/time-based-fairshare/README.md — replace windowSize: 1w with windowSize: 168h
  • examples/time-based-fairshare/README.md — update default value in parameter table

How to reproduce

  1. Follow the time-based-fairshare setup guide
  2. Set windowSize: 1w as shown in the docs
  3. Observe kai-operator crash-looping with the above error

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 31, 2026

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 55eb9a51-80fe-48e8-8d1b-3925befb05cb

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@JheSue JheSue marked this pull request as draft March 31, 2026 02:42
@JheSue JheSue marked this pull request as ready for review March 31, 2026 02:44
…irshare examples

Go's time.ParseDuration does not support the 'w' (week) unit.
Using 'windowSize: 1w' as shown in the documentation causes the
kai-operator to crash-loop with:

  failed to list *v1.SchedulingShard: time: unknown unit "w" in duration "1w"

Replace with the equivalent '168h' (7 * 24h) in all examples and tables.

Signed-off-by: Homer <homer.su@fongcon.com.tw>
@JheSue JheSue force-pushed the fix/docs-windowsize-invalid-duration branch from d3bf57a to 9a950d7 Compare March 31, 2026 03:15
@enoodle enoodle changed the title fix(docs): replace invalid duration "1w" with "168h" in time-based-fairshare examples docs: replace invalid duration "1w" with "168h" in time-based-fairshare examples Mar 31, 2026
@enoodle
Copy link
Copy Markdown
Collaborator

enoodle commented Mar 31, 2026

Thanks @JheSue ! 🙏

@itsomri
Copy link
Copy Markdown
Collaborator

itsomri commented Mar 31, 2026

Hi, this is probably a bug in code, not docs: we were supposed to use prometheus' duration type which allows for w https://pkg.go.dev/github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring@v0.88.0/v1#Duration

I'll take a look when I have the time

@JheSue
Copy link
Copy Markdown
Contributor Author

JheSue commented Apr 1, 2026

Hi, thanks for the pointer! I've investigated the issue and implemented a fix.
I used model.Duration from github.com/prometheus/common/model, which is already an int64 type with built-in UnmarshalJSON/UnmarshalYAML that supports the full Prometheus duration format including w and d.
I've opened a new PR with the fix : #1371
Feel free to close this one.

@JheSue JheSue closed this Apr 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants