Skip to content

feat!: Refactor URLs in status and extend protocol support#412

Open
farshidtz wants to merge 10 commits into
mainfrom
listeners-IENG-2549
Open

feat!: Refactor URLs in status and extend protocol support#412
farshidtz wants to merge 10 commits into
mainfrom
listeners-IENG-2549

Conversation

@farshidtz

@farshidtz farshidtz commented Jul 22, 2026

Copy link
Copy Markdown
Member

Change status data model from:

$ gemma4 status
engine: nvidia-gpu
services:
    server: active
    server-webui: active
endpoints:
    openai: http://127.0.0.1:8336/v1
    webui: http://127.0.0.1:8337/
model:
    name: gemma4-e4b-q4-k-m

To:

$ gemma4 status
engine: nvidia-gpu
services:
    server: active
    server-webui: active
entrypoints:
    openai:
        url: http://127.0.0.1:8336/v1
    webui:
        url: http://127.0.0.1:8337/

And add support for "http+ws", "ws", "ws+unix" protocols in runtime manifests. Example status:

engine: cpu
services:
    llama-server: active
entrypoints:
    logger:
        url: http://localhost:8081/
    openai:
        url: http://0.0.0.0:8080/v1
    whisperlive:
        unix-socket: /run/whisper.sock (ws://unix/realtime)

Example status in JSON:

{
   "engine": "cpu",
   "services": {
    "llama-server": "active"
  },
  "entrypoints": {
    "logger": {
      "url": "http://localhost:8081/"
    },
    "openai": {
      "url": "http://0.0.0.0:8080/v1"
    },
    "whisperlive": {
      "unix-socket": "/run/whisper.sock",
      "unix-socket-url": "ws://unix/realtime"
    }
  },
  "model": {
    "name": "gemma-4-4b-it-int4-fq-ov"
  }
}

Add support for config namespacing in runtime manifests to allow the definition of servers listening on separate ports.

servers:
  openai:
    protocol: http
    base-path: /v1
  kserve:
    protocol: http
    base-path: /v2
  whisperlive:
    protocol: ws+unix
    base-path: /realtime
  logger:  
    protocol: http
    base-path: /
    namespace: logger # new field

The namespace is used to retrieve the configurations for a server entry to construct the corresponding entrypoint. In the above example, the openai and kserve servers URLs will be constructed by using the http.port and http.host configurations (no namespace). For logger, the configuration will be taken from logger.http.port and logger.http.host instead.

BREAKING CHANGE: This is a breaking change for the clients that rely on the endpoints.<key> field in status. They should instead query entrypoints.<key>.url.

Consistent naming conventions

Remove obsolete wrapper func

endpoint cleanup
Copilot AI review requested due to automatic review settings July 22, 2026 15:18

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR replaces the legacy “endpoints” status output with a richer “listeners” representation so server connectivity can be reported for additional protocols (notably WebSocket and WebSocket-over-Unix-socket), while keeping export-status output backward-compatible by continuing to emit endpoints (URLs only).

Changes:

  • Replace Status.Endpoints with Status.Listeners and update SnapStatus + its unit test accordingly.
  • Introduce ServerListeners with support for http/https, ws, and ws+unix, plus comprehensive unit tests; remove the old endpoints implementation and tests.
  • Update chat/webui commands to use OpenAiBaseUrl, and keep export-status emitting legacy endpoints and openai.json.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
cmd/modelctl/common/status.go Switch status payload from endpoints to listeners and fetch listeners via ServerListeners.
cmd/modelctl/common/status_test.go Update assertions to validate listener URL instead of endpoint URL.
cmd/modelctl/common/listeners.go New listeners implementation with extended protocol support + helper accessors.
cmd/modelctl/common/listeners_test.go New tests covering multiple servers, ws/ws+unix, and unsupported protocols.
cmd/modelctl/common/endpoints.go Removed old endpoints implementation (replaced by listeners).
cmd/modelctl/common/endpoints_test.go Removed old endpoints tests (replaced by listeners tests).
cmd/modelctl/commands/webui.go Use OpenAiBaseUrl instead of removed OpenAiEndpoint.
cmd/modelctl/commands/serve-webui.go Use OpenAiBaseUrl instead of removed OpenAiEndpoint.
cmd/modelctl/commands/chat.go Use OpenAiBaseUrl instead of removed OpenAiEndpoint.
cmd/modelctl/commands/export-status.go Convert listeners → legacy endpoints (URLs) for backward compatibility and guard empty openai endpoint.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cmd/modelctl/common/listeners.go Outdated
Comment thread cmd/modelctl/common/listeners.go Outdated
Comment thread cmd/modelctl/common/listeners.go Outdated
Comment thread cmd/modelctl/common/listeners.go Outdated
Comment thread cmd/modelctl/common/listeners_test.go Outdated
Comment thread cmd/modelctl/common/listeners.go Outdated
Comment thread cmd/modelctl/common/listeners.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments.

Comments suppressed due to low confidence (2)

cmd/modelctl/common/endpoints.go:177

  • Same TrimRight issue here: TrimRight uses a character cutset and may trim more than the intended "+unix" suffix. TrimSuffix avoids accidental trimming and better communicates intent.
	protocol := strings.TrimRight(server.Protocol, "+unix") // remove +unix suffix for URL scheme
	unixSocketUrl := fmt.Sprintf("%s://unix%s", protocol, server.BasePath)

cmd/modelctl/commands/status_test.go:88

  • This example calls statusCommand.run(), which starts a progress spinner that writes to stdout and will break Go example output matching. Replace the run() call with statusJson() and print the returned string (requires fmt import).
func Example_statusCommand_printStatusJson() {
	ctx := createTestContextForStatus()
	cmd := statusCommand{
		Context: ctx,
		format:  "json",
	}

	if err := cmd.run(nil, nil); err != nil {
		log.Fatalf("failed to print status in json format: %v", err)
	}

Comment thread cmd/modelctl/common/endpoints.go
Comment thread cmd/modelctl/common/endpoints.go Outdated
Comment thread cmd/modelctl/common/endpoints.go
Comment thread cmd/modelctl/commands/export-status.go
Comment thread cmd/modelctl/commands/status_test.go Outdated
Comment thread cmd/modelctl/commands/status_test.go
Comment thread cmd/modelctl/commands/status_test.go
@farshidtz farshidtz changed the title Add listeners status with extended protocol support feat!: Refactor URLs in status and extend protocol support Jul 23, 2026
@farshidtz
farshidtz marked this pull request as ready for review July 23, 2026 21:08
@farshidtz
farshidtz requested a review from imatrisciano July 23, 2026 21:08

@imatrisciano imatrisciano left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks solid, I've left a couple of comments

Comment on lines 66 to +84
case "http", "https":
httpUrl, err := serverHttpUrl(ctx, serverSettings)
entrypoint, err = serverHttpEntrypoint(ctx, serverSettings)
if err != nil {
return nil, fmt.Errorf("getting server HTTP URL: %v", err)
return nil, fmt.Errorf("constructing HTTP entrypoint: %v", err)
}
case "http+unix", "https+unix":
entrypoint, err = serverHttpOverUnixSocketEntrypoint(ctx, serverSettings)
if err != nil {
return nil, fmt.Errorf("constructing HTTP Unix entrypoint: %v", err)
}
case "ws":
entrypoint, err = serverWsEntrypoint(ctx, serverSettings)
if err != nil {
return nil, fmt.Errorf("constructing WebSocket entrypoint: %v", err)
}
case "ws+unix":
entrypoint, err = serverWsOverUnixSocketEntrypoint(ctx, serverSettings)
if err != nil {
return nil, fmt.Errorf("constructing WebSocket Unix entrypoint: %v", err)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If https is supported, arguably wss should be supported as well

Comment thread pkg/runtimes/validate.go
Comment on lines 128 to 151
func (server Server) validate(name string) error {
if server.Protocol == "" {
return fmt.Errorf("required field is not set for server %s: protocol", name)
}

if server.BasePath == "" {
return fmt.Errorf("required field is not set for server %s: base-path", name)
// base-path is optional
if server.BasePath != "" {
if !strings.HasPrefix(server.BasePath, "/") {
return fmt.Errorf("invalid base-path for server %s: must start with '/'", name)
}

parsed, err := url.Parse(server.BasePath)
if err != nil {
return fmt.Errorf("invalid base-path for server %s: %v", name, err)
}

if parsed.Scheme != "" || parsed.Host != "" || parsed.Opaque != "" {
return fmt.Errorf("invalid base-path for server %s: must be a URL path, not a full URL", name)
}

if parsed.RawQuery != "" || parsed.Fragment != "" {
return fmt.Errorf("invalid base-path for server %s: query and fragment are not allowed", name)
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This validation function should verify that the value of server.Protocol is one of the supported ones, as defined in cmd/modelctl/common/endpoints.go

Since namespaces are used to identify a configuration group, should namespaces containing dots be rejected?

return fmt.Errorf("getting status: %v", err)
}

// Convert Entrypoints to Endpoints (extract URLs) for backward compatibility

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would add a link to this PR for future reference

Suggested change
// Convert Entrypoints to Endpoints (extract URLs) for backward compatibility
// Convert Entrypoints to Endpoints (extract URLs) for backward compatibility,
// see https://github.com/canonical/inference-snaps-cli/pull/412

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.

3 participants