feat(sdk): add BenchmarkRun and AsyncBenchmarkRun classes #712
Conversation
james-rl
left a comment
james-rl
left a comment
There was a problem hiding this comment.
There's a few areas of potential inconsistency or where the naming looks a bit weird, but this looks generally ok.
| # Get info | ||
| info = benchmark_run.get_info() | ||
| assert info.id == run_data.id | ||
| assert info.state in ["queued", "running", "completed", "canceled"] |
There was a problem hiding this comment.
queued doesn't exist as a state?
| def list_scenario_runs( | ||
| self, | ||
| **params: Unpack[SDKBenchmarkRunListScenarioRunsParams], | ||
| ) -> List[ScenarioRunView]: | ||
| """List all scenario runs for this benchmark run. | ||
|
|
||
| :param params: See :typeddict:`~runloop_api_client.sdk._types.SDKBenchmarkRunListScenarioRunsParams` for available parameters | ||
| :return: List of scenario run views | ||
| :rtype: List[ScenarioRunView] | ||
| """ | ||
| page = self._client.benchmarks.runs.list_scenario_runs( | ||
| self._id, | ||
| **params, | ||
| ) | ||
| return list(page) |
There was a problem hiding this comment.
Consider improving the interaction pattern to provide something like a paged iterator (or adding a separate iterator method to do this instead).
| async def get_info( | ||
| self, | ||
| **options: Unpack[BaseRequestOptions], | ||
| ) -> BenchmarkRunView: |
There was a problem hiding this comment.
Can we call this something else instead? get_state or refresh?
sid-rl
force-pushed
the
siddarth/benchmark-sdk
branch
from
04b9a9c to
4650641
Compare
james-rl
left a comment
james-rl
left a comment
There was a problem hiding this comment.
it's a shame that get_info is now a widely supported convention but I agree that it's worse to make a change to only one place.
A minor nit for you in the comments, but this generally looks good
| self._id = run_id | ||
| self._benchmark_id = benchmark_id | ||
|
|
||
| @override |
There was a problem hiding this comment.
I don't think you want @override without a base class
| @override | ||
| def __repr__(self) -> str: | ||
| return f"<BenchmarkRun id={self._id!r}>" |
* update requirements-dev * pyproject formatting nit * feat(sdk): add BenchmarkRun and AsyncBenchmarkRun classes * fixed smoketests * `list_scenario_runs()` now returns a list of ScenarioRun/AsyncScenarioRun objects
Add new SDK classes for managing benchmark runs:
BenchmarkRun: Synchronous class for benchmark run operations
AsyncBenchmarkRun: Async version with the same interface
SDKBenchmarkRunListScenarioRunsParams: TypedDict for list params
Unit tests for both sync and async classes
E2E smoketests validating against real API