Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/commands/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func BuildRootCommand(cfg config.Config, runtimeSpec *spec.RuntimeSpec) *cobra.C
parent.AddCommand(methodCommand)
}

addJobsCustomCommands(root, cfg, runtimeSpec, client)
addJobsCustomCommands(root, cfg, runtimeSpec, client, flags)

return root
}
Expand Down
67 changes: 55 additions & 12 deletions internal/commands/jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ type jobsRunner struct {
cfg config.Config
runtime *spec.RuntimeSpec
client *http.Client
flags *GlobalFlags
createRun *spec.Operation
createUploadURL *spec.Operation
getDirectory *spec.Operation
Expand All @@ -56,8 +57,8 @@ type jobsRunner struct {
listRuns *spec.Operation
}

func addJobsCustomCommands(root *cobra.Command, cfg config.Config, runtimeSpec *spec.RuntimeSpec, client *http.Client) {
runner, ok := newJobsRunner(cfg, runtimeSpec, client)
func addJobsCustomCommands(root *cobra.Command, cfg config.Config, runtimeSpec *spec.RuntimeSpec, client *http.Client, flags *GlobalFlags) {
runner, ok := newJobsRunner(cfg, runtimeSpec, client, flags)
if !ok {
return
}
Expand All @@ -81,7 +82,7 @@ func addJobsCustomCommands(root *cobra.Command, cfg config.Config, runtimeSpec *
root.AddCommand(jobsCmd)
}

func newJobsRunner(cfg config.Config, runtimeSpec *spec.RuntimeSpec, client *http.Client) (*jobsRunner, bool) {
func newJobsRunner(cfg config.Config, runtimeSpec *spec.RuntimeSpec, client *http.Client, flags *GlobalFlags) (*jobsRunner, bool) {
if runtimeSpec == nil {
return nil, false
}
Expand All @@ -90,11 +91,12 @@ func newJobsRunner(cfg config.Config, runtimeSpec *spec.RuntimeSpec, client *htt
cfg: cfg,
runtime: runtimeSpec,
client: client,
flags: flags,
createRun: findOperation(runtimeSpec, "POST", "/runs"),
createUploadURL: findOperation(runtimeSpec, "POST", "/files/upload-url"),
getDirectory: findOperation(runtimeSpec, "GET", "/files/dir"),
getIntegration: findOperation(runtimeSpec, "GET", "/files/integration-dir"),
getConfigHint: findOperation(runtimeSpec, "GET", "/notebooks/config-hint"),
getConfigHint: findOperation(runtimeSpec, "GET", "/me/jupyter/lab/config-hint"),
getMe: findOperation(runtimeSpec, "GET", "/users/me"),
getOrganization: findOperation(runtimeSpec, "GET", "/organization"),
getRun: findOperation(runtimeSpec, "GET", "/runs/{run_id}"),
Expand Down Expand Up @@ -156,9 +158,18 @@ func (r *jobsRunner) newCreateCommand() *cobra.Command {
return fmt.Errorf("invalid --output %q (expected text|json)", output)
}

resolvedScript, err := r.prepareScript(cmd.Context(), script, name, noUpload, uploadPath)
if err != nil {
return err
dryRun := r.flags != nil && r.flags.DryRun

// Skip script upload in dry-run mode: prepareScript would issue
// org/file-store lookups and a presigned-URL upload, none of which
// belong in a "no side effects" preview.
resolvedScript := script
if !dryRun {
var err error
resolvedScript, err = r.prepareScript(cmd.Context(), script, name, noUpload, uploadPath)
if err != nil {
return err
}
}

payload, err := buildRunPayload(resolvedScript, name, runtimeID, timeoutSec, argsRaw, sparkConfigs, depPypi, depFiles, jarMainClass)
Expand All @@ -178,10 +189,13 @@ func (r *jobsRunner) newCreateCommand() *cobra.Command {
query = append(query, executor.QueryPair{Key: "region", Value: runRegion})
}

respBody, err := r.execOnce(cmd.Context(), r.createRun, nil, query, string(payloadJSON))
respBody, err := r.executeOrDryRun(cmd.Context(), cmd.OutOrStdout(), r.createRun, nil, query, string(payloadJSON))
if err != nil {
return err
}
if respBody == nil {
return nil
}

runID := strings.TrimSpace(gjson.GetBytes(respBody, "id").String())
if runID == "" {
Expand Down Expand Up @@ -612,15 +626,20 @@ func (r *jobsRunner) newLogsCommand() *cobra.Command {
return fmt.Errorf("invalid --output %q (expected text|json)", output)
}

if !follow {
dryRun := r.flags != nil && r.flags.DryRun

if !follow || dryRun {
size := 1000
if tail > 0 {
size = tail
}
respBody, err := r.execWithRetry(cmd.Context(), r.getRunLogs, []string{runID}, []executor.QueryPair{{Key: "cursor", Value: "0"}, {Key: "size", Value: fmt.Sprintf("%d", size)}}, "")
respBody, err := r.executeOrDryRun(cmd.Context(), cmd.OutOrStdout(), r.getRunLogs, []string{runID}, []executor.QueryPair{{Key: "cursor", Value: "0"}, {Key: "size", Value: fmt.Sprintf("%d", size)}}, "")
if err != nil {
return err
}
if respBody == nil {
return nil
}

if output == outputJSON {
_, err = fmt.Fprintln(cmd.OutOrStdout(), string(respBody))
Expand Down Expand Up @@ -784,10 +803,13 @@ func (r *jobsRunner) newMetricsCommand() *cobra.Command {
return fmt.Errorf("invalid --output %q (expected text|json)", output)
}

respBody, err := r.execWithRetry(cmd.Context(), r.getRunMetrics, []string{runID}, nil, "")
respBody, err := r.executeOrDryRun(cmd.Context(), cmd.OutOrStdout(), r.getRunMetrics, []string{runID}, nil, "")
if err != nil {
return err
}
if respBody == nil {
return nil
}

if output == outputJSON {
_, err = fmt.Fprintln(cmd.OutOrStdout(), string(respBody))
Expand Down Expand Up @@ -915,10 +937,13 @@ func (r *jobsRunner) executeList(cmd *cobra.Command, statuses []string, name, af
query = append(query, executor.QueryPair{Key: "status", Value: normalized})
}

respBody, err := r.execWithRetry(cmd.Context(), r.listRuns, nil, query, "")
respBody, err := r.executeOrDryRun(cmd.Context(), cmd.OutOrStdout(), r.listRuns, nil, query, "")
if err != nil {
return err
}
if respBody == nil {
return nil
}

if output == outputJSON {
_, err = fmt.Fprintln(cmd.OutOrStdout(), string(respBody))
Expand Down Expand Up @@ -1044,6 +1069,24 @@ func (r *jobsRunner) execOnce(ctx context.Context, op *spec.Operation, pathArgs
return executor.Do(r.client, req)
}

// executeOrDryRun routes the request through execWithRetry, unless the global
// --dry-run flag is set, in which case it prints the equivalent curl to w and
// returns (nil, nil). Callers MUST short-circuit when respBody is nil to avoid
// parsing a missing response.
func (r *jobsRunner) executeOrDryRun(ctx context.Context, w io.Writer, op *spec.Operation, pathArgs []string, query []executor.QueryPair, body string) ([]byte, error) {
if r.flags != nil && r.flags.DryRun {
req, err := executor.BuildRequest(ctx, r.cfg, r.runtime, op, pathArgs, query, body)
if err != nil {
return nil, err
}
if _, err := fmt.Fprintln(w, executor.RenderCurl(req, body)); err != nil {
return nil, err
}
return nil, nil
}
return r.execWithRetry(ctx, op, pathArgs, query, body)
}

func (r *jobsRunner) execWithRetry(ctx context.Context, op *spec.Operation, pathArgs []string, query []executor.QueryPair, body string) ([]byte, error) {
const maxAttempts = 6

Expand Down
78 changes: 72 additions & 6 deletions internal/commands/jobs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"net/http/httptest"
"os"
"strings"
"sync/atomic"
"testing"
"time"

Expand Down Expand Up @@ -832,6 +833,71 @@ func TestFormatMetricValue(t *testing.T) {
}
}

func TestJobsCreateDryRunSkipsRequest(t *testing.T) {
t.Parallel()

var serverHits int32
server := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
atomic.AddInt32(&serverHits, 1)
}))
defer server.Close()

root := buildJobsTestRoot(server.URL)
var out bytes.Buffer
root.SetOut(&out)
root.SetErr(&bytes.Buffer{})
root.SetArgs([]string{
"job-runs", "create", "s3://fake-bucket/x.py",
"--name", "dryrun-test",
"--dry-run",
})

if err := root.Execute(); err != nil {
t.Fatalf("Execute() error = %v", err)
}

if hits := atomic.LoadInt32(&serverHits); hits != 0 {
t.Fatalf("expected no HTTP requests during --dry-run, got %d", hits)
}

got := strings.TrimSpace(out.String())
if !strings.HasPrefix(got, "curl -X POST '"+server.URL+"/runs") {
t.Fatalf("expected curl POST %s/runs, got %q", server.URL, got)
}
if !strings.Contains(got, "dryrun-test") {
t.Fatalf("expected curl output to include job name, got %q", got)
}
}

func TestJobsListDryRunSkipsRequest(t *testing.T) {
t.Parallel()

var serverHits int32
server := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
atomic.AddInt32(&serverHits, 1)
}))
defer server.Close()

root := buildJobsTestRoot(server.URL)
var out bytes.Buffer
root.SetOut(&out)
root.SetErr(&bytes.Buffer{})
root.SetArgs([]string{"job-runs", "list", "--dry-run"})

if err := root.Execute(); err != nil {
t.Fatalf("Execute() error = %v", err)
}

if hits := atomic.LoadInt32(&serverHits); hits != 0 {
t.Fatalf("expected no HTTP requests during --dry-run, got %d", hits)
}

got := strings.TrimSpace(out.String())
if !strings.HasPrefix(got, "curl -X GET '"+server.URL+"/runs") {
t.Fatalf("expected curl GET %s/runs, got %q", server.URL, got)
}
}

func TestBuildRunPayloadRejectsBadDependency(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -893,7 +959,7 @@ func TestCompleteRuntimesSkipsDisabled(t *testing.T) {
t.Parallel()

server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method == http.MethodGet && r.URL.Path == "/notebooks/config-hint" {
if r.Method == http.MethodGet && r.URL.Path == "/me/jupyter/lab/config-hint" {
w.Header().Set("Content-Type", "application/json")
_, _ = io.WriteString(w, `{"runtimes":{"hint":[{"id":"tiny","enabled":true},{"id":"small"},{"id":"x-large","enabled":false}]}}`)
return
Expand All @@ -913,7 +979,7 @@ func TestCompleteRuntimesSkipsDisabled(t *testing.T) {

func newJobsRunnerForTest(baseURL string) *jobsRunner {
cfg := config.Config{AppName: "wherobots", APIKey: "test-key", HTTPTimeout: time.Second}
runner, _ := newJobsRunner(cfg, jobsTestRuntimeSpec(baseURL), http.DefaultClient)
runner, _ := newJobsRunner(cfg, jobsTestRuntimeSpec(baseURL), http.DefaultClient, nil)
return runner
}

Expand Down Expand Up @@ -975,10 +1041,6 @@ func jobsTestRuntimeSpec(baseURL string) *spec.RuntimeSpec {
{Name: "key", Location: "query", Required: true, Type: "string"},
},
},
{
Method: "GET",
Path: "/notebooks/config-hint",
},
{
Method: "POST",
Path: "/runs",
Expand Down Expand Up @@ -1010,6 +1072,10 @@ func jobsTestRuntimeSpec(baseURL string) *spec.RuntimeSpec {
Method: "GET",
Path: "/runs",
},
{
Method: "GET",
Path: "/me/jupyter/lab/config-hint",
},
},
}
}
Expand Down
Loading