Uh oh!
There was an error while loading. Please reload this page.
fix: correct store-ruleset type-ident docs and incident get time-flag gap - #165
Merged
Conversation
store-ruleset-create/list validate --type-ident against the store-type registry (mysql, redis, elasticsearch, kafka, blackbox, mongodb, postgresql, os, minio, etcd), not the datasource-type registry that backs --ds-type elsewhere in the CLI. The flag help, usage example, and skill reference card all called it a "datasource type identifier" and used 'prometheus' as the example value, which the store-type registry never accepts, so the documented example failed every time. Reworded to "store type identifier" and swapped the example to 'redis', which is seeded in the store-type table. This text is generated (internal/cmd/cligen from a vendored OpenAPI spec; the skill card fence via 'go run ./internal/cmd/skilldoc gen' reading the corrected CLI help back out), so the correction here is on the shipped output. The upstream spec description still needs the same fix, or a later cligen regeneration will silently revert this. Also: no CLI verb currently lists valid store-type idents, so there is no in-CLI discovery path for this value today. Adding one is a separate feature, not part of this fix.
--since/--until (list) and --start-time/--end-time (sdp-request-list) are the only time-range filters in the incident card; get <id> [<id2>...] takes ids only. Nothing said so, and get takes any of these flags as an unknown-flag parse error rather than a hint that it isn't supported there, so add a one-line gotcha calling it out.
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix 1 —
store-ruleset-*--type-identnamed the wrong registrystore-ruleset-create/store-ruleset-listvalidate--type-identagainst the store-type registry (mysql,redis,elasticsearch,kafka,blackbox,mongodb,postgresql,os,minio,etcd), not the datasource-type registry used by--ds-typeelsewhere in the CLI. The flag help, the command's own usage example, and the skill reference card all called it a "datasource type identifier" and usedprometheusas the example — a value the store-type registry never accepts, so the documented example failed 100% of the time.Corrected the wording to "store type identifier" and the example to
redis, which is seeded in the store-type table.Where this text really comes from: it's generated —
internal/cmd/cligenbuildsinternal/cli/zz_generated_rule_sets.gofrom the OpenAPI spec vendored in the pinnedgo-flashdutymodule, andgo run ./internal/cmd/skilldoc genderives the skill card fence inskills/flashduty/reference/monit-ruleset.mdby reading that generated CLI help back out. Both files carryDO NOT EDITheaders for exactly that reason. This PR patches the shipped output directly because the description text is wrong for users today, but the durable fix belongs upstream, in the OpenAPI spec that feedscligen— until that's corrected and a newgo-flashdutyversion is pulled in, a futurecligen/skilldoc genregeneration from the same spec will silently revert this change.Is there a CLI verb to discover valid store-type idents? No. The backend has a store-type list endpoint alongside the ruleset endpoints, but it isn't in the OpenAPI spec, so it never made it into the SDK or the CLI. A user has no discoverable path to the right value today. That's a real gap, but adding a command is feature work, not a doc fix, so it's out of scope here — noting it for follow-up.
Verification:
redisis confirmed against the backend's own seed data and validation code path (store-type table seed list + theGetByIdentlookup the create/list handlers call). I could not exercise a live call end-to-end — the backend service isn't reachable in this environment — so that specific path is not live-verified, only statically confirmed.Fix 2 —
incident getsilently had no time-window flags, undocumented--since/--until(onlist) and--start-time/--end-time(onsdp-request-list) are the only time-range filters in the incident surface.incident get <id> [<id2>...]takes ids only and has no time-window flags — passing one is a plain "unknown flag" error, not a filter. Nothing said so. Added one line to the Gotchas section of the incident reference card stating this, verified againstincident get --help(shows no time flags) andincident get <id> --since 24h(errorsunknown flag: --since).Checks
make check(fmt, lint, test -race, build) — all green, no pre-existing failures.go run ./internal/cmd/skilldoc check— cards OK.