Nuclio functions in cvat network #8777
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes in this pull request involve modifications across several files, primarily focusing on error handling in a decorator function, updates to environment variables in Docker configuration, and adjustments in deployment scripts for serverless functions. The Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
🧹 Outside diff range and nitpick comments (1)
cvat/apps/lambda_manager/views.py (1)
986-987: Improved error message handling for HTTP errorsThe changes enhance error reporting by extracting the detailed error message from the response text when available. This provides more informative feedback to clients about what went wrong during HTTP requests.
Consider adding error message sanitization to prevent potential security issues with raw error messages:
- msg = err.response.text if hasattr(err, 'response') else str(err) + msg = str(err) + if hasattr(err, 'response'): + try: + # Attempt to parse and sanitize the error message + msg = err.response.text[:1000] # Limit message length + except Exception: + msg = str(err)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
cvat/apps/lambda_manager/views.py(1 hunks)docker-compose.yml(1 hunks)serverless/deploy_cpu.sh(1 hunks)serverless/deploy_gpu.sh(1 hunks)
🔇 Additional comments (5)
serverless/deploy_gpu.sh (2)
21-23: Environment variable renaming improves clarity
The renaming from CVAT_REDIS_* to FUNCTIONS_REDIS_* better reflects the scope and purpose of these variables in the context of serverless functions.
24-24: Verify network configuration compatibility
The new platform config sets the network to "cvat_cvat". Let's verify this network exists and is properly configured.
serverless/deploy_cpu.sh (1)
29-32: Changes maintain consistency with GPU deployment
The modifications mirror those in deploy_gpu.sh, maintaining consistency across deployment scripts. This is good practice for maintainability.
docker-compose.yml (2)
6-7: Project naming enhances isolation
Adding explicit project name "cvat" is good practice as it:
- Prevents naming conflicts with other compose projects
- Creates predictable network names (cvat_cvat) for Nuclio functions
Line range hint 1-464: Verify Redis service accessibility for Nuclio functions
The Redis services need to be accessible to the Nuclio functions through the "cvat_cvat" network.
Consider documenting the network architecture and service dependencies in the project documentation to help with future maintenance and troubleshooting.
✅ Verification successful
Redis services are properly configured and accessible on the "cvat" network
Both Redis services (cvat_redis_inmem and cvat_redis_ondisk) are correctly configured:
- They are connected to the "cvat" network
- They expose standard Redis ports (6379 for inmem, 6666 for ondisk)
- They are accessible via their container names as hostnames
- The environment variables CVAT_REDIS_INMEM_HOST and CVAT_REDIS_ONDISK_HOST are properly set
- The services are included in the no_proxy list to ensure direct communication
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Redis services configuration
# Expected: Find Redis services on the cvat network with correct ports
# Check Redis service definitions
echo "Checking Redis service configurations..."
rg -A 10 'cvat_redis_(inmem|ondisk):\s*$' docker-compose.yml
# Verify network configuration for Redis services
echo "Checking Redis services network configuration..."
rg -A 5 'networks:\s*- cvat' docker-compose.yml
Length of output: 1377
Script:
#!/bin/bash
# Let's verify the network configuration more specifically for Redis services and Nuclio
# Check Redis services network configuration
echo "=== Redis Services Network Configuration ==="
rg -B 2 -A 2 'networks:.*\s+- cvat' docker-compose.yml
# Check if Redis services are properly defined with network configuration
echo -e "\n=== Redis Services Complete Definition ==="
rg --multiline --multiline-dotall 'cvat_redis_(inmem|ondisk):\s*\{[^}]+\}' docker-compose.yml
# Check if there's any Nuclio-related network configuration
echo -e "\n=== Nuclio Network Configuration ==="
rg -i 'nuclio.*network|network.*nuclio' docker-compose.yml
# Check environment variables related to Redis hosts
echo -e "\n=== Redis Host Environment Variables ==="
rg 'CVAT_REDIS_(INMEM|ONDISK)_HOST' docker-compose.yml
Length of output: 742
Script:
#!/bin/bash
# Let's verify the complete network configuration for Redis services and check the network aliases
# Check complete Redis service definitions including networks
echo "=== Complete Redis Service Definitions ==="
rg -A 15 'cvat_redis_(inmem|ondisk):' docker-compose.yml
# Check all network definitions in the compose file
echo -e "\n=== Network Definitions ==="
rg -A 5 '^networks:' docker-compose.yml
# Check no_proxy settings that might affect Redis connectivity
echo -e "\n=== No Proxy Configuration ==="
rg 'no_proxy:' docker-compose.yml
Length of output: 1767
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #8777 +/- ##
========================================
Coverage 73.92% 73.93%
========================================
Files 409 409
Lines 43957 43957
Branches 3986 3986
========================================
+ Hits 32497 32501 +4
+ Misses 11460 11456 -4
|
|
Motivation and context
How has this been tested?
Checklist
developbranch(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit
Release Notes
Bug Fixes
Configuration Updates
cvat_serverandcvat_grafanaservices, including new settings for better service initialization.