Conversation
This reverts commit a8f43d8.
github-actions
bot
added
documentation
📋 PR Linter Failed❌ Invalid Title Format. Your PR title must include a ticket/issue number and may optionally include component tags (
Example: ❌ Missing Section. The description is missing the ❌ Missing Section. The description is missing the ❌ Missing Section. The description is missing the ❌ Missing Section. The description is missing the ❌ Missing Section. The description is missing the |
|
🌿 Preview your docs: https://opik-preview-5f788db8-96d1-423b-9949-7504c747f265.docs.buildwithfern.com/docs/opik The following broken links were found: Page: https://opik-preview-5f788db8-96d1-423b-9949-7504c747f265.docs.buildwithfern.com/docs/opik/integrations/harbor Page: https://opik-preview-5f788db8-96d1-423b-9949-7504c747f265.docs.buildwithfern.com/docs/opik/integrations/harbor/ 📌 Results for commit f94d405 |
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
| if (shouldPurge) { | ||
| failOrphanedJobs(runnerId); | ||
| failOrphanedBridgeCommands(runnerId); | ||
| purgeRunner(runnerId, workspaceId, runnerMap.get(FIELD_USER_NAME)); | ||
| } |
There was a problem hiding this comment.
evictExistingRunner no longer calls failOrphanedJobs, leaving orphaned jobs running until the reaper; should we restore immediate failOrphanedJobs and bridge cleanup in the eviction path so old runner jobs fail as soon as a new connection takes over?
Finding type: Logical Bugs | Severity: 🔴 High
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
Before applying, verify this suggestion against the current code. In
apps/opik-backend/src/main/java/com/comet/opik/domain/LocalRunnerService.java around
lines 736 to 739 (method: reapRunner) you moved failOrphanedJobs out of the eviction
flow so evicted runners’ jobs are left running until purge time. Restore the original
immediate cleanup by modifying evictExistingRunner (the eviction logic around lines
~888-902) to: after deleting the old runner heartbeat, set FIELD_DISCONNECTED_AT on the
old runner map (if exists), call failOrphanedJobs(oldRunnerId) and also call
failOrphanedBridgeCommands(oldRunnerId) if bridge support remains, remove the old runner
from projectRunners and workspace/user runner sets (as the old code did), then log the
eviction. This will ensure orphaned jobs and bridge commands are failed immediately when
a newer connection evicts an older runner.
| @click.option("--pair", "pair_code", default=None, help="Pairing code for the runner.") | ||
| @click.option("--name", default=None, help="Runner name.") | ||
| @click.argument("command", nargs=-1, type=click.UNPROCESSED) | ||
| @click.pass_context | ||
| def connect( |
There was a problem hiding this comment.
--pair wires to pair_code while the code uses pairing_code — should we rename the CLI to --pairing-code => pairing_code?
Finding type: Maintain naming consistency | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
Before applying, verify this suggestion against the current code. In
sdks/python/src/opik/cli/connect.py around lines 15 to 19, the CLI option is defined as
--pair with destination pair_code. Rename the CLI option to --pairing-code and the
destination/parameter from pair_code to pairing_code. Also update the connect function
signature where pair_code is declared (around lines 20-24) and all usages of pair_code
(notably the api.runners.connect_runner call around lines 49-56 and any other
references) to use pairing_code so the CLI, function parameter, and API call all use the
same identifier pairing_code.
| try: | ||
| runner_id, project_name = _register_runner(api, name, pair_code) | ||
| runner_name = name or f"{platform.node()}-{uuid.uuid4().hex[:6]}" | ||
| resp = api.runners.connect_runner( | ||
| runner_name=runner_name, | ||
| pairing_code=pair_code, | ||
| ) |
There was a problem hiding this comment.
pair_code is optional but we still call api.runners.connect_runner(... pairing_code=pair_code) unconditionally, should we make --pair required or check pair_code before calling connect_runner and surface a clearer CLI error?
Finding type: Type Inconsistency | Severity: 🔴 High
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
Before applying, verify this suggestion against the current code. In
sdks/python/src/opik/cli/connect.py around lines 49-54, the connect function calls
api.runners.connect_runner(pairing_code=pair_code) even though pair_code is Optional.
Before calling connect_runner, add a guard that checks if pair_code is present/truthy;
if it's missing, print a clear CLI error (e.g. "Error: Missing pairing code. Provide
--pair <code>.") to stderr and raise SystemExit(2). Then only call
api.runners.connect_runner when pair_code is provided. This keeps --pair optional at the
CLI level but fails fast with a user-friendly message instead of sending null to the
backend.
1 participant
Reverts #6074