modified MCP servers definition. Added Tool filtering config #1222
Conversation
| continue | ||
| } | ||
| // Use consistent path structure: /etc/mcp/headers/<secretName>/header | ||
| headerValue = path.Join(utils.MCPHeadersMountRoot, header.ValueFrom.SecretRef.Name, utils.MCPSECRETDATAPATH) |
There was a problem hiding this comment.
would it be better to let user configure which key in the secret is the header?
they may prefer host multiple header values in the same secret.
There was a problem hiding this comment.
Ideally yes, but can be more complex. I would preffer to keep it simple for now. Do not expect to have many MCP servers of this type
| .PHONY: manifests | ||
| manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects. | ||
| $(CONTROLLER_GEN) rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases | ||
| $(CONTROLLER_GEN) rbac:roleName=manager-role crd:allowDangerousTypes=true webhook paths="./..." output:crd:artifacts:config=config/crd/bases |
There was a problem hiding this comment.
lol which type is dangerous?
There was a problem hiding this comment.
I have no idea. This is generated
|
/lgtm looks good. |
openshift-ci
bot
added
lgtm
blublinsky
force-pushed
the
new-mcp
branch
2 times, most recently
from
0a1a0b0 to
56469f0
Compare
|
/lgtm |
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
|
/retest |
| // The embedding model is not exposed as it's handled by the container image. | ||
| // +kubebuilder:validation:XValidation:rule="self.alpha >= 0.0 && self.alpha <= 1.0",message="alpha must be between 0.0 and 1.0" | ||
| // +kubebuilder:validation:XValidation:rule="self.threshold >= 0.0 && self.threshold <= 1.0",message="threshold must be between 0.0 and 1.0" | ||
| type ToolFilteringConfig struct { |
There was a problem hiding this comment.
If we are exposing this without a feature gate, it would be great to add a little bit more explanation about what these parameters are and how they are influencing the results.
There was a problem hiding this comment.
Each parameter is clearly annotated. We can create a document on what toolsRag is and how it works. Sort of described here https://github.com/blublinsky/tools_rag. But this file is definetly not the right spot for it
openshift-merge-robot
added
the
needs-rebase
openshift-merge-robot
removed
the
needs-rebase
|
/lgtm |
|
/override "Red Hat Konflux / console-tests-pf5 / ols-bundle" |
|
@blublinsky: Overrode contexts on behalf of blublinsky: Red Hat Konflux / console-tests-pf5 / ols-bundle, Red Hat Konflux / console-tests-pf6 / ols-bundle, Red Hat Konflux / operator-e2e-tests-419 / ols-bundle, Red Hat Konflux / service-e2e-tests-419 / ols-bundle DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: blublinsky 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 |
openshift-ci
bot
added
the
approved
|
@blublinsky: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/unhold |
openshift-ci
bot
removed
the
do-not-merge/hold
openshift-merge-bot
bot
merged commit 4e29cab
into
openshift:main
6 participants
Description
Type of change
Related Tickets & Documents
OLS-2275
Checklist before requesting a review
Testing