feat: implement CLI entrypoints and minimize docker image size#1
feat: implement CLI entrypoints and minimize docker image size#1simonmysun wants to merge 70 commits into
Conversation
…ng dev dependencies and add instructions for usage inside Docker
There was a problem hiding this comment.
Pull request overview
This PR adds command-line entrypoints for multiple services and refactors container/build packaging to ship smaller runtime images by bundling (esbuild) and using multi-stage Docker builds. It also introduces a breaking operational change by separating Neo4j into its own service for graph-rewriting-service (removing supervisord and the embedded DB from that image).
Changes:
- Add CLIs and corresponding tests (SQL assessment, JSONPath mapper, graph rewriting).
- Introduce esbuild-based bundling/build scripts and update package scripts accordingly.
- Rework Dockerfiles (multi-stage builds, slimmer runtime images) and adjust graph rewriting deployment to use a separate Neo4j service.
Reviewed changes
Copilot reviewed 33 out of 36 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| services/sql-assessment-service/test/cli/express-cli-adapter.test.ts | Adds unit tests for extracting/invoking Express routes via CLI adapter. |
| services/sql-assessment-service/test/cli/cli-route-table.test.ts | Adds integration test building a full CLI route table from controllers. |
| services/sql-assessment-service/src/index.ts | Refactors service entrypoint to use a shared bootstrap controller factory. |
| services/sql-assessment-service/src/cli/index.ts | Adds SQL assessment CLI entrypoint (route discovery + request body input). |
| services/sql-assessment-service/src/cli/express-cli-adapter.ts | Implements router introspection and in-process handler invocation for CLI. |
| services/sql-assessment-service/src/bootstrap.ts | Introduces shared dependency wiring via createControllers(). |
| services/sql-assessment-service/package.json | Switches build to esbuild bundling; adds CLI script and build:tsc. |
| services/sql-assessment-service/package-lock.json | Updates lockfile for new build tooling deps (esbuild, ts-node). |
| services/sql-assessment-service/Makefile | Adds cli target and updates .PHONY list. |
| services/sql-assessment-service/import-meta-url.js | Adds an esbuild inject shim for import.meta.url. |
| services/sql-assessment-service/Dockerfile | Converts to multi-stage build with bundled JS runtime. |
| services/sql-assessment-service/.dockerignore | Ignores build/ in Docker context. |
| services/jsonpath-mapper-service/tests/cli.tests.mjs | Adds CLI integration tests (stdin/files/output flag/help/errors). |
| services/jsonpath-mapper-service/src/cli.ts | Adds a Node CLI for mapping JSON via template files. |
| services/jsonpath-mapper-service/src/api/server.ts | Changes API metadata to use env-based version and a constant description. |
| services/jsonpath-mapper-service/scripts/build-api.mjs | Adds esbuild bundling script for API + CLI artifacts. |
| services/jsonpath-mapper-service/rollup.config.mjs | Adds rollup build output for CLI (dist/cli.mjs) with shebang banner. |
| services/jsonpath-mapper-service/package.json | Adds bin entry, build/test scripts for CLI, and esbuild dev dependency. |
| services/jsonpath-mapper-service/import-meta-url.js | Adds an esbuild inject shim for import.meta.url. |
| services/jsonpath-mapper-service/Dockerfile | Switches runtime to a single bundled API file + copied Swagger UI static assets. |
| services/jsonpath-mapper-service/.dockerignore | Restricts Docker build context to minimal build inputs (source/scripts/metadata). |
| services/graph-rewriting-service/supervisord.conf | Removes supervisord configuration (no longer used). |
| services/graph-rewriting-service/src/cli.ts | Adds graph rewriting CLI (transform/find/import + node/edge CRUD) backed by Neo4j. |
| services/graph-rewriting-service/src/cli.test.ts | Adds vitest coverage for CLI parsing, IO helpers, and command handlers. |
| services/graph-rewriting-service/scripts/entrypoint.sh | Removes entrypoint that previously booted Neo4j + supervisord in one container. |
| services/graph-rewriting-service/scripts/build.mjs | Adds esbuild bundling for API+CLI and copies static assets into dist. |
| services/graph-rewriting-service/README.md | Updates docs for dev/prod setups with external Neo4j and new build pipeline. |
| services/graph-rewriting-service/package.json | Switches build to esbuild script, adds bin entry + typecheck + cli script. |
| services/graph-rewriting-service/package-lock.json | Updates lockfile for esbuild addition and bin metadata. |
| services/graph-rewriting-service/Dockerfile | Replaces Neo4j-based image with slim Node runtime; adds healthcheck and env defaults. |
| services/graph-rewriting-service/docker-compose.prod.yml | Adds standalone Neo4j service and wires app service to depend on it. |
| services/graph-rewriting-service/.env.example | Updates env defaults for external Neo4j configuration. |
| services/fermentaladin-service/README.md | Documents Docker usage changes (no uv in runtime; use python directly). |
| services/fermentaladin-service/Dockerfile | Converts to multi-stage build: build venv with uv, run with venv on PATH. |
| services/fermentaladin-service/.dockerignore | Adds dockerignore to reduce build context contents. |
Files not reviewed (3)
- services/graph-rewriting-service/package-lock.json: Language not supported
- services/jsonpath-mapper-service/package-lock.json: Language not supported
- services/sql-assessment-service/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| it('database:analyze-database returns 500 for completely missing body', async () => { | ||
| const route = routes.find( | ||
| (r) => r.command === 'database:analyze-database', | ||
| )!; | ||
| const result = await invokeHandler(route.handler, {}); | ||
| // Controller crashes (validateConnectionInfo gets undefined) → caught as 500 | ||
| expect(result.statusCode).toBe(500); | ||
| expect(result.data).toHaveProperty('message'); | ||
| }); |
There was a problem hiding this comment.
This test currently asserts a 500 when the request body is missing, and notes that the controller crashes. Given the OpenAPI for /api/database/analyze-database declares a required request body, missing/invalid bodies should ideally return a 400 instead of throwing. Recommend fixing the controller validation and updating this assertion so the CLI integration test doesn't lock in a server-side bug.
There was a problem hiding this comment.
The CLI script only adapts the original controller code and follows its implementation.
Possibly fix in services/sql-assessment-service/src/database/database-controller.ts @plc-dev if you agree with the review.
… from package.json
Co-authored-by: Copilot <[email protected]>
|
TODO: integrate embedded databases and make external databases optional
|
…nt routes - inject /api/database execution into generation, grading, description, query - reduce duplicated setup logic across endpoints - add integration tests for API chaining - document updated API workflow in README
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 73 out of 78 changed files in this pull request and generated 2 comments.
Files not reviewed (3)
- services/graph-rewriting-service/package-lock.json: Language not supported
- services/jsonpath-mapper-service/package-lock.json: Language not supported
- services/sql-assessment-service/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ute handler Co-authored-by: Copilot <[email protected]>
…ssing edges Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 75 out of 80 changed files in this pull request and generated 2 comments.
Files not reviewed (3)
- services/graph-rewriting-service/package-lock.json: Language not supported
- services/jsonpath-mapper-service/package-lock.json: Language not supported
- services/sql-assessment-service/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…t absent Replace `if (!sqlContent)` with `if (sqlContent == null)` so only a truly absent value (undefined/null) triggers the skip/init-SQL fallback path. An explicitly supplied empty string now falls through to analyzePGlite, which already validates it and returns 400. Also removes the now-redundant inner `if (!sqlContent)` guard. Adds two regression tests covering sqlContent: '' for both required=false and required=true. Co-authored-by: Copilot <[email protected]>
…432/northwind format Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 75 out of 80 changed files in this pull request and generated 2 comments.
Files not reviewed (3)
- services/graph-rewriting-service/package-lock.json: Language not supported
- services/jsonpath-mapper-service/package-lock.json: Language not supported
- services/sql-assessment-service/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…nd assert HTTP mapping Co-authored-by: Copilot <[email protected]>
…-file without path Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 76 out of 81 changed files in this pull request and generated 3 comments.
Files not reviewed (3)
- services/graph-rewriting-service/package-lock.json: Language not supported
- services/jsonpath-mapper-service/package-lock.json: Language not supported
- services/sql-assessment-service/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ePositionalBody test
… deployment Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 76 out of 81 changed files in this pull request and generated 1 comment.
Files not reviewed (3)
- services/graph-rewriting-service/package-lock.json: Language not supported
- services/jsonpath-mapper-service/package-lock.json: Language not supported
- services/sql-assessment-service/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 76 out of 81 changed files in this pull request and generated no new comments.
Files not reviewed (3)
- services/graph-rewriting-service/package-lock.json: Language not supported
- services/jsonpath-mapper-service/package-lock.json: Language not supported
- services/sql-assessment-service/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This PR includes following changes:
graph-rewriting-serviceis now a stateless service and neo4j has to be additionally deployedAdditional tests are recommended as I don't fully know all their functionalities.
Following errors are observed and not fixed:
sql-assessment-service