Fixes #29667: preserve autoClassificationConfig on classification PUT (1.13 backport)#29735
Fixes #29667: preserve autoClassificationConfig on classification PUT (1.13 backport)#29735IceS2 wants to merge 1 commit into
Conversation
…#29668) Metadata ingestion re-creates source classifications with a bare PUT (name + description only). The ClassificationUpdater treated the absent autoClassificationConfig as a deletion, wiping it on every ingestion and silently disabling the AutoClassification Agent for PII/PersonalData. Preserve the existing autoClassificationConfig when a PUT omits it, mirroring the existing style/lifeCycle handling. Explicit PATCH clearing still deletes it. (cherry picked from commit 0237550)
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
Code Review ✅ ApprovedRestores OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
The Python checkstyle failed. Please run You can install the pre-commit hooks with |
|
🟡 Playwright Results — all passed (12 flaky)✅ 3671 passed · ❌ 0 failed · 🟡 12 flaky · ⏭️ 26 skipped
🟡 12 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
1 participant
Backport of #29668 to
1.13.What
Preserve a classification's
autoClassificationConfigwhen aPUT /v1/classificationsomits it.Why
Metadata connectors re-create source classifications with a full-replace
PUTcarrying onlyname+description.ClassificationUpdater.entitySpecificUpdaterecorded the absentautoClassificationConfigas a deletion, wiping it on every metadata ingestion and silently disabling the AutoClassification Agent forPII,PersonalData, etc. The updater already force-preservesmutuallyExclusive;autoClassificationConfiglacked the equivalent guard.Change
On
Operation.PUT, restoreautoClassificationConfigfrom the original entity when the incoming value isnull— mirroring the existingstyle/lifeCyclehandling. ExplicitPATCHclearing still deletes it.Fixes #29667
Greptile Summary
This backport fixes a regression where metadata ingestion's bare
PUT /v1/classifications(carrying onlyname+description) silently wiped any existingautoClassificationConfig, disabling auto-classification for system classifications likePII/PersonalDataon every ingestion cycle.ClassificationUpdater.entitySpecificUpdatenow callspreserveAutoClassificationConfigOnPut()before the field diff, restoringautoClassificationConfigfrom the stored entity when the incoming PUT value isnull— matching the existing pattern formutuallyExclusive,style, andlifeCycle. Explicit PATCH-based deletion is unaffected.test_putPreservesAutoClassificationConfig_ingestionScenario) directly simulates the ingestion-sink path and asserts both that the bare PUT preserves the config and that an explicit PATCH still removes it.Confidence Score: 5/5
Safe to merge — the change is a minimal, targeted guard that prevents an existing regression without altering any other update path.
The two-line logic in preserveAutoClassificationConfigOnPut is easy to reason about: it acts only when operation is PUT and the incoming value is null, leaving all PATCH paths and non-null PUT payloads completely untouched. The integration test directly simulates the ingestion-sink PUT and also verifies that PATCH-based deletion continues to work, so both sides of the behavior are covered.
No files require special attention; both changed files are small and self-contained.
Important Files Changed
Reviews (1): Last reviewed commit: "Fixes #29667: preserve autoClassificatio..." | Re-trigger Greptile