-
Notifications
You must be signed in to change notification settings - Fork 110
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
Better error message when experiment is terminated by user #10158
base: main
Are you sure you want to change the base?
Conversation
CodSpeed Performance ReportMerging #10158 will not alter performanceComparing Summary
|
9c2e575
to
c0158cd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some questions 🤔
9946c2c
to
7d90b91
Compare
@@ -598,7 +605,8 @@ async def run_ensemble_evaluator_async( | |||
if not self._end_queue.empty(): | |||
logger.debug("Run model canceled - pre evaluation") | |||
self._end_queue.get() | |||
return [] | |||
raise UserCancelled("Experiment cancelled by user in pre evaluation") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we have tests for these different scenarios? (cancelling pre evaluation, during evaluation, and post evaluation)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need specific tests for that 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you manually tested that this works?
7d90b91
to
1f5b386
Compare
Issue
Resolves #9960
Approach
Short description of the approach
(Screenshot of new behavior in GUI if applicable)
git rebase -i main --exec 'just rapid-tests'
)When applicable