Skip to content

skip standalone runner init when --openaiurl is provided#842

Open
ericcurtin wants to merge 1 commit intomainfrom
fix-this
Open

skip standalone runner init when --openaiurl is provided#842
ericcurtin wants to merge 1 commit intomainfrom
fix-this

Conversation

@ericcurtin
Copy link
Copy Markdown
Contributor

Commands wrapped with withStandaloneRunner were unconditionally calling ensureStandaloneRunnerAvailable before RunE executed, causing a Docker socket connection attempt even when --openaiurl was supplied and no local daemon is needed.

Check for the openaiurl flag in the wrapper and bypass runner initialization when it is set.

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="cmd/cli/commands/install-runner.go" line_range="192" />
<code_context>
+		// Skip standalone runner initialization when --openaiurl is provided,
+		// since those commands talk directly to an external endpoint and don't
+		// need (or have) a local Docker daemon.
+		if f := cmd.Flags().Lookup("openaiurl"); f == nil || f.Value.String() == "" {
+			if _, err := ensureStandaloneRunnerAvailable(cmd.Context(), asPrinter(cmd), false); err != nil {
+				return fmt.Errorf("unable to initialize standalone model runner: %w", err)
</code_context>
<issue_to_address>
**issue (bug_risk):** Use cmd.Flag("openaiurl") to correctly handle inherited/persistent flags.

`cmd.Flags().Lookup("openaiurl")` only inspects this command’s local flags. If `--openaiurl` is defined as a persistent/inherited flag on a parent command, it won’t be found and `f` will be `nil`, so the standalone runner is incorrectly initialized even when the flag is set. Using `cmd.Flag("openaiurl")` ensures local, persistent, and inherited flags are all respected for this option.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request modifies the standalone runner initialization logic in cmd/cli/commands/install-runner.go to skip the initialization process when the --openaiurl flag is provided, as these commands interact directly with external endpoints and do not require a local Docker daemon. I have no feedback to provide as there are no review comments to address.

Commands wrapped with withStandaloneRunner were unconditionally calling
ensureStandaloneRunnerAvailable before RunE executed, causing a Docker
socket connection attempt even when --openaiurl was supplied and no
local daemon is needed.

Check for the openaiurl flag in the wrapper and bypass runner
initialization when it is set.

Signed-off-by: Eric Curtin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants