-
Notifications
You must be signed in to change notification settings - Fork 474
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
Adds EP-10494: AI APIs enhancement proposal #10495
Adds EP-10494: AI APIs enhancement proposal #10495
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This proposal LGTM
spec: | ||
ai: | ||
vertexAi: | ||
model: gemini-1.5-flash-001 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we mention what would happen if the URL or the request body contains the model value that's not the same as the model value in the Upstream?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, I'll add a sentence about if the model is not provided as well. We use the Upstream in the case of a mismatch, right?
3ab7979
to
b1fe48b
Compare
name: openai-secret | ||
``` | ||
|
||
Notice that this Upstream does not specify a model, so kgateway will use the model value in the request to determine |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kgateway will use the model value in the request
What does this mean? Might be worth elaborating here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can add a specific request example in a follow up, but for context this field isn't strictly required:
spec:
ai:
openai:
model: my-model
You can specify a model in the request body instead, but if you specify both the Upstream will win out. (This additional note is more implementation detail, but was request in an earlier review to clarify the behavior)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right but my question is how is it specified in the request body?
filters: | ||
- type: ExtensionRef | ||
extensionRef: | ||
group: gateway.kgateway.dev/v1alpha1 | ||
kind: RoutePolicy | ||
name: open-ai-opt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer using targetRef
attachment on the RoutePolicy
rather than ExtensionRef
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the comment:
Line 49 in f7cb884
Note: For the first implementation, a RoutePolicy must be applied to a specific route in the HTTPRoute using an `extensionRef`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is worrying because ExtensionRef is effectively redundant given named route rules.
I think we'll want to consider this as important for the plugin system if things are easier to express as ExtensionRefs instead of via policy attachment.
cc @yuval-k
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes make sense to me
Description
Adds Enhancement Proposal 10494 that proposes adding AI Gateway APIs support.
Supports #10494
Initial APIs can be found in this draft PR: #10493