You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ossie.apache.org by "flyrain (via GitHub)" <gi...@apache.org> on 2026/09/14 17:47:34 UTC

[PR] Migrate converters and Python SDK to flat semantic model documents [ossie]

flyrain opened a new pull request, #396:
URL: https://github.com/apache/ossie/pull/396

   ## Summary
   
   Migrate the Python SDK and all affected converters to the single-model document format in #383. Ossie input/output now puts `name`, `datasets`, `relationships`, and `metrics` directly at the root alongside `version`; the legacy `semantic_model` wrapper is rejected instead of selecting its first entry.
   
   ```yaml
   version: 0.2.0.dev0
   name: sales_analytics
   datasets:
     - name: orders
       source: sales.public.orders
   relationships: []
   metrics: []
   ```
   
   **Depends on #383.** This branch includes that spec change so the migration can be validated before it merges. Review the [migration-only diff](https://github.com/flyrain/ossie/compare/ab4d42e...yf/migrate-converters-flat-model); the shared spec commits will fall out of this PR's diff once #383 is merged with compatible history (otherwise this branch will need rebasing).
   
   ## Behavior
   
   - Flatten the shared `OssieDocument` API and update dbt, Sigma, Wisdom, Databricks, Honeydew, Omni, OrionBelt, Snowflake, GoodData, NVIDIA GSF, Salesforce, and Polaris readers/writers, fixtures, snapshots, and usage documentation.
   - Remove obsolete first-model/drop-extra-model behavior. Preserve vendor-native collections such as dbt's `semantic_models`.
   - Preserve the ontology mapping's embedded `semantic_model` structure and exclude standalone document metadata from that embedded object.
   - Add Polaris `import --output-dir DIR` to emit one document per nonempty namespace with deterministic, collision-safe filenames and no overwrites. Single-file/stdout imports require one nonempty namespace.
   - Trigger converter CI on shared schema/example changes and run shared SDK tests in Validation CI.
   
   This is a breaking SDK and converter format change for the mutable `0.2.0.dev0` specification. Legacy documents must be migrated first; see the spec's migration guidance.
   
   ## Validation
   
   - Passed focused SDK serialization/rejection tests and converter input/output, roundtrip, CLI, relationship, and metadata checks across Python packages. Five dbt snapshots passed.
   - Passed ontology mapping and roundtrip checks; embedded ontology model shape preserved.
   - All 11 checked-in standalone Ossie YAML fixtures/examples validate against the new schema. Migrated converter fixtures preserve their contents exactly after flattening.
   - Python compilation/syntax checks and whitespace checks passed. Existing unrelated unused-import lint findings were not changed.
   - Passed 18 focused Polaris and 18 Salesforce tests, including bulk output and complete conversion-pipeline checks; Java main/test compilation passed. One existing Salesforce vendor-schema check was skipped because its optional manually downloaded schema is unavailable. New Salesforce import tests use an explicit test-only input envelope schema and validate generated Ossie against the canonical schema.
   - Full local regression suites were not run; full regression coverage is left to CI.
   
   ## Checklist
   
   - [x] Converter logic, fixtures, examples, and documentation reflect #383.
   - [x] Breaking behavior and bulk export migration described.
   - [x] Focused regression coverage added or updated.
   - [x] Ontology embedding and vendor-specific model collections preserved.
   - [x] No runtime dependencies added.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Migrate converters and Python SDK to flat semantic model documents [ossie]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #396:
URL: https://github.com/apache/ossie/pull/396#discussion_r4029079627


##########
converters/databricks/src/ossie_databricks/ossie_to_metric_view.py:
##########
@@ -77,13 +77,13 @@ def convert_ossie_to_metric_view(ossie_yaml_str, source=None):
             f"Unsupported Apache Ossie version '{version}'. Supported: {OSSIE_VERSION}"
         )
 
-    models = root.get("semantic_model")
-    if not isinstance(models, list) or not models:
-        raise ConversionError("'semantic_model' must be a non-empty list")
-    if len(models) > 1:
-        _warn("model", "multiple semantic models found; converting only the first")
+    if "semantic_model" in root:
+        raise ConversionError(
+            "Legacy 'semantic_model' wrappers are not supported; "
+            "place the model properties directly at the document root"
+        )

Review Comment:
   The new flat-shape check only rejects the legacy wrapper. Root `dialects` and `vendors` are no longer valid document properties, but this converter passes them through to `_convert_model`, where they are ignored, and a document without the required root `name` is accepted via the `<unnamed>` fallback. Validate both before conversion so invalid flat documents are not silently accepted.



##########
converters/gooddata/src/ossie_gooddata/ossie_to_gooddata.py:
##########
@@ -62,21 +62,28 @@ def ossie_to_gooddata(
     datasets: list[GdDataset] = []
     date_instances: list[GdDateInstance] = []
 
-    for sm in ossie_model.get("semantic_model", []):
-        relationship_map = _build_relationship_map(sm)
-        # Pre-pass: for each Ossie dataset, record whether it is a date instance
-        # and map its physical source columns to the attribute ids that will
-        # be generated. Reference target columns resolve via this map.
-        target_info = _build_target_info(sm)
-
-        for ds in sm.get("datasets", []):
-            gd_ds, date_inst = _convert_ossie_dataset(
-                ds, relationship_map, target_info, data_source_id,
-            )
-            if date_inst:
-                date_instances.append(date_inst)
-            else:
-                datasets.append(gd_ds)
+    if not isinstance(ossie_model, dict):
+        raise ValueError("Ossie input must be a mapping")
+    if "semantic_model" in ossie_model:
+        raise ValueError("Ossie model properties must be at the root; semantic_model wrappers are not supported")
+    if not isinstance(ossie_model.get("name"), str) or not isinstance(ossie_model.get("datasets"), list):
+        raise ValueError("Ossie input requires name and datasets at the root")

Review Comment:
   This validation now rejects the legacy wrapper, but it does not reject the removed root `dialects`/`vendors` properties. `ossie_to_gooddata` then ignores those keys, so a document that is invalid under the new flat schema is accepted with silent metadata loss. Add the root-property check before assigning `sm`.



##########
converters/orionbelt/src/ossie_orionbelt/ossie_to_obml.py:
##########
@@ -111,22 +107,20 @@ def convert(self) -> dict:
         self.warnings = []
         self._unconverted_metrics = []
 
-        # v0.1.x inputs need the legacy shim to promote pre-v0.2
-        # custom_extensions into v0.2 first-class fields before we parse.
-        self._normalize_legacy_v01()
-
-        models = self.ossie.get("semantic_model", [])
-        if not models:
-            raise ValueError("No semantic_model found in Ossie input")
-
-        # Take the first semantic model (OBML is a single-model format)
-        model = models[0]
-        if len(models) > 1:
-            self.warnings.append(
-                f"Ossie contains {len(models)} semantic models; "
-                f"only the first ('{model.get('name')}') is converted."
+        if not isinstance(self.ossie, dict):
+            raise ValueError("Ossie input must be a mapping")
+        if "semantic_model" in self.ossie:
+            raise ValueError(
+                "Legacy 'semantic_model' wrappers are not supported; "
+                "place the model properties directly at the document root"
             )

Review Comment:
   The newly added wrapper rejection does not cover the other removed root properties. `OssietoOBML` proceeds with a document containing `dialects` or `vendors` and silently ignores those keys, even though the standalone v0.2 schema rejects them. Reject these keys alongside the legacy wrapper before normalizing the model.



##########
converters/snowflake/src/ossie_snowflake/converter.py:
##########
@@ -107,25 +107,18 @@ def convert_ossie_to_snowflake(ossie_yaml_str):
             f"Supported: {SUPPORTED_VERSION}"
         )
 
-    semantic_model = root.get("semantic_model")
-    if not isinstance(semantic_model, list) or len(semantic_model) == 0:
+    if "semantic_model" in root:
         raise OssieConversionError(
-            "Invalid Ossie YAML: 'semantic_model' must be a non-empty list"
+            "Legacy 'semantic_model' wrappers are not supported; "
+            "place the model properties directly at the document root"
         )

Review Comment:
   Although this migration rejects `semantic_model`, it still accepts root `dialects` and `vendors` and drops them when building `model`. Those properties were removed from the standalone document schema, so accepting them here creates a silent-loss path instead of rejecting the obsolete shape.



##########
converters/omni/src/ossie_omni/ossie_to_omni.py:
##########
@@ -103,13 +103,13 @@ def convert_ossie_to_omni(ossie_yaml_str, base_view=None, dialect=None):
             f"Unsupported Ossie version '{version}'. Supported: {OSSIE_VERSION}"
         )
 
-    models = root.get("semantic_model")
-    if not isinstance(models, list) or not models:
-        raise ConversionError("'semantic_model' must be a non-empty list")
-    if len(models) > 1:
-        _warn("model", "multiple semantic models found; converting only the first")
+    if "semantic_model" in root:
+        raise ConversionError(
+            "Legacy 'semantic_model' wrappers are not supported; "
+            "place the model properties directly at the document root"
+        )

Review Comment:
   This new flat-document gate rejects `semantic_model`, but it still accepts the removed root `dialects`/`vendors` fields and silently ignores them during conversion. It also lets a document with no `name` reach `_convert_model`, which substitutes `<unnamed>`, so input missing a required root property can convert successfully instead of being rejected. Validate the root metadata and require a string `name` before calling `_convert_model`.



##########
converters/polaris/src/main/java/org/apache/ossie/converter/polaris/OssieModelParser.java:
##########
@@ -51,21 +51,24 @@ public OssieModel parse(Path yamlPath) throws IOException {
     @SuppressWarnings("unchecked")
     public OssieModel parse(InputStream is) {
         Yaml yaml = new Yaml();
-        Map<String, Object> root = yaml.load(is);
+        Object document = yaml.load(is);
+        if (!(document instanceof Map)) {
+            throw new IllegalArgumentException("An Ossie document must be an object");
+        }
+        Map<String, Object> root = (Map<String, Object>) document;
+        if (root.containsKey("semantic_model")) {
+            throw new IllegalArgumentException(
+                    "Legacy semantic_model wrappers are not supported; use version, name, and datasets at the root");
+        }

Review Comment:
   The parser now rejects the legacy wrapper, but it still accepts root `dialects` and `vendors` and simply omits them from the internal model. Since those properties were removed from the standalone Ossie schema, export should reject this input rather than silently discarding it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Migrate converters and Python SDK to flat semantic model documents [ossie]

Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on PR #396:
URL: https://github.com/apache/ossie/pull/396#issuecomment-5702117794

   I would appreciate to review this PR as I'm doing a bunch of work on converters. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Migrate converters and Python SDK to flat semantic model documents [ossie]

Posted by "kayemkim (via GitHub)" <gi...@apache.org>.
kayemkim commented on PR #396:
URL: https://github.com/apache/ossie/pull/396#issuecomment-5673253891

   #393 was settled by removing the root `dialects`/`vendors` arrays from the schema (#397, now on main), and #383 has been asked to drop them in its rebase. This branch still keeps them in a few places that will need the same treatment: `converters/sigma/tests/test_roundtrip.py` asserts `serialized["dialects"] == ["ANSI_SQL", "SIGMA"]` and `serialized["vendors"] == ["SIGMA"]`, the orionbelt `test_schema_rejects_root_dialects_and_vendors` is flipped to `test_schema_accepts_root_dialects_and_vendors`, the nvidia reader allow-lists both keys at the root, the honeydew tests expect a root `vendors`, and the SDK tests and `python/README.md` list them alongside `version`. I merged the schema change with this branch earlier; the conflict is confined to those spots and resolves by dropping them.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Migrate converters and Python SDK to flat semantic model documents [ossie]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on PR #396:
URL: https://github.com/apache/ossie/pull/396#issuecomment-5733405763

   Hi @khush-bhatia @jbonofre, could you take a look at this PR? Don't want the main branch in a inconsistent status for too long. Thanks!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Migrate converters and Python SDK to flat semantic model documents [ossie]

Posted by "khush-bhatia (via GitHub)" <gi...@apache.org>.
khush-bhatia commented on PR #396:
URL: https://github.com/apache/ossie/pull/396#issuecomment-5736066142

   Merging this since this is affecting all converters and CI is red until this merges. cc @jbonofre and @flyrain 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Migrate converters and Python SDK to flat semantic model documents [ossie]

Posted by "khush-bhatia (via GitHub)" <gi...@apache.org>.
khush-bhatia merged PR #396:
URL: https://github.com/apache/ossie/pull/396


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org