Schema v1.1.0 - bug fixes and cli #32
Merged
Conversation
This file contains 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
Added schema v1.0.2 to achieve this - may need to make further changes. Modified error handling in data_file_validator to give better errors for this case. Fixes #30
As we're now checking for more complex version numbers I've added the 'packaging' package to parse and compare for us, and modified the Validator to use `self.schema_version_string` for the string version, and `self.schema_version` for the parsed packaging.version.Version object. Fixes #24
|
Note that currently the CLI and |
|
Just realised @GraemeWatt said this should be v1.1.0 given the added |
Additional resources is a minor change rather than bug fix.
alisonrclarke
changed the title
I've now updated the PR so that |
* Hides validators that are for internal use. * Fixed argument passing to validators so that old schemas work * Added class level docs
This aligns things with the hepdata code.
For use by main hepdata app, so it can preload the allowed remote schemas. Also made custom_data_schemas in DataFileValidator an instance variable so it's clear which schemas have been loaded for which validator.
|
@GraemeWatt I think this is ready for review now following the changes for integration with the main HEPData site. |
Tests add checks for previous schema change as well as checking for multiple errors
Follows pattern previously used for our own data schema but now works for submissions and remote schemas.
Also do all further checks even if there's been a previous error, to ensure all errors are returned.
GraemeWatt
reviewed
Sep 6, 2021
Also added some valid expressions with - to tests to ensure they are allowed
GraemeWatt
requested changes
Sep 6, 2021
There was a problem hiding this comment.
Looks good. I've tried both the CLI and the interface to the web app. Couple more requests:
- The web interface supports
.yaml.gzas an upload file type, i.e. a single YAML file compressed usinggzip. Could this also be supported from the-foption, i.e. uncompress in a temporary directory then validate the single YAML file? It might be better to rename the-z, --zipfileoption as-a, --archive(and similar changes throughout the code) to avoid confusion. - In the validation error messages, is it possible to print just the file name and not the full path? Particularly for archive files extracted to a temporary directory, we don't need to show the full path. Ideally this should be done also for YAML parsing errors where the string representation of the exception contains the full path and not just the file name.
* Rename zipfile to archive in parameters/docs * Allow .yaml.gz files as single file archive
… the submission schema Fixes #34
Added test for this case.
GraemeWatt
approved these changes
Sep 23, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
additional_resourcesValidatorsubclass to check a full submission (in a directory, zipped archive or single file format)