Skip to content

fix(semantic-layer): preserve metadata generation on unchanged configuration - #45907

Merged
aminghadersohi merged 3 commits into
apache:masterfrom
mikebridge:sc-126724-low-batch
Nov 3, 2026
Merged

aminghadersohi merged 3 commits into
apache:masterfrom
mikebridge:sc-126724-low-batch

Conversation

@mikebridge

Copy link
Copy Markdown
Contributor

SUMMARY

TL;DR: Saving an unchanged semantic-layer configuration keeps its metadata cache generation and avoids rewriting the configuration. Actual changes and explicit cache clearing still invalidate metadata.

Background: connection edits can submit the existing configuration again. Previously, simply including that field rotated the generation and discarded reusable metadata. Comparing the validated, unmasked JSON content avoids this unnecessary invalidation. Object key order and formatting are ignored; value types and list order remain significant.

An unchanged configuration is omitted from the database update as well, so a stale no-op save cannot overwrite a newer concurrent configuration. Malformed stored configuration remains repairable through an actual update.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Not applicable; backend change only.

TESTING INSTRUCTIONS

Run python -m pytest tests/unit_tests/semantic_layers/simple_cache_clear_test.py -q (41 passed). The unchanged-save and reordered-JSON regressions failed before the fix. A stale-reader regression failed against the first implementation, then passed after unchanged configuration writes were omitted. Changed-file hooks, including MyPy and pylint, pass.

ADDITIONAL INFORMATION

  • Required feature flag: SEMANTIC_LAYERS.
  • No migration or public API change.
  • Independent of the MetricFlow ordering fix; either can merge first.

…uration

Compare effective configuration content before invalidating metadata. Omit unchanged configuration writes so a stale no-op save cannot overwrite a concurrent edit without a new generation. Preserve explicit clearing and invalidation for actual changes.
@netlify

netlify Bot commented Oct 30, 2026

Copy link
Copy Markdown

✅ Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit f4863c1
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/6ae4f65b7976d8fc54b6e766
😎 Deploy Preview https://deploy-preview-45907--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@codecov

codecov Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.81%. Comparing base (e33b7d9) to head (25406ca).
⚠️ Report is 47 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #45907      +/-   ##
==========================================
+ Coverage   84.73%   84.81%   +0.07%     
==========================================
  Files        3069     3070       +1     
  Lines      205767   207229    +1462     
  Branches    48388    48731     +343     
==========================================
+ Hits       174350   175754    +1404     
+ Misses      28123    28113      -10     
- Partials     3294     3362      +68     
Flag Coverage Δ
hive 34.24% <0.00%> (-0.27%) ⬇️
mysql 52.26% <0.00%> (-0.35%) ⬇️
postgres 52.27% <0.00%> (-0.34%) ⬇️
presto 35.95% <0.00%> (-0.29%) ⬇️
python 87.87% <100.00%> (+0.10%) ⬆️
sqlite 52.02% <0.00%> (-0.35%) ⬇️
unit 82.68% <100.00%> (+0.16%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…nfiguration

The update command validates restored credentials but omits unchanged configuration writes to preserve generation and avoid stale-reader overwrites. Assert that contract instead of requiring a redundant rewrite, retaining checks for restored-secret validation and no cache invalidation.
@mikebridge
mikebridge marked this pull request as ready for review November 2, 2026 15:01
@codeant-ai-for-open-source

codeant-ai-for-open-source Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

CodeAnt  CodeAnt PR Risk

🟢  Low Risk

  • The PR appears safe to merge; malformed configurations are treated as changed, allowing repairs to persist, and regression tests cover this behavior.

Assessed commit: 25406ca93271

Do not treat SQL NULL or an empty string as an existing empty JSON object. Let the existing parse-error handler classify these values as changed so a valid empty configuration is persisted and invalidates metadata. Pin both cases after a database reload.

@aminghadersohi aminghadersohi 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.

Verified at this head: stored NULL and empty-string configuration now persist a {} repair and bump the cache generation, and restoring the old fallback fails both new test cases.

@aminghadersohi
aminghadersohi merged commit 9536b0a into apache:master Nov 3, 2026
74 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants