-
Notifications
You must be signed in to change notification settings - Fork 30
Fix: Add batch upload for prime evals #304
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
base: main
Are you sure you want to change the base?
Conversation
| try: | ||
| total_samples_pushed += future.result() | ||
| except Exception as e: | ||
| errors.append(f"Batch {futures[future] + 1}: {e}") |
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.
Concurrent thread access to non-thread-safe httpx client
Medium Severity
The sync push_samples method uses ThreadPoolExecutor with max_workers=4 by default to upload batches concurrently. Each worker thread calls self.client.request() on the shared client instance. The prime_evals.core.APIClient provided by this package uses httpx.Client, which is documented as not thread-safe. Users following the README's documented pattern of using APIClient with EvalsClient would encounter potential race conditions in the HTTP connection pool, which could cause intermittent failures.
Note
Introduces adaptive, concurrent batching for pushing evaluation samples in both sync and async clients.
push_samplesimplementations splitsamplesinto size-aware batches via_build_batchesand upload concurrently (ThreadPoolExecutorfor sync, semaphore-limited coroutines for async)max_payload_bytes(default 512KB),max_workers/max_concurrentwith validation{"samples_pushed": <count>}and aggregates per-batch errors into a singleEvalsAPIError_upload_batch(sync) and_build_batchesadded; existing create/finalize/list/get/update logic unchanged aside from minor formattingWritten by Cursor Bugbot for commit b67f8cc. This will update automatically on new commits. Configure here.