diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1661fab3..2200fe99 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -98,6 +98,15 @@ jobs: pip install -r requirements-full.txt pip install . + # Fails fast with a clear per-model report if a test fixture model is + # missing or inaccessible, instead of burning the whole matrix on a + # cascade of individual test failures deep into the run. + - name: Check model registry access + run: python scripts/check_model_registry_access.py + env: + WRANGLES_USER: ${{ secrets.WRANGLES_USER }} + WRANGLES_PASSWORD: ${{ secrets.WRANGLES_PASSWORD }} + - name: Run Tests run: pytest env: diff --git a/pytest-local.ini b/pytest-local.ini index 11c64814..cef53cd0 100644 --- a/pytest-local.ini +++ b/pytest-local.ini @@ -2,6 +2,7 @@ testpaths = tests/test_ai_cache.py tests/test_ai_definition.py + tests/test_check_model_registry_access.py tests/test_container_smoke.py tests/test_data.py tests/test_dataframe.py diff --git a/scripts/check_model_registry_access.py b/scripts/check_model_registry_access.py new file mode 100644 index 00000000..edae845f --- /dev/null +++ b/scripts/check_model_registry_access.py @@ -0,0 +1,79 @@ +""" +Checks access to every model in tests/model_registry.py before the +credentialed test suite runs. + +Run this as step one of a live/credentialed test session (it needs +WRANGLES_USER/WRANGLES_PASSWORD or another valid auth path - do not run it +under pytest-local.ini's offline config, which deliberately has no +credentials). Exits non-zero if any model is missing or inaccessible, so a +broken model fixture fails fast with a clear report instead of surfacing as +scattered individual test failures deep into a run. + + python scripts/check_model_registry_access.py +""" +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from wrangles import data as _data +from tests.model_registry import MODEL_REGISTRY + + +def check_all(registry: dict) -> tuple[list, list, list]: + """ + Check access to every model in the registry. + + :return: (accessible, not_found, denied_or_errored) - each a list of + (model_id, note, detail) tuples. + """ + accessible = [] + not_found = [] + denied_or_errored = [] + + for model_id, note in sorted(registry.items()): + try: + _data.model(model_id) + accessible.append((model_id, note, None)) + except _data.ModelNotFoundError as e: + not_found.append((model_id, note, str(e))) + except Exception as e: + denied_or_errored.append((model_id, note, str(e))) + + return accessible, not_found, denied_or_errored + + +def main() -> int: + print(f"Checking access to {len(MODEL_REGISTRY)} registered models...\n") + + accessible, not_found, denied_or_errored = check_all(MODEL_REGISTRY) + + print(f"Accessible: {len(accessible)}/{len(MODEL_REGISTRY)}") + + if not_found: + print(f"\nNOT FOUND ({len(not_found)}) - model no longer exists, " + "tests referencing it need a replacement model or removal:") + for model_id, note, detail in not_found: + print(f" - {model_id} ({note})\n {detail}") + + if denied_or_errored: + print(f"\nACCESS DENIED / ERROR ({len(denied_or_errored)}) - the " + "test-runner account needs access granted, or this ID is " + "stale/mocked-only and should be removed from the registry:") + for model_id, note, detail in denied_or_errored: + print(f" - {model_id} ({note})\n {detail}") + + if not_found or denied_or_errored: + print( + "\nFix these before running the credentialed test suite - each " + "one will otherwise surface as a confusing failure inside " + "whichever test happens to hit it first, instead of here." + ) + return 1 + + print("\nAll registered models are accessible.") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/model_registry.py b/tests/model_registry.py new file mode 100644 index 00000000..89875963 --- /dev/null +++ b/tests/model_registry.py @@ -0,0 +1,66 @@ +""" +Registry of model IDs referenced by the WranglesPY test suite. + +scripts/check_model_registry_access.py checks access to every model +listed here before the credentialed test suite runs, so a missing or +inaccessible model fails fast with a clear report instead of causing +a cascade of individual test failures deep into the run. + +This file was seeded by scanning tests/**/test_*.py for real-looking +model IDs (8-4-4 hex, excluding obvious placeholders like +00000000-0000-0000). It needs human review: remove any ID that is +only ever used against a mocked wrangles.data.model/model_content - +those never hit the network and don't belong in an access-precheck. +""" + +MODEL_REGISTRY = { + "05f6bb73-de04-4cb6": "tests/recipes/wrangles/test_extract.py", + "083ed6fe-a073-4b1a": "tests/connectors/test_train.py", + "0e81f1ad-c0a3-42b4": "tests/recipes/wrangles/test_extract.py (+1 more files)", + "12b7ac66-7418-45b5": "tests/connectors/test_train.py", + "1ac1dddb-fcb5-45f3": "tests/recipes/wrangles/test_main.py", + "1b41d016-7129-4b66": "tests/recipes/test_recipes.py", + "1e13e845-bc3f-4b27": "tests/connectors/test_recipe.py (+2 more files)", + "1eddb7e8-1b2b-4a52": "tests/recipes/wrangles/test_extract.py (+1 more files)", + "1f3ba62b-ce20-486e": "tests/recipes/wrangles/test_extract.py", + "3c8f6707-2de4-4be3": "tests/connectors/test_train.py", + "41789e35-eada-4239": "tests/connectors/test_train.py", + "4202c974-430a-46b9": "tests/connectors/test_train.py", + "42f319a8-0849-4177": "tests/connectors/test_recipe.py (+1 more files)", + "5313d577-0bb6-4174": "tests/connectors/test_train.py", + "6ca4ab44-8c66-40e8": "tests/recipes/wrangles/test_extract.py (+2 more files)", + "6e97bb6c-bfab-402b": "tests/recipes/wrangles/test_main.py", + "73d89595-e5c9-40a4": "tests/recipes/wrangles/test_extract.py", + "829c1a73-1bfd-4ac0": "tests/recipes/wrangles/test_extract.py", + "89637e77-7214-49a0": "tests/connectors/test_train.py", + "8dd00032-d8bb-400c": "tests/recipes/wrangles/test_extract.py", + "8e4ce4c6-9908-4f67": "tests/recipes/wrangles/test_extract.py", + # NOTE: 93d92b4c-9f49-4ff5 (tests/test_data.py) intentionally omitted - + # confirmed mocked-only (FakeResponse harness, never hits the network). + "94674750-f9e1-44af": "tests/connectors/test_train.py", + "a62c7480-500e-480c": "tests/recipes/test_recipes.py (+2 more files)", + "b2cd1a8a-4d99-4be1": "tests/connectors/test_train.py", + # NOTE: bc3ee6a0-e104-4700 (tests/connectors/test_train.py) intentionally + # omitted - confirmed mocked-only (test_missing_columns_error_message + # mocks wrangles.data.model/model_content directly). + "c37af8a6-43d8-4127": "tests/recipes/test_recipes.py", + "c3e6715a-6214-4517": "tests/recipes/wrangles/test_extract.py", + "d168c456-514f-4513": "tests/recipes/wrangles/test_extract.py", + "d188e7a7-9de8-4565": "tests/connectors/test_train.py", + "d7c8270d-f15a-4c9c": "tests/recipes/wrangles/test_extract.py", + "e8658a6f-c694-45d0": "tests/connectors/test_train.py (+2 more files)", + "e954717c-fb9c-4c47": "tests/recipes/test_recipes.py", + "ee320e2b-ccda-47ed": "tests/connectors/test_train.py", + "ee5f020e-d88e-4bd5": "tests/connectors/test_train.py", + "fc7d46e3-057f-47bd": "tests/connectors/test_http.py (+2 more files)", + "fce592c9-26f5-4fd7": "tests/recipes/test_recipes.py (+1 more files)", + "fe730444-1bda-4fcd": "tests/recipes/wrangles/test_main.py (+1 more files)", + # NOTE: fe885889-67f2-4f3a (tests/recipes/test_recipes.py) intentionally + # omitted - not a model_id at all. It's the first three groups of the + # longer version_id fe885889-67f2-4f3a-b33a-1a37ff5c243c, used as + # "c37af8a6-43d8-4127:fe885889-67f2-4f3a-b33a-1a37ff5c243c" in + # test_recipe_by_version_id. The extraction regex (8-4-4 hex) matched + # inside it by accident - confirmed by the checker correctly reporting + # it as inaccessible on its first live CI run. +} + diff --git a/tests/test_check_model_registry_access.py b/tests/test_check_model_registry_access.py new file mode 100644 index 00000000..5e36eefc --- /dev/null +++ b/tests/test_check_model_registry_access.py @@ -0,0 +1,68 @@ +import pytest + +from wrangles import data +from scripts import check_model_registry_access as checker + + +class FakeResponse: + def __init__(self, status_code): + self.status_code = status_code + self.ok = 200 <= status_code < 300 + + def json(self): + return {"id": "some-model"} + + +def _mock_status(monkeypatch, model_id_to_status: dict): + def request_retries(**kwargs): + model_id = kwargs["params"]["id"] + return FakeResponse(model_id_to_status[model_id]) + + monkeypatch.setattr(data._auth, "get_access_token", lambda: "token") + monkeypatch.setattr(data._utils, "request_retries", request_retries) + + +def test_check_all_categorizes_accessible_not_found_and_denied(monkeypatch): + registry = { + "aaaaaaaa-0000-0000": "accessible model", + "bbbbbbbb-0000-0000": "deleted model", + "cccccccc-0000-0000": "forbidden model", + } + _mock_status(monkeypatch, { + "aaaaaaaa-0000-0000": 200, + "bbbbbbbb-0000-0000": 404, + "cccccccc-0000-0000": 403, + }) + + accessible, not_found, denied_or_errored = checker.check_all(registry) + + assert [m for m, _, _ in accessible] == ["aaaaaaaa-0000-0000"] + assert [m for m, _, _ in not_found] == ["bbbbbbbb-0000-0000"] + assert [m for m, _, _ in denied_or_errored] == ["cccccccc-0000-0000"] + + +def test_check_all_empty_registry_returns_empty_results(): + accessible, not_found, denied_or_errored = checker.check_all({}) + assert accessible == [] + assert not_found == [] + assert denied_or_errored == [] + + +def test_main_returns_zero_when_all_accessible(monkeypatch): + _mock_status(monkeypatch, {"aaaaaaaa-0000-0000": 200}) + monkeypatch.setattr(checker, "MODEL_REGISTRY", {"aaaaaaaa-0000-0000": "note"}) + + assert checker.main() == 0 + + +def test_main_returns_nonzero_when_any_model_inaccessible(monkeypatch): + _mock_status(monkeypatch, { + "aaaaaaaa-0000-0000": 200, + "bbbbbbbb-0000-0000": 404, + }) + monkeypatch.setattr(checker, "MODEL_REGISTRY", { + "aaaaaaaa-0000-0000": "note", + "bbbbbbbb-0000-0000": "note", + }) + + assert checker.main() == 1 diff --git a/tests/test_data.py b/tests/test_data.py index dafb26ba..e3fa414b 100644 --- a/tests/test_data.py +++ b/tests/test_data.py @@ -62,6 +62,26 @@ def test_model_endpoints_raise_authorization_error_for_403(monkeypatch, call_mod ) +@pytest.mark.parametrize( + "call_model_endpoint", + [ + lambda: data.model(MODEL_ID), + lambda: data.model_update(MODEL_ID, {"name": "Updated model"}), + lambda: data.model_content(MODEL_ID), + ], +) +def test_model_endpoints_raise_not_found_error_for_404(monkeypatch, call_model_endpoint): + _mock_model_response(monkeypatch, FakeResponse(404)) + + with pytest.raises(data.ModelNotFoundError) as info: + call_model_endpoint() + + assert isinstance(info.value, RuntimeError) + assert str(info.value) == ( + f"Model {MODEL_ID} was not found. Check the model id is correct." + ) + + def test_model_success_returns_metadata(monkeypatch): metadata = {"id": MODEL_ID, "name": "Model"} _mock_model_response(monkeypatch, FakeResponse(200, metadata)) diff --git a/wrangles/data.py b/wrangles/data.py index 25b1eb37..b2ebbb57 100644 --- a/wrangles/data.py +++ b/wrangles/data.py @@ -14,6 +14,10 @@ class AuthorizationError(RuntimeError): pass +class ModelNotFoundError(RuntimeError): + pass + + def _raise_model_response_error(response, id: str, action: str) -> None: if response.status_code == 401: raise AuthenticationError( @@ -24,6 +28,10 @@ def _raise_model_response_error(response, id: str, action: str) -> None: raise AuthorizationError( f"Access denied to model {id}. Check the user's model permissions." ) + if response.status_code == 404: + raise ModelNotFoundError( + f"Model {id} was not found. Check the model id is correct." + ) raise RuntimeError(f'Something went wrong trying to {action} model {id}')