Add common dir #19
Add common dir #19
Conversation
a-s-g93
commented
Apr 16, 2025
a-s-g93
commented
- Pull out common functions from the Cypher MCP into a common package to be used by all MCP servers
- write IT for common
- Update Cypher MCP server with changes
- Test Common and Neo4j Cypher MCP
…reate IT, update cypher server
|
this builds on PR #18 and should be merged after. |
| time.sleep(initial_delay) | ||
| print("\nWaiting for Neo4j to Start...\n", file=sys.stderr) | ||
|
|
||
| while not success and attempts < max_attempts: |
There was a problem hiding this comment.
isn't that more like a test issue than a runtime issue? at runtime the neo4j db should be up and running
should be handled in test infrastructure instead that waits that a server is up (like testcontainers does internally)
| raw_results = await tx.run(query, params) | ||
| eager_results = await raw_results.to_eager_result() | ||
|
|
||
| return [r.data() for r in eager_results.records] |
| A list of dictionaries containing the results of the query or error messages. | ||
| """ | ||
| try: | ||
| async with neo4j_driver.session(database=database) as session: |
There was a problem hiding this comment.
we can use driver.execute_query which handles retries internally and automatically
| A list of dictionaries containing the results of the query or error messages. | ||
| """ | ||
| try: | ||
| async with neo4j_driver.session(database=database) as session: |
There was a problem hiding this comment.
here retries would actually be important, but they are also handled by driver.execute_query
| environment: | ||
| NEO4J_AUTH: neo4j/testingpassword | ||
| NEO4J_ACCEPT_LICENSE_AGREEMENT: "eval" | ||
| NEO4J_PLUGINS: "[\"apoc\"]" No newline at end of file |
There was a problem hiding this comment.
if possible we shouldn't require apoc or enterprise
| from typing import Any | ||
|
|
||
| @pytest.mark.asyncio(loop_scope="session") | ||
| async def test_execute_read_query_with_parameters(async_neo4j_driver: AsyncDriver, healthcheck: Any, init_data: Any): |
There was a problem hiding this comment.
would be good if fixture and use of fixture data were together in the same file
|
|
||
| 3. Run Integration Tests | ||
|
|
||
| **CLOSE ANY LOCAL NEO4J DATABASES BEFORE RUNNING TESTS** |
There was a problem hiding this comment.
we should not require that, but instead use a safe way of running databases that doesn't conflict with existing db's
| "--directory", "parent_of_servers_repo/servers/mcp-neo4j-cypher/src", | ||
| "run", "mcp-neo4j-cypher"], | ||
| "env": { | ||
| "NEO4J_URL": "bolt://localhost", |
| raw_results = await session.execute_write(_write, query, params) | ||
| counters_json_str = json.dumps(raw_results._summary.counters.__dict__, default=str) | ||
| result = await execute_write_query( | ||
| neo4j_driver, query, params, database, logger, False |
There was a problem hiding this comment.
are usually loggers passed to functions in python?
the false should be a named parameter, otherwise the reader doesn't know what it does
| ) -> None: | ||
| logger.info("Starting MCP neo4j Server") | ||
|
|
||
| neo4j_driver = AsyncGraphDatabase.driver( |
There was a problem hiding this comment.
shouldn't this use the common infra ...
2 participants
