Fix external-provider ModelEndpoint example's double /v1 prefix#328
Merged
Conversation
The "Route to External Providers" doc set rewritePath: /v1/ for a Together-backed ModelEndpoint. A ModelService's HTTPRoute only strips the /<namespace>/<service>/ prefix from the client's request, leaving the client's own v1/... segment as the preserved remainder. Prepending /v1/ on top of that produces /v1/v1/..., which 404s against the backend. rewritePath should replace only the namespace/service prefix, same as the reference example for composed replicas (rewritePath: /ml-team/qwen-72b/, no added segment). Set it to / here so the client's v1/... passes through unchanged to Together's own /v1/... endpoint. Fixes modelplaneai#327. Signed-off-by: Nic Cope <nicc@rk0n.org>
There was a problem hiding this comment.
Pull request overview
Fixes the external-provider ModelEndpoint documentation example so that composed HTTPRoute path rewriting no longer produces a double /v1/v1/... prefix when clients call /<namespace>/<service>/v1/..., aligning the docs with the compose-model-service routing behavior.
Changes:
- Update the Together external-provider example to use
rewritePath: /(pass through the client’sv1/...segment unchanged). - Clarify the inline documentation comment explaining that only the
/<namespace>/<service>/prefix is stripped by the composed route. - Apply the same fix to both the rendered doc page and the embedded example manifest.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| docs/manifests/concepts/model-endpoint.yaml | Fix example manifest to use rewritePath: / to avoid /v1/v1/... paths. |
| docs/content/models/model-endpoint.md | Update the “Route to External Providers” example and commentary to match actual path-prefix stripping behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
negz
added
backport release-0.1
documentation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of your changes
Fixes #327
The "Route to External Providers" doc set
rewritePath: /v1/for a Together-backedModelEndpoint. AModelService's composedHTTPRouteonly strips the/<namespace>/<service>/prefix from the client's request path, leaving the client's ownv1/...segment as the preserved remainder. Prepending/v1/on top of that produces/v1/v1/..., which 404s against the backend, exactly as reported in #327.rewritePathshould replace only the namespace/service prefix, the same way the reference example for composed replicas already does (rewritePath: /ml-team/qwen-72b/, no added segment). This fixes the two copies of the Together example (docs/content/models/model-endpoint.mdand the embeddeddocs/manifests/concepts/model-endpoint.yaml) to userewritePath: /, so the client'sv1/...passes through unchanged to Together's own/v1/...endpoint.I have:
nix flake check(or./nix.sh flake check) and made sure it passes.Added or updated tests covering any composition function changes.git commit -s.