Initial OSI <> dbt Semantic Layer converters#116
Conversation
Introduces the shared Python type library for the OSI specification, establishing it as the canonical Python representation of the core spec. All future Python converters will depend on this package rather than defining their own OSI types. Key decisions: - Pydantic v2 native (no shim): OSI has no obligation to support dbt-core's Pydantic v1/v2 compatibility constraints - Python 3.11+ minimum: clean modern syntax, no __future__ annotations - uv + hatchling: consistent with the data tooling ecosystem - PyPI name `osi-python`, import namespace `osi` - `to_osi_yaml()` added alongside `to_osi_json()` since YAML is OSI's native serialization format - OSIRelationship.from_dataset aliased to `from` (YAML/JSON key) via Field(alias="from") with populate_by_name=True so Python callers can use the non-keyword name
Migrates the dbt↔OSI converters from metricflow/metricflow/converters/ into the OSI repository as the osi-dbt package. Source changes from metricflow: - converter_issues.py: direct copy; drop `from __future__ import annotations` - expression_utils.py: direct copy - filter_utils.py: direct copy - msi_to_osi.py: update imports from `metricflow.converters.models` → `osi`; replace all `LazyFormat(...)` calls with f-strings (no metricflow dep) - osi_to_msi.py: same import updates as msi_to_osi.py - cli.py: new file; file-driven CLI for both conversion directions using `parse_manifest_from_dbt_generated_manifest` from metricflow_semantics The `osi-dbt` package depends on `osi-python` (the shared OSI types added in the previous commit) and `metricflow` directly, since `metricflow-semantic-interfaces` is not separately published on PyPI. Build tooling: uv + hatchling, matching the pattern from osi-python.
Migrates tests from tests_metricflow_semantic_interfaces/osi/ into the OSI repository. Import changes from metricflow: - `metricflow.converters.models` → `osi` - `metricflow.converters.msi_to_osi` → `osi_dbt.msi_to_osi` - `metricflow.converters.osi_to_msi` → `osi_dbt.osi_to_msi` - `tests_metricflow_semantic_interfaces.osi.helpers` → `tests.helpers` Snapshot testing: replaced metricflow's `assert_object_snapshot_equal` + `SnapshotConfiguration` with syrupy's `snapshot` fixture. Snapshot assertions compare `result.to_osi_yaml()` (a stable string representation) rather than the raw Pydantic object. Syrupy was chosen over the metricflow helper because it has no dependency on the metricflow test infrastructure and stores snapshots as plain text files in the repository. `[tool.pytest.ini_options]` added to pyproject.toml to point pytest at the `tests/` directory.
Covers installation, CLI usage for both conversion directions, Python API usage, conversion loss notes (what gets dropped and why), and development setup.
b35fd90 to
3a0080b
Compare
Add Ontology & Semantic Interoperability as a top-level current effort with links to discussions apache#22, apache#101, apache#108, apache#68 and PRs apache#124, apache#125. Update existing sections with recently opened discussions, PRs, and converters: metric trees (apache#40), primary key semantics (apache#15, apache#119), reusable datasets (apache#103, apache#109), datatype/is_time reframe (PR apache#113), spatial dimension types (apache#114), default_aggregation (apache#115), positive direction (apache#41), physical metadata (apache#110), and new converter PRs for Salesforce (apache#118), dbt (apache#116), and Databricks (apache#120). Also adds CONTRIBUTING.md (apache#122) and working groups page (apache#123) to Developer Experience, and lists merged converters as existing artifacts. Made-with: Cursor
Add Ontology & Semantic Interoperability as a top-level current effort with links to discussions #22, #101, #108, #68 and PRs #124, #125. Update existing sections with recently opened discussions, PRs, and converters: metric trees (#40), primary key semantics (#15, #119), reusable datasets (#103, #109), datatype/is_time reframe (PR #113), spatial dimension types (#114), default_aggregation (#115), positive direction (#41), physical metadata (#110), and new converter PRs for Salesforce (#118), dbt (#116), and Databricks (#120). Also adds CONTRIBUTING.md (#122) and working groups page (#123) to Developer Experience, and lists merged converters as existing artifacts. Made-with: Cursor
| issues.append( | ||
| ConverterIssue(issue_type=ConverterIssueType.CUMULATIVE_SEMANTICS_LOSS, element_name=metric.name) | ||
| ) | ||
| expr = self._resolve_metric_expression(metric, metric_index, expression_cache) | ||
| osi_metrics.append( | ||
| OSIMetric( | ||
| name=metric.name, | ||
| expression=self._make_expression(expr), | ||
| description=metric.description, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
For cumulative metrics, looks like continue is missing ?
There was a problem hiding this comment.
So for cumulative metrics, we don't skip them, we just drop the extra information about them that we aren't able to represent yet. However we can still produce the basic representation, and then emit a warning about what information was dropped.
khush-bhatia
left a comment
There was a problem hiding this comment.
LGTM, thanks , just left one comment.
MAQL is present in the OSI schema's Dialect definition but was missing from the Python enum, making it impossible to represent GoodData expressions in the Python SDK.
GOODDATA appears in OSI examples but was missing from the enum, causing validation failures for any document that references GoodData extensions.
The OSI project as a whole has advanced to 0.2.0.dev0. Updates all version references across the spec, schema, examples, Python packages, and tests.
jbonofre
left a comment
There was a problem hiding this comment.
LGTM to overall. I left a couple of comments/questions, especially about is_time (to clarify).
|
|
||
| model_config = ConfigDict(frozen=True) | ||
|
|
||
| is_time: bool |
There was a problem hiding this comment.
The spec describes is_time as optional, but here it's required.
I believe we should use Optional[bool] = None here.
WDYT?
There was a problem hiding this comment.
Will make the change! Might be worth reviewing that in the spec later though 🤔 What does the third state give us if None is the same as False?
| metrics=metrics, | ||
| project_configuration=PydanticProjectConfiguration(), | ||
| ), | ||
| issues=[], |
There was a problem hiding this comment.
Correct me if I'm wrong, but ConverterIssue is never thrown here? Should we?
There was a problem hiding this comment.
Presently there are no converter issues going OSI to MSI, issues only exist going MSI to OSI currently. However, I wanted to use the same ConverterResult class to make future work where there are OSI -> MSI converter issues easier to handle without a breaking change. We could add a
if result.issues:
for issue in result.issues:
verb = "was dropped" if issue.issue_type in _DROPPED_ISSUE_TYPES else "was converted with loss"
reason = _ISSUE_REASON[issue.issue_type]
print(f"[WARNING] {issue.issue_type.value}: {issue.element_name} {verb} during conversion because {reason}", file=sys.stderr)block to _cmd_osi_to_msi in converters/dbt/src/osi_dbt/cli.py, but that would be dead code at this time.
The spec marks is_time as optional, but the Pydantic model had it as a required bool. Aligning to Optional[bool] = None so documents that omit is_time validate correctly.
The initial implementation of the OSI <> dbt Semantic Layer converters. This is initially written in python as the simplest path forward required taking MetricFlow as a dependency, though that doesn't need to be the case longer term.