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

add 'complete' message to report at end of run #735

Merged
merged 2 commits into from
Jun 17, 2024
Merged

Conversation

leondz
Copy link
Owner

@leondz leondz commented Jun 13, 2024

resolves #726

@leondz leondz added the reporting Reporting, analysis, and other per-run result functions label Jun 13, 2024
@leondz leondz requested a review from jmartin-tech June 13, 2024 18:46
garak/command.py Outdated
@@ -117,7 +123,7 @@ def end_run():
print(f"📜 report html summary being written to {digest_filename}")
try:
write_report_digest(_config.transient.report_filename, digest_filename)
except Exception as e:
except BaseException as e:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why BaseException?

Catching Exception is very broad already, this only adds GeneratorExit, KeyboardInterrupt and SystemExit as far as I can tell all of which I would expect to immediately terminate vs being able to recover in any meaningful way.

Copy link
Owner Author

Choose a reason for hiding this comment

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

reverted.

Would like to scope this down, actually - I am not sure what is being caught here, because Exception is an uninformative signal

@jmartin-tech jmartin-tech merged commit 05f6734 into main Jun 17, 2024
6 checks passed
@jmartin-tech jmartin-tech deleted the feature/done_msg branch June 17, 2024 13:25
@github-actions github-actions bot locked and limited conversation to collaborators Jun 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
reporting Reporting, analysis, and other per-run result functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add "done" entry to report.jsonl
2 participants