Skip to content

Make chdb an optional dependency for Windows compatibility#145

Open
joe-clickhouse wants to merge 4 commits intomainfrom
joe/make-chdb-optional
Open

Make chdb an optional dependency for Windows compatibility#145
joe-clickhouse wants to merge 4 commits intomainfrom
joe/make-chdb-optional

Conversation

@joe-clickhouse
Copy link
Collaborator

Summary

  • Moves chdb from a hard dependency to an optional extra via pip install mcp-clickhouse[chdb] fixing installation on Windows where chdb has no wheels
  • Conditionally registers chdb tools only when the package is installed and initialization succeeds
  • Clear error messages guiding users to install the extra if it's missing
  • Updates README, fastmcp.json, and test commands to reflect the optional dependency

Closes #143

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Makes chdb an optional extra so the package can install on Windows (where chdb wheels are unavailable), and gates chDB tool registration/runtime behavior based on whether the extra is installed and initialization succeeds.

Changes:

  • Move chdb from a required dependency to an optional extra (mcp-clickhouse[chdb]) in packaging and lockfiles.
  • Conditionally initialize/register chDB tools and improve error surfacing when chDB is enabled but unavailable.
  • Update docs and tests to reflect the optional dependency and conditional behavior.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pyproject.toml Moves chdb into an optional extra (chdb).
uv.lock Updates locked metadata to mark chdb as an extra and advertise the extra.
mcp_clickhouse/mcp_server.py Adds global chDB client/error state, conditional init/registration, and health reporting for chDB failures.
tests/test_optional_chdb.py Adds unit tests for optional-dependency behavior and conditional tool registration.
tests/test_chdb_tool.py Skips chDB tests when chdb isn’t installed.
fastmcp.json Removes chdb from the default dependency list.
README.md Documents the chdb extra and updates test command examples.
Comments suppressed due to low confidence (1)

tests/test_chdb_tool.py:17

  • setUpClass calls create_chdb_client(), which raises ValueError unless CHDB_ENABLED=true. The test is only skipped based on whether chdb is installed, so running pytest tests/test_chdb_tool.py with the extra installed but without CHDB_ENABLED will fail. Consider setting CHDB_ENABLED=true in the test (or extending the skip condition to also require the env var).
@unittest.skipUnless(importlib.util.find_spec("chdb") is not None, "requires chdb extra")
class TestChDBTools(unittest.TestCase):
    @classmethod
    def setUpClass(cls):
        """Set up the environment before chDB tests."""
        cls.client = create_chdb_client()


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

@busyerakli busyerakli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know if I'm allowed to put approvals here, but LGTM

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

mcp_clickhouse/mcp_server.py:138

  • In this branch, the code assumes both ClickHouse and chDB are disabled. However, it’s also reached when CHDB_ENABLED=true but _chdb_client is still None and _chdb_error_message is not set yet (e.g., chDB was enabled after module import, or init hasn’t been attempted). In that case the health check returns a misleading “both disabled” error. Consider handling chdb_config.enabled explicitly here (e.g., attempt _init_chdb_client() once, or return a distinct 503 like “chDB enabled but not initialized”).
            else:
                # Both ClickHouse and chDB are disabled - this is an error
                return PlainTextResponse(
                    "ERROR - Both ClickHouse and chDB are disabled. At least one must be enabled.",
                    status_code=503,

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

Windows is not supported

3 participants