-
Notifications
You must be signed in to change notification settings - Fork 42
Add minLength to name and definition in SelectionList $defs
#971
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
minLength to name and definition in SelectionList $defs
| return ordered_schema | ||
|
|
||
|
|
||
| def strip_nullable_anyof(schema: dict) -> dict: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The meat of the whole PR lives in this one function that is used to post-process a schema.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the handling of optional name and definition fields in SSVC models to ensure they cannot be empty strings while still allowing None values. The changes introduce new mixins with proper validation constraints and reorganize schema utilities into a dedicated module.
Key changes:
- Introduce new
_OptionalBaseand_GenericOptionalSsvcObjectmixins withminLength=1constraints for optional string fields - Replace custom validators with Pydantic's built-in
StringConstraintsfor cleaner validation - Move schema utility functions to a new
ssvc.utils.schemamodule for better code organization
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ssvc/_mixins.py | Adds new mixins with StringConstraints for optional fields and updates imports |
| src/ssvc/selection.py | Refactors Selection and MinimalDecisionPointValue to use new mixins and schema utilities |
| src/ssvc/utils/schema.py | New module containing moved schema utility functions including strip_nullable_anyof |
| src/ssvc/utils/misc.py | Removes schema utility functions that were moved to dedicated module |
| src/test/test_selections.py | Updates tests to verify empty strings raise ValueError while None is accepted |
| data/schema/v2/SelectionList_2_0_0.schema.json | Updates JSON schema to enforce minLength=1 for name and definition fields |
| src/ssvc/registry_demo.py | Updates import to use new schema utility module |
| src/ssvc/doctools.py | Updates imports to use new schema utility module |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
minLengthin JSON schema #904The problem with the first attempt (tagged
fix-834-attempt-1) is that it adds the undesirableAnyOfandnullvalues in the json schema.However, with
fix-834-attempt-2we have a working solution for the SelectionList object at least.We may need to revisit this in the future if we need other objects to do the same "optional but no anyOf" fields, but this works for the current needs.
Copilot Summary
This pull request strengthens schema validation and refactors code related to schema handling and optional fields. The most important changes are the enforcement of non-empty strings for key schema fields, the introduction of a new utility module for schema operations, and the simplification of how optional fields are managed in selection-related models.
Schema validation improvements:
minLength: 1constraints tonameanddefinitionfields in all relevant JSON schema files to ensure these fields cannot be empty strings. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14]Codebase refactoring and modularization:
ssvc.utils.miscto a new modulessvc.utils.schema, and updated all imports accordingly. [1] [2] [3] [4] [5] [6]strip_nullable_anyofinssvc.utils.schemato clean up nullable string representations in generated JSON schemas. [1] [2]Improvements to handling optional fields:
nameanddefinitionfields, removing custom validators and simplifying model logic. [1] [2] [3] [4]These changes collectively improve schema robustness, code maintainability, and clarity around optional fields.