Repository navigation
1919 daps python web support linkml schema - #1921
Merged
Merged
Conversation
…figuration scripts.
Contributor
Reviewer's GuideAdds first-class LinkML schema support across the stack (core server, Python API, CLI, web UI) and comprehensive end-to-end tests, including configuration of an external LinkML schema API, schema type/format handling, and improved metadata validation error reporting. Class diagram for updated Python schema APIs (CommandLib and CLI)classDiagram
class CommandLib {
+schemaCreate(schema_id, definition, definition_file, description, public, system, schema_type, schema_format) SchemaDataReply
+schemaRevise(schema_id, definition, definition_file, description, public, system, schema_type) SchemaDataReply
+schemaUpdate(schema_id, new_id, definition, definition_file, description, public, system, schema_type) SchemaDataReply
-_load_schema_file(path) string
-_validate_json(json_text, context) void
}
class CLI {
+_schemaCreate(schema_id, definition, definition_file, description, public, system, schema_type, schema_format) void
+_schemaRevise(schema_id, definition, definition_file, description, public, system, schema_type) void
+_schemaUpdate(schema_id, new_id, definition, definition_file, description, public, system, schema_type) void
}
class SchemaCreateRequest {
+id string
+def string
+desc string
+public bool
+system bool
+type string
+format string
}
class SchemaReviseRequest {
+id string
+def string
+desc string
+public bool
+system bool
+type string
+format string
}
class SchemaUpdateRequest {
+id string
+id_new string
+def string
+desc string
+public bool
+system bool
+type string
+format string
}
CommandLib --> SchemaCreateRequest : uses
CommandLib --> SchemaReviseRequest : uses
CommandLib --> SchemaUpdateRequest : uses
CLI --> CommandLib : calls
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
CommandLib.schemaUpdate/schemaRevise, JSON validation is still applied wheneverschema_typeisNone, which means updating/revising an existing LinkML schema without explicitly passingschema_type="linkml"will incorrectly force JSON validation; consider deriving the type from the existing schema whenschema_typeis omitted so that engine type is truly "carried forward". - The new
schema_typeargument onschemaUpdate/schemaReviseis only used to gate JSON validation and is not propagated into the protobuf request (unlikeschemaCreatewhich setsmsg.type/msg.format); if callers are allowed to change engine type/format on revise/update, you likely want to set these fields on the message as well or remove the CLI options to avoid a misleading interface.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `CommandLib.schemaUpdate`/`schemaRevise`, JSON validation is still applied whenever `schema_type` is `None`, which means updating/revising an existing LinkML schema without explicitly passing `schema_type="linkml"` will incorrectly force JSON validation; consider deriving the type from the existing schema when `schema_type` is omitted so that engine type is truly "carried forward".
- The new `schema_type` argument on `schemaUpdate`/`schemaRevise` is only used to gate JSON validation and is not propagated into the protobuf request (unlike `schemaCreate` which sets `msg.type`/`msg.format`); if callers are allowed to change engine type/format on revise/update, you likely want to set these fields on the message as well or remove the CLI options to avoid a misleading interface.
## Individual Comments
### Comment 1
<location path="python/datafed_pkg/datafed/CommandLib.py" line_range="339-340" />
<code_context>
# eliminate the need for this workaround.
# =========================================================================
-
def schemaCreate(self, schema_id, definition=None, definition_file=None,
- description=None, public=False, system=False):
+ description=None, public=False, system=False,
+ schema_type="json-schema", schema_format="json"):
"""
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against incompatible schema_type/schema_format combinations in schemaCreate.
Because `schema_type` and `schema_format` are independent, callers can pass incompatible pairs (e.g., `schema_type="json-schema"` with `schema_format="yaml"`). Since validation is only keyed on `schema_type == "json-schema"`, this would try to validate YAML as JSON. Please either restrict allowed type/format pairs and fail fast on invalid combinations, or make validation depend on `schema_format`, aligned with what the backend expects.
</issue_to_address>
### Comment 2
<location path="python/datafed_pkg/datafed/CLI.py" line_range="2064" />
<code_context>
def _schemaCreate(schema_id, definition, definition_file, description, public, system, schema_type, schema_format):
</code_context>
<issue_to_address>
**nitpick:** CLI help text still describes JSON-only schemas despite new LinkML support.
The `_schemaCreate` docstring still states "A JSON schema definition is required" even though the command now supports multiple schema types via `--type` and `--format` (including LinkML/YAML). Please update the text to describe a generic schema definition and briefly reference how it relates to `--type`/`--format` to avoid misleading users about supported formats.
</issue_to_address>
### Comment 3
<location path="tests/end-to-end/test_api_schema.py" line_range="893-902" />
<code_context>
+ # LinkML Schema tests
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding tests for CLI and CommandLib `schema_type` / `schema_format` options to complement the LinkML end-to-end API tests.
The Python API paths are well covered, but the new CLI flags (`--type`, `--format`) and corresponding `CommandLib` parameters for schema type/format aren’t tested. Please add tests that:
* Call `schema create/update/revise` with `--type linkml --format yaml` and assert end-to-end success.
* Verify that `--type linkml` skips JSON validation for YAML input on `schemaUpdate`/`schemaRevise` (YAML that fails JSON parsing should still be accepted).
These can live in a CLI/integration test module separate from the current end-to-end tests to close the coverage gap around the new flags.
Suggested implementation:
```python
# LinkML Schema tests (Python API and CLI/CommandLib coverage)
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_schema_create_view_delete"))
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_schema_create_invalid_yaml"))
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_schema_update_description"))
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_schema_update_definition"))
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_schema_revise"))
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_schema_search"))
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_metadata_validate_pass"))
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_metadata_validate_missing_required"))
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_metadata_validate_wrong_type"))
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_schema_create_from_file"))
# LinkML CLI / CommandLib schema_type & schema_format tests
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_cli_schema_lifecycle_with_type_and_format"))
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_cli_schema_update_revise_yaml_without_json_validation"))
```
To fully implement the new tests you requested, you will also need to:
1. Add the following methods to `TestDataFedPythonAPISchemaCRUD` (or, preferably, to a dedicated CLI/integration test class if that’s how your suite is organised):
- `test_linkml_cli_schema_lifecycle_with_type_and_format`
- Use the CLI (or `CommandLib`) to:
- `schema create` with `--type linkml --format yaml` using a valid LinkML YAML schema.
- `schema view` (or equivalent) to confirm the schema exists and is correctly stored as LinkML.
- `schema delete` (or equivalent) to clean up.
- Assert:
- CLI exit codes are zero for each step.
- The created schema is of type `linkml` and format `yaml` in the returned metadata.
- `test_linkml_cli_schema_update_revise_yaml_without_json_validation`
- Use the CLI (or `CommandLib`) to:
- Create a LinkML schema as above.
- Call `schema update` (and optionally `schema revise`) with:
- `--type linkml --format yaml`
- A YAML schema payload that is valid YAML but invalid JSON (e.g., uses YAML-only features or trailing commas that JSON rejects).
- Ensure the operation succeeds (exit code zero) and the schema is updated/revised.
- Assert:
- The CLI/CommandLib does not attempt JSON validation when `--type linkml` is used.
- The updated schema definition is stored and retrievable.
2. If you choose to place the actual implementations into a separate CLI-focused module (e.g. `tests/cli/test_schema_cli_linkml.py`), either:
- Move these `suite.addTest(...)` entries into that module’s `load_tests`, or
- Import the CLI test class into this file and change `TestDataFedPythonAPISchemaCRUD` to that class name for the new tests.
3. Ensure any required helpers/fixtures used for invoking the CLI or `CommandLib` (e.g. a `CommandLib` instance, temporary workspaces, test schemas on disk) are available to these new test methods and are wired consistently with the rest of your test suite.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
339
to
+340
| def schemaCreate(self, schema_id, definition=None, definition_file=None, | ||
| description=None, public=False, system=False): | ||
| description=None, public=False, system=False, |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Guard against incompatible schema_type/schema_format combinations in schemaCreate.
Because schema_type and schema_format are independent, callers can pass incompatible pairs (e.g., schema_type="json-schema" with schema_format="yaml"). Since validation is only keyed on schema_type == "json-schema", this would try to validate YAML as JSON. Please either restrict allowed type/format pairs and fail fast on invalid combinations, or make validation depend on schema_format, aligned with what the backend expects.
| def _schemaCreate(schema_id, definition, definition_file, description, public, system): | ||
| def _schemaCreate(schema_id, definition, definition_file, description, | ||
| public, system, schema_type, schema_format): | ||
| """ |
Contributor
There was a problem hiding this comment.
nitpick: CLI help text still describes JSON-only schemas despite new LinkML support.
The _schemaCreate docstring still states "A JSON schema definition is required" even though the command now supports multiple schema types via --type and --format (including LinkML/YAML). Please update the text to describe a generic schema definition and briefly reference how it relates to --type/--format to avoid misleading users about supported formats.
Comment on lines
+893
to
+902
| # LinkML Schema tests | ||
| suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_schema_create_view_delete")) | ||
| suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_schema_create_invalid_yaml")) | ||
| suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_schema_update_description")) | ||
| suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_schema_update_definition")) | ||
| suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_schema_revise")) | ||
| suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_schema_search")) | ||
| suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_metadata_validate_pass")) | ||
| suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_metadata_validate_missing_required")) | ||
| suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_metadata_validate_wrong_type")) |
Contributor
There was a problem hiding this comment.
suggestion (testing): Consider adding tests for CLI and CommandLib schema_type / schema_format options to complement the LinkML end-to-end API tests.
The Python API paths are well covered, but the new CLI flags (--type, --format) and corresponding CommandLib parameters for schema type/format aren’t tested. Please add tests that:
- Call
schema create/update/revisewith--type linkml --format yamland assert end-to-end success. - Verify that
--type linkmlskips JSON validation for YAML input onschemaUpdate/schemaRevise(YAML that fails JSON parsing should still be accepted).
These can live in a CLI/integration test module separate from the current end-to-end tests to close the coverage gap around the new flags.
Suggested implementation:
# LinkML Schema tests (Python API and CLI/CommandLib coverage)
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_schema_create_view_delete"))
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_schema_create_invalid_yaml"))
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_schema_update_description"))
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_schema_update_definition"))
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_schema_revise"))
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_schema_search"))
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_metadata_validate_pass"))
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_metadata_validate_missing_required"))
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_metadata_validate_wrong_type"))
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_schema_create_from_file"))
# LinkML CLI / CommandLib schema_type & schema_format tests
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_cli_schema_lifecycle_with_type_and_format"))
suite.addTest(TestDataFedPythonAPISchemaCRUD("test_linkml_cli_schema_update_revise_yaml_without_json_validation"))To fully implement the new tests you requested, you will also need to:
- Add the following methods to
TestDataFedPythonAPISchemaCRUD(or, preferably, to a dedicated CLI/integration test class if that’s how your suite is organised):test_linkml_cli_schema_lifecycle_with_type_and_format- Use the CLI (or
CommandLib) to:schema createwith--type linkml --format yamlusing a valid LinkML YAML schema.schema view(or equivalent) to confirm the schema exists and is correctly stored as LinkML.schema delete(or equivalent) to clean up.
- Assert:
- CLI exit codes are zero for each step.
- The created schema is of type
linkmland formatyamlin the returned metadata.
- Use the CLI (or
test_linkml_cli_schema_update_revise_yaml_without_json_validation- Use the CLI (or
CommandLib) to:- Create a LinkML schema as above.
- Call
schema update(and optionallyschema revise) with:--type linkml --format yaml- A YAML schema payload that is valid YAML but invalid JSON (e.g., uses YAML-only features or trailing commas that JSON rejects).
- Ensure the operation succeeds (exit code zero) and the schema is updated/revised.
- Assert:
- The CLI/CommandLib does not attempt JSON validation when
--type linkmlis used. - The updated schema definition is stored and retrievable.
- The CLI/CommandLib does not attempt JSON validation when
- Use the CLI (or
- If you choose to place the actual implementations into a separate CLI-focused module (e.g.
tests/cli/test_schema_cli_linkml.py), either:- Move these
suite.addTest(...)entries into that module’sload_tests, or - Import the CLI test class into this file and change
TestDataFedPythonAPISchemaCRUDto that class name for the new tests.
- Move these
- Ensure any required helpers/fixtures used for invoking the CLI or
CommandLib(e.g. aCommandLibinstance, temporary workspaces, test schemas on disk) are available to these new test methods and are wired consistently with the rest of your test suite.
nedvedba
approved these changes
Mar 31, 2026
JoshuaSBrown
force-pushed
the
1919-DAPS-python-web-support-linkml-schema
branch
from
March 31, 2026 18:34
ce220a6 to
0366887
Compare
JoshuaSBrown
added a commit
that referenced
this pull request
Mar 31, 2026
* [DAPS-1912] - refactor: register linkml storage engine with schema handler (#1912) * [DAPS-1919] - python, web support linkml schema (#1921) * [DAPS-1923] - update: version numbers bumped. (#1923) --------- Co-authored-by: Austin Hampton <44103380+megatnt1122@users.noreply.github.com> Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
JoshuaSBrown
added a commit
that referenced
this pull request
Apr 1, 2026
* [DAPS-1663] - feature: core, LogContext to dbGetRaw, Correlation_ID to dbMaintenance, metricThread and task_worker (#1885) * [DAPS-1890] - fix: test, mock_core server build (#1890) * [DAPS-1887] - refactor: facility fuse, remove dead code. (#1888) * [DAPS-1857] - feature: python client, tests, support schema functions, to hit feature parity with web ser… (#1859) * [DAPS-1855] - feature, core, add schema factory to decouple schema logic (#1891) * [DAPS-1896] - feature: foxx, schema format and type added to schema create API (#1897) * [DAPS-1893] - feature: common, proto3 add fields for schema type, format, and metadata format (#1894) * [DAPS-1830-1] - refactor: core foxx refactored json schema integration 1 (#1892) * [DAPS-1857-2] - feature: python client schema support (#1895) * [DAPS-1830-2] - refactor: core foxx refactored json schema integration (#1899) * [DAPS-1902] - feature: core, common, get schema api client into compliance with API spec file. (#1903) * [DAPS-1906-1] - feature: add core unit testing to CI (#1907) * [DAPS-1830-3] - refactor: core foxx refactored json schema integration 3 1827 (#1898) * [DAPS-1910] - upgrade: playwright 1.51.1 version. (#1911) * [DAPS-1914] - fix: web, bug in schema id name version mismatch (#1915) * [DAPS-1913] - refactor: foxx ci test scripts consolidate database name and default to different database for tests (#1917) * [DAPS-1916] - tests: add integration test for schema client handler to cmake (#1918) * [DAPS-1912] - refactor: register linkml storage engine with schema handler (#1912) * [DAPS-1919] - feature: python, web support linkml schema (#1921) * [DAPS-1923] - update: version numbers bumped. (#1923) Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> Co-authored-by: Austin Hampton <amh107@latech.edu> Co-authored-by: Blake Nedved <blakeanedved@gmail.com> Co-authored-by: Polina Shpilker <infinite.loopholes@gmail.com> Co-authored-by: JoshuaSBrown <brownjs@ornl.gov>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Add end-to-end support for LinkML-based metadata schemas across the core server, Python client/CLI, and web UI, including configuration of an external LinkML schema engine and improved metadata validation handling.
New Features:
Enhancements:
Build:
Tests: