Conversation
…ecks - Introduced `sanitizeImageUrl` function to ensure proper handling of image URLs during local development. - Updated `AspectCard` and `ProjectLibraryAspect` components to utilize the new sanitization function for image URLs. - Added scripts for worker liveness and readiness checks to enhance monitoring and reliability of Celery workers. - Modified task configurations to include a new CPU queue and improved task acknowledgment settings.
- Updated method parameters in the LivenessProbe class to use underscore-prefixed names for unused variables, improving code readability. - Added type ignore comments for Celery signal imports to suppress type checking warnings.
WalkthroughThis pull request introduces enhanced image URL handling in the frontend and more robust image processing in the backend. It adds a new utility to sanitize URLs, ensuring proper redirection for local development. The image generation functions now use dynamically generated UUIDs and dynamically determined file extensions. Additionally, the Celery worker configuration is updated with new task queues, a liveness probe for heartbeat monitoring, and readiness checks, alongside several new scripts that launch and monitor dedicated workers. Changes
Sequence Diagram(s)sequenceDiagram
participant Component as UI Component
participant Util as sanitizeImageUrl
Component->>Util: Call sanitizeImageUrl(rawUrl)
alt URL starts with "http://minio:9000"
Util-->>Component: Returns transformed URL ("http://localhost:9000...")
else URL does not match
Util-->>Component: Returns original URL
end
Component->>Component: Set <img> src with sanitized URL
sequenceDiagram
participant Worker as Celery Worker
participant Probe as LivenessProbe
participant HB as Heartbeat File
participant RD as Readiness File
Worker->>Probe: Start()
Probe->>HB: Create & update heartbeat file periodically
Worker->>Probe: Emit worker_ready signal
Probe->>RD: Create readiness file
Note over Probe,Worker: Workers execute tasks...
Worker->>Probe: Initiate Shutdown
Probe->>HB: Stop heartbeat & remove file
Probe->>RD: Remove readiness file
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![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 (3)
echo/server/prod-worker-liveness.py (1)
1-18: Bulletproof liveness check implementation! 💯Killer implementation of the worker heartbeat checking. The 60-second threshold for staleness is a reasonable default. Using stat().st_mtime is the right approach for timestamp comparison.
One minor typo in your output message that should be fixed:
- print("Celery Worker liveness file timestamp DOES NOT matches the given constraint.") + print("Celery Worker liveness file timestamp DOES NOT match the given constraint.")echo/server/dembrane/tasks.py (2)
63-64: Unused argument “worker”.
stop(self, worker)doesn’t useworker. Consider removing or referencing it if you need worker context.-def stop(self, worker): +def stop(self, _worker):🧰 Tools
🪛 Ruff (0.8.2)
63-63: Unused method argument:
worker(ARG002)
66-67: Unused argument “worker”.
Same note as above. Either rename to_workeror remove if it’s never utilized.-def update_heartbeat_file(self, worker): +def update_heartbeat_file(self, _worker):🧰 Tools
🪛 Ruff (0.8.2)
66-66: Unused method argument:
worker(ARG002)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
echo/frontend/src/components/aspect/AspectCard.tsx(2 hunks)echo/frontend/src/lib/utils.ts(1 hunks)echo/frontend/src/routes/project/library/ProjectLibraryAspect.tsx(2 hunks)echo/server/dembrane/image_utils.py(2 hunks)echo/server/dembrane/quote_utils.py(1 hunks)echo/server/dembrane/tasks.py(8 hunks)echo/server/dembrane/tasks_config.py(1 hunks)echo/server/prod-worker-cpu.sh(1 hunks)echo/server/prod-worker-liveness.py(1 hunks)echo/server/prod-worker-readiness.py(1 hunks)echo/server/prod-worker.sh(1 hunks)echo/server/run-worker.sh(1 hunks)
🧰 Additional context used
🧬 Code Definitions (4)
echo/frontend/src/routes/project/library/ProjectLibraryAspect.tsx (1)
echo/frontend/src/lib/utils.ts (1)
sanitizeImageUrl(37-43)
echo/server/dembrane/image_utils.py (2)
echo/server/dembrane/utils.py (1)
generate_uuid(13-14)echo/server/dembrane/s3.py (1)
save_to_s3_from_url(84-114)
echo/server/dembrane/quote_utils.py (2)
echo/server/dembrane/s3.py (1)
save_to_s3_from_url(84-114)echo/server/dembrane/utils.py (1)
generate_uuid(13-14)
echo/frontend/src/components/aspect/AspectCard.tsx (1)
echo/frontend/src/lib/utils.ts (1)
sanitizeImageUrl(37-43)
🪛 Ruff (0.8.2)
echo/server/dembrane/tasks.py
63-63: Unused method argument: worker
(ARG002)
66-66: Unused method argument: worker
(ARG002)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: ci-check-server
🔇 Additional comments (30)
echo/frontend/src/lib/utils.ts (1)
37-43: Solid URL sanitization solution 🚀This function elegantly handles the container-to-localhost redirection needed for local dev environments. Clean, focused implementation with proper typing.
echo/frontend/src/routes/project/library/ProjectLibraryAspect.tsx (2)
19-19: Clean import additionImport looks good. Nice modular approach to leverage the utility function.
82-82: URL sanitization properly appliedExcellent implementation of the sanitization function while preserving the fallback placeholder. This ensures consistent image loading behavior across environments.
echo/frontend/src/components/aspect/AspectCard.tsx (2)
2-2: Import looks goodClean import statement extension to include the sanitizeImageUrl utility.
50-50: URL sanitization properly appliedGood implementation mirroring the pattern used in ProjectLibraryAspect. Consistent approach across components.
echo/server/dembrane/image_utils.py (2)
5-5: UUID import addedAppropriate import to support the dynamic UUID generation for image paths.
119-119: Improved S3 storage patternAwesome enhancement to the image storage mechanism. Using a UUID in the path creates unique identifiers, preventing collisions, and the explicit
public=Trueflag ensures proper ACL settings for image accessibility.-image_url = save_to_s3_from_url(image_url) +image_url = save_to_s3_from_url(image_url, "images/" + generate_uuid(), public=True)echo/server/dembrane/quote_utils.py (2)
806-811: Solid error handling for extension extraction 🚀Adding robust error handling for image extension extraction is a killer improvement. Using a try-except block to gracefully fall back to "png" when parsing fails is exactly what a production-ready system needs.
814-816: Sweet dynamic file naming convention 🔥Love the dynamic approach to file naming using UUIDs and the extracted extension. This makes the S3 storage much more organized and the explicit
public=Trueflag ensures proper accessibility.echo/server/prod-worker.sh (1)
5-5: Worker name refactored, clean and consistent naming convention! 🚀Love the standardized worker naming pattern - removing "normal" from the name while keeping the queue type separate is a solid architecture decision. Less redundancy in naming = more maintainable system. Matches perfectly with your new CPU worker implementation.
echo/server/prod-worker-cpu.sh (1)
1-5: Rock-solid CPU worker implementation! LGTM.Excellent work creating a dedicated CPU worker script! The CPU-intensive task isolation pattern is exactly what we need for proper resource optimization. Script follows the same clean pattern as the normal worker, maintaining consistency across your worker fleet.
echo/server/prod-worker-readiness.py (1)
1-10: Clean k8s readiness probe implementation, nice!Solid pythonic approach for the readiness probe. Using Path from pathlib is definitely the way to go for modern Python file operations - much cleaner than the old os.path approach. The script does exactly what it needs to do without overcomplicating things.
echo/server/run-worker.sh (1)
32-32: LGTM! Adding the CPU worker is consistent with the new CPU queue.
This is inline with the PR’s goal of segregating CPU-bound tasks onto a dedicated queue. Keep crushing it!echo/server/dembrane/tasks_config.py (6)
6-6: Confirmed best practice for CPU-bound tasks.
Settingworker_prefetch_multiplier = 1can help ensure tasks are distributed more fairly among workers, especially for CPU-intensive tasks. Looks good.
10-10: Queue for CPU-bound tasks looks legit.
Introducing"cpu"queue aligns with the new worker. Great separation of concerns.
17-17: Check for performance overhead when storing results.
Switching totask_ignore_result = Falsemeans task results are now stored. Awesome for debugging, but keep an eye on resource usage.
19-20: Late ack & requeue can prevent task loss.
task_acks_late = Trueandtask_reject_on_worker_lost = Trueensure tasks rerun if a worker crashes mid-execution. But as you know, it can create duplicates if partial work is done. Use it wisely.
22-26: Robust broker connection settings.
Reconnection and retry settings help keep tasks flowing in ephemeral or shaky network environments. Stay unstoppable.
27-30: Visibility timeout & socket keepalive optimize reliability.
Ensure that your tasks won’t vanish if a worker times out and that connections remain stable. Very SF 100x approach.echo/server/dembrane/tasks.py (11)
3-3: Nice import expansions for liveness checks.
Pulling inPath,bootsteps, and the new signals sets up the readiness and liveness logic. LGTM.Also applies to: 5-5, 7-7
40-45: Using local files for readiness & heartbeat checks.
Writing these files to/tmpis handy. Double-check that ephemeral storage meets your container orchestration environment’s needs.
47-62: The LivenessProbe is a killer addition.
Bootsteps with a timer is a very clean approach to track the worker’s heartbeat. Perfect for monitoring.
88-88: Seamless addition of LivenessProbe to worker steps.
Brilliant. The worker will adopt these readiness and liveness checks automatically.
91-93: Creating readiness file on worker startup.
That’s exactly how you make sure orchestration can detect readiness instantly.
96-98: Cleaning up readiness file on shutdown.
Super thorough. Ensures no stale readiness indicators remain.
153-153: Propagating ignore_result for chunk transcription.
Allowing the chunk transcription tasks to forgo storing results is often wise for large-volume workloads. Nice.
177-177: Routing to “cpu” queue.
Smart to isolate audio-splitting on the CPU queue if it’s heavier processing.
389-389: Targeting “cpu” queue for insight initialization.
Insights often involve heavy-lifting computations. Good call.
494-494: Aspect centroid assigned on CPU queue.
Centroid calculations can be resource-intensive. Good planning.
512-512: Clustering quotes on dedicated CPU queue.
Perfect for heavy clustering logic. Nicely done.
* Implement image URL sanitization and add worker readiness/liveness checks - Introduced `sanitizeImageUrl` function to ensure proper handling of image URLs during local development. - Updated `AspectCard` and `ProjectLibraryAspect` components to utilize the new sanitization function for image URLs. - Added scripts for worker liveness and readiness checks to enhance monitoring and reliability of Celery workers. - Modified task configurations to include a new CPU queue and improved task acknowledgment settings. * Refactor liveness probe task methods for clarity - Updated method parameters in the LivenessProbe class to use underscore-prefixed names for unused variables, improving code readability. - Added type ignore comments for Celery signal imports to suppress type checking warnings.
1 participant
sanitizeImageUrlfunction to ensure proper handling of image URLs during local development.AspectCardandProjectLibraryAspectcomponents to utilize the new sanitization function for image URLs.Summary by CodeRabbit
New Features
Chores