-
[OPIK-2618] [BE] Isolated Subprocess Executor (#3693)
frostbyte_neo 发布于
2025-10-21 10:52:52 +00:00 -
[OPIK-2618] [BE] Java Python integration queue optimization studio
-
Fix implementation
-
OPIK-2618: Address PR review comments (cleanup and docs sync)
-
OPIK-2618: Address unresolved PR comments (import, docs, javadoc)
-
OPIK-2618: Apply @andrescrz review items (docs move, TTL 14d, snake_case, fields, typeref, worker cleanup, 5s timeout)
-
OPIK-2618: Replace process_hello_world with process_optimizer_job across Python and Java tests
-
OPIK-2618: Align TTL defaults to config (no in-code defaults), rely on JsonUtils null exclusion
-
OPIK-2618: Address remaining review items (UUID v7 IDs, simplify Redis connect, rely on JsonUtils for nulls)
-
OPIK-2618: Begin splitting rq_worker into smaller modules; re-export process_optimizer_job
-
OPIK-2618: Extract MetricsWorker and NoOpDeathPenalty to opik_backend.workers; keep rq_worker re-exports
-
OPIK-2618: Use Instant for QueueMessage timestamps; honor provided Instants in RqJobUtils
-
OPIK-2618: Document immutable defaults on JobBuilder (args/kwargs)
-
OPIK-2618: Add queues.useJacksonSerialization flag and surface in config.yml
-
OPIK-2618: Introduce MapStruct RqJobMapper and use in RqJobUtils
-
OPIK-2618: Add fallback short join in RqWorkerManager.stop for forceful shutdown
-
OPIK-2618: Make Redis health_check_interval configurable via REDIS_HEALTH_CHECK_INTERVAL_SECONDS (default 60)
-
OPIK-2618: Let client decode Redis responses (decode_responses=True) and remove internal assumption
-
OPIK-2618: Use Lombok on JobStatus (@Getter, @RequiredArgsConstructor)
-
OPIK-2618: Simplify RQ worker manager - single ping at startup, no reconnect loop; delegate to client
-
OPIK-2618: Remove unused _connect_with_backoff and backoff settings; rely on client
-
OPIK-2618: Add /health/liveness and /health/readiness endpoints; Redis ping for readiness
-
OPIK-2618: Introduce shared Redis client singleton; reuse in worker manager and health endpoints
-
OPIK-2618: Delegate Redis connection to shared singleton; remove worker-managed close and startup ping logic from run loop
-
OPIK-2618: DRY RQ_WORKER_ENABLED access via is_rq_worker_enabled(); gate readiness and startup ping
-
Address comments
-
OPIK-2618: Add utils package marker to fix imports
-
OPIK-2618: Simplify RqJobMapper - rely on same-name mapping; only convert Instants
-
Fix tests
-
Fix tests
-
Fix tests
-
[OPIK-2618] Implement IsolatedSubprocessExecutor with lifecycle management and comprehensive tests
-
Revision 2: Add stack memory limit (20MB) to prevent infinite recursion and update documentation
-
Revision 3: Fix Copilot review comments - add exception variables and initialize process variable
-
Revision 4: Address additional Copilot comments - use 'raise' instead of 'raise e' and clarify process initialization comment
-
Revision 5: Address nitpick comments - shorten inline comment and extract duplicate cleanup logic to helper method
-
Revision 6: Improve readability by using newline variable instead of chr(10) in _create_wrapper_script
-
Update apps/opik-python-backend/src/opik_backend/executor_isolated.py
PR feedback
Co-authored-by: Ido Berkovich ido@comet.com
-
Remove doc
-
Revision 2: Remove dynamic code injection from IsolatedSubprocessExecutor
- Remove _load_code_from_file method that dynamically loaded Python files
- Simplify execute() method to directly use code parameter without file loading
- Remove tests that relied on dynamic file path loading:
- test_execute_with_file_path
- test_execute_auto_detects_file_path
- Clean up unused imports (tempfile, Path)
- The executor now only accepts Python code directly, improving security posture
- Revision 3: Add test for executing Python file contents
- Add test_execute_with_python_file_contents to verify users can read files manually and pass contents
- Clarifies that executor accepts Python code strings from any source
- No automatic file loading from filesystem (security improvement)
- Users can still execute .py files by reading them with open() and passing contents
- Updated docstring to explain the pattern
- Re-added tempfile and Path imports (needed for new test)
- Revert "Revision 3: Add test for executing Python file contents"
This reverts commit 3eed1230f796a26451fcc0705b5667cb9da0c406.
- Revision 4: Refactor executor to accept file paths instead of inline code
- Change execute() signature from code parameter to file_path parameter
- Execute Python files directly with: python /path/to/file.py
- Remove _create_wrapper_script() method - no longer needed
- Data passed via stdin as JSON to the file
- Files read from stdin and output JSON results to stdout
- Update all tests to create temporary Python files and pass file paths
- Update all test code constants to handle stdin/stdout for data passing
- All 56 applicable tests passing (100% success rate)
- More secure: no automatic code injection, explicit file paths only
- Revision 5: Address all PR code review comments
- Move latency tracking to finally block for end-to-end measurement
- Move process removal to finally block to avoid duplication in exception handlers
- Simplify _remove_active_process to encapsulate None check
- Extract log parameters using named formatting (pid=, latency_ms=, etc.)
- Improve log readability with consistent parameter formatting
- Total latency now tracked from start to finish in finally block
- Ensure process cleanup happens in all execution paths (success and error)
- All 19 tests passing
Co-authored-by: Ido Berkovich ido@comet.com
下载附件
-