-
Notifications
You must be signed in to change notification settings - Fork 8
✨ run MCP on Databricks Apps #25
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
Conversation
@smurching I suggest accepting this PR instead of #22 since it's proven to work and provides simpler integration pattern :) |
README.md
Outdated
@@ -76,6 +77,24 @@ the following tools: | |||
|
|||
This server is currently under construction. It is not yet usable, but contributions are welcome! | |||
|
|||
## Deploying the MCP server on Databricks Apps |
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.
@renardeinside sorry I totally missed this, thanks for doing this! cc @aravind-segu. Would you mind moving this up into the Unity Catalog MCP server README section? Since the devtools server doesn't include any code just yet
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.
Let's keep everything in one readme. I'll later refactor it into proper docs via docusaurus, same as for DQX. Having separate files is quite inconvenient.
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.
Oh I meant just move this section up - currently it looks like it's under or associated with
Developer Tools Server
This server is currently under construction. It is not yet usable, but contributions are welcome!
But it only works for the UC server
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.
Definitely +1 to a single README
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 can help make that edit
@@ -0,0 +1,65 @@ | |||
from mcp.server import NotificationOptions, Server |
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.
Nice, any reason not to just replace the existing stdio-based server implementation with this one? We can do it in a follow-up PR, but just curious
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.
Eventually seems like we'd just have one streamable-HTTP-based implementation (once more clients support streamable HTTP)
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.
Also, depending on how much work it is, it's actually now possible to use streamable HTTP to write the server, as of four days ago! https://github.com/modelcontextprotocol/python-sdk/releases/tag/v1.8.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.
I'll update my version then, should work easily.
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.
Done, updated!
@@ -0,0 +1,26 @@ | |||
bundle: |
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.
We should move this into the UC server directory, to enable separately deploying individual servers, I can help with that
message: JSONRPCMessage | ||
|
||
|
||
class InMemoryEventStore(EventStore): |
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.
Curious if we actually need any in-memory event storage for our server, it is stateless right?
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 think we can keep memory for now, and remove it in future if needed. Apps have 6GB ram, ponentially we can also save into temp db
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.
Chatted offline, this setup LGTM!
Implementation for MCP server on Databricks apps.