simplify bbr plugins and update tests accordingly#2783
Conversation
Signed-off-by: Nir Rozenbaum <nrozenba@redhat.com>
k8s-ci-robot
added
the
kind/cleanup
✅ Deploy Preview for gateway-api-inference-extension ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nirrozenbaum The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
approved
| func NewBodyFieldToHeaderPlugin(fieldName, headerName string) (*BodyFieldToHeaderPlugin, error) { | ||
| if fieldName == "" { | ||
| return nil, errors.New("body fieldName is required in BodyFieldToHeader plugin") | ||
| return nil, fmt.Errorf("fieldName is required for plugin '%s'", BodyFieldToHeaderPluginType) |
There was a problem hiding this comment.
may want to define default field and header names instead of erroring. The header should be part of the "protocol" of setting up HTTP routes and the field name is expected based on the API (OpenAI?) so both could have sensible defaults here. Can still error when processing if these are not found in the actual request.
There was a problem hiding this comment.
mmm yes and no.
you are correct that this is very much connected to the http routes (we need to use the header to route).
at the same time, there is no "well defined" BBR protocol anywhere.
This plugin was implemented in a generic way allowing to promote any given body field to any given header.
it feels wrong to put a default value in a general purpose plugin, as it could be error prone.
e.g., what if user defines two plugin instances and defines only fieldName?
both plugins will use the same default header name and will override each other (last wins).
I preferred being explicit here.
Signed-off-by: Nir Rozenbaum <nrozenba@redhat.com>
|
/lgtm |
k8s-ci-robot
added
the
lgtm
…teway-api-inference-extension#2783) * simplify bbr plugins and update tests accordingly Signed-off-by: Nir Rozenbaum <nrozenba@redhat.com> * code review comments Signed-off-by: Nir Rozenbaum <nrozenba@redhat.com> --------- Signed-off-by: Nir Rozenbaum <nrozenba@redhat.com>
3 participants
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
cleanup and simplify bbr plugins.
Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: