-
Notifications
You must be signed in to change notification settings - Fork 55
Integrate Limits MCP server #36
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
base: main
Are you sure you want to change the base?
Conversation
|
Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA). To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application. When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated. If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public. |
0f384f3 to
1dee121
Compare
| subscription_id: Annotated[Optional[str], "Subscription OCID filter"] = None, | ||
| ) -> list[dict]: | ||
| """ | ||
| Maps to GET /20190729/services/{serviceName}/limits/{limitName}/resourceAvailability |
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.
The @mcp.tool description will overwrite this btw
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.
More like a comment
Signed-off-by: Mayur Khandave <mayur.khandave@oracle.com>
Signed-off-by: Mayur Khandave <mayur.khandave@oracle.com>
Signed-off-by: Mayur Khandave <mayur.khandave@oracle.com>
Signed-off-by: Richard Gebhardt <richard.gebhardt@oracle.com>
Signed-off-by: Mayur Khandave <mayur.khandave@oracle.com>
Signed-off-by: Mayur Khandave <mayur.khandave@oracle.com>
Signed-off-by: Mayur Khandave <mayur.khandave@oracle.com>
Signed-off-by: Mayur Khandave <mayur.khandave@oracle.com>
Signed-off-by: will.shope <will.shope@oracle.com>
8701c7a to
fedf1e2
Compare
| "fastmcp==2.12.2", | ||
| "oci==2.160.0" | ||
| ] | ||
|
|
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 you add an authors attribute? feel free to use our project address: https://github.com/oracle/mcp/blob/main/src/oci-api-mcp-server/pyproject.toml#L9-L11; we'll create issues and assign them if ppl contact us.
Other than that, LGTM.
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.
yes
| return oci.limits.LimitsClient(config, signer=signer) | ||
|
|
||
|
|
||
| def get_identity_client(): |
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 be getting identity client here? I think we would want to leave the calls for this to the identity server, no?
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.
I tried approach of letting cline decide if call to availability domain is needed and then figure out calling identity mcp
but it was very slow and most of the time it was not able to find it
|
|
||
|
|
||
| # ---------------------------- | ||
| # Mappers to stable dict shapes |
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.
Put these mappers into a models.py file? Check the standards we're laying out in this PR
#43
| @mcp.tool( | ||
| description="List availability domains for a given compartment needed for limits" | ||
| ) | ||
| def provide_availability_domains_for_limits( |
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 already a tool in our identity server, if it needs to be used Id rather have some other annotation that tells another tool that it would need to consume the list_availabilty_domains tool in some cases
| ) | ||
| def list_services( | ||
| compartment_id: Annotated[str, "OCID of the root compartment (tenancy)"], | ||
| sort_by: Annotated[str, "Sort field: name or description"] = "name", |
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.
Check best practices PR to see how best to annotate and define optional parameters
#43
| @mcp.tool( | ||
| description="Get the full list of resource limit values for the given service" | ||
| ) | ||
| def get_limit_value( |
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.
Name should be list_limit_values rather than get_limit_value, no? Name is super important for LLMs to determine which tool to call
| """ | ||
| try: | ||
| client = get_limits_client() | ||
| items = list_limit_values_with_pagination( |
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.
I did this same thing in some of my earlier PRs where I split things up into a utils function, but we decided as a team to try and keep relevant code near each other. Could we just take the contents of this help file and stick them here?
Edit: I see this function gets reused below, hmmm....
|
Closes #36 |
Description
Integrating Limits MCP server to support fetching limit value and usage
Fixes # 34
Type of change
How Has This Been Tested?
Checklist: