diff --git a/README.md b/README.md index 8cbd409c..9dd71ff6 100644 --- a/README.md +++ b/README.md @@ -54,9 +54,9 @@ irm https://github.com/tracebloc/cli/releases/latest/download/install.ps1 | iex # Per dataset tracebloc data ingest ./my-data \ - --table cats_dogs_train \ - --category image_classification \ - --intent train \ + --name cats_dogs_train \ + --task image_classification \ + --split train \ --label-column label ``` diff --git a/internal/cli/data.go b/internal/cli/data.go index f1912d73..fab42ed8 100644 --- a/internal/cli/data.go +++ b/internal/cli/data.go @@ -91,8 +91,15 @@ func newDataIngestCmd() *cobra.Command { // Ingest-spec flags. image_classification + the tabular / // time-series family are supported today; text + detection + // segmentation land in later increments. - table string - category string + // + // --name/--task are the canonical flags (#180); --table/--category + // stay on as hidden deprecated aliases so existing scripts keep + // working. --intent is unchanged. The wire/spec field names don't + // change — this is a CLI-surface rename only. + name string + tableAlias string + task string + categoryAlias string intent string labelColumn string targetSize string @@ -131,9 +138,9 @@ func newDataIngestCmd() *cobra.Command { Short: "Stage a local dataset into your client's storage", Long: `Stages a local dataset into your client's shared storage, submits an ingestion run to jobs-manager, and watches the ingestor Job -to completion. Supports 9 task categories (image classification, +to completion. Supports 9 tasks (image classification, object/keypoint detection, text classification, masked language -modeling, and the tabular / time-series family); pick one with --category. +modeling, and the tabular / time-series family); pick one with --task. Expected local layout (image_classification shown): @@ -155,13 +162,13 @@ see tracebloc/client#147 non-goals. Exit codes: 0 files staged + ingested successfully (or --detach: just staged + submitted) 2 schema validation failed (synthesized spec rejected) or - v0.1-unsupported category passed + v0.1-unsupported task passed 3 local-layout or kubeconfig error 4 cluster reachable but no tracebloc client / shared storage missing 5 ingestor SA token couldn't be obtained, or jobs-manager rejected the token (401/403) 6 destination table already exists (re-run with --overwrite to - replace it, or pick a different --table) + replace it, or pick a different --name) 7 pre-flight succeeded but staging the files failed (Pod creation, image pull, exec stream, or remote tar error) — or, with --overwrite, removing the old table failed @@ -174,6 +181,24 @@ Exit codes: if len(args) > 0 { localPath = args[0] } + // Resolve the deprecated flag aliases (#180): the canonical + // flag wins; a hidden legacy alias fills in only when the new + // flag wasn't passed, so old scripts keep working without the + // new surface silently shadowing them. The wire/spec field + // names are unchanged — this is a CLI rename only. + nameVal := name + if cmd.Flags().Changed("table") && !cmd.Flags().Changed("name") { + nameVal = tableAlias + } + taskVal := task + if cmd.Flags().Changed("category") && !cmd.Flags().Changed("task") { + taskVal = categoryAlias + } + // Whether the task was chosen at all (via either spelling). + // Dropping --task's old image_classification default means an + // unset task now drives the picker (TTY) or a clear error + // (non-interactive), never a silent image assumption. + taskSet := cmd.Flags().Changed("task") || cmd.Flags().Changed("category") // Guided mode: on a terminal (and unless --no-input), prompt // for whatever's still missing. Off a TTY / with --no-input, // prompter stays nil and runDataIngest keeps flag-only @@ -200,7 +225,7 @@ Exit codes: Context: contextOverride, Namespace: nsOverride, Spec: push.SpecArgs{ - Table: table, Category: category, Intent: intent, + Table: nameVal, Category: taskVal, Intent: intent, LabelColumn: labelColumn, LabelPolicy: labelPolicy, TimeColumn: timeColumn, NumberOfKeypoints: numberOfKeypoints, }, @@ -215,7 +240,7 @@ Exit codes: Printer: printer, Interactive: interactive, Prompter: pr, - CategorySet: cmd.Flags().Changed("category"), + TaskSet: taskSet, OutputJSON: outputJSON, JSONOut: jsonOut, }) @@ -234,16 +259,23 @@ Exit codes: // pre-empts our richer schema-driven diagnostic. Instead, the // schema validator catches missing/empty values with the canonical // JSON-pointer-anchored error. - cmd.Flags().StringVar(&table, "table", "", - "destination table name (MySQL identifier; matches /data/shared// on the PVC)") - cmd.Flags().StringVar(&category, "category", "image_classification", - "task category, one of: "+push.SupportedCategoriesList()) + cmd.Flags().StringVar(&name, "name", "", + "a name for this dataset (letters, digits, underscore) — you'll reference it by this name when you start a training run") + cmd.Flags().StringVar(&tableAlias, "table", "", + "deprecated alias for --name") + _ = cmd.Flags().MarkHidden("table") + cmd.Flags().StringVar(&task, "task", "", + "the task this data is for, one of: "+push.SupportedCategoriesList()+ + ". Omit it on a terminal to pick interactively.") + cmd.Flags().StringVar(&categoryAlias, "category", "", + "deprecated alias for --task") + _ = cmd.Flags().MarkHidden("category") cmd.Flags().StringVar(&intent, "intent", "", - "intent: train|test") + "is this training or test data? train|test (default train)") cmd.Flags().StringVar(&labelColumn, "label-column", "", - "name of the label/target column (in labels.csv for image categories, in the data CSV for tabular)") + "name of the label/target column (in labels.csv for image tasks, in the data CSV for tabular)") cmd.Flags().StringVar(&targetSize, "target-size", "", - "image categories only: resolution as WxH (e.g. 512x512). Default: auto-detected from the first image. "+ + "image tasks only: resolution as WxH (e.g. 512x512). Default: auto-detected from the first image. "+ "All images must share this resolution — the ingestor validates it, it does not resize.") cmd.Flags().StringVar(&schemaFlag, "schema", "", "tabular/time-series only: column types as col:TYPE,col:TYPE (e.g. age:INT,price:FLOAT). "+ @@ -304,12 +336,13 @@ type runDataIngestArgs struct { // Interactive guided mode (#28). When Interactive is true, // runDataIngest prompts (via Prompter) for any missing core inputs - // before validation. CategorySet records whether --category was - // passed explicitly (its non-empty default would otherwise look - // like a deliberate choice). Prompter is nil off a TTY / --no-input. + // before validation. TaskSet records whether the task was passed + // explicitly (via --task or the hidden --category alias); an unset + // task drives the picker rather than assuming a default. Prompter is + // nil off a TTY / --no-input. Interactive bool Prompter prompter - CategorySet bool + TaskSet bool // OutputJSON routes human output to stderr and emits a JSON result // to JSONOut (stdout); set together by the RunE in --output-json @@ -398,7 +431,7 @@ collaborators can train against that table without ever seeing the raw files.`)) // validation. Flags already provided win; non-TTY / --no-input // leaves Prompter nil and skips straight to the flag-only path. if a.Interactive && a.Prompter != nil { - if err := runInteractive(a.Printer, a.Prompter, &a, a.CategorySet); err != nil { + if err := runInteractive(a.Printer, a.Prompter, &a, a.TaskSet); err != nil { if errors.Is(err, errInteractiveCancelled) { a.Printer.Infof("Cancelled — nothing was ingested.") return nil @@ -406,6 +439,13 @@ collaborators can train against that table without ever seeing the raw files.`)) return &exitError{code: 3, err: fmt.Errorf("interactive setup: %w", err)} } } + // --intent defaults to train. Applied after the interactive block so + // the guided flow still asks "training or test?" (it prompts on an + // empty value); a non-interactive run that omits --intent gets train + // without erroring (RFC-0002 §5). The wire field stays "intent". + if a.Spec.Intent == "" { + a.Spec.Intent = "train" + } if a.LocalPath == "" { return &exitError{code: 3, err: errors.New( "local dataset path is required — pass it as an argument, or run " + @@ -442,8 +482,14 @@ collaborators can train against that table without ever seeing the raw files.`)) // list rather than the schema's 11-option enum dump. switch { case a.Spec.Category == "": - // Left empty by a caller; let the schema produce the canonical - // "category is required" error downstream. + // No task chosen. In guided mode the picker already filled this; + // reaching here means a non-interactive run (or --no-input / + // --output-json) that omitted --task. Give a clear, actionable + // error instead of silently assuming images (the old default). + return &exitError{code: 2, err: fmt.Errorf( + "which task is this data for? pass --task — one of: %s. "+ + "(On a terminal without --no-input, tracebloc asks you to pick.)", + push.SupportedCategoriesList())} case push.IsCLISupported(a.Spec.Category): // supported case push.IsKnown(a.Spec.Category): @@ -455,11 +501,11 @@ collaborators can train against that table without ever seeing the raw files.`)) // caught above, so IsKnown here means known-but-unsupported. spec, _ := push.Lookup(a.Spec.Category) return &exitError{code: 2, err: fmt.Errorf( - "category %q isn't supported by the CLI yet (%s). Supported categories: %s.", + "task %q isn't supported by the CLI yet (%s). Supported tasks: %s.", a.Spec.Category, spec.UnsupportedNote, push.SupportedCategoriesList())} default: return &exitError{code: 2, err: fmt.Errorf( - "category %q isn't a recognized task category. Supported categories: %s.", + "task %q isn't a recognized task. Supported tasks: %s.", a.Spec.Category, push.SupportedCategoriesList())} } @@ -681,7 +727,7 @@ collaborators can train against that table without ever seeing the raw files.`)) return &exitError{code: 6, err: fmt.Errorf( "table %q already exists in this client. Re-ingesting the same table doesn't merge or replace — "+ "the run would fail after uploading everything. Re-run with --overwrite to replace it, "+ - "or pick a different --table. (`tracebloc data delete %s` also removes it.)", + "or pick a different --name. (`tracebloc data delete %s` also removes it.)", existingTable, existingTable)} } if tableExists && a.Overwrite { @@ -1024,8 +1070,8 @@ func printLocalSummary(p *ui.Printer, layout *push.LocalLayout, spec map[string] p.Field("total size", push.HumanBytes(layout.TotalBytes)) p.Section("Ingest settings") - p.Field("table", fmt.Sprintf("%v", spec["table"])) - p.Field("category", fmt.Sprintf("%v", spec["category"])) + p.Field("name", fmt.Sprintf("%v", spec["table"])) + p.Field("task", fmt.Sprintf("%v", spec["category"])) p.Field("intent", fmt.Sprintf("%v", spec["intent"])) switch lbl := spec["label"].(type) { case string: diff --git a/internal/cli/data_test.go b/internal/cli/data_test.go index 49716e1c..0796307b 100644 --- a/internal/cli/data_test.go +++ b/internal/cli/data_test.go @@ -101,13 +101,13 @@ func TestDataIngest_UnsupportedCategory_ExitsTwo(t *testing.T) { t.Run(badCategory, func(t *testing.T) { code, _, _ := execDataIngest(t, []string{ root, - "--table=t1", - "--category=" + badCategory, + "--name=t1", + "--task=" + badCategory, "--intent=train", "--label-column=label", }) if code != 2 { - t.Fatalf("expected exit 2 for unsupported category %q, got %d", badCategory, code) + t.Fatalf("expected exit 2 for unsupported task %q, got %d", badCategory, code) } }) } @@ -126,18 +126,18 @@ func TestDataIngest_KnownUnsupportedCategory_PendingNote(t *testing.T) { rootCmd.SetErr(&bytes.Buffer{}) rootCmd.SetArgs([]string{"data", "ingest", "--kubeconfig=/tmp/tracebloc-cli-test-nonexistent-" + t.Name(), - root, "--table=t1", "--category=causal_language_modeling", + root, "--name=t1", "--task=causal_language_modeling", "--intent=train", "--label-column=label"}) err := rootCmd.Execute() if err == nil { - t.Fatal("expected an error for a known-but-unsupported category") + t.Fatal("expected an error for a known-but-unsupported task") } if got := ExitCodeFromError(err); got != 2 { t.Fatalf("exit code = %d, want 2", got) } msg := err.Error() - if strings.Contains(msg, "isn't a recognized task category") { - t.Errorf("known category misrouted to the unrecognized-category branch:\n%s", msg) + if strings.Contains(msg, "isn't a recognized task") { + t.Errorf("known task misrouted to the unrecognized-task branch:\n%s", msg) } if !strings.Contains(msg, "isn't supported by the CLI yet") { t.Errorf("want the registry pending-support note, got:\n%s", msg) @@ -145,9 +145,9 @@ func TestDataIngest_KnownUnsupportedCategory_PendingNote(t *testing.T) { } // TestDataIngest_TraversalTableName_ExitsTwo is the security -// regression pin at the CLI layer. --table=../../etc must be +// regression pin at the CLI layer. --name=../../etc must be // rejected with exit 2 BEFORE any spec synthesis or cluster work — -// the table name flows into the /data/shared/
/ PVC path, +// the name flows into the /data/shared/
/ PVC path, // and a traversal value would let PR-b's stage Pod escape that // subtree. Bugbot flagged this on PR #8 commit 4240097. func TestDataIngest_TraversalTableName_ExitsTwo(t *testing.T) { @@ -156,8 +156,8 @@ func TestDataIngest_TraversalTableName_ExitsTwo(t *testing.T) { t.Run(bad, func(t *testing.T) { code, _, _ := execDataIngest(t, []string{ root, - "--table=" + bad, - "--category=image_classification", + "--name=" + bad, + "--task=image_classification", "--intent=train", "--label-column=label", }) @@ -168,23 +168,23 @@ func TestDataIngest_TraversalTableName_ExitsTwo(t *testing.T) { } } -// TestDataIngest_MissingIntent_ExitsTwo: pins the "intent is -// required" diagnostic path — different schema violation but the -// same exit-code class. -func TestDataIngest_MissingIntent_ExitsTwo(t *testing.T) { +// TestDataIngest_OmittedIntent_DefaultsToTrain: --intent defaults to +// "train", so omitting it no longer fails schema validation (exit 2). +// The run gets past the spec checks and stops at the injected bad +// kubeconfig (exit 3) — the same fall-through point as +// TestDataIngest_BadKubeconfig_ExitsThree, which proves the default was +// applied rather than the value being rejected as missing. +func TestDataIngest_OmittedIntent_DefaultsToTrain(t *testing.T) { root := imgcLayout(t) - code, _, stderr := execDataIngest(t, []string{ + code, _, _ := execDataIngest(t, []string{ root, - "--table=t1", - "--category=image_classification", - // intent omitted + "--name=t1", + "--task=image_classification", + // intent omitted → defaults to train "--label-column=label", }) - if code != 2 { - t.Fatalf("expected exit 2 for missing intent, got %d", code) - } - if !strings.Contains(stderr, "intent") { - t.Errorf("expected stderr to mention 'intent', got:\n%s", stderr) + if code != 3 { + t.Fatalf("expected exit 3 (default intent applied, then bad kubeconfig), got %d", code) } } @@ -202,8 +202,8 @@ func TestDataIngest_MissingIntent_ExitsTwo(t *testing.T) { func TestDataIngest_NonexistentLocalPath_ExitsThree(t *testing.T) { code, _, _ := execDataIngest(t, []string{ "/tmp/tracebloc-cli-test-no-such-dir-" + t.Name(), - "--table=t1", - "--category=image_classification", + "--name=t1", + "--task=image_classification", "--intent=train", "--label-column=label", }) @@ -230,8 +230,8 @@ func TestDataIngest_MissingLabelsCSV_ExitsThree(t *testing.T) { code, _, _ := execDataIngest(t, []string{ root, - "--table=t1", - "--category=image_classification", + "--name=t1", + "--task=image_classification", "--intent=train", "--label-column=label", }) @@ -249,8 +249,8 @@ func TestDataIngest_BadKubeconfig_ExitsThree(t *testing.T) { root := imgcLayout(t) code, _, _ := execDataIngest(t, []string{ root, - "--table=t1", - "--category=image_classification", + "--name=t1", + "--task=image_classification", "--intent=train", "--label-column=label", }) @@ -270,7 +270,7 @@ func TestDataIngest_RequiresExactlyOneArg(t *testing.T) { { name: "no positional", args: []string{ - "--table=t1", "--category=image_classification", + "--name=t1", "--task=image_classification", "--intent=train", "--label-column=label", }, }, @@ -278,7 +278,7 @@ func TestDataIngest_RequiresExactlyOneArg(t *testing.T) { name: "two positionals", args: []string{ "./a", "./b", - "--table=t1", "--category=image_classification", + "--name=t1", "--task=image_classification", "--intent=train", "--label-column=label", }, }, @@ -293,6 +293,69 @@ func TestDataIngest_RequiresExactlyOneArg(t *testing.T) { } } +// TestDataIngest_DeprecatedFlagAliases pins that the pre-#180 flag names +// still resolve through their hidden aliases so existing scripts don't +// break: --table→--name and --category→--task. A valid +// spec via the old names must fall through the local checks to the +// injected bad kubeconfig (exit 3) exactly as the canonical names do; a +// bad value via --category must still reach the task gate (exit 2), +// proving the aliased value flows through rather than being ignored. +func TestDataIngest_DeprecatedFlagAliases(t *testing.T) { + root := imgcLayout(t) + + t.Run("valid via old names falls through to kubeconfig", func(t *testing.T) { + code, _, _ := execDataIngest(t, []string{ + root, + "--table=t1", + "--category=image_classification", + "--intent=train", + "--label-column=label", + }) + if code != 3 { + t.Fatalf("expected exit 3 (aliases resolved, then bad kubeconfig), got %d", code) + } + }) + + t.Run("bad value via --category reaches the task gate", func(t *testing.T) { + code, _, _ := execDataIngest(t, []string{ + root, + "--table=t1", + "--category=definitely-not-a-task", + "--intent=train", + "--label-column=label", + }) + if code != 2 { + t.Fatalf("expected exit 2 (aliased task value hit the gate), got %d", code) + } + }) +} + +// TestDataIngest_OmitTask_NonInteractive_Errors: dropping --task's old +// image_classification default means a non-interactive run that omits the +// task no longer silently assumes images. Off a TTY (as in tests) the +// picker can't run, so the task gate returns a clear exit-2 error naming +// --task. execDataIngest discards the error, so run the command directly +// and inspect it (mirrors TestDataIngest_KnownUnsupportedCategory_PendingNote). +func TestDataIngest_OmitTask_NonInteractive_Errors(t *testing.T) { + root := imgcLayout(t) + rootCmd := NewRootCmd(BuildInfo{Version: "test"}) + rootCmd.SetOut(&bytes.Buffer{}) + rootCmd.SetErr(&bytes.Buffer{}) + rootCmd.SetArgs([]string{"data", "ingest", + "--kubeconfig=/tmp/tracebloc-cli-test-nonexistent-" + t.Name(), + root, "--name=t1", "--intent=train", "--label-column=label"}) + err := rootCmd.Execute() + if err == nil { + t.Fatal("expected an error when --task is omitted non-interactively") + } + if got := ExitCodeFromError(err); got != 2 { + t.Fatalf("exit code = %d, want 2", got) + } + if !strings.Contains(err.Error(), "--task") { + t.Errorf("error should tell the user to pass --task, got:\n%s", err.Error()) + } +} + // TestAliasResolution verifies that the deprecated aliases still dispatch // to the same handlers as the canonical names: // - "dataset" → same as "data" diff --git a/internal/cli/interactive.go b/internal/cli/interactive.go index d945de91..3b561008 100644 --- a/internal/cli/interactive.go +++ b/internal/cli/interactive.go @@ -104,13 +104,13 @@ func isInteractiveTTY() bool { // runInteractive fills the gaps in a's core ingest fields by prompting, // then returns. It only prompts for what's still missing, so flags the -// user already passed win. categorySet says whether --category was set -// explicitly (vs left at its non-empty default), which would otherwise -// hide "the user didn't actually choose a category." +// user already passed win. taskSet says whether the task was passed +// explicitly (via --task or the hidden --category alias); when it wasn't, +// the picker runs rather than assuming a default. // -// Mutates a through the pointer. PR-b adds category-specific prompts +// Mutates a through the pointer. PR-b adds task-specific prompts // (target-size, schema, number-of-keypoints) + a confirm screen. -func runInteractive(p *ui.Printer, pr prompter, a *runDataIngestArgs, categorySet bool) error { +func runInteractive(p *ui.Printer, pr prompter, a *runDataIngestArgs, taskSet bool) error { p.PromptHeader("Let's set up your data ingest") p.Hintf("Press Enter to accept a default; Ctrl-C to cancel.") prompted := false @@ -125,9 +125,9 @@ func runInteractive(p *ui.Printer, pr prompter, a *runDataIngestArgs, categorySe prompted = true } - if !categorySet { + if !taskSet { p.PromptHint("What kind of task your data is for — this drives how it's validated and loaded.") - ans, err := pr.Select("Task category", "what kind of data this is", + ans, err := pr.Select("Task", "what kind of data this is", promptCategories, a.Spec.Category) if err != nil { return err @@ -150,7 +150,7 @@ func runInteractive(p *ui.Printer, pr prompter, a *runDataIngestArgs, categorySe if a.Spec.Intent == "" { p.PromptHint("Whether this split is used to train the model or to evaluate it.") - ans, err := pr.Select("Intent", "which split this data is", + ans, err := pr.Select("Is this training or test data?", "which split this data is", []string{"train", "test"}, "train") if err != nil { return err @@ -262,8 +262,8 @@ func promptCategorySpecific(p *ui.Printer, pr prompter, a *runDataIngestArgs) (b func renderReview(p *ui.Printer, a *runDataIngestArgs) { p.Section("Review") p.Field("path", a.LocalPath) - p.Field("category", a.Spec.Category) - p.Field("table", a.Spec.Table) + p.Field("task", a.Spec.Category) + p.Field("name", a.Spec.Table) p.Field("intent", a.Spec.Intent) if a.Spec.LabelColumn != "" { p.Field("label column", a.Spec.LabelColumn) diff --git a/internal/cli/interactive_test.go b/internal/cli/interactive_test.go index d1daf298..18b24d40 100644 --- a/internal/cli/interactive_test.go +++ b/internal/cli/interactive_test.go @@ -56,9 +56,9 @@ func discardPrinter() *ui.Printer { return ui.New(&bytes.Buffer{}) } func TestRunInteractive_FillsAllWhenEmpty(t *testing.T) { f := &fakePrompter{answers: map[string]string{ "Path to your dataset directory": "./data", - "Task category": "tabular_classification", + "Task": "tabular_classification", "Destination table name": "churn_train", - "Intent": "test", + "Is this training or test data?": "test", "Label column": "churned", }} a := &runDataIngestArgs{Spec: push.SpecArgs{Category: "image_classification"}} @@ -96,7 +96,7 @@ func TestRunInteractive_ShowsExampleHints(t *testing.T) { var buf bytes.Buffer p := ui.New(&buf, ui.WithColor(false)) - if err := runInteractive(p, f, a, true /*categorySet*/); err != nil { + if err := runInteractive(p, f, a, true /*taskSet*/); err != nil { t.Fatalf("runInteractive: %v", err) } out := buf.String() @@ -113,18 +113,18 @@ func TestRunInteractive_ShowsExampleHints(t *testing.T) { } // TestRunInteractive_SkipsProvidedValues: flags already set (and an -// explicit --category) mean nothing is prompted. +// explicit --task) mean nothing is prompted. func TestRunInteractive_SkipsProvidedValues(t *testing.T) { f := &fakePrompter{answers: map[string]string{}} - // text_classification has no category-specific prompts, so with all - // core fields set + an explicit --category, nothing is asked. + // text_classification has no task-specific prompts, so with all + // core fields set + an explicit --task, nothing is asked. a := &runDataIngestArgs{ LocalPath: "./data", Spec: push.SpecArgs{ Category: "text_classification", Table: "t", Intent: "train", LabelColumn: "label", }, } - if err := runInteractive(discardPrinter(), f, a, true /*categorySet*/); err != nil { + if err := runInteractive(discardPrinter(), f, a, true /*taskSet*/); err != nil { t.Fatalf("runInteractive: %v", err) } if len(f.asked) != 0 { @@ -192,8 +192,8 @@ func TestRunInteractive_Cancel(t *testing.T) { // label column, so it must not be prompted. func TestRunInteractive_MLMSkipsLabel(t *testing.T) { f := &fakePrompter{answers: map[string]string{ - "Destination table name": "mlm_train", - "Intent": "train", + "Destination table name": "mlm_train", + "Is this training or test data?": "train", }} a := &runDataIngestArgs{ LocalPath: "./data", diff --git a/internal/push/category.go b/internal/push/category.go index eef4a2e3..32fa2e92 100644 --- a/internal/push/category.go +++ b/internal/push/category.go @@ -10,7 +10,7 @@ import "strings" // with what the ingestor actually resolves. // // Everything category-shaped derives from the registry below — the -// family predicates, the `--category` help text, the interactive +// family predicates, the `--task` help text, the interactive // picker, and the push accept-gate — so the enumerations can't drift // apart (they used to: the flag help listed 5 of 9, cli#74). type CategorySpec struct { @@ -130,7 +130,7 @@ func IsText(category string) bool { func IsRegressionClass(category string) bool { return categoryByID[category].RegressionClass } // SupportedCategoryIDs returns the ids `dataset push` supports, in display -// order. Used to build the --category help, the interactive picker, and +// order. Used to build the --task help, the interactive picker, and // the accept-gate's "Supported:" lists from one place. func SupportedCategoryIDs() []string { ids := make([]string, 0, len(categoryRegistry)) diff --git a/internal/push/spec.go b/internal/push/spec.go index 1d485c10..dbee3fe0 100644 --- a/internal/push/spec.go +++ b/internal/push/spec.go @@ -30,15 +30,22 @@ import ( // 2. A safe single path segment — the name becomes the // /data/shared/
/ subdirectory on the PVC. // -// The intersection of "MySQL identifier" and "single safe path -// component" is [A-Za-z0-9_]: letters, digits, underscore. No -// slashes, no dots — which is what closes the path-traversal hole -// (see ValidateTableName). +// The name must START with a letter or underscore, then letters, +// digits, and underscores: [A-Za-z_][A-Za-z0-9_]*. No slashes, no +// dots — which is what closes the path-traversal hole (see +// ValidateTableName) — and no leading digit. +// +// This mirrors the ingestor's own check (the source of truth): +// tracebloc/data-ingestors' validators/table_name_validator.py +// requires ^[a-zA-Z_][a-zA-Z0-9_]*$. Any name accepted here is +// therefore accepted in-cluster; the looser old pattern let +// leading-digit / all-digit names ("123", "1data") through the CLI +// only for the cluster to reject them post-upload. // // All the real-world example tables (chest_xrays_train, // cats_dogs_train) match this; it's the conventional snake_case // table-naming style anyway. -var tableNamePattern = regexp.MustCompile(`^[A-Za-z0-9_]+$`) +var tableNamePattern = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`) // MaxTableNameLength caps `--table` at 63 chars. Two hard limits // agree on this: @@ -74,7 +81,7 @@ const MaxTableNameLength = 63 // both of which assume a validated name. func ValidateTableName(table string) error { if table == "" { - return fmt.Errorf("table name is required (set --table)") + return fmt.Errorf("dataset name is required (set --name)") } if len(table) > MaxTableNameLength { return fmt.Errorf( @@ -86,12 +93,13 @@ func ValidateTableName(table string) error { } if !tableNamePattern.MatchString(table) { return fmt.Errorf( - "table name %q is invalid: must match [A-Za-z0-9_]+ "+ - "(letters, digits, underscore only). The table name is "+ + "table name %q is invalid: must start with a letter or "+ + "underscore, then letters, digits, and underscores only "+ + "(matches [A-Za-z_][A-Za-z0-9_]*). The table name is "+ "used both as the MySQL table identifier and as the "+ "/data/shared/
/ subdirectory on the cluster PVC, "+ - "so slashes, dots, and path-traversal sequences are "+ - "rejected.", + "so a leading digit, slashes, dots, and path-traversal "+ + "sequences are rejected.", table) } return nil diff --git a/internal/push/spec_test.go b/internal/push/spec_test.go index 72d8a791..27c7cb1d 100644 --- a/internal/push/spec_test.go +++ b/internal/push/spec_test.go @@ -368,7 +368,6 @@ func TestValidateTableName_Accepts(t *testing.T) { "ABC", "table_123", "_leading_underscore", - "9starts_with_digit", // valid MySQL identifier + safe path segment } { if err := ValidateTableName(name); err != nil { t.Errorf("ValidateTableName(%q) = %v, want nil", name, err) @@ -376,6 +375,26 @@ func TestValidateTableName_Accepts(t *testing.T) { } } +// TestValidateTableName_LeadingDigit mirrors the ingestor's table-name +// rule (tracebloc/data-ingestors' validators/table_name_validator.py, +// ^[a-zA-Z_][a-zA-Z0-9_]*$, the source of truth): a name must start +// with a letter or underscore. The old CLI pattern was looser and let +// leading-digit / all-digit names through only for the cluster to +// reject them post-upload — this pins the CLI to the ingestor so any +// name accepted here is accepted in-cluster. +func TestValidateTableName_LeadingDigit(t *testing.T) { + for _, name := range []string{"1data", "123"} { + if err := ValidateTableName(name); err == nil { + t.Errorf("ValidateTableName(%q) = nil, want a leading-digit rejection", name) + } + } + for _, name := range []string{"_data", "Data1", "chest_xrays_train"} { + if err := ValidateTableName(name); err != nil { + t.Errorf("ValidateTableName(%q) = %v, want nil", name, err) + } + } +} + // TestValidateTableName_RejectsTooLong: K8s label values are // capped at 63 chars, and the stage Pod carries the raw table // name as the tracebloc.io/table label. Without this rejection,