Skip to content

Move executor categorization enable flag under errorCategories.enabled#4892

Merged
dejanzele merged 2 commits intoarmadaproject:masterfrom
dejanzele:move-categorization-flag
May 6, 2026
Merged

Move executor categorization enable flag under errorCategories.enabled#4892
dejanzele merged 2 commits intoarmadaproject:masterfrom
dejanzele:move-categorization-flag

Conversation

@dejanzele
Copy link
Copy Markdown
Member

@dejanzele dejanzele commented Apr 30, 2026

Summary

Move the executor's failure-classification feature flag from application.enableJobErrorCategorization to application.errorCategories.enabled. The flag now lives with the rules it gates rather than as a separate top-level boolean.

Shipped default in config/executor/config.yaml is enabled: false.

Why

The flag was added in a separate refactor PR after the categorizer config already existed at the top level, which left the on/off switch sitting next to (rather than inside) the thing it controls. The nested form is cohesive: an operator reading or writing categorization config sees enabled as a sibling of defaultCategory, categories, etc.

Code-side, this also lets future validation (e.g. "if enabled: true then categories must be non-empty") live inside categorizer.ErrorCategoriesConfig.Validate() rather than spanning two structs.

Migration

This is a breaking config change for any operator deployment that sets enableJobErrorCategorization: true today.

Before:

application:
  enableJobErrorCategorization: true
  errorCategories:
    defaultCategory: "uncategorized"
    categories: [...]

After:

application:
  errorCategories:
    enabled: true
    defaultCategory: "uncategorized"
    categories: [...]

If errorCategories.enabled is unset/false, the executor skips classifier construction entirely (same behavior as the old flag being off). The shipped default in config/executor/config.yaml is enabled: false, so categorization remains opt-in.

Validation

  • go test ./internal/executor/... passes.
  • golangci-lint run ./internal/executor/... clean.
  • e2e/config/executor_config.yaml updated to the new shape (still enabled in e2e where rules are exercised).
  • config/executor/config.yaml adds errorCategories.enabled: false as the explicit shipped default.

Signed-off-by: Dejan Zele Pejchev <pejcev.dejan@gmail.com>
@dejanzele dejanzele force-pushed the move-categorization-flag branch from 72f022e to bec644f Compare April 30, 2026 13:17
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 30, 2026

Greptile Summary

This PR relocates the executor's failure-classification feature flag from ApplicationConfiguration.EnableJobErrorCategorization (a top-level bool) into ErrorCategoriesConfig.Enabled, so the on/off switch lives alongside the rules it gates. It is a straightforward, purely mechanical rename with no behaviour change — the shipped default remains enabled: false.

  • internal/executor/categorizer/types.go: adds Enabled bool as the first field of ErrorCategoriesConfig; the flag is now co-located with DefaultCategory, Categories, etc.
  • internal/executor/application.go: guard condition updated from EnableJobErrorCategorization to ErrorCategories.Enabled — identical runtime semantics.
  • Config files: config/executor/config.yaml gains an explicit enabled: false default; e2e/config/executor_config.yaml is migrated to enabled: true so categorization remains active in e2e runs.

Confidence Score: 5/5

Safe to merge; the change is a pure config-field rename with identical runtime behavior, and all call sites have been updated.

Every reference to the old EnableJobErrorCategorization field has been removed and replaced consistently. The new Enabled field defaults to false (zero value for bool), so any existing deployment that does not update its config will simply have categorization off — the same as if the old field were absent. The e2e config is correctly migrated to keep tests passing, and the base config gains an explicit enabled: false to document the default.

No files require special attention. All changed files are consistent and the migration is complete.

Important Files Changed

Filename Overview
internal/executor/configuration/types.go Removes EnableJobErrorCategorization field and updates the comment on ErrorCategories to reflect that Enabled now lives inside the struct.
internal/executor/categorizer/types.go Adds Enabled bool as the first field of ErrorCategoriesConfig, consolidating the feature-gate inside the config struct it controls.
internal/executor/application.go Updates the classifier construction guard from EnableJobErrorCategorization to ErrorCategories.Enabled; no logic change.
e2e/config/executor_config.yaml Migrates e2e config from top-level enableJobErrorCategorization: true to errorCategories.enabled: true, keeping categorization active in e2e.
config/executor/config.yaml Adds explicit errorCategories.enabled: false shipped default, making the opt-in nature of the feature visible in the base config.
internal/executor/categorizer/doc.go Updates the package-level doc example to include enabled: true, keeping documentation consistent with the new config shape.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[ExecutorConfiguration] --> B[ApplicationConfiguration]
    B --> C[ErrorCategoriesConfig]
    C --> D["Enabled bool (NEW — was EnableJobErrorCategorization)"]
    C --> E[DefaultCategory string]
    C --> F[DefaultSubcategory string]
    C --> G["Categories []CategoryConfig"]

    H[application.go setupExecutorApiComponents] -->|"if ErrorCategories.Enabled"| I[categorizer.NewClassifier]
    I --> J[Classifier used in event reporting]
    H -->|"Enabled == false"| K[classifier = nil, no categorization]
Loading

Reviews (2): Last reviewed commit: "Merge branch 'master' into move-categori..." | Re-trigger Greptile

@dejanzele dejanzele enabled auto-merge (squash) May 6, 2026 13:31
@dejanzele dejanzele merged commit 863ad7b into armadaproject:master May 6, 2026
17 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.

2 participants