Skip to content

add extra error messages and fix response schema #253

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.

Sign up for GitHub

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

Open
Mahhheshh wants to merge 2 commits into
base: main
Choose a base branch
from
Open

add extra error messages and fix response schema #253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
696717a
add extra error messages and fix response schema
Mahhheshh Feb 22, 2026
a7a942c
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/core/errors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,10 @@ class DatasetError(IntEnum):
NOT_FOUND = 111
NO_ACCESS = 112
NO_DATA_FILE = 113


class QualityError(IntEnum):
UNKNOWN_DATASET = 361
NO_QUALITIES = 362
NOT_PROCESSED = 363
PROCESSED_WITH_ERROR = 364
42 changes: 31 additions & 11 deletions src/routers/openml/qualities.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import database.datasets
import database.qualities
from core.access import _user_has_access
from core.errors import DatasetError
from core.errors import QualityError
from database.users import User
from routers.dependencies import expdb_connection, fetch_user
from schemas.datasets.openml import Quality
Expand All @@ -32,18 +32,38 @@ def get_qualities(
dataset_id: int,
user: Annotated[User | None, Depends(fetch_user)],
expdb: Annotated[Connection, Depends(expdb_connection)],
) -> list[Quality]:
) -> dict[Literal["data_qualities"], dict[Literal["quality"], list[Quality]]]:
dataset = database.datasets.get(dataset_id, expdb)
if not dataset or not _user_has_access(dataset, user):
raise HTTPException(
status_code=HTTPStatus.PRECONDITION_FAILED,
detail={"code": DatasetError.NO_DATA_FILE, "message": "Unknown dataset"},
detail={"code": QualityError.UNKNOWN_DATASET, "message": "Unknown dataset"},
) from None
return database.qualities.get_for_dataset(dataset_id, expdb)
# The PHP API provided (sometime) helpful error messages
# if not qualities:
# check if dataset exists: error 360
# check if user has access: error 361
# check if there is a data processed entry and forward the error: 364
# if nothing in process table: 363
# otherwise: error 362

processing = database.datasets.get_latest_processing_update(dataset_id, expdb)
if processing is None:
raise HTTPException(
status_code=HTTPStatus.PRECONDITION_FAILED,
detail={"code": QualityError.NOT_PROCESSED, "message": "Dataset not processed yet"},
)
if processing.error:
raise HTTPException(
status_code=HTTPStatus.PRECONDITION_FAILED,
detail={
"code": QualityError.PROCESSED_WITH_ERROR,
"message": "Dataset processed with error",
},
)

qualities = database.qualities.get_for_dataset(dataset_id, expdb)
if not qualities:
raise HTTPException(
status_code=HTTPStatus.PRECONDITION_FAILED,
detail={"code": QualityError.NO_QUALITIES, "message": "No qualities found"},
)

return {
"data_qualities": {
"quality": qualities,
},
}
3 changes: 1 addition & 2 deletions tests/routers/openml/qualities_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,9 +306,8 @@ def test_get_quality_identical_error(
py_api: TestClient,
php_api: httpx.Client,
) -> None:
if data_id in [55, 56, 59]:
pytest.skip("Detailed error for code 364 (failed processing) not yet supported.")
if data_id in [116]:
# skipping 116 is not valid case for 362
pytest.skip("Detailed error for code 362 (no qualities) not yet supported.")
Comment on lines 309 to 311
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Misleading skip message and unnecessary list literal in in-check.

Two nits:

  1. The skip message says "not yet supported" but the PR TODO clarifies that 116 is simply not a valid dataset for testing error code 362 — the feature is implemented, only the fixture is missing. The message should reflect reality.
  2. data_id in [116] constructs a throwaway list for a single-element membership test; prefer data_id == 116.
♻️ Suggested fix 
-    if data_id in [116]:
-        # skipping 116 is not valid case for 362
-        pytest.skip("Detailed error for code 362 (no qualities) not yet supported.")
+    if data_id == 116:
+        # 116 is not a valid fixture for error code 362; skip until a suitable dataset_id is identified
+        pytest.skip("No suitable dataset_id for error code 362 (NO_QUALITIES) identified yet.")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if data_id in [116]:
# skipping 116 is not valid case for 362
pytest.skip("Detailed error for code 362 (no qualities) not yet supported.")
if data_id == 116:
# 116 is not a valid fixture for error code 362; skip until a suitable dataset_id is identified
pytest.skip("No suitable dataset_id for error code 362 (NO_QUALITIES) identified yet.")
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/routers/openml/qualities_test.py` around lines 309 - 311, Update the
test skip to use a direct equality check and a truthful message: replace the
membership test `data_id in [116]` with `data_id == 116`, and change the
pytest.skip message called in the same block to say that dataset 116 is not a
valid dataset for testing error code 362 (fixture missing) rather than "not yet
supported" so it accurately reflects the TODO note; keep the skip invocation
using pytest.skip and leave surrounding test logic unchanged.

php_response = php_api.get(f"/data/qualities/{data_id}")
python_response = py_api.get(f"/datasets/qualities/{data_id}")
Expand Down