Skip to content

Add dev tools #17

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

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open

Conversation

JordiNeil
Copy link

@JordiNeil JordiNeil commented Apr 27, 2025

Related Issues/PRs

These changes are related to issue #5 regarding adding dev tools

I tried to follow current structure for UC mcp

What changes are proposed in this pull request?

Add:

  • Job Tools:
    • List jobs: List all jobs in workspace
    • Get Job: Get job details by id
  • SQL Query Tool:
    • Execute sql query
  • Test Connection Tool
  • Dockerfile

How is this PR tested?

  • Existing unit/integration tests
  • New unit/integration tests
  • Manual tests

image

image

Does this PR require documentation update?

  • No. You can skip the rest of this section.
  • Yes. I've updated the relevant server README.md

Release Notes

Is this a user-facing change?

  • No. You can skip the rest of this section.
  • Yes. Give a description of this change to be included in the release notes.

Added new tools for jobs, sql query and test connection.
Added docker

How should the PR be classified in the release notes? Choose one:

  • rn/none - No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" section
  • rn/breaking-change - The PR will be mentioned in the "Breaking Changes" section
  • rn/feature - A new user-facing feature worth mentioning in the release notes
  • rn/bug-fix - A user-facing bug fix worth mentioning in the release notes
  • rn/documentation - A user-facing documentation change worth mentioning in the release notes

@JordiNeil JordiNeil marked this pull request as ready for review April 27, 2025 00:31
@smurching smurching requested a review from jai-behl April 28, 2025 05:04
@smurching
Copy link
Contributor

Thanks @JordiNeil ! Would you mind adding unit tests for some of the tools you introduced? You can see the tests for the Unity Catalog MCP server in https://github.com/databrickslabs/mcp/tree/master/servers/unity_catalog/tests as an example - cc also @jai-behl @annzhang-db to take a look

@JordiNeil
Copy link
Author

Do you want to keep a single pyproject.toml for the entire repo? Or do you want to separate each mcp server?

@smurching
Copy link
Contributor

cc @renardeinside to comment here - initially we had multiple pyproject.toml, but @renardeinside helped update to a single one to match the standard of other Databricks Labs projects - I imagine we'd want to stick with that pattern here

@renardeinside
Copy link
Contributor

Dear team,
having separate project.toml files is not the recommended approach with uv.
there are projects with tons of dependencies who don't introduce it like this.

To manage separated dependencies please use dependency groups:

uv add --group=dev-tools my-dependency

And not separate pyproject.toml files.

@renardeinside
Copy link
Contributor

Same on the topic of separate readme.mds -> please keep everything inside one README.md in the repository root.
In 2-3 weeks we'll introduce proper documentation (like for dqx) and it would be simpler for refactoring if we keep it all together.

@renardeinside
Copy link
Contributor

As for the PR in general:

  • please merge it with recent master branch (btw @smurching we need to rename it to main)
  • make sure folder structure follows what we have currently (src/databricks/labs/mcp/servers/developer_tools <- put dev tools here)

When PR is ready please tag @smurching and me for review.

I would like to highlight that It's a great piece of work, please don't consider my comments as nitpicks - just helping to structure and unify labs approaches across projects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants