Adding tool to fetch the public IP for databases, plus some small fixes in descriptions#107
Adding tool to fetch the public IP for databases, plus some small fixes in descriptions#107shyam2511 wants to merge 1 commit intooracle:mainfrom
Conversation
oracle-contributor-agreement
bot
added
the
OCA Verified
shyam2511
force-pushed
the
database-mcp-changes
branch
2 times, most recently
from
030c3ed to
6b3fcd0
Compare
| None, | ||
| description="**[Required]** Gets the compartment_id of this PluggableDatabase. The `OCID`__ of the compartment. __ https://docs.cloud.oracle.com/Content/General/Concepts/identifiers.htm", | ||
| ) | ||
| connection_strings: Optional[str] = Field(None, description="") |
There was a problem hiding this comment.
Can we add a description here? Also I think this should have a type associated with it instead of Any
| ) | ||
| connection_strings: Optional[str] = Field(None, description="") | ||
| connection_strings: Optional[Any] = Field(None, description="") | ||
| container_database_id: Optional[str] = Field( |
There was a problem hiding this comment.
container_database_id field is marked as optional but the description it says its required. I think we need to adjust one of these
| freeform_tags: Optional[str] = Field( | ||
| freeform_tags: Optional[dict] = Field( | ||
| None, | ||
| description='Free-form tags for this resource. Each tag is a simple key-value pair with no predefined name, type, or namespace. For more information, see `Resource Tags`__. Example: `{"Department": "Finance"}` __ https://docs.cloud.oracle.com/Content/General/Concepts/resourcetags.htm', |
There was a problem hiding this comment.
Any examples should be in the examples argument. See https://docs.pydantic.dev/2.0/api/fields/#pydantic.fields.Field
There was a problem hiding this comment.
This model is a pojo that is never visible to the agent, hence the description shouldn't matter here
| return map_pluggabledatabase(response.data) | ||
|
|
||
|
|
||
| def get_public_ip_for_db(database_id, region=None): |
There was a problem hiding this comment.
Can we add unit tests for these changes
|
|
||
| try: | ||
| # Fetch VNIC details | ||
| get_vnic_response = virtual_network_client.get_vnic(node.vnic_id) |
There was a problem hiding this comment.
We already have a tool in the networking mcp server called get_vnic. Is it worth it to just use that instead?
There was a problem hiding this comment.
We require this tool to be in our server itself, that's why we have added it. Otherwise the user will have to install both servers, and the agent would have no way to know how to actually get the public IPs even when given these tools
shyam2511
force-pushed
the
database-mcp-changes
branch
from
6b3fcd0 to
2a33f60
Compare
…es in descriptions.
shyam2511
force-pushed
the
database-mcp-changes
branch
from
2a33f60 to
1a7c01c
Compare
shyam2511
requested review from
AlaaShaker,
gebhardtr and
krisrice
as code owners
shyam2511
changed the title
2 participants
Description
Adding tool to fetch the public IP for databases, plus some small fixes in descriptions.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Checklist: