Skip to content

WIP - Sunny/pipelines - #1

Draft
sunny2get wants to merge 25 commits into
mainfrom
sunny/pipelines
Draft

WIP - Sunny/pipelines#1
sunny2get wants to merge 25 commits into
mainfrom
sunny/pipelines

Conversation

@sunny2get

Copy link
Copy Markdown
Owner

RATIONALE

CHANGES

PR Automation

Comment-Commands: Trigger CI by commenting on the PR:

  • /trigger-smoke-test or /trigger-test-smoke - Run smoke tests
  • /trigger-install-test or /trigger-test-install - Run installation tests

Labels: Apply labels to trigger workflows:

  • run-smoke-tests or go - Run smoke tests on demand (only works for non-forked PRs)

Important

For Forked PRs: The run-smoke-tests label won't work. A required Smoke Tests check will block merge until a maintainer acts:

  • A maintainer uses /approve-smoke-tests to run smoke tests (results will set the check)
  • A maintainer uses /skip-smoke-tests to bypass the check without running tests

Please comment requesting a maintainer review if you need smoke tests to run.

@sunny2get sunny2get changed the title Sunny/pipelines WIP - Sunny/pipelines May 4, 2026

@ajalon1 ajalon1 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.

Some very nice work so far. I'm looking at this from a very high level, mostly in regards to:

  1. general implementation as an API client
  2. implementation patterns that dovetail with the overall CLI, or can be used to improve the overall CLI

Obviously not a comprehensive review.

The only thing I would actually require as a change would be to rename the command to "pipeline" and permit "pipelines" as an alias.

Comment thread cmd/pipelines/cmd.go Outdated

func Cmd() *cobra.Command {
cmd := &cobra.Command{
Use: "pipelines",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
Use: "pipelines",
Use: "pipeline",
Aliases: []string{"pipelines"},

to keep things consistent in CLI help.

Comment thread docs/commands/pipelines-reference.md Outdated
@@ -0,0 +1,178 @@
<!-- TOOD: remove this file, just to keep track internally while development is happening -->

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I appreciate this reference here. These summary docs really help when reviewing large PRs like this.


flags.Bind(cmd)
cmd.Flags().IntVar(&offset, "offset", 0, "Pagination offset")
cmd.Flags().IntVar(&limit, "limit", 0, "Maximum number of inputs to return")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I noticed limit default of 0 in a lot of the list commands. I would expect that all of the list commands have a defined default limit -- it's just good practice for any API client.

Comment on lines +54 to +56
if pipelineID == "" {
return errors.New("--pipeline is required")
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You can use MarkFlagRequired() to not have to reimplement this yourself.

Comment on lines +25 to +33
// Flags holds the values backing the shared --pipeline / --scope /
// --version flags. Bind() registers them on a cobra command and
// Resolve(cmd) turns them into a (Scope, *version) pair via
// pipelines.ResolveScope.
type Flags struct {
PipelineID string
Scope string
Version int
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I actually really like this pattern. I wonder if it's possible for this to be used more consistently throughout the pipeline commands.

Comment on lines +27 to +32
func TestHandleDeleteError_404IsSuppressed(t *testing.T) {
httpErr := &drapi.HTTPError{StatusCode: http.StatusNotFound, URL: "http://x/api/v2/pipelines/abc"}

err := handleDeleteError(httpErr, "abc")
assert.NoError(t, err)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why do you return an error on 404 for some commands, and suppress it for others?


if d.VersionID != nil {
scope = "locked"
versionDisplay = "v" + strconv.Itoa(*d.VersionID)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why have the extra v?

dependabot Bot and others added 17 commits May 22, 2026 13:52
…oss#522)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
- Replace UUID IDs with 24-char MongoDB ObjectIds in help text, test
  fixtures, and docs (pipeline_id, dispatch_id, input_id, schedule_id,
  environment_id now use ObjectId format per pipelines-api commit 65c86cd)
- Add internal/pipelines/Time type with flexible UnmarshalJSON to handle
  the API's bare UTC datetime strings (no timezone suffix) alongside
  standard RFC3339, fixing the time-parse crash on pipelines list/get
- Update Graph structs: GraphNode.ID and GraphEdge.Source/Target are now
  int (not string), GraphPipeline drops ID, adds PythonVersion; add extra
  edge/node fields for full --output json pass-through
- Fix demo-pipelines Taskfile target to use correct port 8100

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Covalent is not available in the local port-forward environment, so
environment create/update time out. Mirrors the existing SKIP_SCHEDULES
flag. Both default to false; pass SKIP_ENVIRONMENTS=true to skip the
8 environment steps (total drops to 27 when combined with SKIP_SCHEDULES).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
API now returns RFC3339Nano timestamps with Z suffix, so the custom
pipeline.Time wrapper is no longer needed. Replace all Time fields in
pipeline.go with standard time.Time and delete time.go.

Also default DEMO_SKIP_SCHEDULES and DEMO_SKIP_ENVIRONMENTS to true in
the demo-pipelines Taskfile task (k8s/Covalent not available locally),
and pipe demo output to output.txt via tee.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
API is now RFC3339 compliant; replace string datetime fields with
time.Time across run, input, schedule, and environment structs.
Update render helpers to format with .UTC().Format(time.RFC3339),
consistent with the pipeline/version commands.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add cmd/pipelines/outputfmt: OutputFormat type + AddOutputFlag()
  with --output-format flag, mirroring internal/workload/flags.go
- Add cmd/pipelines/pipelineutil: RenderPipeline/RenderPipelines with
  lipgloss/table list view and tabwriter detail view
- Rewrite all *util/render.go files: add Render*() routing functions,
  replace tabwriter lists with lipgloss/table, switch to tabwriter
  for detail views, add DTOs for input/schedule/environment/version,
  adopt "2006-01-02 15:04 UTC" timestamp format
- Update all 23 verb cmd files: use outputfmt.AddOutputFlag, remove
  scattered --output StringVar and manual validation, call Render*()

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sunny2get and others added 8 commits May 22, 2026 21:37
…pattern

Consolidates all *util sibling packages (runutil, inpututil, scheduleutil,
envutil, versionutil, pipelineutil, outputfmt) into internal/pipelines/,
mirroring how internal/workload/ owns its own output helpers. Each cmd verb
now imports only internal/pipelines and calls e.g. pipelines.RenderRuns().

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-056

- camelCase all JSON tags to match DrBaseModel alias_generator (action 056)
- Primary resource IDs now decode as "id" on the wire
- Wrap all list endpoints in DataPage[T] envelope (action 056)
- Remove PipelineName (lattice_name) from PipelineVersion — dropped by action 055
- Rename electron_names → taskNames, python_version → pythonVersion, etc.
- Update all test mock responses and DataPage[ListItem] in list cmd tests
- Fix pre-existing invalid "t" timestamps in environment_test.go

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Renames cmd/pipelines/ → cmd/pipeline/, updates the cobra Use field,
feature gate key (DATAROBOT_CLI_FEATURE_PIPELINE), and all example
strings. Adds "pipelines" as a backward-compat alias per naming conventions.
Docs and demo.sh updated accordingly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…lient

Replace the duplicated inline token+header setup and &http.Client{}
construction in Get, Post, Patch, and Delete with the AuthorizeRequest
and NewHTTPClient helpers introduced in client.go. Also fixes a bug in
Post/Delete where config.GetAPIKey was called directly, bypassing the
--skip-auth flag. Migrates raw viper imports to viperx in get.go and
affected test files to satisfy depguard.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…k and header consistency

- Move AuthorizeRequest from client.go to auth.go (its natural home); remove
  the now-redundant SetAuthHeaders wrapper and update filesapi callers to call
  AuthorizeRequest directly
- Fix --skip-auth bypass: auth.go previously called config.GetAPIKey directly,
  bypassing resolveToken(); now all callers go through getToken() which honours
  skip_auth
- Fix GetJSON body leak: use defer resp.Body.Close() so body is closed on
  decode error
- Fix Header.Add → Header.Set for Content-Type in delete.go, post.go, patch.go
- Replace hand-rolled fakeBody (missing io.EOF) with strings.NewReader in
  client_test.go

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pipelines-api action 059 renamed the graph JSON key from "lattice" to
"pipeline" (PipelineGraphLattice → PipelineGraphMeta). Update the Go
struct tag and test fixtures to match the new wire format.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Renames the Go package from `internal/pipelines` to `internal/pipeline`
to match the singular command name (`dr pipeline`) and updates all cmd
imports, tests, and telemetry calls accordingly. Also adds telemetry
TrackWith calls across all pipeline subcommands.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

2 participants