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/22 01:32:44 UTC
[PR] Require complete core documents in ontology mappings [ossie]
flyrain opened a new pull request, #441:
URL: https://github.com/apache/ossie/pull/441
## Summary
Ontology maps currently reference `$defs/SemanticModel`, so their embedded models omit the document metadata required by the flat core format introduced in #383. Reference the core schema root instead, keeping the existing raw GitHub retrieval URL and the `semantic_model` property.
**Breaking change to the unreleased `0.2.0.dev0` schema:** each `ontology_mappings[*].semantic_model` must now contain its own `version: 0.2.0.dev0`. The outer ontology version does not supply it. Migration adds that field while preserving the model contents.
- Preserve the embedded version through ontology parsing and export; reject missing or unsupported versions. Newly generated Palantir output uses the current ontology and core document versions.
- Update the flights example, vendored fixture, round-trip snapshot, and migration documentation together.
- Add schema tests using a local reference registry and converter regression tests. Trigger schema and converter CI for ontology changes.
## Related discussion
Follow-up to [the review comment on #383](https://github.com/apache/ossie/pull/383#discussion_r4011033522) and the [agreed implementation scope](https://github.com/apache/ossie/pull/383#discussion_r4011213820).
## Validation
Passed on Python 3.14:
- 15 focused schema tests: `validation/tests/test_ontology.py` and `validation/tests/test_validate.py::test_semantic_model_definition_does_not_require_document_version`.
- 38 focused converter tests: `test_ossie_parser.py`, `test_flights_snapshot.py`, `test_examples_in_sync.py`, and the Palantir generated-version and field round-trip tests.
- Python compilation for changed source/test files and `git diff --check`.
Full local regression suites were not run; broader coverage is left to CI.
## Applicable checklist
- [x] Ontology schema, core documentation, and migration guidance agree.
- [x] Breaking change documented above and in the ontology specification.
- [x] Converter behavior, examples, and snapshots updated together.
- [x] Missing/invalid versions, core document shapes, and round trips covered by tests.
- [x] ASF license header included on the new test file; no dependencies added.
- [ ] Dev mailing list discussion.
- [ ] Full CI passes.
--
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] Require complete core documents in ontology mappings [ossie]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #441:
URL: https://github.com/apache/ossie/pull/441#discussion_r4074867720
##########
ontology/ontology.md:
##########
@@ -588,6 +604,7 @@ though `Store` plays a role in three of the relationships.
## Version History
- **0.2.0.dev0** (2026-05-29): Basic support for ontologies and logical schema mappings
+ - Breaking: each ontology map's `semantic_model` now requires its own `version`.
Review Comment:
It's a trade-off by reusing the core model spec. I think it's worth it. I'd love to get feedback from the community though.
As a side effect, embedded semantic models can have different versions now. We could mitigate that by introducing version inheritance, so that the default versions are still with the ontology version, but it is more of impl details, from the the spec perspective, versions are required in embedded semantic models.
cc @khush-bhatia
--
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] Reference the core schema root in ontology mappings [ossie]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #441:
URL: https://github.com/apache/ossie/pull/441#discussion_r4089932627
##########
ontology/ontology.json:
##########
@@ -297,7 +297,7 @@
"description": "Human-readable description of this ontology map"
},
"semantic_model": {
- "$ref": "https://raw.githubusercontent.com/apache/ossie/main/core-spec/ossie-schema.json#/$defs/SemanticModel"
+ "$ref": "https://raw.githubusercontent.com/apache/ossie/main/core-spec/ossie-schema.json"
Review Comment:
Widening this `$ref` to the whole `ossie-schema.json` document (rather than `#/$defs/SemanticModel`) means `validation/validate.py` `validate_schema()` now needs to resolve the full document. But it builds no local registry and falls back to a deprecated automatic live HTTPS fetch.
On a document with `ontology_mappings`, running the tool own documented usage (`python validation/validate.py <file> --schema ontology/ontology.json`) from an offline machine or no-egress CI raises an uncaught `_WrappedReferencingError: Unresolvable: https://raw.githubusercontent....` instead of producing a validation report.
This PR new test (`validation/tests/test_ontology.py`) already builds a local `Registry` for the same schema.
I think that fix should be applied to `validate.py` itself, not just the tests.
##########
converters/ontology/src/ossie_ontology/model.py:
##########
@@ -704,7 +704,9 @@ def __init__(
description: str | None = None,
ai_context: AiContext | None = None,
custom_extensions: list[CustomExtension] | None = None,
+ version: str = "0.2.0.dev0",
Review Comment:
I believe this `version` param is an unconstrained `str` at construction, but is fed into a `Literal["0.2.0.dev0"]` field at export time.
I think we should consider validating (or at least documenting) the accepted value here.
##########
converters/ontology/src/ossie_ontology/spec.py:
##########
@@ -175,11 +175,8 @@ class Metric(OssieObject):
class SemanticModel(OssieObject):
- """Model contents: datasets plus the join paths and metrics defined over them.
-
- OntologyMapping embeds these contents under ``semantic_model``; standalone
- core documents place them at the root alongside document metadata.
- """
+ """Complete core document embedded under OntologyMapping.semantic_model."""
+ version: Literal["0.2.0.dev0"]
Review Comment:
Now that `version` is strictly validated here, note the `datasets` field a few lines below (currently `datasets: list[Dataset] = Field(default_factory=list), no min_length=1`) still allows an empty list.
That contradicts `ontology/ontology.json` root schema (`minItems: 1`) which this PR repoints to `$ref` to use, and the new sentence in `ontology/ontology.md` saying the embedded model needs "at least one dataset".
I think it's worth tightening in this PR since it's the same class you are already changing.
--
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] Require complete core documents in ontology mappings [ossie]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #441:
URL: https://github.com/apache/ossie/pull/441#discussion_r4074373776
##########
ontology/ontology.json:
##########
@@ -297,7 +297,7 @@
"description": "Human-readable description of this ontology map"
},
"semantic_model": {
- "$ref": "https://raw.githubusercontent.com/apache/ossie/main/core-spec/ossie-schema.json#/$defs/SemanticModel"
+ "$ref": "https://raw.githubusercontent.com/apache/ossie/main/core-spec/ossie-schema.json"
Review Comment:
Fixed in the new commit.
--
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] Require complete core documents in ontology mappings [ossie]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #441:
URL: https://github.com/apache/ossie/pull/441#discussion_r4067794488
##########
ontology/ontology.json:
##########
@@ -297,7 +297,7 @@
"description": "Human-readable description of this ontology map"
},
"semantic_model": {
- "$ref": "https://raw.githubusercontent.com/apache/ossie/main/core-spec/ossie-schema.json#/$defs/SemanticModel"
+ "$ref": "https://raw.githubusercontent.com/apache/ossie/main/core-spec/ossie-schema.json"
Review Comment:
This root reference now requires the embedded `version`, but `converters/orionbelt/src/ossie_orionbelt/ontology.py:109` still removes it with `sem_model.pop("version", None)`, and `converters/orionbelt/tests/test_ossie_converter_ontology.py:104` explicitly asserts that it is absent. The OrionBelt converter will therefore continue emitting ontology documents that fail this schema; retain the core version and update its test and migration documentation before landing.
--
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] Reference the core schema root in ontology mappings [ossie]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre merged PR #441:
URL: https://github.com/apache/ossie/pull/441
--
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] Reference the core schema root in ontology mappings [ossie]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on PR #441:
URL: https://github.com/apache/ossie/pull/441#issuecomment-5823532692
Thanks @jbonofre for the review, fixed all your comments. PTAL!
--
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] Reference the core schema root in ontology mappings [ossie]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on PR #441:
URL: https://github.com/apache/ossie/pull/441#issuecomment-5875326425
Thanks @jbonofre and @khush-bhatia for the review. Will address the rest comments in followups.
--
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