Repository navigation
Add doctl insights dashboard commands - #2102
Merged
Merged
Conversation
Add the six dashboard methods from godo v1.236.0 to do.InsightsService as context.TODO() pass-throughs, with the mock regenerated by mockgen v0.6.0. Related: COBS-1892
Add doctl insights dashboard (list, get, create, update, delete, schema) backed by the Insights dashboards API. A dashboard is defined by a Perses JSON document that the API owns; doctl does not model it. `schema` prints the JSON Schema the document must satisfy. `create` and `update` read the document from --spec-file (JSON or YAML, or - for stdin) and send it as written, so a file that validates locally reaches the server byte-for-byte. JSON output embeds `spec` as an object, so `get --output json | jq .dashboard.spec` round- trips back into --spec-file unchanged. Page/per-page validation is factored into insightsPageOptions and reused by the resource list command. API errors are passed through unchanged. Related: COBS-1892 Co-authored-by: Cursor <cursoragent@cursor.com>
Cover flag validation, spec file reading (JSON, YAML, stdin, rejection of non-object documents), request construction against the mocked InsightsService, the shared page option helper, and text and JSON display of dashboards and pagination. Related: COBS-1892 Co-authored-by: Cursor <cursoragent@cursor.com>
adityasingh-beep
force-pushed
the
adityasingh/COBS-1892-Implement-doctl-Insights-dashboard-commands
branch
from
November 5, 2026 05:36
0ea75df to
4af6b6a
Compare
adityasingh-beep
marked this pull request as ready for review
November 5, 2026 05:36
dmamidala-glitch
approved these changes
Nov 5, 2026
| if len(spec) == 0 { | ||
| return nil, errInsightsDashboardSpecNotObject | ||
| } | ||
| if spec[0] != '{' { |
Contributor
There was a problem hiding this comment.
Valid flow-style YAML can start with {, for example {kind: Dashboard}. This skips YAML conversion and is incorrectly parsed as JSON. Could we preserve the input when json.Valid(spec) succeeds; otherwise convert it with yaml.YAMLToJSON? Please add a regression test.
Contributor
Author
There was a problem hiding this comment.
Good catch, thanks! You're right, {kind: Dashboard} was getting treated as JSON and blowing up. Fixed in 9f14c73 — it now checks json.Valid like you suggested, and I added a test for the flow-style case.
One small extra: while poking at this I noticed YAMLToJSON quietly ignores anything after the first document, so something like cat a.json b.json | ... --spec-file - would've silently sent just a.json. Added a quick single-document check and a couple of tests so that can't slip through.
readInsightsDashboardSpec chose the parser by the first byte, so
flow-style YAML such as {kind: Dashboard} was sent to the JSON decoder
and rejected. Decide on json.Valid instead: valid JSON objects pass
through byte-for-byte, everything else is converted from YAML.
yaml.YAMLToJSON only converts the first document and silently drops the
rest, so decode with the underlying yaml.v2 parser first and require
exactly one document. This keeps rejecting concatenated JSON objects and
now also rejects multi-document YAML.
Addresses review feedback on digitalocean#2102.
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
RohithSangati
approved these changes
Nov 6, 2026
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Adds
doctl insights dashboardwithlist,get,create,update,delete, andschema, backed by the Insights dashboards API added in godo #1201 (OpenAPI: digitalocean/openapi#1298).Related: COBS-1892 (parent COBS-1866)
Design
A dashboard is defined by a Perses JSON document that the API owns; doctl does not model it.
schemaprints the JSON Schema the document must satisfy, passed through from the API verbatim.create/updateread the document from--spec-file(JSON or YAML, or-for stdin) and send it as written, so a file that validates locally reaches the server byte-for-byte. Anything that is not a single JSON object is rejected before the request is made.specas an object rather than the string-encoded form the API transports, soget --output json | jq .dashboard.specround-trips back into--spec-fileunchanged.nameanddescriptionare server-derived fromspec.display;updateis a full replace.listsupports--page,--per-page(max 200),--sort(name | created_at | updated_at, omitted = newest first) and--sort-direction(asc | desc, requires--sort, matching server behaviour).insightsPageOptionsand reused byinsights resource list.Dependency
Requires godo
v1.236.0(contains #1201), whichmainalready pins. This PR adds no dependency or vendor changes.Testing
make gofmt_check,go vet,make check_focusedclean.make test_unit: all insights tests pass (102); the only failure is the pre-existingTestRegistryLogout, which requires a native keychain (macOS local-run artefact, passes in CI on Linux).do/mocks/InsightsService.goregenerated with mockgen v0.6.0 (byte-identical to generator output).spec.display.name.