Lbaas 4091 add load balancer tools #123
Conversation
| lbType, _ := args["Type"].(string) | ||
| network, _ := args["Network"].(string) | ||
| sizeUnit, _ := args["SizeUnit"].(float64) | ||
| networkStack, _ := args["NetworkStack"].(string) | ||
| projectID, _ := args["ProjectID"].(string) |
There was a problem hiding this comment.
should we be validating each field here?
There was a problem hiding this comment.
These fields are optional so I don't think validation is necessary. I'll add a comment for clarity.
| FailoverThreshold: uint32(failoverThreshold), | ||
| } | ||
| } | ||
| } else { |
There was a problem hiding this comment.
Is it accepted to assume that the LLM will provide either GLOBAL or default to REGIONAL?
I think we may need to be explicit in our check here:
Perhaps check if either GLOBAL, REGIONAL or fallback to a default case which returns an error.
There was a problem hiding this comment.
if type is not provided, the api defaults to the regional type. So, if type is not provided, all the other args will be used to create regional type load balancer. Maybe I could add the defaults in the descriptions of the fields?
| { | ||
| Handler: l.createLoadBalancer, | ||
| Tool: mcp.NewTool("load-balancer-create", | ||
| mcp.WithDescription("Create a new Load Balancer"), |
There was a problem hiding this comment.
For complex structs, I'd suggest using schema generation. See apps/specs/ and apps-create-app-from-spec as an example
| mcp.WithString("LoadBalancerID", mcp.Required(), mcp.Description("ID of the load balancer")), | ||
| mcp.WithString("Name", mcp.Required(), mcp.Description("Name of the load balancer")), | ||
| mcp.WithString("Region", mcp.Description("Region slug (e.g., nyc3)")), | ||
| mcp.WithArray("DropletIDs", mcp.Description("IDs of the Droplets assigned to the load balancer")), | ||
| mcp.WithString("Tag", mcp.Description("Droplet tag corresponding to Droplets assigned to the load balancer")), | ||
| mcp.WithArray("ForwardingRules", mcp.Description("Forwarding rules for a load balancer")), | ||
| mcp.WithString("Type", mcp.Required(), mcp.Description("Type of the load balancer (REGIONAL, REGIONAL_NETWORK, GLOBAL)")), | ||
| mcp.WithString("Network", mcp.Description("Network type of the load balancer (EXTERNAL, INTERNAL)")), | ||
| mcp.WithNumber("SizeUnit", mcp.DefaultNumber(2), mcp.Description("Size of the load balancer in units appropriate to its type")), | ||
| mcp.WithString("NetworkStack", mcp.Description("Network stack of the load balancer (IPV4, DUALSTACK)")), | ||
| mcp.WithString("ProjectID", mcp.Description("Project ID to which the load balancer will be assigned")), | ||
| mcp.WithArray("TargetLoadBalancerIDs", mcp.Description("IDs of the target regional load balancers for a global load balancer")), | ||
| mcp.WithObject("GLBSettings", mcp.Description("Forward configurations for a global load balancer")), |
There was a problem hiding this comment.
Same comment about generating this type instead.
IshitaBadole
force-pushed
the
LBAAS-4091-add-load-balancer-tools
branch
3 times, most recently
from
055d3bb to
67eabc7
Compare
|
@IshitaBadole, could you fix the conflicts before I merge this one? |
IshitaBadole
force-pushed
the
LBAAS-4091-add-load-balancer-tools
branch
from
0c4bfe7 to
4e34658
Compare
Hi, I've rebased my changes |
|
There's one conflict I see. I think this can be solved by re-generating mocks.go within networking. |
IshitaBadole
force-pushed
the
LBAAS-4091-add-load-balancer-tools
branch
from
4e34658 to
2f4e394
Compare
Thanks I missed that. It's updated now. |
2 participants
Tools have been locally tested using mcp inspector.
Verified tests are passing with
make test.