-
Notifications
You must be signed in to change notification settings - Fork 149
feat(rivetkit): add runner override option to getOrCreate and create #4064
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
Allows specifying a runner per-call instead of only at client config level.
Usage:
```ts
// getOrCreate with runner override
client.actor.getOrCreate(['key'], {
createOnRunner: 'my-specific-runner'
});
// create with runner override
await client.actor.create(['key'], {
runner: 'my-specific-runner'
});
```
Changes:
- client.ts: Add createOnRunner to GetOrCreateOptions, runner to CreateOptions
- protocol/query.ts: Add runner field to schemas
- driver.ts: Add runner to input interfaces
- actor-query.ts: Pass runner through to manager driver
|
🚅 Deployed to the rivet-pr-4064 environment in rivet-frontend
|
Code Review: feat(rivetkit): add runner override option to getOrCreate and createSummaryThis PR adds the ability to specify a runner on a per-call basis for ✅ Positives
🐛 Critical Issue: Missing Implementation in Remote Manager DriverThe PR has a critical bug where the Problem locations:
Current code: // In getOrCreateWithKey (line 195-204)
const { actor, created } = await getOrCreateActor(this.#config, {
datacenter: region,
name,
key: serializeActorKey(key),
runner_name_selector: this.#config.runnerName, // ❌ Wrong - ignores input.runner
input: actorInput ? uint8ArrayToBase64(cbor.encode(actorInput)) : undefined,
crash_policy: "sleep",
});
// In createActor (line 232-239)
const result = await createActor(this.#config, {
datacenter: region,
name,
runner_name_selector: this.#config.runnerName, // ❌ Wrong - ignores input.runner
key: serializeActorKey(key),
input: input ? uint8ArrayToBase64(cbor.encode(input)) : undefined,
crash_policy: "sleep",
});Fix needed: // In getOrCreateWithKey
const { actor, created } = await getOrCreateActor(this.#config, {
datacenter: region,
name,
key: serializeActorKey(key),
runner_name_selector: runner ?? this.#config.runnerName, // ✅ Use parameter or fall back to config
input: actorInput ? uint8ArrayToBase64(cbor.encode(actorInput)) : undefined,
crash_policy: "sleep",
});
// In createActor
const result = await createActor(this.#config, {
datacenter: region,
name,
runner_name_selector: runner ?? this.#config.runnerName, // ✅ Use parameter or fall back to config
key: serializeActorKey(key),
input: input ? uint8ArrayToBase64(cbor.encode(input)) : undefined,
crash_policy: "sleep",
});📝 Recommendations
🔒 SecurityNo security concerns identified. The runner parameter is validated by Zod schemas and follows the same pattern as other optional parameters. ⚡ PerformanceNo performance concerns. The changes add minimal overhead (one optional parameter check). 📊 Test CoverageMissing: No tests were added for this new functionality. Suggest adding:
VerdictNeeds Changes - The PR introduces a valuable feature but has a critical implementation gap where the |
Summary
Allows specifying a runner per-call instead of only at client config level.
Usage
Changes
client.ts: AddcreateOnRunnertoGetOrCreateOptions,runnertoCreateOptionsprotocol/query.ts: Addrunnerfield toCreateRequestSchemaandGetOrCreateRequestSchemadriver.ts: AddrunnertoGetOrCreateWithKeyInputandCreateInputinterfacesactor-query.ts: Pass runner through to manager driver callsNote
This adds the client-side support. The manager/engine implementation to actually route to the specified runner may need additional work depending on how runner selection is handled server-side.