Conversation
|
#393 was settled by removing the root |
b2bfb27 to
9c2a506
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved validation gaps and Polaris edge cases block approval.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Migrates the Python SDK and converters to flat semantic-model documents, with Polaris bulk import support and updated tests, documentation, and CI.
Changes:
- Flattens
OssieDocumentand updates converter handling. - Updates fixtures, snapshots, documentation, and validation workflows.
- Adds deterministic Polaris namespace output support.
File summaries
| File | Reviewed change |
|---|---|
python/tests/test_models.py |
Flat model and legacy-shape tests |
python/src/ossie/models.py |
Flat document model |
python/README.md |
SDK format documentation |
docs/index.md |
Format overview updates |
converters/wisdom/tests/test_wisdom_to_ossie.py |
Wisdom conversion tests |
converters/wisdom/tests/test_ossie_to_wisdom.py |
Wisdom reverse-conversion tests |
converters/wisdom/src/ossie_wisdom/wisdom_to_ossie.py |
Wisdom flat-document output |
converters/wisdom/src/ossie_wisdom/ossie_to_wisdom.py |
Wisdom flat-document input |
converters/wisdom/src/ossie_wisdom/converter_issues.py |
Wisdom conversion issues |
converters/wisdom/src/ossie_wisdom/cli.py |
Wisdom CLI handling |
converters/wisdom/README.md |
Wisdom documentation |
converters/snowflake/tests/test_ossie_to_snowflake_yaml_converter.py |
Snowflake conversion tests |
converters/snowflake/src/ossie_snowflake/converter.py |
Snowflake flat input conversion |
converters/snowflake/README.md |
Snowflake documentation |
converters/sigma/tests/test_sigma_to_ossie.py |
Sigma import tests |
converters/sigma/tests/test_roundtrip.py |
Sigma round-trip tests |
converters/sigma/tests/test_ossie_to_sigma.py |
Sigma export tests |
converters/sigma/src/ossie_sigma/sigma_to_ossie.py |
Sigma flat-document import |
converters/sigma/src/ossie_sigma/ossie_to_sigma.py |
Sigma flat-document export |
converters/sigma/src/ossie_sigma/converter_issues.py |
Sigma conversion issues |
converters/sigma/README.md |
Sigma documentation |
converters/sigma/LIMITATIONS.md |
Sigma limitations |
converters/salesforce/src/test/resources/schemas/salesforce-input-fixture-schema.json |
Salesforce fixture schema |
converters/salesforce/src/test/java/org/apache/ossie/SalesforceToOssieConverterTest.java |
Salesforce conversion tests |
converters/salesforce/src/test/java/org/apache/ossie/converter/FlatDocumentConversionTest.java |
Flat-document conversion tests |
converters/salesforce/src/main/java/org/apache/ossie/converter/ConverterImpl.java |
Salesforce conversion implementation |
converters/salesforce/src/main/java/org/apache/ossie/converter/ConverterConstants.java |
Salesforce converter constants |
converters/salesforce/src/main/java/org/apache/ossie/converter/Converter.java |
Salesforce converter interface |
converters/salesforce/README.md |
Salesforce documentation |
converters/README.md |
Converter documentation |
converters/polaris/src/main/java/org/apache/ossie/converter/polaris/PolarisImporter.java |
Polaris import handling |
converters/polaris/src/main/java/org/apache/ossie/converter/polaris/PolarisExporter.java |
Polaris export handling |
converters/polaris/src/main/java/org/apache/ossie/converter/polaris/OssieYamlGenerator.java |
Ossie YAML generation |
converters/polaris/src/main/java/org/apache/ossie/converter/polaris/OssiePolarisConverter.java |
Polaris conversion and bulk output |
converters/polaris/src/main/java/org/apache/ossie/converter/polaris/OssieModelParser.java |
Flat YAML parsing |
converters/polaris/src/main/java/org/apache/ossie/converter/polaris/model/OssieModel.java |
Polaris Ossie model |
converters/polaris/README.md |
Polaris documentation |
converters/orionbelt/tests/test_ossie_metric_no_silent_loss.py |
Metric preservation tests |
converters/orionbelt/tests/test_ossie_converter_vendors.py |
Vendor conversion tests |
converters/orionbelt/tests/test_ossie_converter_trend_v26.py |
Trend conversion tests |
converters/orionbelt/tests/test_ossie_converter_roundtrip_robustness.py |
Round-trip robustness tests |
converters/orionbelt/tests/test_ossie_converter_pop.py |
Population conversion tests |
converters/orionbelt/tests/test_ossie_converter_ontology.py |
Ontology conversion tests |
converters/orionbelt/tests/test_ossie_converter_measure_overrides.py |
Measure override tests |
converters/orionbelt/tests/test_ossie_converter_filters.py |
Filter conversion tests |
converters/orionbelt/tests/test_ossie_converter_cumulative.py |
Cumulative conversion tests |
converters/orionbelt/src/ossie_orionbelt/validation.py |
OrionBelt validation |
converters/orionbelt/src/ossie_orionbelt/ossie_to_obml.py |
Ossie-to-OBML conversion |
converters/orionbelt/src/ossie_orionbelt/ontology.py |
OrionBelt ontology handling |
converters/orionbelt/src/ossie_orionbelt/obml_to_ossie.py |
OBML-to-Ossie conversion |
converters/orionbelt/README.md |
OrionBelt documentation |
converters/orionbelt/ossie_obml_ontology_mapping_analysis.md |
Ontology mapping analysis |
converters/orionbelt/ossie_obml_mapping_analysis.md |
OBML mapping analysis |
converters/ontology/src/ossie_ontology/spec.py |
Ontology specification |
converters/omni/tests/test_real_world_layout.py |
Omni layout tests |
converters/omni/tests/test_ossie_to_omni.py |
Omni export tests |
converters/omni/tests/test_omni_to_ossie.py |
Omni import tests |
converters/omni/tests/fixtures/fixtureA_ossie.yaml |
Omni fixture |
converters/omni/tests/_util.py |
Omni test utilities |
converters/omni/tests/_roundtrip_helpers.py |
Omni round-trip helpers |
converters/omni/src/ossie_omni/ossie_to_omni.py |
Omni flat-document export |
converters/omni/src/ossie_omni/omni_to_ossie.py |
Omni flat-document import |
converters/omni/README.md |
Omni documentation |
converters/nvidia/tests/test_converter.py |
NVIDIA converter tests |
converters/nvidia/tests/fixtures/sales.ossie.yaml |
NVIDIA fixture |
converters/nvidia/src/ossie_nvidia_gsf/native_converter.py |
NVIDIA native conversion |
converters/nvidia/README.md |
NVIDIA documentation |
converters/microsoft/tests/test_tom_integration.py |
Microsoft TOM integration tests |
converters/microsoft/tests/test_semantic_model_to_ossie.py |
Microsoft import tests |
converters/microsoft/tests/test_ossie_to_semantic_model.py |
Microsoft export tests |
converters/microsoft/tests/test_edge_cases.py |
Microsoft edge-case tests |
converters/microsoft/tests/conftest.py |
Microsoft test configuration |
converters/microsoft/src/ossie_microsoft/semantic_model_to_ossie.py |
Microsoft flat-document import |
converters/microsoft/src/ossie_microsoft/ossie_to_semantic_model.py |
Microsoft flat-document export |
converters/microsoft/README.md |
Microsoft documentation |
converters/honeydew/tests/test_ossie_honeydew_converter.py |
Honeydew converter tests |
converters/honeydew/src/ossie_honeydew/converter.py |
Honeydew conversion |
converters/honeydew/README.md |
Honeydew documentation |
converters/gooddata/tests/test_roundtrip.py |
GoodData round-trip tests |
converters/gooddata/tests/test_ossie_to_gooddata.py |
GoodData export tests |
converters/gooddata/tests/test_gooddata_to_ossie.py |
GoodData import tests |
converters/gooddata/src/ossie_gooddata/ossie_to_gooddata.py |
GoodData flat-document export |
converters/gooddata/src/ossie_gooddata/models.py |
GoodData models |
converters/gooddata/src/ossie_gooddata/gooddata_to_ossie.py |
GoodData flat-document import |
converters/gooddata/README.md |
GoodData documentation |
converters/dbt/tests/test_ossie_to_msi.py |
dbt export tests |
converters/dbt/tests/test_msi_to_ossie.py |
dbt import tests |
converters/dbt/tests/helpers.py |
dbt test helpers |
converters/dbt/tests/__snapshots__/test_ossie_to_msi.ambr |
dbt export snapshots |
converters/dbt/tests/__snapshots__/test_msi_to_ossie.ambr |
dbt import snapshots |
converters/dbt/src/ossie_dbt/ossie_to_msi.py |
dbt flat-document export |
converters/dbt/src/ossie_dbt/msi_to_ossie.py |
dbt flat-document import |
converters/dbt/README.md |
dbt documentation |
converters/databricks/tests/test_metric_view_to_ossie.py |
Databricks import tests |
converters/databricks/tests/fixtures/tpcds_ossie.yaml |
Databricks TPC-DS fixture |
converters/databricks/tests/fixtures/fixtureB_ossie.yaml |
Databricks fixture |
converters/databricks/tests/fixtures/fixtureA_ossie.yaml |
Databricks fixture |
converters/databricks/tests/_util.py |
Databricks test utilities |
converters/databricks/tests/_roundtrip_helpers.py |
Databricks round-trip helpers |
converters/databricks/src/ossie_databricks/ossie_to_metric_view.py |
Databricks export |
converters/databricks/src/ossie_databricks/metric_view_to_ossie.py |
Databricks import |
converters/databricks/README.md |
Databricks documentation |
.github/workflows/validation-ci.yml |
Shared validation workflow |
.github/workflows/converter-wisdom-ci.yml |
Wisdom CI workflow |
.github/workflows/converter-snowflake-ci.yml |
Snowflake CI workflow |
.github/workflows/converter-sigma-ci.yml |
Sigma CI workflow |
.github/workflows/converter-salesforce-ci.yml |
Salesforce CI workflow |
.github/workflows/converter-polaris-ci.yml |
Polaris CI workflow |
.github/workflows/converter-orionbelt-ci.yml |
OrionBelt CI workflow |
.github/workflows/converter-ontology-ci.yml |
Ontology CI workflow |
.github/workflows/converter-omni-ci.yml |
Omni CI workflow |
.github/workflows/converter-nvidia-ci.yml |
NVIDIA CI workflow |
.github/workflows/converter-microsoft-ci.yml |
Microsoft CI workflow |
.github/workflows/converter-honeydew-ci.yml |
Honeydew CI workflow |
.github/workflows/converter-gooddata-ci.yml |
GoodData CI workflow |
.github/workflows/converter-dbt-ci.yml |
dbt CI workflow |
.github/workflows/converter-databricks-ci.yml |
Databricks CI workflow |
Review details
Suppressed comments (3)
converters/orionbelt/src/ossie_orionbelt/obml_to_ossie.py:146
- This comment says root-level dialect/vendor advertisement arrays remain optional, but the migrated schema removes those properties (
additionalProperties: false) and the converter above rejects them. Please describe them as unsupported rather than optional so the implementation contract is not contradicted.
converters/polaris/src/main/java/org/apache/ossie/converter/polaris/OssiePolarisConverter.java:148 - The new
--output-dirpath is skipped when the catalog has no nonempty namespaces because this early return also handles single-file and stdout imports. The documented contract says those modes require exactly one nonempty namespace, so an empty catalog should fail instead of returning success without producing output.
converters/polaris/src/main/java/org/apache/ossie/converter/polaris/OssiePolarisConverter.java:164 - The new bulk-import path writes the generator output without constraining catalog identifiers, but
OssieYamlGeneratoremits names and other identifiers as unquoted scalars. A valid Polaris namespace such astruebecomesname: true, which SnakeYAML reads as a boolean andOssieModelParserthen rejects, leaving an emitted file that cannot be re-imported. Quote all generated string fields (or use a YAML serializer) before writing these documents.
- Files reviewed: 125/125 changed files
- Comments generated: 6
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
I would appreciate to review this PR as I'm doing a bunch of work on converters. |
Follows apache#383, which moved the semantic model's fields to the document root and removed the `semantic_model` wrapper. `ossie-schema.json` now requires `version`, `name` and `datasets` at the root and forbids additional properties, so the converter's emitted documents stopped validating the moment the branch was rebased. Emit side is `{"version": ..., **semantic_model}`. The spread cannot clobber `version`: the model dict only ever holds name/description/datasets/ relationships/metrics plus the `custom_extensions` the stash writes, and no path on it emits a `version` key. Read side rejects the old wrapper by name rather than lifting its first entry, matching the treatment in apache#396. Reading entry zero would convert silently while discarding any later model, and a document old enough to carry the wrapper may have moved on elsewhere too; naming the one thing the reader must change is more useful than a best-effort guess. `datasets` is `minItems: 1` upstream, so the emit side now reports TS-MODEL-NO-DATASETS at ERROR when a model yields none. Without it the two legs disagreed: `to-ossie` exited 0 having written a document that fails the schema and that this converter's own `to-tml` refuses. Both legs now exit 1 on the same input. Both fixtures were regenerated by running the migrated converter over their own TML and asserting the result deep-equals the old expected document with its wrapper lifted -- so the 1,151 changed fixture lines are a re-rooting, not a content change. 823 tests pass. The README's mapping row is now pinned by a test, having been verified by nothing: reverting it left the whole suite green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Summary
Migrate the Python SDK and all affected converters to the single-model document format merged in #383. Ossie input/output now puts
name,datasets,relationships, andmetricsdirectly at the root alongsideversion; the legacysemantic_modelwrapper is rejected instead of selecting its first entry.Rebased onto current main after #383 merged. This PR now contains only the SDK/converter migration and its CI/documentation updates. It also incorporates #397: root-level
dialectsandvendorsare removed; expression dialects and vendor custom extensions remain supported.Behavior
OssieDocumentAPI and update dbt, Sigma, Wisdom, Databricks, Honeydew, Omni, OrionBelt, Snowflake, GoodData, NVIDIA GSF, Microsoft/Power BI, Salesforce, and Polaris readers/writers, fixtures, snapshots, and usage documentation.dialects/vendorsfields and first-model/drop-extra-model behavior. Preserve vendor-native collections such as dbt'ssemantic_models.semantic_modelstructure and exclude standalone document metadata from that embedded object.import --output-dir DIRto emit one document per nonempty namespace with deterministic, collision-safe filenames and no overwrites. Single-file/stdout imports require one nonempty namespace.This is a breaking SDK and converter format change for the mutable
0.2.0.dev0specification. Legacy documents must be migrated first; see the spec's migration guidance.Checklist
Validation