Skip to content

fix: Ensure .env loaded before config init during mcp dev startup #30

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

Merged
merged 1 commit into from
Apr 15, 2025

Conversation

sealpp
Copy link
Contributor

@sealpp sealpp commented Apr 9, 2025

Problem Description

When attempting to start the service using mcp dev mcp_clickhouse/mcp_server.py, the following ValueError occurs:

ValueError: Missing required environment variables: CLICKHOUSE_HOST, CLICKHOUSE_USER, 
CLICKHOUSE_PASSWORD

This error appears even if the project's root .env file is correctly configured.

Root Cause

The issue stems from the fact that the mcp_env.py file directly instantiates the ClickHouseConfig class at the module's top level (near the end of the file). When mcp_server.py executes from mcp_clickhouse.mcp_env import ..., it triggers the execution of code within mcp_env.py, including the immediate instantiation of ClickHouseConfig.

During its initialization (__init__), ClickHouseConfig attempts to read the ClickHouse-related environment variables. However, at this point, the load_dotenv() function in mcp_server.py (responsible for loading variables from the .env file) might not have executed yet. This results in missing environment variables, thus raising the ValueError.

Solution

To resolve this issue and adopt a more robust pattern, this change introduces a Lazy Loading mechanism for the ClickHouseConfig instantiation:

  1. Modify mcp_env.py:

    • Removed the direct instantiation config = ClickHouseConfig() at the module level.
    • Added a module-level variable _CONFIG_INSTANCE initialized to None.
    • Implemented a get_config() function (using the standard global keyword to modify the module-level _CONFIG_INSTANCE). This function creates the ClickHouseConfig instance upon its first call, stores it in _CONFIG_INSTANCE, and returns the cached instance on subsequent calls.
  2. Modify mcp_server.py:

    • Ensured the load_dotenv() call happens after all import statements but before the configuration is potentially needed.
    • Changed the import from mcp_clickhouse.mcp_env to import the get_config function instead of the (now removed) config instance.
    • In places where the configuration is needed (e.g., within the create_clickhouse_client function), call the get_config() function to retrieve the configuration instance.

Benefits

  • Reliability: Guarantees that environment variables from .env are loaded before the configuration object attempts to access them during the mcp dev startup flow.
  • Code Clarity: Keeps the import block cleaner and follows standard Python practices regarding module initialization.
  • Deferred Initialization: The creation of the configuration object is postponed until it's actually required by the application logic.

How to Test

  1. Ensure the root .env file is correctly configured according to README.md.
  2. Run mcp dev mcp_clickhouse/mcp_server.py.
  3. The service should start normally without the previous ValueError.

@iskakaushik iskakaushik requested a review from serprex April 15, 2025 13:44
@iskakaushik iskakaushik merged commit 7f0805c into ClickHouse:main Apr 15, 2025
1 check passed
emanb29 pushed a commit to hydrolix/mcp-hydrolix that referenced this pull request Apr 21, 2025
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.

3 participants