Fixes open-metadata/openmetadata-collate#5995: remove external S3 sample-data storage support - #33143
akashverma0786 wants to merge 17 commits into
Conversation
…ple-data storage support Strip the AWS S3 branch and its bucket/prefix/path-pattern/credential fields from the shared sample-data storage schema, and delete the now-dead ingestion code that read them: the upload_sample_storage_config wiring, the whole sample_data_handler.py upload path, and the orphaned get_storage_config_for_table/get_sample_storage_config helpers (zero callers, confirmed by grep and by live instrumentation against a real MySQL + MinIO run). OpenMetadata-hosted sample-data storage, storeSampleData/readSampleData, and classification sampling are unaffected — verified live: with S3 config fully set, OM's own sample data still populated and MinIO received zero writes, both before and after this change.
akashverma0786
added
the
safe to test
❌ 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 |
|
The Python checkstyle failed. Please run You can install the pre-commit hooks with |
1 similar comment
|
The Python checkstyle failed. Please run You can install the pre-commit hooks with |
✅ Generated Sources Auto-UpdatedThe generated TypeScript types ( |
|
Too many files changed for review (111 files, 100 file limit). Bypass the limit by tagging |
🔄 Playwright impact map auto-refreshedThis PR touched specs or UI source that changed the source→spec routing map. I regenerated What is this file? It is the auto-generated half of Playwright's PR planner. It routes "if source X changes, run specs Y" by walking spec imports and cross-referencing What if I want to regenerate locally instead? Run this before pushing your next change to skip the bot commit: python3 .github/scripts/generate_playwright_impact_map.py
git add .github/playwright/impact-map.generated.json
git commit --amend --no-edit # or a separate commit |
|
The Python checkstyle failed. Please run You can install the pre-commit hooks with |
1 similar comment
|
The Python checkstyle failed. Please run You can install the pre-commit hooks with |
Jest test CoverageUI tests summary
|
🔴 Playwright Results — workflow failedValidated commit ✅ 4502 passed · ❌ 1 failed · 🟡 6 flaky · ⏭️ 1 skipped · 🧰 0 lifecycle flaky ❌ Action needed: test(s) failed on every attempt against this PR’s validated commit — see Genuine Failures below. These are test failures, not CI budget or infrastructure issues. PerformanceBlocking 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) 55m 33s ⏱️ Max setup 4m 12s · max shard execution 22m 43s · max shard-job elapsed before upload 25m 54s · reporting 20s 🌐 221.15 requests/attempt · 2.23 app boots/UI scenario · 46.59% common-shard skew Optimization targets still in progress:
Genuine Failures (failed on all attempts)❌
|
Collapsing storageConfig to a plain empty object routed it away from CoreOneOfField into the generic object-field template, whose shouldRenderNullTemplate returns null for any non-root object with zero properties and additionalProperties:false — the "OpenMetadata Storage" placeholder rendered as nothing under Sample Data Storage Config. Wrapping it back in a single-item oneOf routes it through CoreOneOfField instead, which renders the branch title even with one option (no dropdown, per its own test coverage) rather than going through the null-returning path.
✅ Generated Sources Auto-UpdatedThe generated TypeScript types ( |
…d S3-removal migration - Flatten sampleDataStorageConfig.config to a top-level [No Sample Data Storage Config, OpenMetadata Storage] dropdown and delete the vestigial dataStorageConfig nesting. The prior single-branch storageConfig oneOf rendered no selector and an empty panel; the flattened two-option dropdown shows a real, selectable choice. - Drop the now-unused DataStorageConfig plumbing from the TimescaleDB sampler. - Add a MySQL+Postgres migration stripping the legacy S3 sampleDataStorageConfig shape from stored service/profiler configs (backward-compat; never writes externally). - Update the ConnectionConfigLayout Playwright test to the no-S3 form. - Regenerate dereferenced connection schemas (public jsons).
🔄 Playwright impact map auto-refreshedThis PR touched specs or UI source that changed the source→spec routing map. I regenerated What is this file? It is the auto-generated half of Playwright's PR planner. It routes "if source X changes, run specs Y" by walking spec imports and cross-referencing What if I want to regenerate locally instead? Run this before pushing your next change to skip the bot commit: python3 .github/scripts/generate_playwright_impact_map.py
git add .github/playwright/impact-map.generated.json
git commit --amend --no-edit # or a separate commit |
✅ Generated Sources Auto-UpdatedThe generated TypeScript types ( |
# Conflicts: # .github/playwright/impact-map.generated.json # bootstrap/sql/migrations/native/2.1.0/mysql/postDataMigrationSQLScript.sql # bootstrap/sql/migrations/native/2.1.0/postgres/postDataMigrationSQLScript.sql
🔄 Playwright impact map auto-refreshedThis PR touched specs or UI source that changed the source→spec routing map. I regenerated What is this file? It is the auto-generated half of Playwright's PR planner. It routes "if source X changes, run specs Y" by walking spec imports and cross-referencing What if I want to regenerate locally instead? Run this before pushing your next change to skip the bot commit: python3 .github/scripts/generate_playwright_impact_map.py
git add .github/playwright/impact-map.generated.json
git commit --amend --no-edit # or a separate commit |
no-positional-locator suppressions total is 1241 after prior fixes, but the test still asserted the pre-fix count of 1243.
lodash startCase splits OpenMetadata into Open Metadata at the lower-to-upper boundary, so the sampleDataStorageConfig oneOf option titled "OpenMetadata Storage" rendered as "Open Metadata Storage". Playwright's exact-text option lookup never matched, timing out.
After external S3 sample-data storage was removed, sampleDataStorageConfig.config
was left as a oneOf with two structurally identical empty branches ("No Sample Data
Storage Config" and "OpenMetadata Storage") — an invalid oneOf that also renders a
redundant, do-nothing selector. Collapse it to a single empty "OpenMetadata Storage"
object.
OpenMetadata-hosted storage has no settable fields, so hide the now-fieldless
sampleDataStorageConfig group from every connection form via DEF_UI_SCHEMA (same
pattern as policyAgentConfig); the empty box no longer renders.
Update the connection-config E2E to assert the field is hidden, and drop the
OpenMetadata label fix and its unit test that only existed to render the now-hidden
"OpenMetadata Storage" option.
✅ Generated Sources Auto-UpdatedThe generated TypeScript types ( |
Code Review ✅ Approved 1 resolved / 1 findingsRemoves external S3 sample-data storage support by stripping S3 fields from ✅ 1 resolved✅ Bug: sampleDataStorageConfig oneOf has two identical empty branches
OptionsDisplay: compact → Counting what did not apply, without listing it. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
|
|
2 participants
Summary
Removes external S3 sample-data storage support per open-metadata/openmetadata-collate#5995.
connectionBasicType.json): strippedbucketName,prefix,filePathPattern,overwriteData, and the AWS S3 branch (with itsawsCredentials.jsonref) from the shareddataStorageConfigdefinition.sampleDataStorageConfigitself — the on/off toggle every connector$refs — is untouched.sample_data_handler.py(the Parquet-to-S3 upload path), theupload_sample_storage_configwiring insampler_config.py/sampler_interface.py, and the orphanedget_storage_config_for_table/get_sample_storage_confighelpers insampler/config.py.All of the removed ingestion code had zero callers anywhere in the codebase — confirmed by repo-wide grep and by live instrumentation against a real
metadata classifyrun (MySQL source, MinIO as the configured S3 target):upload_sample_storage_configwas alwaysNoneat runtime,upload_sample_data()was never entered even with a fully valid, reachable S3 config set.What's retained, verified live (same MinIO test)
storeSampleData/readSampleData— unaffectedsampleData, the Sample Data tab) — unaffected; populated correctly with S3 config set and with it removedTest plan
ruff checkclean on all touched filestest_sampler_interface.py— dropped the two tests exercising the deleted S3-upload branch) and passing (20/20)metadata classify, with a valid S3/MinIOsampleDataStorageConfigset — before and after this change, MinIO bucket stayed empty, OM's own sample data populated correctlymake generate+mvn clean install -pl openmetadata-specto regenerate Java/Python/TS models from the schema change (not yet run in this PR)