feat(evaluation) 9/15: run start, dispatch and cancellation - #820
feat(evaluation) 9/15: run start, dispatch and cancellation#820Ahmath-Gadji wants to merge 2 commits into
Conversation
|
Warning Review limit reached
Next review available in: 59 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ed1feca to
03026f3
Compare
e0a85b2 to
f15c899
Compare
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 57 minutes. |
03026f3 to
224a71c
Compare
f15c899 to
5400f80
Compare
224a71c to
b0f87a9
Compare
5400f80 to
ea0f369
Compare
b0f87a9 to
58ad1d0
Compare
ea0f369 to
43925aa
Compare
The run half of `EvaluationService`, dispatched through the `EvaluationRunner` port so the orchestrator stays Ray-free. Setup and teardown of a run's identity live here rather than in the worker, because creating users and partitions is orchestration the API layer already owns. The worker receives a partition it may write to and a token it may use, and nothing else about the system. Ordering in `start_run` is deliberate and each step is regression-tested: 1. Ping the runner first. Dispatch is fire-and-forget, so an unreachable worker would otherwise strand the run in QUEUED with a partition and a token provisioned for nobody. It surfaces as 503. 2. Insert the run row before provisioning anything. The partial unique index is the mutual exclusion; a read-then-insert would let two racing requests both regenerate the shared eval user's token, the second revoking the credentials the first is still indexing with. The loser gets a 409 before touching anything. 3. On a failed provision, release the row and drop the partition. A run left in an active status holds the lock and would block every later run. Runs authenticate as one long-lived non-admin service user (`__openrag_eval__`) whose token is regenerated at the start of every run, so no usable plaintext token is ever stored at rest. Cancellation asks the worker first. A worker that owns the run writes its own terminal status, including metrics. `False` means nobody owns it — the run was orphaned by an actor restart — so the row is reaped here instead, otherwise it would block every subsequent run forever.
`__eval_*` is now rejected on the public creation path (part 1/15), so a run has to opt in to the namespace it owns. Without this the first run fails at `create_partition` with RESERVED_PARTITION_NAME. The unit tests did not catch it: `FakePartitionService` stands in for the whole service, so it accepted a name the real one refuses. The fake now asserts the flag, which is the only thing that keeps the two in step.
58ad1d0 to
5acd338
Compare
43925aa to
cc3263c
Compare
2 participants
Part 9 of 15 of the split of #811. Targets
eval/08-runner-port(#819).What
The run half of
EvaluationService—start_run,cancel_run, and the provisioning around them — dispatched through theEvaluationRunnerport so the orchestrator never imports Ray.Setup and teardown of a run's identity live here rather than in the worker, because creating users and partitions is orchestration the API layer already owns. The worker receives a partition it may write to and a token it may use, and nothing else about the system.
Notable
The ordering inside
start_runis the whole design. Each step exists because of a specific failure, and each has a regression test:QUEUEDwith a partition and a token provisioned for nobody. Surfaces as503. →test_start_run_refuses_when_the_runner_cannot_be_reached409before touching anything. →test_a_second_start_is_refused_before_the_token_is_regeneratedassertsregenerated == 0.test_a_failed_provision_releases_the_run_lockAuth. Runs authenticate as one long-lived non-admin service user,
__openrag_eval__, whose token is regenerated at the start of every run — a single service account in the DB, and no usable plaintext token at rest. It getsfile_quota=-1because a corpus is re-uploaded on every run and a quota would fail the second one for reasons unrelated to the eval.Cancellation asks the worker first. A worker that owns the run writes its own terminal status, including the metrics — so the service must not overwrite it.
Falsemeans nobody owns the run (an actor restart orphaned it); the row is reaped here instead, because nothing else would ever move it out of an active status. An unreachable runner is logged, not raised — an orphan still has to be reapable.Testing
10 unit tests, all against in-memory fakes — no Ray, no actor, no worker process, which is exactly what the port bought. Covers the three orderings above, both cancellation paths, the already-finished 409, and that
api_base_urlcomes fromSettingsrather than the environment.ruff, format check and the layer-import guard pass.