DTS manifest schema #209
DTS manifest schema #209
Conversation
staging_service/app.py
Outdated
| if Path._DTS_MANIFEST_SCHEMA_PATH is None: | ||
| raise Exception("Please provide DTS_MANIFEST_SCHEMA in the config file") | ||
| global _DTS_MANIFEST_SCHEMA | ||
| _DTS_MANIFEST_SCHEMA = DTS_MANIFEST_SCHEMA_PATH |
There was a problem hiding this comment.
Shouldn't you load / parse the schema here to fail early?
There was a problem hiding this comment.
Can you add a comment to the config / schema parsing code noting that there's no tests, it's all tested manually, and if you make changes you need to add tests or test manually?
staging_service/app.py
Outdated
| try: | ||
| jsonschema.Draft202012Validator.check_schema(dts_schema) | ||
| except jsonschema.exceptions.SchemaError as err: | ||
| raise Exception(f"Schema file {schema_path} is not a valid JSON schema: {err.message}") |
There was a problem hiding this comment.
| raise Exception(f"Schema file {schema_path} is not a valid JSON schema: {err.message}") | |
| raise Exception(f"Schema file {schema_path} is not a valid JSON schema: {err.message}") from err |
There was a problem hiding this comment.
Right, knew I forgot something.
staging_service/app.py
Outdated
|
|
||
| Path._DATA_DIR = DATA_DIR | ||
| Path._META_DIR = META_DIR | ||
| Path._CONCIERGE_PATH = CONCIERGE_PATH | ||
| Path._DTS_MANIFEST_SCHEMA_PATH = DTS_MANIFEST_SCHEMA_PATH |
There was a problem hiding this comment.
Is having this stored in the Path class still useful?
There was a problem hiding this comment.
I only left it there to keep the config pattern. But since it's not used, I suppose not.
| errors.append(Error(ErrorType.PARSE_FAIL, "Manifest is not a dictionary", spcsrc)) | ||
|
|
||
| return _error(Error(ErrorType.PARSE_FAIL, "Manifest is not a dictionary", spcsrc)) | ||
| validator = jsonschema.Draft202012Validator(dts_manifest_schema) |
There was a problem hiding this comment.
Can this line also be moved to the config section, so this function expects a validator class? Then the warning about invalid schemas should be moot
| @pytest.mark.parametrize("bad_schema", [None, 1, [], {"foo"}]) | ||
| def test_dts_manifest_bad_schema(bad_schema): | ||
| f = _get_test_file("manifest_small.json") | ||
| with pytest.raises(Exception): | ||
| parse_dts_manifest(f, bad_schema) |
There was a problem hiding this comment.
No checking the error message?
There was a problem hiding this comment.
Moot with the change to pass in the validator.
|
(changes look big - they're mostly Pipfile.lock)
Adds the DTS manifest JSON schema, does some basic validation with it and returns errors.
To do that, here's the short version of what this PR does:
import_specificationsActual parsing via the schema comes in the next PR. This is mostly about setup and validation.