Add bidirectional Apache Ossie <-> Cube converter - #289
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new converters/cube/ Python converter that round-trips between Apache Ossie semantic models and Cube YAML data models, including a CLI and extensive tests/fixtures, and registers CUBE in the supported-vendors list.
Changes:
- Introduces bidirectional conversion logic (
convert_cube_to_ossie/convert_ossie_to_cube) with stash/parking mechanisms to preserve unmapped constructs and enable lossless round-trips. - Adds a full test suite (fixtures, property-based tests with Hypothesis fallback, CLI tests) plus Cube converter documentation and packaging (
pyproject.toml). - Adds a dedicated GitHub Actions workflow to run Cube converter tests in CI and updates
converters/README.mdto listCUBE.
Reviewed changes
Copilot reviewed 28 out of 29 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| converters/README.md | Registers CUBE as a supported vendor extension. |
| converters/cube/src/ossie_cube/_common.py | Shared conversion utilities (YAML handling, stash protocol, expression translation, mappings). |
| converters/cube/src/ossie_cube/cube_to_osi.py | Cube → Ossie conversion implementation (datasets/fields/relationships/metrics + preservation). |
| converters/cube/src/ossie_cube/osi_to_cube.py | Ossie → Cube export implementation (layout, meta parking, joins/measures/view generation). |
| converters/cube/src/ossie_cube/converter_issues.py | Structured issue types + issue log for lossy/unsafe conversions. |
| converters/cube/src/ossie_cube/cli.py | ossie-cube CLI: import/export behavior, IO, error reporting. |
| converters/cube/src/ossie_cube/init.py | Public API surface for the converter package. |
| converters/cube/README.md | Converter documentation: mapping table, fan-out semantics, usage, limitations. |
| converters/cube/pyproject.toml | Packaging + dev dependencies/test config for the converter. |
| converters/cube/tests/** | Comprehensive unit, fixture round-trip, property-based, edge-case, and CLI tests. |
| converters/cube/tests/fixtures/** | Cube model fixtures used for round-trip and baseline tests (including TPC-DS). |
| .github/workflows/converter-cube-ci.yml | CI workflow for the Cube converter test suite. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Both from the Copilot review on apache#289. Dimension names were sanitized separately in _convert_model (to decide which members a cube has) and again in _build_dimensions (to name them). The first pass used a fresh `taken` set per field, so a collision was silently swallowed by a set comprehension there and only rejected later in the second pass -- meaning the member set that decides `{CUBE.member}` vs `{CUBE}.column`, and where a measure lands, could be short a name while measures were being placed. Demonstrated: "Order Status" and "order status" collapsed to one name with no error. Now resolved once in _resolve_dimension_names and reused. That also fixes a defect the review did not mention: the old set included the two halves of a split geo dimension (location_latitude, location_longitude), which never exist as Cube dimensions since they merge back into `location`, so a metric referencing one would emit an unresolvable `{CUBE.location_…}`. The halves now resolve to the dimension they merge into. _HypothesisRnd.chance() ignored its `p` argument and always drew an unweighted boolean, so the Hypothesis driver explored a different distribution than the seeded one despite the docstring claiming they share a generator. Now weighted, and drawn so the minimal value means False -- shrinking toward the smallest model rather than the largest. 231 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Both addressed in d362c9e.
Names are now resolved once in This also fixed something not mentioned: the old set included both halves of a split
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 32 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
converters/cube/src/ossie_cube/osi_to_cube.py:721
- Using
queue.pop(0)makes this BFS O(n²) due to repeated list shifting; on larger relationship graphs this can become unnecessarily slow. Use an index cursor (or a deque) to avoid O(n) pops from the front.
queue = [base]
while queue:
current = queue.pop(0)
for neighbor in adjacency.get(current, []):
if neighbor in paths:
continue
paths[neighbor] = f"{paths[current]}.{neighbor}"
entries.append({"join_path": paths[neighbor], "includes": "*"})
queue.append(neighbor)
converters/cube/tests/test_roundtrip.py:42
- Typo: "licence" is misspelled here (the rest of the repo uses "license").
licence headers on the fixtures) are not part of the data model, and key order
Scaffolds converters/cube/ following the osi-omni and osi-databricks
converters: a pure offline YAML transform with no Cube deployment, API
token, or network access required.
This commit lands the import direction (Cube -> Ossie). Cubes become
datasets, cube joins become relationships, cube measures are hoisted to
model-level metrics, and the mapped view supplies the model's name,
description, and AI context -- the view is the model boundary because
Cube users are view-first and Cube's agent reads meta.ai_context only
from views and members, not cubes.
Design decisions worth calling out:
- Fan-out. Cube corrects row multiplication at query time by
deduplicating on declared primary keys, which a static Ossie
expression cannot inherit. So a bare `type: count` maps to
COUNT(DISTINCT <pk>) -- exactly equal to both forms Cube renders, and
correct in every join context -- and a non-idempotent aggregate on a
dataset the graph fans out is refused by default, mirroring Cube's own
refusal. --no-strict-fanout downgrades it to a recorded issue.
- Calculated measures inline their {other_measure} references, because
that is what Cube itself does; Ossie has no metric-to-metric
reference. Cycles are rejected.
- Measure filters fold into CASE WHEN ... END inside the aggregate,
matching Cube's own applyMeasureFilters rendering.
- Dimension `type: number` omits Ossie `datatype` rather than assert a
precision the model does not carry; `type: geo` splits into two
fields since an Ossie field holds one expression.
- Losses that cannot be avoided surface as structured ConverterIssues
rather than bare warnings, following the osi-dbt converter, so a
pipeline can gate on them.
Jinja-templated YAML, .js/.ts models, and `extends` are refused or
preserved verbatim rather than half-converted. Everything Cube-only
round-trips through custom_extensions[CUBE].
49 tests pass; the fixture output validates against core-spec/osi-schema.json
via validation/validate.py.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both directions are now implemented and losslessly round-trip.
Export emits one `model/cubes/<name>.yml` per dataset plus a
`model/views/<name>.yml` for the model itself -- the view is always
emitted, not optional, because it is the model boundary for view-first
Cube users. A model imported from Cube restores its original file paths,
view curation, segments, pre-aggregations, hierarchies, and every other
Cube-only construct from the stash; a hand-authored Ossie model gets a
view generated at the FK sink with each cube addressed by its join path.
Because Cube has a `meta` field at every level, Ossie constructs Cube has
no slot for (`unique_keys`, foreign-vendor `custom_extensions`, the
structured form of `ai_context`) are parked under `meta.ossie` instead of
being dropped. So Ossie -> Cube -> Ossie is lossless too, not just
Cube -> Ossie -> Cube.
Reference forms follow Cube's semantics rather than being uniform:
`{CUBE.member}` when the dataset declares a field of that name (reusing
the member's SQL, compile-time checked), `{CUBE}.column` for a raw
physical column, and `{other_cube.member}` across cubes -- which is also
what gives a cross-dataset metric its implicit join. The cube's own name
is never spelled out, so the model survives `extends`.
COUNT(DISTINCT <primary key>) converts back to Cube's bare `type: count`,
closing the loop on the fan-out mapping in both directions.
Tests (160): per-direction unit tests, fixture round-trips including a
TPC-DS model generated from examples/tpcds_semantic_model.yaml as the
guide asks, core-spec JSON Schema validation of every emitted Ossie
document, and Hypothesis property-based round-trips over generated Cube
models with a seeded fallback when hypothesis is unavailable.
The property tests found two real defects: a `{{` anywhere in a file
disqualifies it as Jinja (matching Cube's own file-level check), which
could leave a join pointing at a cube that was never converted -- the
error now names the skipped file instead of just reporting a missing
cube.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Coverage was 91% with several load-bearing branches never executed. Adds test_edge_cases.py (45 tests) for the paths the fixtures and property tests cannot reach, since those generate inside the round-trippable subset by design: - composite primary keys -- the COUNT(DISTINCT CONCAT(CAST(...))) form is central to the fan-out mapping and was never run in either direction; - `count` with `sql` (COUNT(x)), including that it is fan-out-unsafe where a bare count is not; - the export side of the one_to_many flip -- import flipping it was tested, export flipping it back was not; - off-layout file grouping: several cubes in one oddly-named file have to return to that same file, not be split into the canonical layout; - the JavaScript-style mapping form of dimensions/measures/joins; - legacy belongsTo/hasMany/hasOne spellings; - unconvertible joins restored at their original positions; - geo dimension extras, measure `title`, `ai_context.examples`, a bare string `ai_context`, multiple views, and the malformed-input errors. Two dead paths removed, both found by the same pass: - member-level Jinja handling was unreachable. JINJA_RE is checked per file (as Cube's own CubeSchemaConverter does), so a templated member's file never reaches the per-member branch. Renamed the issue type TEMPLATED_MEMBER_DROPPED -> TEMPLATED_FILE_SKIPPED to match what it actually reports, and dropped the matching `extra_dimensions` restore. - `is_cube_name` was never called. Coverage 91% -> 96%; the remaining 40 lines are defensive ConversionError branches on malformed input. 201 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Answering "what happens if I pass only a view file?" turned up three things. The behavior was right -- a Cube view projects members from cubes and defines none, so it cannot become an Ossie model on its own and the conversion is refused. But the message said "no convertible cubes found", which reads as if the file was not recognized at all. It now says a view was found, explains why that is not enough, and names the cubes its join_paths reference so the user knows which files to add. Pointing `-i` at a single `.yml` was refused as "not a directory". There is nothing ambiguous about a single model file, so it is now accepted. And cli.py had no tests at all -- 0% coverage. Adds test_cli.py (15 tests) covering the input shapes people reach for first (directory, single file, view-only), that stdout stays pipeable while issues go to stderr, that a fan-out refusal exits non-zero and --no-strict-fanout downgrades it, that node_modules and dotfiles are skipped, and a CLI round trip that reproduces the TPC-DS fixture. 216 tests; coverage 96% -> 97%, cli.py 0% -> 99%. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Cube itself has a single model root (`CUBEJS_SCHEMA_PATH` is one string, default `model`), so pointing at that root is the idiomatic whole-project case and the recursive walk already handles files spread across subdirectories under it. But *requiring* one path was friction: converting two cubes out of fifty, or files that live in separate trees, meant assembling a directory first just to satisfy the CLI. `-i` now takes any number of files and/or directories, so globs work too. Files are keyed relative to the deepest directory containing every input, because those keys decide where export writes them back. That generalization is deliberately behavior-preserving: one directory anchors to itself and one file to its own directory, so existing round trips key exactly as before. Two files from `cubes/` and `views/` key as `cubes/orders.yml` and `views/sales.yml`, and export reproduces that tree. Overlapping inputs (a directory plus a file inside it) are now an error rather than reading the same file twice. Verified on a real Cube model passed as two explicit file paths: round trip is content-identical with the original filenames preserved, the Ossie output passes validation/validate.py, and the fan-out guard correctly refuses both measures on the joined cube's `one` side. 226 tests, 96% line coverage. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The converter emits vendor_name: CUBE in custom_extensions, so it belongs in the table converters/README.md keeps of vendors with defined extensions. Deliberately not touching the parallel list in core-spec/spec.md: vendor_name is a free-form string, so no spec change is needed, and edits under core-spec/ carry the heavier review process. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every other bidirectional converter in the repo pairs a vendor fixture with an Ossie one (databricks, omni, gooddata, orionbelt); this converter was the only one keeping its Ossie inputs as inline Python strings. Adds fixtureA_ossie.yaml and tpcds_ossie.yaml, and moves the hand-authored Ossie model out of test_roundtrip.py into hand_authored_ossie.yaml. The point is not just tidiness. Following the databricks pattern, the fixtures are asserted as whole-document snapshots in both directions: import must reproduce the Ossie fixture, and exporting that fixture must reproduce the Cube fixture. Field-level assertions cannot see an unintended change elsewhere in the document; a snapshot shows it as a readable diff. Each fixture carries the command to regenerate it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both from the Copilot review on apache#289. Dimension names were sanitized separately in _convert_model (to decide which members a cube has) and again in _build_dimensions (to name them). The first pass used a fresh `taken` set per field, so a collision was silently swallowed by a set comprehension there and only rejected later in the second pass -- meaning the member set that decides `{CUBE.member}` vs `{CUBE}.column`, and where a measure lands, could be short a name while measures were being placed. Demonstrated: "Order Status" and "order status" collapsed to one name with no error. Now resolved once in _resolve_dimension_names and reused. That also fixes a defect the review did not mention: the old set included the two halves of a split geo dimension (location_latitude, location_longitude), which never exist as Cube dimensions since they merge back into `location`, so a metric referencing one would emit an unresolvable `{CUBE.location_…}`. The halves now resolve to the dimension they merge into. _HypothesisRnd.chance() ignored its `p` argument and always drew an unweighted boolean, so the Hypothesis driver explored a different distribution than the seeded one despite the docstring claiming they share a generator. Now weighted, and drawn so the minimal value means False -- shrinking toward the smallest model rather than the largest. 231 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A Cube `type: geo` dimension holds two SQL expressions where an Ossie
field holds one, so import splits it into `<name>_latitude` /
`<name>_longitude`. Export merges them back, so the round trip was already
exact.
But those half names exist only in Ossie. Cube has neither a column nor a
member called `home_latitude` -- the halves merge into `home` -- so a
metric or field expression referencing one had nothing valid to emit:
AVG(users.home_latitude) -> sql: '{CUBE}.home_latitude'
which names a column that does not exist (the column is `lat`). The
member-reference form would have been just as wrong, failing at Cube
compile time instead of in the database.
The half's real SQL is already in the stash, so a reference to one is now
replaced by that SQL:
AVG(users.home_latitude) -> AVG({CUBE}.lat)
AVG(users.home_latitude) - MIN(orders.amt) -> AVG({users}.lat) - MIN({CUBE.amt})
`{CUBE}` means "the cube this is declared on", so an inlined snippet is
requalified to name its original cube when it crosses into another cube's
SQL -- otherwise it would silently rebind to the wrong cube.
One normalization follows and is documented: after a round trip such a
metric names the column the half actually reads (`users.lat`) rather than
the Ossie-only field name. Same reference, and the only form Cube can
express.
234 tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both from the Copilot review on apache#289. The BFS popped from the front of a list, which is O(n) per pop; a deque makes it O(1). Semantic models are small enough that this was never going to matter in practice, but the deque is also the more idiomatic form. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
508c9f5 to
7d981c9
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 32 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
converters/cube/src/ossie_cube/osi_to_cube.py:685
- When the model has foreign-vendor
custom_extensionsbut the imported Cube model had multiple views and none was selected (mapped_viewis missing), export currently drops those extensions while loggingPARKED_IN_META. This causes avoidable data loss and the issue type/message is inconsistent ("parked" vs "dropped"). Prefer parking the extensions on a deterministic view (or failing fast) so Ossie -> Cube stays lossless even in the "no mapped view" case.
if foreign and mapped is None:
issues.add(IssueType.PARKED_IN_META, "model",
"no mapped view to park foreign-vendor custom_extensions on; "
"they have no Cube home and are dropped")
converters/cube/README.md:279
- The README hard-codes an exact test count ("234 tests"), but the PR description claims a different number. Since this value will drift over time, it’s better to avoid a specific count (or generate it automatically) to prevent documentation from becoming stale.
234 tests at 96% line coverage: example-based unit tests per direction, CLI
behavior tests, fixture round-trip tests (including the
Summary
Adds a bidirectional converter between Apache Ossie semantic models and Cube data models, under
converters/cube/. Pure offline YAML transform — no Cube deployment, API token, or network access required, matching the other Python converters in this repo.ossie-cube import): Cube files → Ossie. Cube-only constructs (segments, pre-aggregations, hierarchies, folders, view curation, formats, access policies, …) are preserved incustom_extensions[CUBE], so Cube → Ossie → Cube is lossless.ossie-cube export): Ossie → Cube files. Cube has ametafield at every level, so Ossie constructs Cube has no slot for (unique_keys, foreign-vendorcustom_extensions, the structured form ofai_context) are parked undermeta.ossierather than dropped — making Ossie → Cube → Ossie lossless too.The Ossie
semantic_modelmaps to a Cube view, not a cube. Cube users are view-first, and Cube's own AI agent readsmeta.ai_contextonly from views and individual members — cube-level AI context is explicitly not consumed — so the view is the natural model boundary.Fan-out semantics
Cube corrects for join row-multiplication at query time: when a cube sits on the multiplied side of a join it builds
SELECT DISTINCT <primary key> FROM <join>, joins that key set back to the measure's own cube, and aggregates there, so each source row is counted once. A static Ossie expression has no way to inherit that. Cube also refuses outright when the measures themselves span cubes that fan out.So the converter emits the fan-out-safe form wherever one exists, and refuses to emit a silently-wrong one:
countCOUNT(DISTINCT <pk>)count(pk)normally andcount(distinct pk)when multiplied;COUNT(DISTINCT pk)equals bothcount_distinct/count_distinct_approxCOUNT(DISTINCT x)/APPROX_COUNT_DISTINCT(x)min/maxMIN(x)/MAX(x)sum,avg,count+sqlSUM(x),AVG(x),COUNT(x)Only the last row is at risk, and only when its cube is the
to(one) side of a relationship in the model. That is computable from the Ossie graph, and the converter refuses by default — mirroring Cube's own refusal.--no-strict-fanoutdowngrades it to a structured issue naming the metric, dataset, and responsible relationship.This points at a spec gap. Ossie has no additivity or grain declaration to record non-additivity properly. dbt's
non_additive_dimensionis the nearest precedent, and this repo's dbt converter already loses the same information (osi_to_msi.pyhard-codesnon_additive_dimension=None, with a namedCUMULATIVE_SEMANTICS_LOSSissue type). Raised separately as #290.Other design notes
{CUBE.member}when the dataset declares a field of that name (reuses the member's SQL, compile-time checked),{CUBE}.columnfor a raw physical column, and{other_cube.member}across cubes — which is also what gives a cross-dataset Ossie metric its implicit join. The cube's own name is never emitted, so models surviveextends.{other_measure}references, because that is what Cube itself does; Ossie has no metric-to-metric reference. Cycles are rejected.filtersfold intoCASE WHEN … ENDinside the aggregate, matching Cube's ownapplyMeasureFiltersrendering and the filtered-aggregation idiom the Ossie expression language endorses.type: numberomits Ossiedatatyperather than asserting a precision the model does not carry — Cube collapses Integer/Decimal/Float into one type, and the spec says to omit when unknown. The original type is stashed.type: geodimensions split into<name>_latitude/<name>_longitude, since an Ossie field holds one expression and a geo dimension has two.Unsupported constructs
extends— resolving it means reproducing Cube's definition-merge semantics exactly, so it is refused rather than half-applied..js/.tsmodels — preserved verbatim and never half-converted. Jinja is detected per file, the same rule Cube's ownCubeSchemaConverteruses for the Rollup Designer.Losses the converter can absorb are returned as structured
ConverterIssues (following theosi-dbtconverter) rather than printed to stderr and forgotten, so a pipeline can gate on them.Testing
226 tests, 96% line coverage:
examples/tpcds_semantic_model.yamlas the converter guide asks;core-spec/osi-schema.json;hypothesisis unavailable.Also verified by hand against a real production Cube model: round trip content-identical with original filenames preserved, output passing
validation/validate.py, and the fan-out guard correctly flagging the two measures on the joined cube'soneside.Related Issues
Related to #290 (spec has no way to declare a non-additive metric).
Checklist
Specification
core-spec/changesOntology
ontology/changesConverters
CUBEregistered in the supported-vendors table inconverters/README.mdValidation
validation/changes; emitted models are validated by the existingvalidation/validate.pyDocumentation
converters/cube/README.mddocuments the full mapping, the fan-out behavior, requirements, and limitationsExamples
Tests
Compliance
jsonschemadev dependency already ships inconverters/orionbelt/AI assistance
This contribution was developed with AI assistance (Claude). I have reviewed the code and tests and take responsibility for them, per the ASF Generative Tooling Guidance.