feat(docker): update Temporal setup and add worker configuration #28
Conversation
- Updated the base image for the Temporal Dockerfile to version 1.27.2. - Changed database configuration in `docker-compose.yml` to use `postgres12` and added a new environment variable for visibility database. - Introduced a new Dockerfile for the Temporal worker, defining multi-stage builds for development and production environments. - Enhanced healthcheck command in the Temporal Dockerfile for improved service monitoring. - Added error handling in the main worker script to ensure unhandled errors are logged. These changes improve the setup and configuration of the Temporal service and its workers, enhancing reliability and maintainability.
|
Warning Rate limit exceeded@anatolyshipitz has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 11 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 Walkthrough""" WalkthroughThis update modernizes Temporal service deployment by upgrading the base image, refactoring PostgreSQL configuration to use environment variables, and simplifying health checks. It introduces a multi-stage Dockerfile for the Temporal worker, adds a dedicated worker service to Docker Compose, and ensures robust worker process error handling with immediate invocation and error logging in the worker entrypoint. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant Docker Compose
participant Temporal Service
participant Temporal Worker
Developer->>Docker Compose: up
Docker Compose->>Temporal Service: start with env vars for PostgreSQL
Docker Compose->>Temporal Worker: start (dev mode, watches for changes)
Temporal Worker->>Temporal Worker: run() (immediately invoked)
Temporal Worker->>Temporal Worker: Log error and exit if run() fails
Temporal Worker->>Temporal Service: Connects to Temporal via network
Possibly related PRs
Suggested reviewers
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
🔍 Vulnerabilities of
|
| digest | sha256:f1f160ca92d755a1cc3608d43e17979574bc9d1f928190e2294aa1001ce72cfc |
| vulnerabilities |     |
| platform | linux/amd64 |
| size | 243 MB |
| packages | 1628 |
📦 Base Image node:20-alpine
| also known as |
|
| digest | sha256:37a5a350292926f98d48de9af160b0a3f7fcb141566117ee452742739500a5bd |
| vulnerabilities |   |
Description
| ||||||||||||
Description
| ||||||||||||
Description
| ||||||||||||
Description
| ||||||||||||
Description
| ||||||||||||
Description
| ||||||||||||
Description
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Dockerfile.temporal (1)
23-23: Fixed port exposure.Exposing a fixed port (7233) instead of using the TEMPORAL_PORT variable might simplify some configurations, but it reduces flexibility.
Consider using the TEMPORAL_PORT variable for more flexibility:
-EXPOSE 7233 +EXPOSE $TEMPORAL_PORT
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Dockerfile.temporal(1 hunks)Dockerfile.temporal-worker-main(1 hunks)docker-compose.yml(2 hunks)workers/main/src/index.ts(1 hunks)
🔇 Additional comments (10)
workers/main/src/index.ts (1)
9-12: Good addition of error handling for the worker process.The immediate invocation of the exported
runfunction with error handling ensures that unhandled promise rejections are properly caught, logged, and result in a non-zero exit code. This is crucial for containerized worker processes as it allows the orchestration system to detect failures.Consider these optional enhancements:
- Add more structured logging with additional context
- Implement graceful shutdown by handling process signals (SIGTERM, SIGINT)
run().catch((err) => { - console.error('Unhandled error in main:', err); + console.error('Unhandled error in main worker process:', err.message, { stack: err.stack }); + // Allow any pending logs to be flushed before exiting process.exit(1); }); +// Handle graceful shutdown +process.on('SIGTERM', async () => { + console.log('Received SIGTERM, shutting down gracefully'); + // Add cleanup code here (close connections, etc.) + process.exit(0); +});docker-compose.yml (2)
91-92: Database configuration improvements.Updating the DB type to
postgres12and adding the visibility database configuration improves the Temporal setup by explicitly configuring the visibility store.
149-168: Well-structured Temporal worker service configuration.The new
temporal-worker-mainservice follows Docker Compose best practices:
- Uses health check dependency to ensure proper startup order
- Mounts source directories for live development
- Properly isolates node_modules
- Includes watch configuration for automatic rebuilds
Consider adding environment variables for the worker service, especially for connecting to the Temporal server:
temporal-worker-main: container_name: temporal-worker-main build: context: . dockerfile: Dockerfile.temporal-worker-main target: dev depends_on: temporal: condition: service_healthy + environment: + - TEMPORAL_ADDRESS=temporal:${TEMPORAL_PORT:-7233} + - TEMPORAL_NAMESPACE=default volumes: - ./workers/main:/app/main - ./workers/common:/app/common - /app/main/node_modules networks: - app-networkDockerfile.temporal-worker-main (4)
1-7: Well-designed dependency stage for efficient caching.The deps stage properly separates dependency installation from code copying, which optimizes build caching. Installing netcat enables network connectivity checks that could be useful for health checks.
8-17: Good development environment setup with hot reloading.The dev stage correctly sets up a development environment with NODE_ENV and debugging enabled. Using nodemon with TypeScript support enables efficient development workflow by watching both local and shared code directories.
18-25: Clean build stage with proper dependency inheritance.The build stage correctly copies dependencies from the deps stage and builds the TypeScript application.
26-36: Security-focused production stage using distroless.Using a distroless image for production is an excellent security practice that reduces the attack surface by eliminating unnecessary packages. Running as a non-root user further enhances security.
There's a potential path inconsistency between the COPY destination and the CMD path:
COPY --from=build /app/main/dist ./build COPY --from=build /app/main/node_modules ./node_modules USER node -CMD ["node", "build/worker.js"] +CMD ["node", "build/index.js"]Also consider adding a health check for the production stage:
USER node +HEALTHCHECK --interval=10s --timeout=5s --retries=3 \ + CMD ["node", "-e", "const http=require('http');const options={host:'localhost',port:process.env.HEALTH_PORT||3000,path:'/health',timeout:2000};const req=http.request(options,r=>{if(r.statusCode==200){process.exit(0)}process.exit(1)});req.on('error',()=>process.exit(1));req.end()"] CMD ["node", "build/worker.js"]Dockerfile.temporal (3)
1-1: Good update to newer Temporal base image.Updating to version 1.27.2 brings in the latest features, performance improvements, and security fixes from Temporal.
5-15: Improved configuration with explicit environment variables.Adding build arguments and corresponding environment variables for PostgreSQL configuration makes the setup more flexible and customizable. This is a good practice for containerized applications.
17-18: Simplified and improved health check.The health check command has been simplified and timing parameters have been adjusted for quicker health status detection. This will improve the reliability of service dependencies.
Consider using variables instead of hardcoded values in the health check command:
HEALTHCHECK --interval=10s --timeout=5s --start-period=30s --retries=5 \ - CMD sh -c "tctl --address temporal:7233 cluster health" + CMD sh -c "tctl --address $HOST:$TEMPORAL_PORT cluster health"
…tests - Extracted error handling logic into a separate function `handleRunError` for better modularity and reusability. - Updated the main script to use the new error handling function. - Added unit tests for `handleRunError` to ensure proper logging and exit behavior on unhandled errors. These changes improve the robustness of error handling in the application and enhance test coverage for critical functionality.
- Modified ESLint rules to allow console error logging while maintaining warnings for other console methods. - Refactored test imports for better organization and clarity. - Enhanced the `handleRunError` test to ensure proper error logging and process exit behavior, improving test reliability. These changes improve code quality and testing practices, ensuring better adherence to ESLint rules and more robust error handling tests.
- Removed unnecessary installation of netcat from the Dockerfile for all stages. - Streamlined the build process by eliminating redundant commands, focusing on npm installations. These changes enhance the efficiency of the Dockerfile, reducing build time and complexity.
feat(dependencies): update package dependencies and TypeScript configuration - Added new dependencies: `@temporalio/client`, `@temporalio/worker`, `mysql2`, and `zod` to `package.json`. - Updated `@types/node` version to `22.15.21` for improved type definitions. - Adjusted `tsconfig.json` to change the `rootDir` to `..` and include TypeScript files from the `../common` directory. These changes enhance the project's dependency management and TypeScript configuration, ensuring compatibility with the latest packages and improving code organization.
- Introduced `DefaultLogger` for consistent error logging in the main worker script. - Updated `handleRunError` to utilize the logger for error messages and added a delay before exiting the process. - Refactored the `run` function to simplify its return statement. - Enhanced unit tests to verify the new logging behavior and ensure proper process exit. These changes improve the clarity and reliability of error handling in the application, ensuring that errors are logged consistently and the process exits gracefully.
- Removed commented-out code related to timer behavior in the `handleRunError` tests. - Improved clarity and focus of the tests by eliminating unnecessary comments. These changes enhance the readability and maintainability of the test suite, ensuring that the tests are concise and relevant.
… workflow - Changed the dependency installation command from `npm ci` to `npm install` in the code quality workflow for better compatibility with the project setup. - This adjustment ensures that the workflow installs the latest dependencies as specified in `package.json`, improving the reliability of the build process.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
.github/workflows/code-quality.yml (2)
45-48: 🛠️ Refactor suggestionAdd linting for the new Temporal worker Dockerfile.
Since you've added a new Dockerfile for the Temporal worker, you should add a linting step for it to maintain consistency with your existing Docker linting practices.
- name: Lint Dockerfile.temporal run: docker run --rm -i hadolint/hadolint < Dockerfile.temporal + - name: Lint Dockerfile.temporal-worker-main + run: docker run --rm -i hadolint/hadolint < Dockerfile.temporal-worker-main
57-63: 🛠️ Refactor suggestionAdd the new Temporal worker to the Docker security scanning matrix.
Since you've added a new Dockerfile for the Temporal worker, you should add it to the Docker Scout security scanning matrix for comprehensive security coverage.
service: - name: n8n dockerfile: Dockerfile.n8n tag: n8n-test:latest - name: temporal dockerfile: Dockerfile.temporal tag: temporal-test:latest + - name: temporal-worker-main + dockerfile: Dockerfile.temporal-worker-main + tag: temporal-worker-main-test:latest
🧹 Nitpick comments (2)
.github/workflows/code-quality.yml (1)
18-18: Consider usingnpm ciinstead ofnpm installfor CI environments.The change from
npm citonpm installin the CI workflow may lead to inconsistent builds sincenpm installcan modify package-lock.json and install different versions based on semver ranges.npm ciis generally recommended for CI environments as it ensures reproducible builds by installing exact versions from package-lock.json.If there's a specific reason for using
npm install(such as compatibility with new dependencies), please document this reasoning. Otherwise, consider reverting tonpm ci:- run: cd workers/main && npm install + run: cd workers/main && npm ciAlso applies to: 31-31
workers/main/src/index.ts (1)
13-17: Consider extending the timeout before process exit.The current 100ms timeout before process exit might not be sufficient for all logs to be flushed or for cleanup operations to complete. Consider increasing this value to ensure graceful shutdown.
export function handleRunError(err: Error): never { logger.error(`Unhandled error in main: ${err.message}`); - setTimeout(() => process.exit(1), 100); + setTimeout(() => process.exit(1), 1000); // Allow more time for logs to flush and cleanup throw err; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
workers/main/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.github/workflows/code-quality.yml(2 hunks)workers/main/package.json(1 hunks)workers/main/src/__tests__/index.test.ts(1 hunks)workers/main/src/index.ts(1 hunks)workers/main/tsconfig.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- workers/main/src/tests/index.test.ts
🔇 Additional comments (6)
workers/main/tsconfig.json (1)
4-4:❓ Verification inconclusive
Configuration supports shared code structure but verify module resolution.
The changes to rootDir and include patterns support sharing code with a common directory, which aligns well with the multi-stage Dockerfile approach. However, ensure that module paths will resolve correctly when referenced from different file locations.
Run this script to verify TypeScript module resolution is working correctly:
Also applies to: 11-11
🏁 Script executed:
#!/bin/bash # Verify TypeScript compilation with the new configuration cd workers/main # Attempt to compile with the new configuration npx tsc --noEmit # Check if there are any TypeScript files importing from the common directory echo "Files importing from common directory:" grep -r "from '../common" . --include="*.ts"Length of output: 800
🏁 Script executed:
#!/bin/bash # Check for TypeScript in dependencies echo "Root package.json TypeScript entries:" grep -R "\"typescript\"" -n package.json || echo "No TypeScript entry in root package.json" echo "" echo "workers/main/package.json TypeScript entries:" if [ -f workers/main/package.json ]; then grep -R "\"typescript\"" -n workers/main/package.json || echo "No TypeScript entry in workers/main/package.json" else echo "workers/main/package.json not found" fi echo "" echo "workers/main/tsconfig.json (first 200 lines):" sed -n '1,200p' workers/main/tsconfig.jsonLength of output: 901
Verify TypeScript compilation and module resolution in workers/main
The
rootDir: ".."andinclude: ["./**/*.ts","../common/**/*.ts"]allow shared code, but we couldn’t confirm a successful compile in the sandbox. Please ensure the local compiler is installed and that imports from../commonresolve without errors.• In workers/main/package.json you have
"typescript": "5.8.3"
• Run in workers/main:npm install npx tsc --noEmitand address any errors.
• Grep your source forfrom '../common(or other shared-code paths) to catch unresolved imports.
• If you prefer cleaner module paths, consider adding"paths"mappings in tsconfig.workers/main/src/index.ts (3)
1-3: Good addition of structured logging.Using Temporal's DefaultLogger is a proper approach for integrating with the Temporal ecosystem. Setting the log level to ERROR is appropriate for production environments.
9-11: Run function is a placeholder that needs implementation.The
runfunction is currently just a placeholder that returns true without doing any actual work. This needs to be implemented to start the Temporal worker.The function should register workflows and activities with the Temporal worker and start it. For example:
export async function run(): Promise<boolean> { - return true; + const worker = await Worker.create({ + workflowsPath: require.resolve('./workflows'), + activities: require('./activities'), + taskQueue: 'main-task-queue', + connection: await NativeConnection.connect({ + address: process.env.TEMPORAL_ADDRESS || 'localhost:7233', + }), + }); + + await worker.run(); + return true; }
19-19: Good pattern for immediate worker invocation with error handling.The immediate invocation of the run function with proper error handling ensures that any unhandled errors will be properly logged before the process exits, which aligns with the PR objective for improving error handling.
workers/main/package.json (2)
22-22: Fixing ts-node version is a good practice for stability.Changing from "^10.9.1" to "10.9.1" for ts-node ensures a consistent version is used across all environments, which improves build stability.
12-12:✅ Verification successful
Verify compatibility with Node.js 22.x types.
Upgrading @types/node from version 20.x to 22.x is a major version jump that might introduce compatibility issues. Ensure your code doesn't use APIs that have changed between these versions.
🏁 Script executed:
#!/bin/bash # Check if the project is using Node.js 22.x features or deprecated APIs cd workers/main # Check Node.js version in use echo "Current Node.js version:" node -v # Look for potential compatibility issues by checking for common Node.js APIs that # might have changed between v20 and v22 echo "Checking for potential compatibility issues..." grep -r "fs\." --include="*.ts" . grep -r "path\." --include="*.ts" . grep -r "process\." --include="*.ts" . grep -r "buffer\." --include="*.ts" . grep -r "stream\." --include="*.ts" .Length of output: 501
Compatibility with Node.js types v22 verified
- CI/runtime is using Node.js v24.0.2, so it’s newer than the types.
- A grep for common Node.js APIs (
fs,path,buffer,stream,process) only found aprocess.exit(1)call, which remains unchanged between v20→v22.- No deprecated or removed APIs are in use.
It’s safe to upgrade
@types/nodefrom 20.x to 22.x.
- Added `@temporalio/activity`, `@temporalio/workflow`, and `source-map-support` to both `package.json` and `package-lock.json`. - Updated the versioning for existing Temporal dependencies to ensure compatibility. These changes enhance the project's functionality by integrating additional Temporal features and improving error stack trace support.
…pendencies - Added new dependencies including `@emnapi/core`, `@emnapi/runtime`, and various `@esbuild` modules to `package-lock.json`. - Updated existing dependencies such as `@grpc/grpc-js` and `minimatch` to newer versions. - Removed outdated dependencies and ensured compatibility with the latest package versions. These changes enhance the project's dependency management, ensuring that the application utilizes the latest features and improvements from the updated packages.
|
docker-compose.ymlto usepostgres12and added a new environment variable for visibility database.These changes improve the setup and configuration of the Temporal service and its workers, enhancing reliability and maintainability.