Skip to content

fix(service): preserve tag recognizer config on ingestion PUT - #31205

Merged
IceS2 merged 4 commits into
mainfrom
asses-tag-update-break-autoclass
Aug 10, 2026
Merged

fix(service): preserve tag recognizer config on ingestion PUT#31205
IceS2 merged 4 commits into
mainfrom
asses-tag-update-break-autoclass

Conversation

@IceS2

@IceS2 IceS2 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Describe your changes:

Fixes #

Metadata ingestion upserts source tags with a bare CreateTag carrying only classification, name and description. CreateTag defaults recognizers to an empty list and autoClassificationEnabled to false, so TagUpdater could not tell a caller that never heard of those fields from one deliberately clearing them — every ingestion run stripped the tag's recognizers and switched auto-classification off.

PII.Sensitive ships 39 recognizers and is reachable this way: tag_utils with system_tags=True (unitycatalog, databricks, bigquery) matches source tag names against system tags case-insensitively, so a source PII/sensitive pair upserts straight over it.

This is the tag-level twin of the classification-level bug fixed in #29668; that guard covered autoClassificationConfig on Classification, but nothing protected the equivalent fields on Tag.

Fix: treat the recognizer configuration as one unit that a PUT only touches when it names recognizers.

if (operation != Operation.PUT || !nullOrEmpty(updated.getRecognizers())) {
  return;
}
updated.setRecognizers(original.getRecognizers());
updated.setAutoClassificationEnabled(original.getAutoClassificationEnabled());
updated.setAutoClassificationPriority(original.getAutoClassificationPriority());

Independent null-guards would not work: autoClassificationEnabled has a schema default of false, so "disable this" and "I never heard of this field" are identical on the wire. Anchoring all three fields to whether recognizers was named gives the server a signal it can actually read. The check is nullOrEmpty rather than == null because nulls are dropped on store and read back as an empty list.

A PUT that does name recognizers still replaces them, and clearing via PATCH is unaffected — that is the path the UI uses.

Type of change:

  • Bug fix

Metadata ingestion upserts source tags with a bare CreateTag carrying only
classification, name and description. CreateTag defaults recognizers to an
empty list and autoClassificationEnabled to false, so the updater could not
tell a caller that never heard of these fields from one deliberately clearing
them, and every ingestion run stripped the tag's recognizers and switched
auto-classification off. PII.Sensitive ships 39 recognizers and is reachable
this way through the system-tag name match in tag_utils.

Treat the recognizer configuration as one unit that a PUT only touches when
it names recognizers. Clearing via PATCH is unaffected, which is the path the
UI uses. This mirrors the classification-level guard added in #29668.
@github-actions github-actions Bot added Ingestion safe to test Add this label to run secure Github workflows on PRs labels Aug 7, 2026
@IceS2
IceS2 marked this pull request as ready for review August 7, 2026 15:09
@IceS2
IceS2 requested a review from a team as a code owner August 7, 2026 15:09
Copilot AI lite review requested due to automatic review settings August 7, 2026 15:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

  • No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

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 skip-pr-checks label.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit 64ab4b2f49adfa93a77b0e131b7e6051d1f3b3df in Playwright run 31377548954, attempt 1.

✅ 550 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky

Performance

Blocking targets: ✅ met · Optimization targets: 🟡 in progress

Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting.

🕒 Full workflow signal wall (to summary) 50m 30s

⏱️ Max setup 3m 3s · max shard execution 17m 22s · max shard-job elapsed before upload 20m 34s · reporting 3s

🌐 199.90 requests/attempt · 2.83 app boots/UI scenario · 2.02% common-shard skew

Optimization targets still in progress:

  • Application boot ratio was 2.83 per UI scenario (1617 boots / 571 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
✅ Shard chromium-01 132 0 0 0 0 0
✅ Shard chromium-02 128 0 0 0 0 0
✅ Shard chromium-03 139 0 0 0 0 0
✅ Shard data-asset-rules-01 61 0 0 0 0 0
✅ Shard domain-isolation-01 14 0 0 0 0 0
✅ Shard global-state-01 34 0 0 0 0 0
✅ Shard ingestion-01 1 0 0 0 0 0
✅ Shard reindex-01 2 0 0 0 0 0
✅ Shard search-01 10 0 0 0 0 0
✅ Shard search-rbac-01 29 0 0 0 0 0

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

Copilot AI review requested due to automatic review settings August 10, 2026 06:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings August 10, 2026 10:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@gitar-bot

gitar-bot Bot commented Aug 10, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Preserves tag recognizer configuration and auto-classification settings on ingestion PUT operations by anchoring them to whether recognizers are explicitly provided. No issues found.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source

@IceS2
IceS2 added this pull request to the merge queue Aug 10, 2026
Merged via the queue into main with commit fd94194 Aug 10, 2026
104 of 106 checks passed
@IceS2
IceS2 deleted the asses-tag-update-break-autoclass branch August 10, 2026 14:50
IceS2 added a commit that referenced this pull request Aug 10, 2026
#31304)

Metadata ingestion upserts source tags with a bare CreateTag carrying only
classification, name and description. CreateTag defaults recognizers to an
empty list and autoClassificationEnabled to false, so the updater could not
tell a caller that never heard of these fields from one deliberately clearing
them, and every ingestion run stripped the tag's recognizers and switched
auto-classification off. PII.Sensitive ships 39 recognizers and is reachable
this way through the system-tag name match in tag_utils.

Treat the recognizer configuration as one unit that a PUT only touches when
it names recognizers. Clearing via PATCH is unaffected, which is the path the
UI uses. This mirrors the classification-level guard added in #29668.

(cherry picked from commit fd94194)
IceS2 added a commit that referenced this pull request Aug 10, 2026
#31305)

Metadata ingestion upserts source tags with a bare CreateTag carrying only
classification, name and description. CreateTag defaults recognizers to an
empty list and autoClassificationEnabled to false, so the updater could not
tell a caller that never heard of these fields from one deliberately clearing
them, and every ingestion run stripped the tag's recognizers and switched
auto-classification off. PII.Sensitive ships 39 recognizers and is reachable
this way through the system-tag name match in tag_utils.

Treat the recognizer configuration as one unit that a PUT only touches when
it names recognizers. Clearing via PATCH is unaffected, which is the path the
UI uses. This mirrors the classification-level guard added in #29668.

(cherry picked from commit fd94194)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ingestion safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants