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

Conversation

aybruhm
Copy link
Member

@aybruhm aybruhm commented Dec 2, 2023

Description

This PR achieves the following:

  • Creates a dummy app and testset (not visible to users)
  • Loads the dummy testset at application restart
  • Implements backend logic to assign the dummy test set to evaluations (in use)
  • Utilizes alertPopup component to remove testset(s) in the UI, etc

Related Issue

Closes #863 x #1005

@aybruhm aybruhm changed the title Fix: Resolving Internal Server Error Triggered by Testset Removal in Evaluation Page [Bug Fix]: Resolves Internal Server Error Triggered by Testset Removal in Evaluation Page Dec 2, 2023
@aybruhm
Copy link
Member Author

aybruhm commented Dec 4, 2023

@bekossy could you help run prettier, commit and push? Thank you.

@aybruhm aybruhm requested a review from mmabrouk December 4, 2023 06:52
@aybruhm aybruhm marked this pull request as ready for review December 4, 2023 06:52
@aybruhm aybruhm requested a review from MohammedMaaz December 12, 2023 08:28
@aybruhm aybruhm assigned aybruhm and bekossy and unassigned aybruhm and bekossy Dec 12, 2023
@aakrem
Copy link
Collaborator

aakrem commented Dec 13, 2023

@aybruhm could this create a conflict with the default testsets ?

@aybruhm
Copy link
Member Author

aybruhm commented Dec 13, 2023

@aybruhm could this create a conflict with the default testsets ?

No, it will not.

Copy link
Collaborator

@aakrem aakrem left a comment

Choose a reason for hiding this comment

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

@aybruhm

  • I would also give the option to delete the corresponding evaluations.
  • The modal text should be improved to better mention that the evaluation will be referenced to an empty testset instead of dummy.

Approving for now. Please consider improving these two points.

@aakrem
Copy link
Collaborator

aakrem commented Dec 18, 2023

getting internal server error when deleting the testset

@aakrem
Copy link
Collaborator

aakrem commented Dec 21, 2023

closing this as, as agreed, we want to prevent deleting a testset instead of creating dummy one.
Re-open if needed

@aakrem aakrem closed this Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Removing a Testset that is referenced by an Evaluation Model causes an Internal Server Error
5 participants