Skip to content

[CFX-6308] refactor(workload): wrap workload list JSON in envelope object#613

Merged
ajalon1 merged 2 commits into
datarobot-oss:mainfrom
ajalon1:aj/CFX-6308-workload-list-json-envelope
Jul 1, 2026
Merged

[CFX-6308] refactor(workload): wrap workload list JSON in envelope object#613
ajalon1 merged 2 commits into
datarobot-oss:mainfrom
ajalon1:aj/CFX-6308-workload-list-json-envelope

Conversation

@ajalon1

@ajalon1 ajalon1 commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Rationale

dr workload list --output-format json currently emits a bare JSON array:

Before:

[
  {
    "id": "wl-001",
    "name": "my-workload",
    "status": "running",
    "type": "custom",
    "importance": "high",
    "artifactID": "art-001",
    "endpoint": "https://...",
    "createdAt": "2026-04-01T08:00:00Z",
    "updatedAt": "2026-04-01T08:00:00Z"
  }
]

This PR wraps it in a consistent JSON envelope object using the existing PrintJSONEnvelope helper (introduced in #582):

After:

{
  "workloads": [
    {
      "id": "wl-001",
      "name": "my-workload",
      "status": "running",
      "type": "custom",
      "importance": "high",
      "artifactID": "art-001",
      "endpoint": "https://...",
      "createdAt": "2026-04-01T08:00:00Z",
      "updatedAt": "2026-04-01T08:00:00Z"
    }
  ]
}

Changes

  • internal/workload/output.go: Replaced json.MarshalIndent in printWorkloadsJSON() with outputformat.PrintJSONEnvelope(os.Stdout, "workloads", outputs)

Notes

No test changes needed (no existing tests for printWorkloadsJSON). This is a breaking change for JSON consumers. Please confirm the new shape is acceptable for your integrations. Part of CFX-6308.


Note

Medium Risk
Breaking JSON contract for workload list consumers; change is localized to list JSON formatting with no auth or data-path impact.

Overview
dr workload list --output-format json no longer prints a bare array; it now uses outputformat.PrintJSONEnvelope with the workloads key, matching other list commands (e.g. components, plugins, tasks).

In printWorkloadsJSON, the manual json.MarshalIndent + fmt.Println path is replaced by a single call to PrintJSONEnvelope(os.Stdout, "workloads", outputs). Single-workload JSON and text/table output are unchanged.

This is a breaking change for scripts or integrations that expect a top-level JSON array.

Reviewed by Cursor Bugbot for commit fe31cd9. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit fe31cd9. Configure here.

Comment thread internal/workload/output.go
@ajalon1 ajalon1 changed the title fix: wrap workload list JSON in envelope object [CFX-6308] refactor(workload): wrap workload list JSON in envelope object Jun 27, 2026
Comment thread internal/workload/output.go
@ajalon1 ajalon1 requested a review from a team June 29, 2026 17:03

@wojtekwdr wojtekwdr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, fix failing tests

Replace bare JSON array output with PrintJSONEnvelope wrapper so
dr workload list --output-format json outputs
{"workloads": [...]} instead of a bare [...]. Part of CFX-6308.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
@ajalon1

ajalon1 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

LGTM, fix failing tests

Fixed on rebase
ETA: Fixed in 6f18ce6

@ajalon1 ajalon1 force-pushed the aj/CFX-6308-workload-list-json-envelope branch from fe31cd9 to 4fea51d Compare July 1, 2026 16:43
@ajalon1

ajalon1 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

/approve-smoke-tests

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

🔐 Fork PR smoke tests triggered by @ajalon1

⚠️ Security Notice: This will run tests with access to repository secrets.

What happens next:

  1. Security scans will run automatically (Trivy, gosec)
  2. If security scans pass, smoke tests will run
  3. Results will be posted as PR comments

⚠️ Important: Review the PR code carefully before approving!

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

🔐 Fork smoke tests started by maintainer

⏳ Security scans passed. Running smoke tests...

Commit: 4fea51dde668a49fcabd2e9f4cd052d67d816e7a
View run

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

All smoke tests passed! (Fork PR)

✅ Security Scan: success
✅ Linux: success
✅ Windows: success

View run details

@ajalon1

ajalon1 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

/skip-smoke-tests

@ajalon1

ajalon1 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

Skipping bc my only recent change was to unit tests

@ajalon1 ajalon1 merged commit ffc12eb into datarobot-oss:main Jul 1, 2026
19 checks passed
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.

3 participants