Add MCP server support to Retool Helm chart#285
Conversation
|
@codex review this bad boy |
|
Some initial AI review comments: Full review comments:
|
|
main thing: this template is 374 lines, multiplayer is 252, and most of the diff is stuff we don't actually need in v1. ideally this just mirrors stuff i'd drop unless there's a concrete reason:
if you cut those, the template lands close to multiplayer in size and shape and they'll be much easier to keep in sync going forward. couple of bugs worth fixing regardless:
ingress + httproute additions look fine, they match the existing multiplayer pattern. |
77de358 to
c44a907
Compare
c44a907 to
db5e260
Compare
db5e260 to
d618c4d
Compare
|
|
||
| # Paths to route to MCP server pods. Defaults include both the MCP endpoint | ||
| # and OAuth protected resource metadata endpoint. | ||
| ingress: |
There was a problem hiding this comment.
i might be dumb but also confused why there's two ingress/mcp blocks for the mcp-sever? this would probably read very weird to customers
There was a problem hiding this comment.
service_backend_api is defining the backend 3001 port as a service, and these two nested blocks route /.well-known/oauth requests to that service. The remaining endpoints are routed as normal.
Kind of odd, I agree... any better ways to do this
| @@ -1,3 +1,10 @@ | |||
| {{- $needsBackendApi := eq (include "retool.mcp.needsBackendApi" .) "true" -}} | |||
There was a problem hiding this comment.
nit: but can you change the variable name here to be indicative that it's mcp-related?
Merge activity
|
Adds optional MCP server support to the Retool Helm chart, disabled by default.
Main changes:
Validation performed: