Skip to content

Improve reporting of deserializer validation errors #550

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

Draft
dbutenhof wants to merge 2 commits into
base: main
Choose a base branch
from
+14 −10
Draft

Improve reporting of deserializer validation errors #550

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
72d2554
Improve reporting of validation errors
dbutenhof Jan 21, 2026
21240f0
Merge branch 'main' into fix/badparam
dbutenhof Jan 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
24 changes: 14 additions & 10 deletions src/guidellm/__main__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,17 +462,21 @@ def run(**kwargs): # noqa: C901

if uvloop is not None:
asyncio.set_event_loop_policy(uvloop.EventLoopPolicy())
asyncio.run(
benchmark_generative_text(
args=args,
progress=(
GenerativeConsoleBenchmarkerProgress()
if not disable_console_interactive
else None
),
console=console,

try:
asyncio.run(
benchmark_generative_text(
args=args,
progress=(
GenerativeConsoleBenchmarkerProgress()
if not disable_console_interactive
else None
),
console=console,
)
)
)
except (ValidationError, ValueError) as err:
Copy link
Collaborator

@sjmonson sjmonson Jan 22, 2026

Choose a reason for hiding this comment

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

My biggest concern here is we lose debugging information for a large class of errors. ValueError is especially generic and could be emitted in any number of places. Maybe instead we define a custom error type for these runtime argument errors and catch that specific exception here?

Copy link
Collaborator Author

@dbutenhof dbutenhof Jan 22, 2026

Choose a reason for hiding this comment

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

Hmm... yeah, it actually looks like ValueError is used a lot more than I would have expected. And if any of them can occur after "initial startup" we wouldn't want to hide the traceback. So much for a "simple touch", but it was in any case an interesting excursion.

Yeah, we could use another exception for "static startup validation" errors. Perhaps a better solution would be to refactor the deserializers with a validation method that can be called during option parsing rather than let this sort of thing wait until run. And that'll require a lot more thought ...

raise click.BadParameter(str(err)) from err


@benchmark.command(
Expand Down