Add OrionBelt (OBML) converter - #153
Conversation
jbonofre
left a comment
jbonofre
left a comment
There was a problem hiding this comment.
LGTM, I just left some comments about the fact we are loosing part of the model during convertion (whereas the README implies not 😄 ).
|
Thanks for the careful review, @jbonofre - you were right, and the "we lose part of the model on import" catch was a good one. The root cause was a test blind spot: our corpus only ever round-tripped our own output, which always emits Pushed a fix to this branch that turns both drop paths into preserve-and-warn, plus a bit more:
Two follow-on edge cases got hardened while in here: preserved metrics now contribute their dialects/vendors to the top-level v0.2 Docs updated to scope the fidelity claim, and new regression tests cover both former drop paths, the round trip, the metadata arrays, the name collision, and idempotency (134 passing). Thanks again! |
|
Follow-up: aligned this branch with #148 (drop non-spec root While addressing the metric-loss feedback I noticed the OrionBelt converter was emitting root-level The converter now emits neither root array. Dialects stay per-expression in Note this supersedes the first inline finding above (advertise restored metrics in the top-level This also keeps us pointed at the single-document-dialect direction in #52. 135 tests passing. |
Bidirectional OSI <-> OrionBelt OBML converter, plus a ROADMAP entry. OSI -> OBML and OBML -> OSI with roundtrip fidelity via vendor custom_extensions. Highlights from review on this PR: - Reads ANSI_SQL, SNOWFLAKE, and DATABRICKS metric expressions (their aggregations are ANSI-compatible); MDX/TABLEAU/MAQL are not parsed as SQL. - Resolves dataset.column references against the OSI dataset/field map case-insensitively and with SQL quoting stripped; decimal literals are kept literal. Unmappable references preserve the metric verbatim. - Metrics with no OBML representation are preserved verbatim under an OSI-vendor customExtension and re-emitted on OBML -> OSI, with a loud LOSSY: warning, so the OSI -> OBML -> OSI roundtrip stays lossless. - Emits schema-conformant OSI: no root-level dialects/vendors (those live per-expression and per-entity), matching the published core schema. - Name-collision safety and idempotent convert() on both directions.
35faca4 to
b1bf415
Compare
Sync the vendored OBML JSON Schema with the canonical OrionBelt schema: remove the snake_case duplicate properties (max_staleness, intent_tags) so the contract is camelCase-only. Converter test suite green (141 passed).
Sync the vendored converter with the upstream OrionBelt package, which was decomposed from a single 2967-line converter.py into a thin facade plus focused modules. Public API (osi_orionbelt.converter.*) and behavior are unchanged; the converter test suite passes (141). - converter.py: now a facade re-exporting every public name + main() - _common.py: shared constants / type maps - osi_to_obml.py: OSItoOBML (OSI -> OBML) - obml_to_osi.py: OBMLtoOSI (OBML -> OSI) - ontology.py: OBMLtoOSIOntology - validation.py: ValidationResult + validate_obml/osi/osi_ontology
|
Hi @jbonofre, gentle nudge on this one. Since your approval the diff has grown a fair bit in response to review, so I'd value a fresh look before merge:
The PR is @khush-bhatia flagging for merge once the re-approval lands. Thanks both! |
Mirrors the canonical converter (packages/osi-orionbelt in the orionbelt-semantic-layer repo): the OBML count-synthesis knobs (countable, countLabel on a dataObject; exposeCounts, countLabelPattern on the model) now roundtrip via the ORIONBELT vendor custom_extensions. The auto-synthesized count measures themselves are derived on read and are never exported, so there is no OSI spec or dataset/metric change. - obml_to_osi.py: emit the four knobs into the ORIONBELT extension. - osi_to_obml.py: restore them (is-not-None guards so exposeCounts:false and countable:false survive). - schemas/obml-schema.json: add the four fields (vendored snapshot). - tests: TestCountSynthesisKnobs roundtrip + no-leak coverage.
# Conflicts: # ROADMAP.md
…ng consistency Align with the OSI->Ossie incubation rename applied to the other converters: package dir osi_orionbelt -> ossie_orionbelt, dist name apache-ossie-orionbelt, CLI ossie-orionbelt. Keeps osi in spec-format file/class names (osi_to_obml.py, OSItoOBML) as upstream did for dbt/gooddata.
|
Rebased on latest I also renamed the converter to match the incubation naming used by the other converters (dbt, gooddata): package |
| { | ||
| "$schema": "https://json-schema.org/draft/2020-12/schema", | ||
| "$id": "https://github.com/open-semantic-interchange/OSI/core-spec/osi-schema.json", | ||
| "title": "OSI Core Metadata Specification", | ||
| "description": "JSON Schema for validating OSI (Open Semantic Interoperability) semantic model definitions", |
There was a problem hiding this comment.
Why make a copy of the schema ? This becomes one more file that needs to be updated for any changes to the spec.
For existing converters, we are planning to consolidate the java/python and other language specific bindings to be generated from the spec and remove the duplication across different language bindings.
There was a problem hiding this comment.
Good point, thanks. Rather than keep a copy, the converter now resolves the OSI core schema via a search path (validation._osi_schema_path): a vendored copy if present, otherwise core-spec/osi-schema.json from the enclosing checkout. I've deleted the vendored osi-schema.json, so OSI-core validation now links the single canonical core-spec/ copy with nothing to keep in sync. I also dropped the vendored osi-ontology-schema.json and made validate_osi_ontology semantic-only, since ossie ships no ontology schema and the converter emits ontology documents but never consumes them. This lines up with your plan to generate the bindings from the spec. Pushed.
| r"\s*\.\s*" | ||
| r'(?P<col>[A-Za-z_]\w*|"[^"]+"|`[^`]+`|\[[^\]]+\])' | ||
| ) | ||
| _OSI_KNOWN_VENDORS = ( |
There was a problem hiding this comment.
Where is this used ? Trying to understand the need for maintaining a list of known vendors within the scope of a converter
There was a problem hiding this comment.
You're right, that list didn't belong here. It was dead: only re-exported, never used. The converter preserves any vendor outside its own internal set (_INTERNAL_VENDORS) verbatim in both directions, so there's nothing to enumerate. Removed both _OSI_KNOWN_VENDORS and the equally unused _OSI_KNOWN_DIALECTS. Pushed.
Addresses two review comments on apache/ossie#153: 1. Resolve the OSI core + ontology schemas via a search path instead of a hard-coded vendored path (validation.py::_osi_schema_path): prefer a copy vendored beside the package (needed by the self-contained PyPI wheel and the product /v1/convert API, which validates OSI input against it), otherwise fall back to core-spec/<name> in an enclosing Ossie monorepo checkout. This lets the in-tree converter drop its vendored copy and link the single canonical schema. No behaviour change here (the copy is still present), so OSI schema validation and its tests keep passing. 2. Remove the unused _OSI_KNOWN_VENDORS and _OSI_KNOWN_DIALECTS constants (only re-exported, never used; vendor passthrough keys off _INTERNAL_VENDORS, dialect parsing off _SQL_PARSEABLE_DIALECTS). 145 package tests plus the /v1/convert input-validation integration tests pass; ruff and mypy clean.
…ead constants Addresses review feedback on this PR: 1. Stop vendoring a copy of the OSI spec. validation._osi_schema_path resolves the OSI core schema from a search path: a copy vendored beside the package if present, otherwise core-spec/osi-schema.json from the enclosing checkout. The vendored osi-schema.json is deleted, so OSI-core validation now links the single canonical core-spec copy with nothing to keep in sync. 2. Omit ontology-schema validation. ossie ships no OSI ontology schema and the converter emits ontology documents but never consumes them, so the vendored osi-ontology-schema.json is removed and validate_osi_ontology runs semantic checks only (unique concepts, reference integrity). 3. Remove the unused _OSI_KNOWN_VENDORS and _OSI_KNOWN_DIALECTS constants (only re-exported; passthrough uses _INTERNAL_VENDORS, dialect parsing uses _SQL_PARSEABLE_DIALECTS). Only the OBML schema (OrionBelt's own format) stays vendored. mypy override added for the optional orionbelt engine import. 145 tests pass; ruff clean; mypy clean with the dev extra.
…spec The vendored osi-schema.json copy was removed so the source tree carries no duplicate of the spec. But a pip-installed wheel has no core-spec/ alongside it, so validate_osi would silently skip schema validation. force-include the single canonical core-spec/osi-schema.json into the built wheel: no tracked duplicate, yet a pip install still validates OSI documents self-contained. In-tree runs still resolve it from core-spec/ via validation._osi_schema_path.
…200) Addresses two review comments on apache/ossie#153: 1. Resolve the OSI core + ontology schemas via a search path instead of a hard-coded vendored path (validation.py::_osi_schema_path): prefer a copy vendored beside the package (needed by the self-contained PyPI wheel and the product /v1/convert API, which validates OSI input against it), otherwise fall back to core-spec/<name> in an enclosing Ossie monorepo checkout. This lets the in-tree converter drop its vendored copy and link the single canonical schema. No behaviour change here (the copy is still present), so OSI schema validation and its tests keep passing. 2. Remove the unused _OSI_KNOWN_VENDORS and _OSI_KNOWN_DIALECTS constants (only re-exported, never used; vendor passthrough keys off _INTERNAL_VENDORS, dialect parsing off _SQL_PARSEABLE_DIALECTS). 145 package tests plus the /v1/convert input-validation integration tests pass; ruff and mypy clean.
…ess (#201) (#202) * osi-orionbelt: fix converter round-trip fidelity + validation robustness (#201) Fixes the three pre-existing converter bugs found during the apache/ossie#153 review: - P1 metric round-trip: OSItoOBML now resolves metric references by BOTH the OSI dataset/field name and the physical code (source-table code and bare-identifier field expression). The OBMLtoOSI emitter writes metric SQL against the physical code (e.g. SUM(fact_orders.amount)), so resolving names only dropped such metrics on the return trip when a data object's code differed from its display name. - P2 dimension collision: _extract_dimensions no longer keys dimensions by bare field name. When the same field name occurs in more than one dataset (Orders.date, Invoices.date) the later one is qualified with its data object and a warning is recorded, instead of silently overwriting the first. - P2 validation robustness: validate_osi guards its semantic loops against malformed structures (datasets/fields that are not lists of dicts) so it returns schema errors instead of raising AttributeError. Adds tests/test_osi_converter_roundtrip_robustness.py (7 regression tests). 152 package tests pass; ruff and mypy clean; API convert integration tests pass. * osi-orionbelt: make metric ref resolution space-safe (review fix) Addresses a review finding on the P1 fix: resolving a physical code to a display field name containing a space (e.g. net_amount -> 'Net Amount') spliced 'Orders.Net Amount' into the intermediate SQL string, and the downstream \w+.\w+ parsers split it on the space, emitting invalid OBML ('{[Orders].[Net]} Amount') with no LOSSY warning. _resolve_column_refs now bracket-quotes any resolved name that is not a bare SQL word, and the dataset.column parsers (_parse_simple_agg, _parse_expr_agg, _decompose_complex_metric, _sql_refs_to_obml) accept bracketed identifiers and unquote them. A metric over a spaced display-name field now converts to a valid single-column measure (column 'Net Amount') instead of dangling. Adds a regression test for a physical code mapped to a display field with a space. 153 package tests pass; ruff and mypy clean. * osi-orionbelt: resolve quoted physical identifiers in metric refs (review P3) Unquote single-identifier field expressions and source table codes before indexing them for metric-reference resolution. A Snowflake/Databricks quoted physical column (field expression "net_amount") or quoted source table (WH.PUBLIC."fact_orders") referenced by its bare code now converts to a queryable measure instead of being preserved as a LOSSY unconverted metric. Adds a regression test for quoted source table + field expression. 154 package tests pass; ruff and mypy clean.
3 participants
Summary
Adds a bidirectional converter between OSI and OrionBelt's OBML (OrionBelt Markup Language) under
converters/orionbelt/, following the hub-and-spoke guidance inconverters/index.md.ORIONBELTvendor incustom_extensionsso nothing is dropped.ai_context) via anOSIvendor stash, so a full OSI to OBML to OSI roundtrip is lossless. Third-party vendor extensions (SNOWFLAKE,DBT,SALESFORCE,GOODDATA, ...) are preserved verbatim at every level.Layout
Mirrors the existing Python converters (
gooddata,dbt):src/osi_orionbelt/(converter, CLI, vendored OSI/OBML/ontology JSON schemas for self-contained builds)tests/with 120 tests, including a TPC-DS baseline againstexamples/tpcds_semantic_model.yaml(byte-identical fixture)pyproject.toml,README.md, and OBML <-> OSI mapping analysis docsCLI (
osi-orionbelt) exposesobml-to-osiandosi-to-obmlsubcommands, mirroringosi-dbt.Spec impact
None.
vendor_nameis free-form in the core spec, so theORIONBELTvendor tag needs no schema or specification change. This is a converter-only (non-spec) contribution.Notes
The canonical converter source is maintained in the orionbelt-semantic-layer repository (
packages/osi-orionbelt) and published to PyPI asosi-orionbelt(Apache-2.0). Documented limitations (many-to-many joins, named secondary join paths, ontology-layer value concepts) are carried losslessly incustom_extensionsand listed in the README.