closes #4 Refactor codebase into a modular structure with backward compatibility#8
closes #4 Refactor codebase into a modular structure with backward compatibility#8
Conversation
abutbul
commented
May 23, 2025
- Updated .gitignore to exclude build artifacts and cache files.
- Created MANIFEST.in to include necessary files in the package.
- Added MODULAR_REFACTORING.md to document the refactoring process.
- Enhanced README.md to reflect the new modular design and usage instructions.
- Introduced compatibility_example.py to demonstrate usage of both original and modular approaches.
- Added programmatic_usage.py to showcase how to use the package programmatically.
- Updated generator.py to support modular imports and fallback to original implementation.
- Created mcp_generator.py as a new entry point for the modular structure.
- Established init.py to define the package interface.
- Developed cli.py for command-line interface functionality.
- Implemented generator.py for core generation logic.
- Added generators.py for generating tool and resource definitions.
- Created http.py for HTTP client utilities.
- Developed parser.py for parsing OpenAPI specifications.
- Introduced project.py for project directory and file management.
- Updated pyproject.toml for package metadata and dependencies.
- Created setup.py for package installation.
- Updated .gitignore to exclude build artifacts and cache files. - Created MANIFEST.in to include necessary files in the package. - Added MODULAR_REFACTORING.md to document the refactoring process. - Enhanced README.md to reflect the new modular design and usage instructions. - Introduced compatibility_example.py to demonstrate usage of both original and modular approaches. - Added programmatic_usage.py to showcase how to use the package programmatically. - Updated generator.py to support modular imports and fallback to original implementation. - Created mcp_generator.py as a new entry point for the modular structure. - Established __init__.py to define the package interface. - Developed cli.py for command-line interface functionality. - Implemented generator.py for core generation logic. - Added generators.py for generating tool and resource definitions. - Created http.py for HTTP client utilities. - Developed parser.py for parsing OpenAPI specifications. - Introduced project.py for project directory and file management. - Updated pyproject.toml for package metadata and dependencies. - Created setup.py for package installation.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the codebase into a modular Python package while maintaining backward compatibility. Key changes include new package structure and entry points, updated build configuration files, and enhanced documentation reflecting these changes.
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| setup.py, pyproject.toml, MANIFEST.in | New and updated packaging files for package installation |
| openapi_mcp_generator/* | Modular reorganization of core functionality into a package |
| generator.py, mcp_generator.py, cli.py | New and alternative entry points with CLI improvements and fallbacks |
| README.md, MODULAR_REFACTORING.md | Updated documentation and usage instructions reflecting modular design |
| examples/* | Examples updated to showcase multiple usage methods |
great suggestion. missed out on that Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
Refactors the project into a modular Python package while preserving the original script-based workflow for backward compatibility.
- Introduces
openapi_mcp_generatorpackage with separated modules (CLI, generator, parser, HTTP utils, project builder). - Adds new entry point (
mcp_generator.py) alongside the originalgenerator.pyto support both workflows. - Updates packaging (setup.py, pyproject.toml, MANIFEST.in), documentation (README.md, MODULAR_REFACTORING.md), and CI to install and test the package.
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| generator.py | Kept original CLI script and added fallback to modular implementation |
| mcp_generator.py | New wrapper entry point invoking the modular CLI |
| openapi_mcp_generator/generator.py | Core modular generation logic and template context creator |
| openapi_mcp_generator/cli.py | New CLI module |
| setup.py, pyproject.toml, MANIFEST.in | Packaging updates for proper package installation |
| README.md, MODULAR_REFACTORING.md | Documentation updated to explain modular design |
| tests/test_generated_server.py | New tests for validating generated server output |
Comments suppressed due to low confidence (5)
setup.py:13
- Ensure
setup.py's install_requires matchespyproject.tomldependencies—addmcp[cli]>=0.1.0to avoid missing runtime requirements.
"install_requires=[
.github/workflows/generate-and-test.yml:25
- CI references a non-existent
requirements.txt; switch to installing viapip install .or usingpyproject.toml.
pip install -r requirements.txt
openapi_mcp_generator/generator.py:118
- Don’t hardcode
auth_typein the template context—use the passed-inauth_typeparameter to support all authentication modes.
"auth_type": "bearer", # Always use bearer auth
tests/test_generated_server.py:1
- [nitpick] Consider adding tests for the new CLI entry point and for error paths in
parse_openapi_specto improve coverage of the modular code.
import importlib.util
openapi_mcp_generator/generators.py:59
- The
Contexttype is not imported; addfrom mcp import Contextor fully qualify the type to prevent NameError at runtime.
parameters_definitions.append("ctx: Context")
openapi_mcp_generator/cli.py
Outdated
| project_dir = generate_mcp_server( | ||
| args.openapi_file, | ||
| args.output_dir, | ||
| args.api_url, | ||
| args.auth_type, | ||
| args.api_token, | ||
| args.api_username, | ||
| args.api_password | ||
| ) | ||
|
|
||
| print(f"MCP server generated successfully in: {project_dir}") | ||
| print(f"To build and run the Docker container:") | ||
| print(f" cd {project_dir}") | ||
| print(f" ./docker.sh build") | ||
| print(f" ./docker.sh start --transport=sse") | ||
| return 0 |
There was a problem hiding this comment.
There’s unreachable duplicate code in the except block after the return; remove the redundant lines to simplify the control flow.
| project_dir = generate_mcp_server( | |
| args.openapi_file, | |
| args.output_dir, | |
| args.api_url, | |
| args.auth_type, | |
| args.api_token, | |
| args.api_username, | |
| args.api_password | |
| ) | |
| print(f"MCP server generated successfully in: {project_dir}") | |
| print(f"To build and run the Docker container:") | |
| print(f" cd {project_dir}") | |
| print(f" ./docker.sh build") | |
| print(f" ./docker.sh start --transport=sse") | |
| return 0 |
| from pathlib import Path | ||
|
|
There was a problem hiding this comment.
The Path import is unused—remove it to clean up imports.
| from pathlib import Path |
| ```python | ||
| from openapi_mcp_generator import generator | ||
|
|
||
| generator.generate('samples/TriliumNext/etapi.openapi.yaml') |
There was a problem hiding this comment.
Update the example to call generate_mcp_server (not generate), matching the actual API function name.
| generator.generate('samples/TriliumNext/etapi.openapi.yaml') | |
| generator.generate_mcp_server('samples/TriliumNext/etapi.openapi.yaml') |
| @@ -0,0 +1,4 @@ | |||
| include LICENSE | |||
| include README.md | |||
| include requirements.txt | |||
There was a problem hiding this comment.
Remove or update the reference to requirements.txt—that file doesn’t exist at the project root.
| include requirements.txt |
| SystemExit: If the file cannot be read or parsed | ||
| """ | ||
| if not os.path.exists(filepath): | ||
| print(f"Error: OpenAPI specification file not found: {filepath}") |
There was a problem hiding this comment.
Error messages should go to stderr (e.g., print(..., file=sys.stderr)) instead of stdout to separate error logging from normal output.
| print(f"Error: OpenAPI specification file not found: {filepath}") | |
| print(f"Error: OpenAPI specification file not found: {filepath}", file=sys.stderr) |