Add Lightdash converter#253
Conversation
Bidirectional converter between Ossie documents and Lightdash semantic definitions (dbt schema.yml meta blocks): datasets/fields/metrics/ relationships map to models/columns/meta.metrics/meta.joins, and Lightdash presentation attributes round-trip through custom_extensions with vendor_name "lightdash". Follows the apache-ossie-<vendor> uv packaging layout and ships tests off the TPC-DS example, including a structural round-trip.
…elity
- Add converter-lightdash-ci.yml mirroring the other converters (tests
were otherwise not exercised by CI)
- Rewrite ${other_table.column} references into cross-dataset
references on import, not just ${TABLE}.column
- Keep inexpressible metric types (percentile) in the extension even
when the metric carries custom SQL, on both column-level and
model-level metrics
- Drop the redundant per-converter .gitignore (covered by the root
one) and unused helpers; document the not-yet-carried model-level
meta keys in the README
There was a problem hiding this comment.
Pull request overview
Adds a new Python-based converter package (apache-ossie-lightdash) that enables bidirectional translation between Ossie OSI documents and Lightdash’s dbt schema.yml-embedded semantic definitions (meta.dimension, meta.metrics, meta.joins), including a CLI and round-trip tests.
Changes:
- Introduces
osi_to_lightdashandlightdash_to_osiconverters plus shared SQL/aggregation rewriting utilities. - Adds a CLI (
ossie-lightdash) for export/import workflows and a structured issue-reporting model (ConverterIssueType). - Adds a Lightdash-focused test suite including a structural TPC-DS round-trip.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| converters/lightdash/uv.lock | Adds pinned dependency lockfile for the new converter package. |
| converters/lightdash/pyproject.toml | Defines the apache-ossie-lightdash package, dependencies, and CLI entry point. |
| converters/lightdash/.gitignore | Ignores local venv and Python cache artifacts for the new converter. |
| converters/lightdash/README.md | Documents mapping rules, usage, and limitations for Lightdash conversion. |
| converters/lightdash/src/ossie_lightdash/init.py | Exposes the converter public API from the package root. |
| converters/lightdash/src/ossie_lightdash/cli.py | Implements ossie-lightdash import/export commands and output formatting. |
| converters/lightdash/src/ossie_lightdash/converter_issues.py | Adds typed conversion-issue reporting used by both directions. |
| converters/lightdash/src/ossie_lightdash/expression_utils.py | Provides SQL rewriting and simple-aggregation parsing/building helpers. |
| converters/lightdash/src/ossie_lightdash/osi_to_lightdash.py | Implements OSI → Lightdash dbt schema.yml export (dimensions/metrics/joins). |
| converters/lightdash/src/ossie_lightdash/lightdash_to_osi.py | Implements Lightdash dbt schema.yml → OSI import (datasets/fields/metrics/relationships). |
| converters/lightdash/tests/init.py | Establishes the test package with ASF header. |
| converters/lightdash/tests/test_osi_to_lightdash.py | Unit tests for OSI → Lightdash mapping (dimensions/metrics/joins/issues). |
| converters/lightdash/tests/test_lightdash_to_osi.py | Unit tests for Lightdash → OSI mapping (sources, metrics, joins, issues). |
| converters/lightdash/tests/test_tpcds_roundtrip.py | End-to-end structural round-trip test using the in-repo TPC-DS example. |
Comments suppressed due to low confidence (1)
converters/lightdash/src/ossie_lightdash/osi_to_lightdash.py:205
definition.update(extension_data)allows vendor extension keys (e.g.sql,description, ortype) to override the OSI-derived structural fields. This can let a stale extension value win over the OSI expression/description, which the PR description explicitly aims to prevent.
definition["type"] = extension_data.get("type", "number")
definition["sql"] = osi_sql_to_lightdash(expression, target_dataset)
target_column = None
definition.update(extension_data)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| expression = _pick_expression(field.expression, self._dialect) | ||
| if expression and expression != field.name: | ||
| dimension["sql"] = osi_sql_to_lightdash(expression, dataset.name) | ||
| dimension.update(_lightdash_extension_data(field, issues)) |
There was a problem hiding this comment.
Good catch — fixed in 80c775d. The export overlay now drops structural keys (sql/label on dimensions, sql/description on metrics) so a hand-authored extension can never override the OSI-derived definition; type on metrics deliberately stays overridable as the documented channel for types OSI expressions can't express (percentile). Regression test added.
| sql_on = " AND ".join( | ||
| f"${{{from_model_name}.{from_column}}} = ${{{to_model_name}.{to_column}}}" | ||
| for from_column, to_column in zip( | ||
| relationship.from_columns, relationship.to_columns | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Fixed in 80c775d — mismatched from_columns/to_columns now skip the relationship with a RELATIONSHIP_COLUMNS_MISMATCHED issue instead of silently truncating. I went with the issue-reporting pattern (rather than raising like the Snowflake converter) to stay consistent with this converter's ConverterResult contract, where malformed elements degrade gracefully with a report — happy to switch to a hard error if reviewers prefer the Snowflake behavior. Regression test added.
| - **Cross-dataset metrics are dropped on export** (with a | ||
| `CROSS_DATASET_METRIC_DROPPED` issue): a Lightdash model metric cannot | ||
| reference other tables. | ||
| - **Percentile metrics** keep `type` / `percentile` in the `lightdash` | ||
| extension (Ossie expressions cannot express them faithfully) and re-export | ||
| as model-level metrics. |
There was a problem hiding this comment.
Agreed — documented in 80c775d as a known limitation: dataset names are not preserved when they differ from the source table name (references inside expressions and relationships are rewritten consistently, but name-stable round-trips are not guaranteed).
…ip columns - Drop structural keys (sql/label on dimensions, sql/description on metrics) from the extension overlay on export, so a hand-authored extension can never override the OSI-derived definition; metric type stays overridable as the documented channel for inexpressible types - Skip relationships whose from_columns/to_columns lengths differ, reporting RELATIONSHIP_COLUMNS_MISMATCHED instead of silently truncating the join - Document that dataset names are not preserved when they differ from the source table name
| if extension.vendor_name == LIGHTDASH_VENDOR_NAME: | ||
| try: | ||
| data.update(json.loads(extension.data)) | ||
| except (TypeError, ValueError): | ||
| pass |
There was a problem hiding this comment.
Fixed in bb09df3 — unparseable extension data now reports an EXTENSION_DATA_INVALID issue, with a regression test.
| def _convert_sql_metric( | ||
| self, metric_name: str, definition: Dict[str, Any], *, dataset_name: str | ||
| ) -> OSIMetric: | ||
| sql = definition.get("sql") or "" | ||
| expression = lightdash_sql_to_osi(sql, dataset_name) | ||
| return self._build_metric( |
There was a problem hiding this comment.
Fixed in bb09df3 — model-level metrics without sql are skipped with a METRIC_SQL_MISSING issue instead of importing an empty expression, with a regression test.
| [tool.uv.sources] | ||
| # apache-ossie is not yet published to PyPI; resolve it from the in-repo | ||
| # package for now. Remove this block once apache-ossie published to PyPI. | ||
| apache-ossie = { path = "../../python", editable = true} |
There was a problem hiding this comment.
Fixed in bb09df3 — added the [tool.uv] block mirroring the other converters, and verified from a clean checkout that uv sync + uv run pytest passes (27 tests).
… model metrics, add [tool.uv] - Surface unparseable lightdash extension data as an EXTENSION_DATA_INVALID issue instead of silently dropping it - Skip model-level metrics without sql on import, reporting METRIC_SQL_MISSING rather than emitting an empty OSI expression - Add the [tool.uv] block (required-version, default-groups) used by the other converters so uv sync reliably installs the dev group in CI
Resolves #227 (proposed there and welcomed by @khush-bhatia — thank you!)
What
Bidirectional converter between Ossie documents and Lightdash semantic definitions, which live as
metablocks in dbtschema.ymlfiles.osi_to_lightdash): Ossie → Lightdash-flavouredschema.ymldict (columns withmeta.dimension, column/modelmeta.metrics,meta.joins)lightdash_to_osi): existing Lightdash definitions → Ossie, as a migration path into Ossielabel,format,round,group_label,hidden,percentile, ...) round-trip throughcustom_extensionswith a stablevendor_name: "lightdash"(JSON-stringdata, per the model contract). Structural keys (sql/type/description) are deliberately excluded from the extension so a stale copy can never win over the OSI expression on export.meta.joins(sql_onis built from / parsed back into column pairs).Conventions
Follows the layout being standardized on dev@: package
apache-ossie-lightdash, uv packaging (mirroringconverters/dbt),osi_to_lightdash.py/lightdash_to_osi.pymodule naming,ossie-lightdashCLI entry point, ASF headers throughout.Verification
CROSS_DATASET_METRIC_DROPPEDissue, as documented — a Lightdash model metric cannot reference other tables)ossie-lightdash export examples/tpcds_semantic_model.yaml ...→importback reproduces 5 datasets / 3 single-dataset metrics / 4 relationshipsNotes
primary_key/ai_contextnot carried, standalone Lightdash YAML mode out of scope for now).--databaseso one document stays valid across environments ([Feature] Allow database-less OSI dataset sources (schema.table) to bind by schema + alias, so one OSI document works across environments dbt-labs/dbt-core#15649).