From 8b4af54e8333afc6acf08c8131b4fddd0f4eff2a Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Mon, 9 Dec 2024 13:41:20 -0500 Subject: [PATCH 01/18] change dts as a kvp to just presence --- staging_service/app.py | 10 ++++------ tests/test_app.py | 6 +++--- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/staging_service/app.py b/staging_service/app.py index b89dc049..6c091850 100644 --- a/staging_service/app.py +++ b/staging_service/app.py @@ -122,23 +122,21 @@ async def bulk_specification(request: web.Request) -> web.json_response: :param request: contains the URL parameters for the request. Expected to have the following: * files (required) - a comma separated list of files, e.g. folder1/file1.txt,file2.txt - * dts (optional) - if present, and has the value "1", this will treat all of the given - files as DTS manifest files, and attempt to parse them accordingly. + * dts (optional) - if present this will treat all of the given files as DTS manifest files, + and attempt to parse them accordingly. """ username = await authorize_request(request) - params = parse_qs(request.query_string) - files = params.get("files", []) + files = parse_qs(request.query_string).get("files", []) files = files[0].split(",") if files else [] files = [f.strip() for f in files if f.strip()] paths = {} for f in files: p = Path.validate_path(username, f) paths[PathPy(p.full_path)] = PathPy(p.user_path) - as_dts = params.get("dts", ["0"])[0] == "1" # list(dict) returns a list of the dict keys in insertion order (py3.7+) file_type_resolver = _file_type_resolver - if as_dts: + if "dts" in request.query: file_type_resolver = _make_dts_file_resolver() res = parse_import_specifications( tuple(list(paths)), diff --git a/tests/test_app.py b/tests/test_app.py index c1fcd8d8..ff007f84 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -1108,7 +1108,7 @@ async def test_bulk_specification_dts_success(): json.dump(manifest_1_dict, f) with open(base / manifest_2, "w", encoding="utf-8") as f: json.dump(manifest_2_dict, f) - resp = await cli.get(f"bulk_specification/?files={manifest_1} , {manifest_2}&dts=1") + resp = await cli.get(f"bulk_specification/?files={manifest_1} , {manifest_2}&dts") jsn = await resp.json() # fails for now. will update when schema/parser is properly finished. assert jsn == { @@ -1171,7 +1171,7 @@ async def test_bulk_specification_dts_fail_json_without_dts(): } with open(base / manifest_1, "w", encoding="utf-8") as f: json.dump(manifest_1_dict, f) - resp = await cli.get(f"bulk_specification/?files={manifest_1}&dts=0") + resp = await cli.get(f"bulk_specification/?files={manifest_1}") jsn = await resp.json() assert jsn == { "errors": [ @@ -1195,7 +1195,7 @@ async def test_bulk_specification_dts_fail_wrong_format(): manifest_data = ["wrong", "format"] with open(base / manifest, "w", encoding="utf-8") as f: json.dump(manifest_data, f) - resp = await cli.get(f"bulk_specification/?files={manifest}&dts=1") + resp = await cli.get(f"bulk_specification/?files={manifest}&dts") jsn = await resp.json() assert jsn == { "errors": [ From 6874ba6c546bce6dbca2daba549020153c1741ac Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Mon, 9 Dec 2024 14:25:38 -0500 Subject: [PATCH 02/18] put test files in the right place --- tests/test_app.py | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/tests/test_app.py b/tests/test_app.py index ff007f84..6a150bc1 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -1052,8 +1052,10 @@ async def test_bulk_specification_success(): async def test_bulk_specification_dts_success(): async with AppClient(config) as cli: with FileUtil() as fu: - fu.make_dir("testuser/dts_folder") # testuser is hardcoded in the auth mock - base = Path(fu.base_dir) / "testuser" + sub_dir = "dts_folder" + dts_dir = f"testuser/{sub_dir}" + fu.make_dir(dts_dir) # testuser is hardcoded in the auth mock + base = Path(fu.base_dir) / "testuser" / sub_dir manifest_1 = "test_manifest_1.json" manifest_1_dict = { "resources": [], @@ -1108,20 +1110,20 @@ async def test_bulk_specification_dts_success(): json.dump(manifest_1_dict, f) with open(base / manifest_2, "w", encoding="utf-8") as f: json.dump(manifest_2_dict, f) - resp = await cli.get(f"bulk_specification/?files={manifest_1} , {manifest_2}&dts") + resp = await cli.get(f"bulk_specification/?files={sub_dir}/{manifest_1} , {sub_dir}/{manifest_2}&dts") jsn = await resp.json() # fails for now. will update when schema/parser is properly finished. assert jsn == { "errors": [ { "type": "cannot_parse_file", - "file": f"testuser/{manifest_1}", + "file": f"{dts_dir}/{manifest_1}", "message": "No import specification data in file", "tab": None, }, { "type": "cannot_parse_file", - "file": f"testuser/{manifest_2}", + "file": f"{dts_dir}/{manifest_2}", "message": "No import specification data in file", "tab": None, }, @@ -1150,8 +1152,10 @@ async def test_bulk_specification_dts_success(): async def test_bulk_specification_dts_fail_json_without_dts(): async with AppClient(config) as cli: with FileUtil() as fu: - fu.make_dir("testuser/dts_folder") # testuser is hardcoded in the auth mock - base = Path(fu.base_dir) / "testuser" + sub_dir = "dts_folder" + dts_dir = f"testuser/{sub_dir}" + fu.make_dir(dts_dir) # testuser is hardcoded in the auth mock + base = Path(fu.base_dir) / "testuser" / sub_dir manifest_1 = "test_manifest_1.json" manifest_1_dict = { "resources": [], @@ -1171,13 +1175,13 @@ async def test_bulk_specification_dts_fail_json_without_dts(): } with open(base / manifest_1, "w", encoding="utf-8") as f: json.dump(manifest_1_dict, f) - resp = await cli.get(f"bulk_specification/?files={manifest_1}") + resp = await cli.get(f"bulk_specification/?files={sub_dir}/{manifest_1}") jsn = await resp.json() assert jsn == { "errors": [ { "type": "cannot_parse_file", - "file": f"testuser/{manifest_1}", + "file": f"{dts_dir}/{manifest_1}", "message": "json is not a supported file type for import specifications", "tab": None, } @@ -1189,19 +1193,21 @@ async def test_bulk_specification_dts_fail_json_without_dts(): async def test_bulk_specification_dts_fail_wrong_format(): async with AppClient(config) as cli: with FileUtil() as fu: - fu.make_dir("testuser/dts_folder") # testuser is hardcoded in the auth mock - base = Path(fu.base_dir) / "testuser" + sub_dir = "dts_folder" + dts_dir = f"testuser/{sub_dir}" + fu.make_dir(dts_dir) # testuser is hardcoded in the auth mock + base = Path(fu.base_dir) / "testuser" / sub_dir manifest = "test_manifest.json" manifest_data = ["wrong", "format"] with open(base / manifest, "w", encoding="utf-8") as f: json.dump(manifest_data, f) - resp = await cli.get(f"bulk_specification/?files={manifest}&dts") + resp = await cli.get(f"bulk_specification/?files={sub_dir}/{manifest}&dts") jsn = await resp.json() assert jsn == { "errors": [ { "type": "cannot_parse_file", - "file": f"testuser/{manifest}", + "file": f"{dts_dir}/{manifest}", "message": "Manifest is not a dictionary", "tab": None, } From a4e3949d9b133039a88f7824d7f9f3a06c46cf6f Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Mon, 9 Dec 2024 15:13:38 -0500 Subject: [PATCH 03/18] enforce JSON file extensions only for dts import specs --- staging_service/app.py | 9 ++++++--- tests/test_app.py | 26 ++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/staging_service/app.py b/staging_service/app.py index 6c091850..c4b2f8b2 100644 --- a/staging_service/app.py +++ b/staging_service/app.py @@ -106,10 +106,13 @@ def _file_type_resolver(path: PathPy) -> FileTypeResolution: def _make_dts_file_resolver() -> Callable[[Path], FileTypeResolution]: """Makes a DTS file resolver.""" - - def dts_file_resolver(_: PathPy): + def dts_file_resolver(path: PathPy): + # must be a ".json" file + file_parts = str(path).split(".") + ext = file_parts[-1] + if len(file_parts) < 2 or ext.lower() != "json": + return FileTypeResolution(unsupported_type=ext) return FileTypeResolution(parser=parse_dts_manifest) - return dts_file_resolver diff --git a/tests/test_app.py b/tests/test_app.py index 6a150bc1..53a38392 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -1216,6 +1216,32 @@ async def test_bulk_specification_dts_fail_wrong_format(): assert resp.status == 400 +async def test_bulk_specification_dts_fail_wrong_extension(): + async with AppClient(config) as cli: + with FileUtil() as fu: + sub_dir = "dts_folder" + dts_dir = f"testuser/{sub_dir}" + fu.make_dir(dts_dir) # testuser is hardcoded in the auth mock + base = Path(fu.base_dir) / "testuser" / sub_dir + manifest = "test_manifest.foo" + manifest_data = {"resources": [], "instructions": {}} + with open(base / manifest, "w", encoding="utf-8") as f: + json.dump(manifest_data, f) + resp = await cli.get(f"bulk_specification/?files={sub_dir}/{manifest}&dts") + jsn = await resp.json() + assert jsn == { + "errors": [ + { + "type": "cannot_parse_file", + "file": f"{dts_dir}/{manifest}", + "message": "foo is not a supported file type for import specifications", + "tab": None, + } + ] + } + assert resp.status == 400 + + async def test_bulk_specification_fail_no_files(): async with AppClient(config) as cli: for f in ["", "?files=", "?files= , ,, , "]: From b3613ea6cc3cb043f265364fb36d07876d946995 Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Mon, 9 Dec 2024 15:14:04 -0500 Subject: [PATCH 04/18] ruff formatting --- staging_service/app.py | 2 ++ tests/test_app.py | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/staging_service/app.py b/staging_service/app.py index c4b2f8b2..494ce46d 100644 --- a/staging_service/app.py +++ b/staging_service/app.py @@ -106,6 +106,7 @@ def _file_type_resolver(path: PathPy) -> FileTypeResolution: def _make_dts_file_resolver() -> Callable[[Path], FileTypeResolution]: """Makes a DTS file resolver.""" + def dts_file_resolver(path: PathPy): # must be a ".json" file file_parts = str(path).split(".") @@ -113,6 +114,7 @@ def dts_file_resolver(path: PathPy): if len(file_parts) < 2 or ext.lower() != "json": return FileTypeResolution(unsupported_type=ext) return FileTypeResolution(parser=parse_dts_manifest) + return dts_file_resolver diff --git a/tests/test_app.py b/tests/test_app.py index 53a38392..6e7e450f 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -1110,7 +1110,9 @@ async def test_bulk_specification_dts_success(): json.dump(manifest_1_dict, f) with open(base / manifest_2, "w", encoding="utf-8") as f: json.dump(manifest_2_dict, f) - resp = await cli.get(f"bulk_specification/?files={sub_dir}/{manifest_1} , {sub_dir}/{manifest_2}&dts") + resp = await cli.get( + f"bulk_specification/?files={sub_dir}/{manifest_1} , {sub_dir}/{manifest_2}&dts" + ) jsn = await resp.json() # fails for now. will update when schema/parser is properly finished. assert jsn == { From bb76d499ce95540d233fb8260175a32e01f68335 Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Mon, 9 Dec 2024 17:04:37 -0500 Subject: [PATCH 05/18] update json checking, add tests --- staging_service/app.py | 10 +++++----- tests/test_app.py | 15 ++++++++++++--- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/staging_service/app.py b/staging_service/app.py index 494ce46d..cd65d1f1 100644 --- a/staging_service/app.py +++ b/staging_service/app.py @@ -13,7 +13,7 @@ from .app_error_formatter import format_import_spec_errors from .auth2Client import KBaseAuth2 -from .autodetect.Mappings import CSV, EXCEL, TSV +from .autodetect.Mappings import CSV, EXCEL, JSON, TSV from .AutoDetectUtils import AutoDetectUtils from .globus import assert_globusid_exists, is_globusid from .import_specifications.file_parser import ( @@ -53,6 +53,7 @@ EXCEL: write_excel, } +NO_EXTENSION = "missing extension" @routes.get("/importer_filetypes/") async def importer_filetypes(_: web.Request) -> web.json_response: @@ -109,10 +110,9 @@ def _make_dts_file_resolver() -> Callable[[Path], FileTypeResolution]: def dts_file_resolver(path: PathPy): # must be a ".json" file - file_parts = str(path).split(".") - ext = file_parts[-1] - if len(file_parts) < 2 or ext.lower() != "json": - return FileTypeResolution(unsupported_type=ext) + suffix = path.suffix[1:] if path.suffix else NO_EXTENSION + if suffix.lower() != JSON.lower(): + return FileTypeResolution(unsupported_type=suffix) return FileTypeResolution(parser=parse_dts_manifest) return dts_file_resolver diff --git a/tests/test_app.py b/tests/test_app.py index 6e7e450f..f8147154 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -4,6 +4,7 @@ import json import os import platform +import pytest import shutil import string import time @@ -1218,14 +1219,22 @@ async def test_bulk_specification_dts_fail_wrong_format(): assert resp.status == 400 -async def test_bulk_specification_dts_fail_wrong_extension(): +@pytest.mark.parametrize( + "manifest,expected", + [ + ("test_manifest.foo", "foo"), + ("json", app.NO_EXTENSION), + (".json", app.NO_EXTENSION), + ("some_manifest", app.NO_EXTENSION), + ], +) +async def test_bulk_specification_dts_fail_wrong_extension(manifest: str, expected: str): async with AppClient(config) as cli: with FileUtil() as fu: sub_dir = "dts_folder" dts_dir = f"testuser/{sub_dir}" fu.make_dir(dts_dir) # testuser is hardcoded in the auth mock base = Path(fu.base_dir) / "testuser" / sub_dir - manifest = "test_manifest.foo" manifest_data = {"resources": [], "instructions": {}} with open(base / manifest, "w", encoding="utf-8") as f: json.dump(manifest_data, f) @@ -1236,7 +1245,7 @@ async def test_bulk_specification_dts_fail_wrong_extension(): { "type": "cannot_parse_file", "file": f"{dts_dir}/{manifest}", - "message": "foo is not a supported file type for import specifications", + "message": f"{expected} is not a supported file type for import specifications", "tab": None, } ] From a1d2b02c5441b89ce95aabd6abe43ba0d853dfd4 Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Mon, 9 Dec 2024 17:07:46 -0500 Subject: [PATCH 06/18] make constants more clear --- staging_service/app.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/staging_service/app.py b/staging_service/app.py index cd65d1f1..598179e8 100644 --- a/staging_service/app.py +++ b/staging_service/app.py @@ -13,7 +13,7 @@ from .app_error_formatter import format_import_spec_errors from .auth2Client import KBaseAuth2 -from .autodetect.Mappings import CSV, EXCEL, JSON, TSV +from .autodetect.Mappings import CSV, EXCEL, TSV from .AutoDetectUtils import AutoDetectUtils from .globus import assert_globusid_exists, is_globusid from .import_specifications.file_parser import ( @@ -53,8 +53,10 @@ EXCEL: write_excel, } +JSON_EXTENSION = "json" NO_EXTENSION = "missing extension" + @routes.get("/importer_filetypes/") async def importer_filetypes(_: web.Request) -> web.json_response: """ @@ -108,10 +110,10 @@ def _file_type_resolver(path: PathPy) -> FileTypeResolution: def _make_dts_file_resolver() -> Callable[[Path], FileTypeResolution]: """Makes a DTS file resolver.""" - def dts_file_resolver(path: PathPy): + def dts_file_resolver(path: PathPy) -> FileTypeResolution: # must be a ".json" file suffix = path.suffix[1:] if path.suffix else NO_EXTENSION - if suffix.lower() != JSON.lower(): + if suffix.lower() != JSON_EXTENSION: return FileTypeResolution(unsupported_type=suffix) return FileTypeResolution(parser=parse_dts_manifest) From 15be5832ec6864298eec34ae53762da3cbfa03dc Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Mon, 9 Dec 2024 17:09:17 -0500 Subject: [PATCH 07/18] add comment --- staging_service/app.py | 1 + 1 file changed, 1 insertion(+) diff --git a/staging_service/app.py b/staging_service/app.py index 598179e8..167e83cd 100644 --- a/staging_service/app.py +++ b/staging_service/app.py @@ -53,6 +53,7 @@ EXCEL: write_excel, } +# The constant in autodetect.Mappings isn't guaranteed to be the string we want. JSON_EXTENSION = "json" NO_EXTENSION = "missing extension" From 37a73c15bc3bbee6abe338376cd1aefa78ebf6b4 Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Mon, 9 Dec 2024 20:51:56 -0500 Subject: [PATCH 08/18] fix commit error --- staging_service/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/staging_service/app.py b/staging_service/app.py index 0a559480..e607641f 100644 --- a/staging_service/app.py +++ b/staging_service/app.py @@ -124,7 +124,7 @@ def dts_file_resolver(path: PathPy) -> FileTypeResolution: suffix = path.suffix[1:] if path.suffix else NO_EXTENSION if suffix.lower() != JSON_EXTENSION: return FileTypeResolution(unsupported_type=suffix) - return FileTypeResolution(parser=parse_dts_manifest) + return FileTypeResolution(parser=lambda p: parse_dts_manifest(p, dts_schema)) return dts_file_resolver From 90b209dede7657cc68a4b0ac7c86a912fe56213d Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Mon, 9 Dec 2024 21:13:59 -0500 Subject: [PATCH 09/18] update jsonschema validation error handling --- .../import_specifications/individual_parsers.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/staging_service/import_specifications/individual_parsers.py b/staging_service/import_specifications/individual_parsers.py index 3b1dde5d..c3f4b0c5 100644 --- a/staging_service/import_specifications/individual_parsers.py +++ b/staging_service/import_specifications/individual_parsers.py @@ -361,9 +361,20 @@ def parse_dts_manifest(path: Path, dts_manifest_schema: dict) -> ParseResults: err_str = err.message err_path = err.absolute_path if err_path: - if isinstance(err_path[-1], int): - err_path[-1] = f"item {err_path[-1]}" - err_str += f" at {'/'.join(err_path)}" + # paths can look like, say, ["instructions", "objects", 0, "data_type"] + # convert that '0' to "item 0" to be slightly more readable to users. + # kind of a mouthful below, but does that conversion in place + # The error above gets translated into: + # " at instructions/objects/item 0/data_type" + # + # Another example would be if the path is just ["instructions"] and it's missing + # a property, the error might be: + # "missing property 'foo' for 'instructions'" + err_path = [f"item {elem}" if isinstance(elem, int) else elem for elem in err_path] + prep = "for" + if len(err_path) > 1: + prep = "at" + err_str += f" {prep} {'/'.join(err_path)}" errors.append(Error(ErrorType.PARSE_FAIL, err_str, spcsrc)) except jsonschema.exceptions.SchemaError: return _error(Error(ErrorType.OTHER, "Manifest schema is invalid", spcsrc)) From 748380c056913268fa72481c2ad35b7263f64807 Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Tue, 10 Dec 2024 08:57:46 -0500 Subject: [PATCH 10/18] update schema --- import_specifications/schema/dts_manifest.json | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/import_specifications/schema/dts_manifest.json b/import_specifications/schema/dts_manifest.json index fc70d7ef..07578596 100644 --- a/import_specifications/schema/dts_manifest.json +++ b/import_specifications/schema/dts_manifest.json @@ -13,7 +13,20 @@ "type": "object", "properties": { "data_type": {"type": "string"}, - "parameters": {"type": "object"} + "parameters": { + "type": "object", + "patternProperties": { + ".*": { + "oneOf": [ + {"type": "string"}, + {"type": "number"}, + {"type": "boolean"}, + {"type": "null"} + ] + } + }, + "additionalProperties": false + } }, "required": ["data_type", "parameters"] } From 528bb40edd3e6d4af1412449a855861dbaa22355 Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Wed, 11 Dec 2024 15:56:08 -0500 Subject: [PATCH 11/18] remove fancy error texts --- .../schema/dts_manifest.json | 24 +++++++++---------- .../individual_parsers.py | 17 ++----------- .../test_individual_parsers.py | 2 +- 3 files changed, 15 insertions(+), 28 deletions(-) diff --git a/import_specifications/schema/dts_manifest.json b/import_specifications/schema/dts_manifest.json index 07578596..1bf6dc80 100644 --- a/import_specifications/schema/dts_manifest.json +++ b/import_specifications/schema/dts_manifest.json @@ -6,7 +6,10 @@ "instructions": { "type": "object", "properties": { - "protocol": {"type": "string"}, + "protocol": { + "type": "string", + "const": "KBase narrative import" + }, "objects": { "type": "array", "items": { @@ -15,17 +18,14 @@ "data_type": {"type": "string"}, "parameters": { "type": "object", - "patternProperties": { - ".*": { - "oneOf": [ - {"type": "string"}, - {"type": "number"}, - {"type": "boolean"}, - {"type": "null"} - ] - } - }, - "additionalProperties": false + "additionalProperties": { + "oneOf": [ + {"type": "string"}, + {"type": "number"}, + {"type": "boolean"}, + {"type": "null"} + ] + } } }, "required": ["data_type", "parameters"] diff --git a/staging_service/import_specifications/individual_parsers.py b/staging_service/import_specifications/individual_parsers.py index c3f4b0c5..f5ca0b64 100644 --- a/staging_service/import_specifications/individual_parsers.py +++ b/staging_service/import_specifications/individual_parsers.py @@ -359,22 +359,9 @@ def parse_dts_manifest(path: Path, dts_manifest_schema: dict) -> ParseResults: validator = jsonschema.Draft202012Validator(dts_manifest_schema) for err in validator.iter_errors(manifest_json): err_str = err.message - err_path = err.absolute_path + err_path = list(err.absolute_path) if err_path: - # paths can look like, say, ["instructions", "objects", 0, "data_type"] - # convert that '0' to "item 0" to be slightly more readable to users. - # kind of a mouthful below, but does that conversion in place - # The error above gets translated into: - # " at instructions/objects/item 0/data_type" - # - # Another example would be if the path is just ["instructions"] and it's missing - # a property, the error might be: - # "missing property 'foo' for 'instructions'" - err_path = [f"item {elem}" if isinstance(elem, int) else elem for elem in err_path] - prep = "for" - if len(err_path) > 1: - prep = "at" - err_str += f" {prep} {'/'.join(err_path)}" + err_str += f" at {err_path}" errors.append(Error(ErrorType.PARSE_FAIL, err_str, spcsrc)) except jsonschema.exceptions.SchemaError: return _error(Error(ErrorType.OTHER, "Manifest schema is invalid", spcsrc)) diff --git a/tests/import_specifications/test_individual_parsers.py b/tests/import_specifications/test_individual_parsers.py index 4ef05d8b..87a63d41 100644 --- a/tests/import_specifications/test_individual_parsers.py +++ b/tests/import_specifications/test_individual_parsers.py @@ -926,7 +926,7 @@ def test_dts_manifest_fail_with_path( [ Error( ErrorType.PARSE_FAIL, - "'data_type' is a required property at instructions/objects/item 1", + "'data_type' is a required property at ['instructions', 'objects', 1]", SpecificationSource(manifest_path), ) ], From 86c77ad3ae2842682cf3c1081fe5d6e9a4307143 Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Wed, 11 Dec 2024 16:31:28 -0500 Subject: [PATCH 12/18] move schema loading/validating to app config --- staging_service/app.py | 27 ++++++++++++------- .../individual_parsers.py | 6 ++--- .../test_individual_parsers.py | 7 ++--- 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/staging_service/app.py b/staging_service/app.py index e607641f..9ff2484f 100644 --- a/staging_service/app.py +++ b/staging_service/app.py @@ -6,10 +6,12 @@ import sys from collections import defaultdict from pathlib import Path as PathPy +from typing import Any from urllib.parse import parse_qs, unquote import aiohttp_cors from aiohttp import web +import jsonschema from .app_error_formatter import format_import_spec_errors from .auth2Client import KBaseAuth2 @@ -110,21 +112,16 @@ def _file_type_resolver(path: PathPy) -> FileTypeResolution: def _make_dts_file_resolver() -> Callable[[Path], FileTypeResolution]: - """Makes a DTS file resolver. + """Makes a DTS file resolver - This looks a little goofy, but it ensures that the DTS manifest schema file - only gets loaded once per API call, no matter how many DTS manifest files are - expected to be parsed. It also prevents having it stick around in memory. + This injects the DTS schema into the FileTypeResolution's parser call. """ - with open(_DTS_MANIFEST_SCHEMA) as schema_file: - dts_schema = json.load(schema_file) - def dts_file_resolver(path: PathPy) -> FileTypeResolution: # must be a ".json" file suffix = path.suffix[1:] if path.suffix else NO_EXTENSION if suffix.lower() != JSON_EXTENSION: return FileTypeResolution(unsupported_type=suffix) - return FileTypeResolution(parser=lambda p: parse_dts_manifest(p, dts_schema)) + return FileTypeResolution(parser=lambda p: parse_dts_manifest(p, _DTS_MANIFEST_SCHEMA)) return dts_file_resolver @@ -611,6 +608,16 @@ async def authorize_request(request): return username +def load_and_validate_schema(schema_path: PathPy) -> dict[str, Any]: + with open(schema_path) as schema_file: + dts_schema = json.load(schema_file) + try: + jsonschema.Draft202012Validator.check_schema(dts_schema) + except jsonschema.exceptions.SchemaError as err: + Exception(f"Schema file {schema_path} is not a valid JSON schema: {err.message}") + return dts_schema + + def inject_config_dependencies(config): """ # TODO this is pretty hacky dependency injection @@ -656,8 +663,10 @@ def inject_config_dependencies(config): 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 + # will raise an Exception if the schema is invalid + _DTS_MANIFEST_SCHEMA = load_and_validate_schema(DTS_MANIFEST_SCHEMA_PATH) if FILE_EXTENSION_MAPPINGS is None: raise Exception("Please provide FILE_EXTENSION_MAPPINGS in the config file ") diff --git a/staging_service/import_specifications/individual_parsers.py b/staging_service/import_specifications/individual_parsers.py index f5ca0b64..4573afe8 100644 --- a/staging_service/import_specifications/individual_parsers.py +++ b/staging_service/import_specifications/individual_parsers.py @@ -344,6 +344,9 @@ def parse_dts_manifest(path: Path, dts_manifest_schema: dict) -> ParseResults: and its value will be a Tuple of frozendicts of the parameters. Also, in keeping with the xsv parsers, each parameter value is expected to be a PRIMITIVE_TYPE. + Note that the dts_manifest_schema is expected to be valid, and may throw an + unexpected exception otherwise. + TODO: include further details here, and in separate documentation - ADR? """ spcsrc = SpecificationSource(path) @@ -351,7 +354,6 @@ def parse_dts_manifest(path: Path, dts_manifest_schema: dict) -> ParseResults: # dummy for now results = {} try: - jsonschema.Draft202012Validator.check_schema(dts_manifest_schema) with open(path, "r") as manifest: manifest_json = json.load(manifest) if not isinstance(manifest_json, dict): @@ -363,8 +365,6 @@ def parse_dts_manifest(path: Path, dts_manifest_schema: dict) -> ParseResults: if err_path: err_str += f" at {err_path}" errors.append(Error(ErrorType.PARSE_FAIL, err_str, spcsrc)) - except jsonschema.exceptions.SchemaError: - return _error(Error(ErrorType.OTHER, "Manifest schema is invalid", spcsrc)) except json.JSONDecodeError: return _error(Error(ErrorType.PARSE_FAIL, "File must be in JSON format", spcsrc)) except FileNotFoundError: diff --git a/tests/import_specifications/test_individual_parsers.py b/tests/import_specifications/test_individual_parsers.py index 87a63d41..3d9e4aeb 100644 --- a/tests/import_specifications/test_individual_parsers.py +++ b/tests/import_specifications/test_individual_parsers.py @@ -879,11 +879,8 @@ def test_dts_manifest_file_is_directory( @pytest.mark.parametrize("bad_schema", [None, 1, [], {"foo"}]) def test_dts_manifest_bad_schema(bad_schema): f = _get_test_file("manifest_small.json") - _dts_manifest_parse_fail( - f, - bad_schema, - [Error(ErrorType.OTHER, "Manifest schema is invalid", SpecificationSource(f))], - ) + with pytest.raises(Exception): + parse_dts_manifest(f, bad_schema) def test_dts_manifest_no_top_level_keys( From 5469ce7643b39911a3dfc0d5e210e41c691cd043 Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Wed, 11 Dec 2024 17:16:04 -0500 Subject: [PATCH 13/18] add empty tests for schema --- staging_service/app.py | 2 +- tests/test_app.py | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/staging_service/app.py b/staging_service/app.py index 9ff2484f..adf255aa 100644 --- a/staging_service/app.py +++ b/staging_service/app.py @@ -614,7 +614,7 @@ def load_and_validate_schema(schema_path: PathPy) -> dict[str, Any]: try: jsonschema.Draft202012Validator.check_schema(dts_schema) except jsonschema.exceptions.SchemaError as err: - 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}") return dts_schema diff --git a/tests/test_app.py b/tests/test_app.py index f8147154..1f46171b 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -1253,6 +1253,17 @@ async def test_bulk_specification_dts_fail_wrong_extension(manifest: str, expect assert resp.status == 400 +def test_bulk_specification_dts_fail_bad_schema(): + # TODO: This is tested manually, as there's no good way to inject bad configs + # to individual tests right now. + # TODO: automated tests for: + # * missing schema config + # * missing schema file + # * malformed schema file (i.e. not json) + # * bad schema (good JSON, invalid as json schema) + pass + + async def test_bulk_specification_fail_no_files(): async with AppClient(config) as cli: for f in ["", "?files=", "?files= , ,, , "]: From a10dc0351af393ba4dab41ad63bc2b615caaa975 Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Wed, 11 Dec 2024 17:17:07 -0500 Subject: [PATCH 14/18] rename schema file --- .../schema/{dts_manifest.json => dts_manifest_schema.json} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename import_specifications/schema/{dts_manifest.json => dts_manifest_schema.json} (100%) diff --git a/import_specifications/schema/dts_manifest.json b/import_specifications/schema/dts_manifest_schema.json similarity index 100% rename from import_specifications/schema/dts_manifest.json rename to import_specifications/schema/dts_manifest_schema.json From b010cded4a3ebb47047abdf20ceaac91744193d6 Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Wed, 11 Dec 2024 17:19:26 -0500 Subject: [PATCH 15/18] ruff formatting --- staging_service/app.py | 1 + 1 file changed, 1 insertion(+) diff --git a/staging_service/app.py b/staging_service/app.py index adf255aa..e5b10a9e 100644 --- a/staging_service/app.py +++ b/staging_service/app.py @@ -116,6 +116,7 @@ def _make_dts_file_resolver() -> Callable[[Path], FileTypeResolution]: This injects the DTS schema into the FileTypeResolution's parser call. """ + def dts_file_resolver(path: PathPy) -> FileTypeResolution: # must be a ".json" file suffix = path.suffix[1:] if path.suffix else NO_EXTENSION From af30bf28202fd8a3571b3a4fba65ee0cb2f0ce39 Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Wed, 11 Dec 2024 17:21:53 -0500 Subject: [PATCH 16/18] fix configs --- deployment/conf/deployment.cfg | 2 +- deployment/conf/testing.cfg | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/deployment/conf/deployment.cfg b/deployment/conf/deployment.cfg index 5ec9ac74..b6622c58 100644 --- a/deployment/conf/deployment.cfg +++ b/deployment/conf/deployment.cfg @@ -4,4 +4,4 @@ DATA_DIR = /kb/deployment/lib/src/data/bulk/ AUTH_URL = https://ci.kbase.us/services/auth/api/V2/token CONCIERGE_PATH = /kbaseconcierge FILE_EXTENSION_MAPPINGS = /kb/deployment/conf/supported_apps_w_extensions.json -DTS_MANIFEST_SCHEMA = /kb/deployment/import_specifications/schema/dts_manifest.json +DTS_MANIFEST_SCHEMA = /kb/deployment/import_specifications/schema/dts_manifest_schema.json diff --git a/deployment/conf/testing.cfg b/deployment/conf/testing.cfg index 41f067bb..5e016215 100644 --- a/deployment/conf/testing.cfg +++ b/deployment/conf/testing.cfg @@ -5,4 +5,4 @@ AUTH_URL = https://ci.kbase.us/services/auth/api/V2/token CONCIERGE_PATH = /kbaseconcierge FILE_EXTENSION_MAPPINGS = ./deployment/conf/supported_apps_w_extensions.json ;FILE_EXTENSION_MAPPINGS_PYCHARM = ../deployment/conf/supported_apps_w_extensions.json -DTS_MANIFEST_SCHEMA = ./import_specifications/schema/dts_manifest.json +DTS_MANIFEST_SCHEMA = ./import_specifications/schema/dts_manifest_schema.json From ebf25e74d0a5a4803589589f7f21a7aa2c3f26af Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Wed, 11 Dec 2024 21:58:23 -0500 Subject: [PATCH 17/18] updates to config, validator, etc. --- staging_service/app.py | 19 ++++---- .../individual_parsers.py | 11 ++--- .../test_individual_parsers.py | 46 ++++++++----------- tests/test_app.py | 41 +++++++++++++++++ 4 files changed, 73 insertions(+), 44 deletions(-) diff --git a/staging_service/app.py b/staging_service/app.py index e5b10a9e..14922cd6 100644 --- a/staging_service/app.py +++ b/staging_service/app.py @@ -6,7 +6,6 @@ import sys from collections import defaultdict from pathlib import Path as PathPy -from typing import Any from urllib.parse import parse_qs, unquote import aiohttp_cors @@ -44,7 +43,7 @@ VERSION = "1.3.6" _DATATYPE_MAPPINGS = None -_DTS_MANIFEST_SCHEMA = None +_DTS_MANIFEST_VALIDATOR: jsonschema.Draft202012Validator | None = None _APP_JSON = "application/json" @@ -122,7 +121,7 @@ def dts_file_resolver(path: PathPy) -> FileTypeResolution: suffix = path.suffix[1:] if path.suffix else NO_EXTENSION if suffix.lower() != JSON_EXTENSION: return FileTypeResolution(unsupported_type=suffix) - return FileTypeResolution(parser=lambda p: parse_dts_manifest(p, _DTS_MANIFEST_SCHEMA)) + return FileTypeResolution(parser=lambda p: parse_dts_manifest(p, _DTS_MANIFEST_VALIDATOR)) return dts_file_resolver @@ -609,14 +608,14 @@ async def authorize_request(request): return username -def load_and_validate_schema(schema_path: PathPy) -> dict[str, Any]: +def load_and_validate_schema(schema_path: PathPy) -> jsonschema.Draft202012Validator: with open(schema_path) as schema_file: dts_schema = json.load(schema_file) 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}") - return dts_schema + raise Exception(f"Schema file {schema_path} is not a valid JSON schema: {err.message}") from err + return jsonschema.Draft202012Validator(dts_schema) def inject_config_dependencies(config): @@ -651,7 +650,7 @@ def inject_config_dependencies(config): Path._DATA_DIR = DATA_DIR Path._META_DIR = META_DIR Path._CONCIERGE_PATH = CONCIERGE_PATH - Path._DTS_MANIFEST_SCHEMA_PATH = DTS_MANIFEST_SCHEMA_PATH + _DTS_MANIFEST_SCHEMA_PATH = DTS_MANIFEST_SCHEMA_PATH if Path._DATA_DIR is None: raise Exception("Please provide DATA_DIR in the config file ") @@ -662,12 +661,12 @@ def inject_config_dependencies(config): if Path._CONCIERGE_PATH is None: raise Exception("Please provide CONCIERGE_PATH in the config file ") - if Path._DTS_MANIFEST_SCHEMA_PATH is None: + if _DTS_MANIFEST_SCHEMA_PATH is None: raise Exception("Please provide DTS_MANIFEST_SCHEMA in the config file") - global _DTS_MANIFEST_SCHEMA + global _DTS_MANIFEST_VALIDATOR # will raise an Exception if the schema is invalid - _DTS_MANIFEST_SCHEMA = load_and_validate_schema(DTS_MANIFEST_SCHEMA_PATH) + _DTS_MANIFEST_VALIDATOR = load_and_validate_schema(DTS_MANIFEST_SCHEMA_PATH) if FILE_EXTENSION_MAPPINGS is None: raise Exception("Please provide FILE_EXTENSION_MAPPINGS in the config file ") diff --git a/staging_service/import_specifications/individual_parsers.py b/staging_service/import_specifications/individual_parsers.py index 4573afe8..e0379caf 100644 --- a/staging_service/import_specifications/individual_parsers.py +++ b/staging_service/import_specifications/individual_parsers.py @@ -4,13 +4,12 @@ import csv import json -import jsonschema import math import re from pathlib import Path from typing import Any, Optional, Tuple, Union -import jsonschema.exceptions +from jsonschema import Draft202012Validator import magic import pandas from frozendict import frozendict @@ -321,7 +320,7 @@ def parse_excel(path: Path) -> ParseResults: return _error(Error(ErrorType.PARSE_FAIL, "No non-header data in file", spcsrc)) -def parse_dts_manifest(path: Path, dts_manifest_schema: dict) -> ParseResults: +def parse_dts_manifest(path: Path, validator: Draft202012Validator) -> ParseResults: """ Parse the provided DTS manifest file. Expected to be JSON, and will fail otherwise. The manifest should have this format, with expected keys included: @@ -344,10 +343,7 @@ def parse_dts_manifest(path: Path, dts_manifest_schema: dict) -> ParseResults: and its value will be a Tuple of frozendicts of the parameters. Also, in keeping with the xsv parsers, each parameter value is expected to be a PRIMITIVE_TYPE. - Note that the dts_manifest_schema is expected to be valid, and may throw an - unexpected exception otherwise. - - TODO: include further details here, and in separate documentation - ADR? + TODO: include further details in separate documentation """ spcsrc = SpecificationSource(path) errors = [] @@ -358,7 +354,6 @@ def parse_dts_manifest(path: Path, dts_manifest_schema: dict) -> ParseResults: manifest_json = json.load(manifest) if not isinstance(manifest_json, dict): return _error(Error(ErrorType.PARSE_FAIL, "Manifest is not a dictionary", spcsrc)) - validator = jsonschema.Draft202012Validator(dts_manifest_schema) for err in validator.iter_errors(manifest_json): err_str = err.message err_path = list(err.absolute_path) diff --git a/tests/import_specifications/test_individual_parsers.py b/tests/import_specifications/test_individual_parsers.py index 3d9e4aeb..a719e5ea 100644 --- a/tests/import_specifications/test_individual_parsers.py +++ b/tests/import_specifications/test_individual_parsers.py @@ -4,7 +4,6 @@ import uuid from collections.abc import Callable, Generator from pathlib import Path -from typing import Any # TODO update to C impl when fixed: https://github.com/Marco-Sulla/python-frozendict/issues/26 from frozendict import frozendict @@ -21,6 +20,8 @@ ) from tests.test_app import FileUtil from tests.test_utils import bootstrap_config +from jsonschema import Draft202012Validator + _TEST_DATA_DIR = (Path(__file__).parent / "test_data").resolve() @@ -36,11 +37,11 @@ def temp_dir_fixture() -> Generator[Path, None, None]: @pytest.fixture(scope="module") -def dts_schema() -> Generator[dict[str, Any], None, None]: +def dts_validator() -> Generator[Draft202012Validator, None, None]: config = bootstrap_config() with open(config["staging_service"]["DTS_MANIFEST_SCHEMA"]) as dts_schema_file: schema = json.load(dts_schema_file) - yield schema + yield Draft202012Validator(schema) ########################################## @@ -778,9 +779,9 @@ def test_excel_parse_fail_unequal_rows(): ) -def test_dts_manifest_parse_success(dts_schema: dict[str, Any]): +def test_dts_manifest_parse_success(dts_validator: Draft202012Validator): f = _get_test_file("manifest_small.json") - res = parse_dts_manifest(f, dts_schema) + res = parse_dts_manifest(f, dts_validator) # fails for now assert res.results is None assert res.errors == tuple( @@ -805,25 +806,25 @@ def manifest_writer(input_json: dict | list) -> Path: return manifest_writer -def _dts_manifest_parse_fail(input_file: Path, schema: dict, errors: list[Error]): +def _dts_manifest_parse_fail(input_file: Path, validator: Draft202012Validator, errors: list[Error]): """ Tests a failing DTS manifest parse. input_file - the path to the input file. Might be a directory or not exist. errors - a list of Error objects expected to be in the order generated by the call to parse_dts_manifest. """ - res = parse_dts_manifest(input_file, schema) + res = parse_dts_manifest(input_file, validator) assert res.results is None assert res.errors == tuple(errors) -def test_dts_manifest_non_json(temp_dir: Generator[Path, None, None], dts_schema: dict[str, Any]): +def test_dts_manifest_non_json(temp_dir: Generator[Path, None, None], dts_validator: Draft202012Validator): test_file = temp_dir / str(uuid.uuid4()) with open(test_file, "w", encoding="utf-8") as outfile: outfile.write("totally not json") _dts_manifest_parse_fail( test_file, - dts_schema, + dts_validator, [ Error( ErrorType.PARSE_FAIL, "File must be in JSON format", SpecificationSource(test_file) @@ -833,12 +834,12 @@ def test_dts_manifest_non_json(temp_dir: Generator[Path, None, None], dts_schema def test_dts_manifest_non_dict( - write_dts_manifest: Callable[[dict | list], Path], dts_schema: dict[str, Any] + write_dts_manifest: Callable[[dict | list], Path], dts_validator: Draft202012Validator ): manifest_path = write_dts_manifest(["wrong_format"]) _dts_manifest_parse_fail( manifest_path, - dts_schema, + dts_validator, [ Error( ErrorType.PARSE_FAIL, @@ -849,23 +850,23 @@ def test_dts_manifest_non_dict( ) -def test_dts_manifest_not_found(temp_dir: Generator[Path, None, None], dts_schema: dict[str, Any]): +def test_dts_manifest_not_found(temp_dir: Generator[Path, None, None], dts_validator: Draft202012Validator): manifest_path = temp_dir / "not_a_file" _dts_manifest_parse_fail( manifest_path, - dts_schema, + dts_validator, [Error(ErrorType.FILE_NOT_FOUND, source_1=SpecificationSource(manifest_path))], ) def test_dts_manifest_file_is_directory( - temp_dir: Generator[Path, None, None], dts_schema: dict[str, Any] + temp_dir: Generator[Path, None, None], dts_validator: Draft202012Validator ): test_file = temp_dir / "testdir.json" os.makedirs(test_file, exist_ok=True) _dts_manifest_parse_fail( test_file, - dts_schema, + dts_validator, [ Error( ErrorType.PARSE_FAIL, @@ -876,20 +877,13 @@ def test_dts_manifest_file_is_directory( ) -@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) - - def test_dts_manifest_no_top_level_keys( - write_dts_manifest: Callable[[dict | list], Path], dts_schema: dict[str, Any] + write_dts_manifest: Callable[[dict | list], Path], dts_validator: Draft202012Validator ): manifest_path = write_dts_manifest({"missing": "stuff"}) _dts_manifest_parse_fail( manifest_path, - dts_schema, + dts_validator, [ Error( ErrorType.PARSE_FAIL, @@ -906,7 +900,7 @@ def test_dts_manifest_no_top_level_keys( def test_dts_manifest_fail_with_path( - write_dts_manifest: Callable[[dict | list], Path], dts_schema: dict[str, Any] + write_dts_manifest: Callable[[dict | list], Path], dts_validator: Draft202012Validator ): manifest_path = write_dts_manifest( { @@ -919,7 +913,7 @@ def test_dts_manifest_fail_with_path( ) _dts_manifest_parse_fail( manifest_path, - dts_schema, + dts_validator, [ Error( ErrorType.PARSE_FAIL, diff --git a/tests/test_app.py b/tests/test_app.py index 1f46171b..899c1c6a 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -4,6 +4,8 @@ import json import os import platform +import uuid +import jsonschema import pytest import shutil import string @@ -1264,6 +1266,45 @@ def test_bulk_specification_dts_fail_bad_schema(): pass +def test_load_and_validate_schema_good(): + # TODO: update this after updating how config is handled + schema_file = config["staging_service"]["DTS_MANIFEST_SCHEMA"] + validator = app.load_and_validate_schema(schema_file) + assert isinstance(validator, jsonschema.Draft202012Validator) + + +def test_load_and_validate_schema_missing_file(): + not_real_file = Path("not_real") + while not_real_file.exists(): + not_real_file = Path(str(uuid.uuid4())) + with pytest.raises(FileNotFoundError, match="No such file or directory"): + app.load_and_validate_schema(not_real_file) + + +def test_load_and_validate_schema_malformed_file(tmp_path: Path): + # TODO: migrate FileUtil and import_specifications.test_individual_parsers.temp_path_fixture + # into conftest.py, and resolve everywhere else that FileUtil gets used. + # Until then, the built-in tmp_path is appropriate for these tests + wrong_schema = "not valid json" + schema_file = tmp_path / f"{uuid.uuid4()}.json" + schema_file.write_text(wrong_schema, encoding="utf-8") + with pytest.raises(json.JSONDecodeError, match="Expecting value: line 1 column 1"): + app.load_and_validate_schema(schema_file) + + +def test_load_and_validate_schema_bad(tmp_path: Path): + invalid = { + "properties": { + "some_prop": { "type": "not_real"} + } + } + schema_file = tmp_path / f"{uuid.uuid4()}.json" + schema_file.write_text(json.dumps(invalid), encoding="utf-8") + exp_err = f"Schema file {schema_file} is not a valid JSON schema: 'not_real' is not valid" + with pytest.raises(Exception, match=exp_err): + app.load_and_validate_schema(schema_file) + + async def test_bulk_specification_fail_no_files(): async with AppClient(config) as cli: for f in ["", "?files=", "?files= , ,, , "]: From cb4b95a0287c36adb1c9827acffc7ab3025469db Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Wed, 11 Dec 2024 21:58:47 -0500 Subject: [PATCH 18/18] ruff formatting --- staging_service/app.py | 4 +++- .../import_specifications/test_individual_parsers.py | 12 +++++++++--- tests/test_app.py | 6 +----- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/staging_service/app.py b/staging_service/app.py index 14922cd6..4c34f841 100644 --- a/staging_service/app.py +++ b/staging_service/app.py @@ -614,7 +614,9 @@ def load_and_validate_schema(schema_path: PathPy) -> jsonschema.Draft202012Valid 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}") from err + raise Exception( + f"Schema file {schema_path} is not a valid JSON schema: {err.message}" + ) from err return jsonschema.Draft202012Validator(dts_schema) diff --git a/tests/import_specifications/test_individual_parsers.py b/tests/import_specifications/test_individual_parsers.py index a719e5ea..8c6c3af4 100644 --- a/tests/import_specifications/test_individual_parsers.py +++ b/tests/import_specifications/test_individual_parsers.py @@ -806,7 +806,9 @@ def manifest_writer(input_json: dict | list) -> Path: return manifest_writer -def _dts_manifest_parse_fail(input_file: Path, validator: Draft202012Validator, errors: list[Error]): +def _dts_manifest_parse_fail( + input_file: Path, validator: Draft202012Validator, errors: list[Error] +): """ Tests a failing DTS manifest parse. input_file - the path to the input file. Might be a directory or not exist. @@ -818,7 +820,9 @@ def _dts_manifest_parse_fail(input_file: Path, validator: Draft202012Validator, assert res.errors == tuple(errors) -def test_dts_manifest_non_json(temp_dir: Generator[Path, None, None], dts_validator: Draft202012Validator): +def test_dts_manifest_non_json( + temp_dir: Generator[Path, None, None], dts_validator: Draft202012Validator +): test_file = temp_dir / str(uuid.uuid4()) with open(test_file, "w", encoding="utf-8") as outfile: outfile.write("totally not json") @@ -850,7 +854,9 @@ def test_dts_manifest_non_dict( ) -def test_dts_manifest_not_found(temp_dir: Generator[Path, None, None], dts_validator: Draft202012Validator): +def test_dts_manifest_not_found( + temp_dir: Generator[Path, None, None], dts_validator: Draft202012Validator +): manifest_path = temp_dir / "not_a_file" _dts_manifest_parse_fail( manifest_path, diff --git a/tests/test_app.py b/tests/test_app.py index 899c1c6a..07d167db 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -1293,11 +1293,7 @@ def test_load_and_validate_schema_malformed_file(tmp_path: Path): def test_load_and_validate_schema_bad(tmp_path: Path): - invalid = { - "properties": { - "some_prop": { "type": "not_real"} - } - } + invalid = {"properties": {"some_prop": {"type": "not_real"}}} schema_file = tmp_path / f"{uuid.uuid4()}.json" schema_file.write_text(json.dumps(invalid), encoding="utf-8") exp_err = f"Schema file {schema_file} is not a valid JSON schema: 'not_real' is not valid"