Skip to content
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.

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

[Bug Fix]: Resolves Internal Server Error Triggered by Testset Removal in Evaluation Page #986

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
751af34
Feat - created confirm testset delete modal component
aybruhm Dec 2, 2023
226d444
Update - added confirm testset delete modal to testsets/index.tsx page
aybruhm Dec 2, 2023
8396afb
Feat: Added axios function for converting testsets to dummy if in use
aybruhm Dec 2, 2023
b377ba5
:art: Format - ran format-fix
aybruhm Dec 2, 2023
2448a50
Update - added is_visible field to app and testset db models
aybruhm Dec 4, 2023
17c6507
Feat - created db functions to create dummy testset, app, etc
aybruhm Dec 4, 2023
90e36e5
Feat - implemented assign dummy testset to evaluations service
aybruhm Dec 4, 2023
e139c85
Update - created api model for TestsetsToConvert
aybruhm Dec 4, 2023
2a570a9
Feat - created convert testsets to dummy api router
aybruhm Dec 4, 2023
06c90c8
Update - refactor conver testsets to dummy if in use api (axios) logic
aybruhm Dec 4, 2023
e06bafe
Update - added db function - create_dummy_testset to application life…
aybruhm Dec 4, 2023
59b7928
Update - modified handleConfirmOK async function
aybruhm Dec 4, 2023
14ba043
:art: Format - ran black and format-fix
aybruhm Dec 4, 2023
1dc216b
format fix
bekossy Dec 4, 2023
b5bb693
Merge branch 'main' into gh/bug-removing-a-testset-internalservererror
bekossy Dec 5, 2023
bdcadc3
Refactor - removed convert testset(s) to dummy api router and axios l…
aybruhm Dec 12, 2023
7b09b48
Update - remove confirm testset delete modal component
aybruhm Dec 12, 2023
06f167f
Refactor & Cleanup - make use of alertpopup component for testset del…
aybruhm Dec 12, 2023
dfb9afe
:art: Format - ran prettier --write .
aybruhm Dec 12, 2023
8264dad
Update - remove className from div element to resolve build error
aybruhm Dec 12, 2023
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
4 changes: 3 additions & 1 deletion agenta-backend/agenta_backend/main.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import os
from contextlib import asynccontextmanager

from agenta_backend.config import settings
from agenta_backend.services import db_manager
from agenta_backend.routers import (
app_router,
container_router,
Expand Down Expand Up @@ -41,6 +41,8 @@ async def lifespan(application: FastAPI, cache=True):
application: FastAPI application.
cache: A boolean value that indicates whether to use the cached data or not.
"""

await db_manager.create_dummy_testset()
await templates_manager.update_and_sync_templates(cache=cache)
yield

Expand Down
4 changes: 4 additions & 0 deletions agenta-backend/agenta_backend/models/api/testset_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ class DeleteTestsets(BaseModel):
testset_ids: List[str]


class TestsetsToConvert(DeleteTestsets):
pass


# The NewTestset class represents a new data set.
# Each row is a dictionary with column names as keys and column values as values.
# csvdata = [
Expand Down
2 changes: 2 additions & 0 deletions agenta-backend/agenta_backend/models/db_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ class AppDB(Model):
app_name: str
organization: OrganizationDB = Reference(key_name="organization")
user: UserDB = Reference(key_name="user")
is_visible: bool = Field(default=True)
created_at: Optional[datetime] = Field(default=datetime.utcnow())
updated_at: Optional[datetime] = Field(default=datetime.utcnow())

Expand Down Expand Up @@ -184,6 +185,7 @@ class TestSetDB(Model):
name: str
app: AppDB = Reference(key_name="app")
csvdata: List[Dict[str, str]]
is_visible: bool = Field(default=True)
user: UserDB = Reference(key_name="user")
organization: OrganizationDB = Reference(key_name="organization")
created_at: Optional[datetime] = Field(default=datetime.utcnow())
Expand Down
12 changes: 8 additions & 4 deletions agenta-backend/agenta_backend/routers/testset_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@
from agenta_backend.models.api.testset_model import (
TestSetSimpleResponse,
DeleteTestsets,
TestsetsToConvert,
NewTestset,
TestSetOutputResponse,
)
from agenta_backend.utils.common import engine, check_access_to_app
from agenta_backend.models.db_models import TestSetDB
from agenta_backend.services.db_manager import get_user
from agenta_backend.services import db_manager
from agenta_backend.services import db_manager, evaluation_service
from agenta_backend.models.converters import testset_db_to_pydantic

upload_folder = "./path/to/upload/folder"
Expand Down Expand Up @@ -369,7 +370,7 @@ async def get_testset(

@router.delete("/", response_model=List[str])
async def delete_testsets(
delete_testsets: DeleteTestsets,
payload: DeleteTestsets,
request: Request,
):
"""
Expand All @@ -381,11 +382,14 @@ async def delete_testsets(
Returns:
A list of the deleted testsets' IDs.
"""

user_org_data: dict = await get_user_and_org_id(request.state.user_id)
await evaluation_service.assign_dummy_testset_to_evaluations(
request.state.user_id, payload.testset_ids
)

deleted_ids = []

for testset_id in delete_testsets.testset_ids:
for testset_id in payload.testset_ids:
test_set = await db_manager.fetch_testset_by_id(testset_id=testset_id)
if test_set is None:
raise HTTPException(status_code=404, detail="testset not found")
Expand Down
95 changes: 90 additions & 5 deletions agenta-backend/agenta_backend/services/db_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,17 @@ async def fetch_app_by_name(
"""
if not organization_id:
user = await get_user(user_uid=user_org_data["uid"])
query_expression = (AppDB.app_name == app_name) & (AppDB.user == user.id)
query_expression = (
(AppDB.app_name == app_name)
& (AppDB.user == user.id)
& (AppDB.is_visible == True)
)
app = await engine.find_one(AppDB, query_expression)
else:
query_expression = (AppDB.app_name == app_name) & (
AppDB.organization == ObjectId(organization_id)
query_expression = (
(AppDB.app_name == app_name)
& (AppDB.organization == ObjectId(organization_id))
& (AppDB.is_visible == True)
)
app = await engine.find_one(AppDB, query_expression)
return app
Expand Down Expand Up @@ -716,6 +722,20 @@ async def get_app_instance_by_id(app_id: str) -> AppDB:
return app


async def get_app_instance_by_name(app_name: str) -> AppDB:
"""Get the app object from the database with the provided app name.

Arguments:
app_name (str): The app name

Returns:
AppDB: instance of app object
"""

app = await engine.find_one(AppDB, AppDB.app_name == app_name)
return app


async def add_variant_from_base_and_config(
base_db: VariantBaseDB,
new_config_name: str,
Expand Down Expand Up @@ -800,7 +820,8 @@ async def list_apps(
organization_access = await check_user_org_access(user_org_data, org_id)
if organization_access:
apps: List[AppDB] = await engine.find(
AppDB, AppDB.organization == ObjectId(org_id)
AppDB,
*(AppDB.organization == ObjectId(org_id), AppDB.is_visible == True),
)
return [app_db_to_pydantic(app) for app in apps]

Expand All @@ -811,7 +832,9 @@ async def list_apps(
)

else:
apps: List[AppVariantDB] = await engine.find(AppDB, AppDB.user == user.id)
apps: List[AppVariantDB] = await engine.find(
AppDB, *(AppDB.user == user.id, AppDB.is_visible == True)
)
return [app_db_to_pydantic(app) for app in apps]


Expand Down Expand Up @@ -1253,6 +1276,21 @@ async def fetch_testsets_by_app_id(app_id: str) -> List[TestSetDB]:
return testsets


async def fetch_user_list_evaluations_db(user_id: str) -> List[EvaluationDB]:
"""Fetches a list of evaluations for the given user.

Args:
user_id (str): The user Id

Returns:
List[EvaluationDB]: A list of evaluations.
"""

user = await get_user(user_id)
evaluations_db = await engine.find(EvaluationDB, EvaluationDB.user == user.id)
return evaluations_db


async def fetch_evaluation_by_id(evaluation_id: str) -> Optional[EvaluationDB]:
"""Fetches a evaluation by its ID.
Args:
Expand Down Expand Up @@ -1431,6 +1469,53 @@ async def get_templates() -> List[Template]:
return templates_db_to_pydantic(templates)


async def create_dummy_app() -> None:
"""Creates a dummy app."""

# reason for this is because "0" will serve as the default
# user that the dummy app will belong to
user_db = await get_user(user_uid="0")
org_db = await get_organization_object(str(user_db.organizations[0]))
app_db = AppDB(
app_name="dummy_app", organization=org_db, user=user_db, is_visible=False
)
await engine.save(app_db)


async def get_dummy_testset() -> TestSetDB:
"""Fetch the dummy testset."""

testset = await engine.find_one(TestSetDB, TestSetDB.name == "dummy_testset")
return testset


async def create_dummy_testset() -> None:
"""Creates a dummy testset."""

dummy_app = await get_app_instance_by_name("dummy_app")
if dummy_app is None:
print("============= Step 1: Create dummy app. =============")
await create_dummy_app()

testset = await engine.find_one(TestSetDB, TestSetDB.name == "dummy_testset")
if testset is None:
print("============= Step 2: Create dummy testset. =============")
app_db = await get_app_instance_by_name("dummy_app")
testset = {
"name": "dummy_testset",
"created_at": datetime.now().isoformat(),
"csvdata": [{}],
}
testset = TestSetDB(
**testset,
app=app_db,
is_visible=False,
user=app_db.user,
organization=app_db.organization,
)
await engine.save(testset)


async def count_apps(**user_org_data: dict) -> int:
"""
Counts all the unique app names from the database
Expand Down
21 changes: 21 additions & 0 deletions agenta-backend/agenta_backend/services/evaluation_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,27 @@ async def fetch_list_evaluations(
]


async def assign_dummy_testset_to_evaluations(
user_id: str, testsets: List[str]
) -> None:
"""Assigns a dummy testset to evaluations in use of testset(s).

Args:
user_id (str): The user Id
testsets (List[str]): The id of the testsets in use
"""

evaluations = await db_manager.fetch_user_list_evaluations_db(user_id)
dummy_testset = await db_manager.get_dummy_testset()
for evaluation in evaluations:
evaluation_testset_id = str(evaluation.testset.id)
if evaluation_testset_id in testsets:
evaluation.testset = dummy_testset
await engine.save(evaluation)
logger.info("Updated evaluation in use of testset to dummy testset.")
logger.info("Completed assigning dummy testset to evaluations.")


async def fetch_evaluation(evaluation_id: str, **user_org_data: dict) -> Evaluation:
"""
Fetches a single evaluation based on its ID.
Expand Down
30 changes: 24 additions & 6 deletions agenta-web/src/pages/apps/[app_id]/testsets/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {deleteTestsets, useLoadTestsetsList} from "@/lib/services/api"
import {createUseStyles} from "react-jss"
import {testset} from "@/lib/Types"
import {isDemo} from "@/lib/helpers/utils"
import AlertPopup from "@/components/AlertPopup/AlertPopup"

const useStyles = createUseStyles({
container: {
Expand Down Expand Up @@ -69,12 +70,29 @@ export default function Testsets() {
}

const onDelete = async () => {
const testsetsIds = selectedRowKeys.map((key) => key.toString())
try {
await deleteTestsets(testsetsIds)
mutate()
setSelectedRowKeys([])
} catch {}
const testsetIds = selectedRowKeys.map((key) => key.toString())
AlertPopup({
title: "Confirm Testset(s) Deletion",
message: (
<div>
<p data-cy="testset-name-reqd-error">
Deleting these testset(s) will change the evaluation in use of it/them to
use a dummy testset. Are you sure you want to proceed?
</p>
</div>
),
okButtonProps: {
type: "primary",
danger: true,
},
onOk: async () => {
try {
await deleteTestsets(testsetIds)
mutate()
setSelectedRowKeys([])
} catch {}
},
})
}

return (
Expand Down
Loading