updated tool naming as per suggestion #125
Conversation
modelcontextprotocol/server.py
Outdated
| from tools.glossary import ( | ||
| create_glossary_category_assets as create_glossary_category_assets_impl, | ||
| create_glossary_assets as create_glossary_assets_impl, | ||
| create_glossary_term_assets as create_glossary_term_assets_impl, | ||
| ) |
There was a problem hiding this comment.
why are we importing the tools separately? can we follow the previous import style.
There was a problem hiding this comment.
This is because we have to import it as implementation function.
we could change the names of functions directly in modules to avoid importing like this.
but as a standard in python we prefer Import Aliasing
Why current approach is better:
- Preserves Original API: The implementation functions keep their intended names
- Context-Specific Naming: The alias reflects the usage context, not the implementation
- Less Invasive: Doesn't require changing the actual function definitions
- Flexibility: Different modules can import with different aliases as needed
- Clear Intent: Shows that the renaming is for disambiguation, not because the function is "internal"
There was a problem hiding this comment.
no no, i meant why not
from tools import (
search_assets as search,
get_assets_by_dsl as fetch_assets_by_dsl,
...
)
There was a problem hiding this comment.
yup that will be cleaner, I will make that change.
modelcontextprotocol/server.py
Outdated
| create_glossary_term_assets as create_glossary_term_assets_impl, | ||
| ) | ||
| from tools.search import search_assets as search_assets_impl | ||
| from tools.dsl import get_assets_by_dsl as get_assets_by_dsl_impl |
There was a problem hiding this comment.
_impl doesn't sound nice. lets improve the code readability here please.
There was a problem hiding this comment.
In Python, _impl is a naming convention
When you see _impl in a Python codebase, it often signifies that the associated code (e.g., a function, class, or module) is an internal implementation detail and is not intended for direct public use.
we need to do this in order to not have naming conflicts of tool names with implementation functions in respective modules.
This approach is widely used in professional Python codebases
There was a problem hiding this comment.
_impl is a java convention and not python, please check
| from tools.glossary import ( | ||
| create_glossary_category_assets as create_glossary_category_assets_impl, | ||
| create_glossary_assets as create_glossary_assets_impl, | ||
| create_glossary_term_assets as create_glossary_term_assets_impl, | ||
| ) | ||
| from tools.search import search_assets as search_assets_impl | ||
| from tools.dsl import get_assets_by_dsl as get_assets_by_dsl_impl | ||
| from tools.lineage import traverse_lineage as traverse_lineage_impl | ||
| from tools.assets import update_assets as update_assets_impl |
There was a problem hiding this comment.
responded there.
🔧 Rename MCP Tools to Follow Clean Naming Convention
Resolved #75
Summary
Renamed MCP tool functions from
*_toolsuffix to clean<action>_<type>format to improve API consistency and remove redundant naming.Changes Made
Tool Renames:
search_assets_tool→search_assetsget_assets_by_dsl_tool→get_assets_by_dsltraverse_lineage_tool→traverse_lineageupdate_assets_tool→update_assetsImplementation Improvements:
_implsuffix to avoid naming conflictsDocumentation Updates:
README.mdtool restriction examples and available tools list.cursor/rules/project-structure.mdcarchitecture documentationTesting
✅ All 7 tools properly registered and functional
✅ No old tool names remaining
✅ Server imports and starts successfully
✅ No linting errors
Breaking Change
Migration: