Conversation
- Add webhook_service.py with Slack integration - Implement threshold-based alerts (temp high/low, humidity high/low) - Add periodic status updates via APScheduler - Add webhook management API endpoints (/api/webhook/*) - Include retry logic with exponential backoff - Add 5-minute cooldown to prevent alert spam - Add webhook documentation and quickstart guide - Add test files for webhook and periodic updates - Update requirements.txt with requests and APScheduler 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Token should be generated manually and stored in .env file. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Update CLAUDE.md with webhook endpoints and configuration - Update README.md with webhook setup instructions - Update Dockerfile for new dependencies - Add handoff documentation directory 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add Flask-RESTX for improved API structure and Swagger documentation - Introduce new webhook management endpoints: GET, PUT, POST for configuration, enabling, and disabling webhooks - Update requirements.txt to include flask-restx dependency - Refactor existing webhook routes to utilize Flask-RESTX resources This update enhances the API's usability and maintainability while providing better documentation for webhook interactions.
- Change 'url' field in webhook_config_input model to be optional for partial updates - Update description for 'url' to clarify its requirement during new webhook creation - Remove global security setting from API definition to allow public access to Swagger UI, while maintaining endpoint protection via decorators This update improves the flexibility of webhook configuration and enhances API accessibility.
Co-authored-by: baz-reviewer[bot] <174234987+baz-reviewer[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Created comprehensive test suite for Flask-RESTX webhook configuration API to ensure the bug fix at line 495 works correctly. Test coverage includes: - Creating webhook config when service doesn't exist (critical bug fix test) - Creating webhook config with missing URL (validation test) - Updating existing webhook configuration - Getting webhook config (exists and not exists scenarios) - Creating webhook with alert thresholds - Invalid threshold validation - Authentication and authorization tests All 9 tests pass successfully, verifying the AttributeError fix prevents crashes when creating new webhook service via API. Related to commit 9ffd7cb
Replace return statements with webhooks_ns.abort() in webhook endpoints to prevent @marshal_with decorator from dropping error keys during serialization. Affected endpoints: - POST /api/webhook/test (400 and 500 responses) - POST /api/webhook/enable (400 response) - POST /api/webhook/disable (400 response)
- Check for existing URL before allowing partial updates without URL - Preserve existing config values during partial updates instead of using hardcoded defaults - Prevents creating WebhookConfig with empty URL when webhook service exists but has no config
feat: Integrate Flask-RESTX for webhook management API
Flask-RESTX validation returns {message, errors} format which differs
from the existing {error, details} format that clients expect.
Manual validation in the handler still runs with consistent error format.
Hotfix/restx
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…ature.py Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Flask-RESTX min/max constraints are OpenAPI docs only and not enforced at runtime. Add validate_webhook_config() to enforce: - retry_count: 1-10 - retry_delay: 1-60 seconds - timeout: 5-120 seconds Returns 400 with clear error message on violation.
…t of project restructuring. These documents are no longer needed for the current implementation and have been replaced by updated documentation practices.
…or project - Added CLAUDE.md to provide guidance on project architecture, API endpoints, and development commands. - Updated README.md to reflect the transition to Raspberry Pi 4 and included production deployment strategies. - Enhanced Dockerfile and docker-compose.yml with health checks and resource limits for optimized performance. - Introduced WSGI entry point for production deployment and a startup script for easier service management. - Added systemd service configuration for automated deployment and monitoring. - Included detailed production deployment guide in docs/PI4_DEPLOYMENT.md. - Updated various files to ensure consistency with the new Raspberry Pi 4 setup and improved documentation practices.
- Add generate_error_id() for tracking errors across logs and responses - Use logging.exception() to capture full stack traces in logs - Return error_id instead of internal exception details to clients - Improves security by not exposing internals while aiding debugging
- Updated API response to mask webhook URLs, revealing only the scheme and host to prevent exposure of sensitive tokens. - Introduced a new function to handle URL masking in the WebhookService and updated relevant logging statements. - Added unit tests to verify that webhook URLs are masked correctly in API responses.
…ng and logging - Added new log entries in temp_monitor.log for webhook service configuration issues and API access attempts with invalid tokens. - Enhanced error handling in temp_monitor.py by replacing specific error messages with a generic 'Internal server error' for health and metrics endpoints, while using logging.exception for better error tracking.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
- App now exits with code 1 if BEARER_TOKEN is not set instead of running in a degraded state with broken API endpoints - Fixed README documentation that incorrectly stated token was "auto-generated if not provided"
Convert test_webhook.py from placeholder print statements to proper unittest framework with 29 tests across 8 test classes: - Use unittest.mock.patch to capture payloads without network calls - Verify Slack payload structure (attachments, color, text, ts, fields) - Assert field content, ordering, and short flags - Confirm requests.post is never called when enabled=False - Test all message types: basic, alerts, status updates, system events
…, locator, pattern finder, web search researcher, and commit commands. These files are no longer needed as part of the project restructuring and have been replaced by updated documentation practices.
…or project - Introduced AGENTS.md to provide detailed guidance on project architecture, API endpoints, and development commands. - Documented core layers including Flask application, webhook service, and API models. - Outlined public and protected routes, configuration settings, key design patterns, and testing strategies. - Included common issues and solutions to assist developers in troubleshooting.
- Deleted the 'mem_reservation' setting for the service, simplifying resource configuration.
- Deleted the 'mem_limit' setting for the service, further simplifying resource configuration.
- Deleted the unused test.yml file to clean up the repository. - Ensured consistent formatting in docker-compose.yml by maintaining the existing profile entry.
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
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. Comment |
|
@OpenHands please fix the failing actions on PR #36 at branch |
|
I'm on it! fakebizprez can track my progress at all-hands.dev |
|
| Filename | Overview |
|---|---|
| temp_monitor.py | Critical syntax error: import statement misplaced inside function body (line 143). Application will fail to start. |
| test_webhook.py | Syntax error in shebang line with extra 'f' character. Test script may not execute correctly. |
| webhook_service.py | New webhook service implementation with alert thresholds, retry logic, and URL masking. Clean implementation with thread safety. |
| api_models.py | Flask-RESTX API models with proper validation constraints. Clean separation of input/output models. |
| .env.example | Environment configuration template expanded with webhook and alert settings. BEARER_TOKEN now required but empty value could be clearer. |
| .github/workflows/ci.yml | CI/CD workflow with tests and self-hosted deployment. Good security controls preventing untrusted fork execution. |
Sequence Diagram
sequenceDiagram
participant Client as API Client
participant Flask as Flask App
participant Auth as Authentication
participant Monitor as Sensor Monitor Thread
participant Webhook as WebhookService
participant Slack as Slack API
Note over Monitor: Background thread every 60s
Monitor->>Monitor: get_compensated_temperature()
Monitor->>Monitor: get_humidity()
alt Thresholds exceeded
Monitor->>Webhook: check_and_alert(temp, humidity)
Webhook->>Webhook: Check alert cooldown (5 min)
alt Cooldown expired
Webhook->>Slack: POST webhook with retry
Slack-->>Webhook: HTTP response
Webhook->>Webhook: Mark alert sent
end
end
alt Periodic status enabled
Monitor->>Webhook: send_status_update()
Webhook->>Slack: POST status message
Slack-->>Webhook: HTTP response
end
Note over Client: API Configuration Flow
Client->>Flask: PUT /api/webhook/config
Flask->>Auth: Check credentials
Auth-->>Flask: Authorized
Flask->>Flask: validate_webhook_config()
Flask->>Flask: validate_thresholds()
Flask->>Webhook: set_webhook_config()
Flask->>Webhook: set_alert_thresholds()
Flask-->>Client: Updated config response
Client->>Flask: POST /api/webhook/test
Flask->>Auth: Check credentials
Auth-->>Flask: Authorized
Flask->>Webhook: send_status_update()
Webhook->>Slack: POST test message
Slack-->>Webhook: HTTP response
Flask-->>Client: Test result
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (23)
deployment/systemd/temp-monitor-compose.service (2)
9-11: Document that these values must be customized for each deployment.The hardcoded
User=pi,Group=pi, andWorkingDirectory=/home/pi/temp_monitorwill need to be adjusted for different installations. Consider adding a comment in the file or ensuring the README mentions these values must be updated during setup.💡 Suggested improvement
Add a comment to guide users:
[Service] Type=oneshot RemainAfterExit=yes +# Update User, Group, and WorkingDirectory for your environment User=pi Group=pi WorkingDirectory=/home/pi/temp_monitor
14-14: Consider using a finite timeout instead of disabling it.
TimeoutStartSec=0disables the startup timeout entirely, which could cause systemd to hang indefinitely if Docker Compose encounters issues. Consider using a generous but finite timeout like300seconds to handle image pulls while still catching genuine failures.💡 Proposed change
-TimeoutStartSec=0 +TimeoutStartSec=300AGENTS.MD (1)
198-198: Consider removing the hardcoded line number reference.The reference to "line 191" may become outdated as
temp_monitor.pyevolves. Consider removing the specific line number and just referencing the function name, or noting it's approximate.💡 Suggested change
-Adjust `factor` in `get_compensated_temperature()` (line 191) based on actual readings +Adjust the `factor` parameter in `get_compensated_temperature()` based on actual readings.github/workflows/ci.yml (1)
72-103: Consider grouping redirects for cleaner shell script.The multiple
echostatements work correctly but could be consolidated using grouped commands, as suggested by shellcheck. This is a minor style improvement.💡 Proposed refactor
- name: Create .env from GitHub Secrets run: | # Validate required secrets (BEARER_TOKEN is required for API auth) if [ -z "${BEARER_TOKEN}" ]; then echo "Missing required secret: BEARER_TOKEN" exit 1 fi - # Create .env file with required configuration - echo "BEARER_TOKEN=${BEARER_TOKEN}" > .env - - # Add optional Cloudflare Tunnel token if configured - if [ -n "${CLOUDFLARED_TOKEN}" ]; then - echo "CLOUDFLARED_TOKEN=${CLOUDFLARED_TOKEN}" >> .env - fi - - # Add optional Slack webhook if configured (enables alerts/status updates) - if [ -n "${SLACK_WEBHOOK_URL}" ]; then - echo "SLACK_WEBHOOK_URL=${SLACK_WEBHOOK_URL}" >> .env - echo "WEBHOOK_ENABLED=true" >> .env - fi - - # Production monitoring defaults (intentionally enabled for production, - # differs from local development defaults in .env.example) - echo "ALERT_TEMP_MIN_C=15.0" >> .env - echo "ALERT_TEMP_MAX_C=27.0" >> .env - echo "ALERT_HUMIDITY_MIN=30.0" >> .env - echo "ALERT_HUMIDITY_MAX=70.0" >> .env - echo "STATUS_UPDATE_ENABLED=true" >> .env - echo "STATUS_UPDATE_INTERVAL=3600" >> .env - echo "STATUS_UPDATE_ON_STARTUP=true" >> .env + # Create .env file with required configuration + { + echo "BEARER_TOKEN=${BEARER_TOKEN}" + + # Add optional Cloudflare Tunnel token if configured + if [ -n "${CLOUDFLARED_TOKEN}" ]; then + echo "CLOUDFLARED_TOKEN=${CLOUDFLARED_TOKEN}" + fi + + # Add optional Slack webhook if configured (enables alerts/status updates) + if [ -n "${SLACK_WEBHOOK_URL}" ]; then + echo "SLACK_WEBHOOK_URL=${SLACK_WEBHOOK_URL}" + echo "WEBHOOK_ENABLED=true" + fi + + # Production monitoring defaults (intentionally enabled for production, + # differs from local development defaults in .env.example) + echo "ALERT_TEMP_MIN_C=15.0" + echo "ALERT_TEMP_MAX_C=27.0" + echo "ALERT_HUMIDITY_MIN=30.0" + echo "ALERT_HUMIDITY_MAX=70.0" + echo "STATUS_UPDATE_ENABLED=true" + echo "STATUS_UPDATE_INTERVAL=3600" + echo "STATUS_UPDATE_ON_STARTUP=true" + } > .env chmod 600 .envCLAUDE.md (1)
198-198: Consider removing the hardcoded line number reference.The reference to "line 191" may become outdated as
temp_monitor.pyevolves. Consider removing the specific line number and just referencing the function name.💡 Suggested change
-Adjust `factor` in `get_compensated_temperature()` (line 191) based on actual readings +Adjust the `factor` parameter in `get_compensated_temperature()` based on actual readingsREADME.md (1)
260-262: Add language identifier to code fence.The code fence would benefit from a language identifier for proper syntax highlighting. Consider using
pythonortextdepending on the intended formatting.💡 Suggested change
-``` +```python compensated_temp = raw_temp - ((cpu_temp - raw_temp) * factor)</details> </blockquote></details> <details> <summary>webhook_service.py (4)</summary><blockquote> `64-68`: **Catch a more specific exception or log the exception details.** The bare `Exception` catch is flagged by static analysis. While this is defensive code for URL masking, consider catching a more specific exception like `ValueError` or using `logging.exception` to preserve the stack trace. <details> <summary>🔎 Proposed fix</summary> ```diff - except Exception as e: - logging.warning(f"Error masking webhook URL: {e}") + except (ValueError, AttributeError) as e: + logging.warning(f"Error masking webhook URL: {e}")
115-118: Uselogging.exceptionto preserve stack traces on errors.Per static analysis hints,
logging.exceptionautomatically includes the exception traceback, which is valuable for debugging webhook failures in production.🔎 Proposed fix
except requests.exceptions.Timeout: - logging.error(f"Webhook timeout (attempt {attempt + 1}/{self.webhook_config.retry_count})") + logging.exception(f"Webhook timeout (attempt {attempt + 1}/{self.webhook_config.retry_count})") except requests.exceptions.RequestException as e: - logging.error(f"Webhook request failed (attempt {attempt + 1}/{self.webhook_config.retry_count}): {e}") + logging.exception(f"Webhook request failed (attempt {attempt + 1}/{self.webhook_config.retry_count}): {e}")
107-109: Consider accepting additional 2xx status codes.Slack webhooks typically return
200 OK, but other webhook endpoints may return201 Createdor204 No Content. This could cause false negatives when integrating with non-Slack webhooks.🔎 Proposed fix
- if response.status_code == 200: + if response.ok: # Accepts any 2xx status logging.info(f"Webhook sent successfully to {self._mask_url(url)}") return True
202-204: Remove extraneous f-string prefix.Static analysis correctly identifies f-strings without placeholders on lines 203, 237, 268, and 299. These are regular strings with emojis.
🔎 Proposed fix for all occurrences
- success = self.send_slack_message( - text=f"🔥 *Temperature Alert: HIGH*", + success = self.send_slack_message( + text="🔥 *Temperature Alert: HIGH*",- success = self.send_slack_message( - text=f"❄️ *Temperature Alert: LOW*", + success = self.send_slack_message( + text="❄️ *Temperature Alert: LOW*",- success = self.send_slack_message( - text=f"💧 *Humidity Alert: HIGH*", + success = self.send_slack_message( + text="💧 *Humidity Alert: HIGH*",- success = self.send_slack_message( - text=f"🏜️ *Humidity Alert: LOW*", + success = self.send_slack_message( + text="🏜️ *Humidity Alert: LOW*",thoughts/tasks/issue-24-pydantic-validation/2025-12-31-research.md (1)
303-320: Add language specifier to fenced code blocks for consistency.Per markdownlint, fenced code blocks should have a language specified. These ASCII diagrams could use
textorplaintextas the language specifier.🔎 Proposed fix
-``` +```text Request JSON | vDockerfile (1)
38-40: Consider usingcurlfor a lighter-weight health check.The current health check spawns a Python interpreter for each check, which has higher overhead than using
curl. Sincepython:3.9-slim-bullseyedoesn't include curl by default, this approach works, but consider adding curl to the apt-get install if you want lower overhead.🔎 Alternative using curl (requires adding curl to apt-get install)
# Health check for monitoring and load balancers HEALTHCHECK --interval=30s --timeout=10s --start-period=10s --retries=3 \ - CMD python -c "import requests; requests.get('http://localhost:8080/health', timeout=5)" || exit 1 + CMD curl -f http://localhost:8080/health || exit 1And add to apt-get:
RUN apt-get update && \ apt-get install -y \ libatlas-base-dev \ i2c-tools \ python3-dev \ build-essential \ cmake \ + curl \ git && \wsgi.py (1)
30-31: Uselogging.exceptionto include stack trace.Per static analysis,
logging.exceptionautomatically includes the traceback, which is valuable for diagnosing startup failures.🔎 Proposed fix
except Exception as e: - logger.error(f"Failed to start sensor thread: {e}") + logger.exception(f"Failed to start sensor thread: {e}") raisestart_production.sh (1)
34-38: Consider validating sense-hat or sense_hat_emu package.The dependency check verifies
waitress,psutil, andflask, but doesn't verify the Sense HAT libraries. If running on actual Pi hardware, missing sense-hat could cause runtime failures.🔎 Proposed enhancement
# Verify required modules are installed echo "Checking dependencies..." -python -c "import waitress, psutil, flask" || { +python -c "import waitress, psutil, flask, requests" || { echo "Error: Required packages not installed" echo "Run: pip install -r requirements.txt" exit 1 }Note: Sense HAT validation would require different logic since it may use a mock/emulator in non-Pi environments.
docs/PI4_DEPLOYMENT.md (1)
380-391: Add language specifier to logrotate config block.Per markdownlint, add a language identifier for the logrotate configuration block.
🔎 Proposed fix
Create `/etc/logrotate.d/temp-monitor`: -``` +```text /var/log/temp-monitor/*.log { daily rotate 7deployment/systemd/temp-monitor.service (1)
3-3: Update the placeholder documentation URL.The documentation URL contains a placeholder
your-repothat should be replaced with the actual repository pathfreightCognition/temp_monitorbased on the PR context.🔎 Proposed fix
-Documentation=https://github.com/your-repo/temp_monitor +Documentation=https://github.com/freightCognition/temp_monitordocker-compose.yml (2)
17-23: Healthcheck implementation is functional.The Python-based healthcheck is working correctly. The 5-second timeout in the Python command is appropriately less than the 10-second Docker healthcheck timeout.
If you prefer a simpler approach and if curl/wget are available in the container, you could use:
test: ["CMD-SHELL", "curl -f http://localhost:8080/health || exit 1"]
25-32: Pin the cloudflared image to a specific version.Using
cloudflare/cloudflared:latestcan lead to unexpected behavior if the image updates. Replace with a specific version (e.g.,cloudflare/cloudflared:2024.1.0) for reproducible deployments.test_periodic_updates.py (2)
14-52: Consider using idiomatic boolean assertions.The test logic is correct. For more Pythonic style, consider using
assert not status_update_enabledinstead ofassert status_update_enabled == False. This is optional and a matter of style preference.
201-244: Consider using unittest framework for consistency.This test file uses a custom test runner with simple function calls, while
test_webhook.pyuses theunittestframework withTestLoaderandTextTestRunner. For consistency across the project's test suite, consider refactoring to useunittest.TestCaseclasses.api_models.py (1)
148-166: Consider adding individual field range validation.The cross-field validation (min < max) is correct, but individual field ranges defined in
alert_thresholds_input(temp: -50 to 100, humidity: 0 to 100) are not enforced here. Flask-RESTX'smin/maxon fields are for OpenAPI documentation only and don't perform runtime validation.🔎 Proposed enhancement
def validate_thresholds(thresholds: dict) -> tuple: """ Validate threshold relationships (cross-field validation). Args: thresholds: Dictionary with threshold values Returns: Tuple of (is_valid: bool, error_message: str) """ + # Validate temperature ranges + for key in ['temp_min_c', 'temp_max_c']: + if thresholds.get(key) is not None: + if not (-50 <= thresholds[key] <= 100): + return False, f'{key} must be between -50 and 100' + + # Validate humidity ranges + for key in ['humidity_min', 'humidity_max']: + if thresholds.get(key) is not None: + if not (0 <= thresholds[key] <= 100): + return False, f'{key} must be between 0 and 100' + if thresholds.get('temp_min_c') is not None and thresholds.get('temp_max_c') is not None: if thresholds['temp_min_c'] >= thresholds['temp_max_c']: return False, 'temp_min_c must be less than temp_max_c' if thresholds.get('humidity_min') is not None and thresholds.get('humidity_max') is not None: if thresholds['humidity_min'] >= thresholds['humidity_max']: return False, 'humidity_min must be less than humidity_max' return True, ''temp_monitor.py (2)
160-185: Consider reusingWebhookService._mask_urlto avoid duplication.This function duplicates
WebhookService._mask_urlfromwebhook_service.py(lines 12-31 in the snippet). Consider exposing a public method or utility function to maintain DRY principle.🔎 Suggested approach
Either make
_mask_urlpublic inWebhookService:# In webhook_service.py, rename _mask_url to mask_url def mask_url(self, url: str) -> str:Or extract a standalone utility function that both can use:
# In a shared utils.py def mask_webhook_url(url: str) -> str: ...Then import and reuse in
temp_monitor.py.
277-343: LGTM on sensor update loop!The background thread correctly handles:
- Threshold checking with webhook alerts
- Periodic status updates with proper timing logic
- Error handling with short retry intervals
- Timestamp update in
finallyblock to prevent retry stormsOne minor note: the
global last_status_updatedeclaration at line 307 could be moved to the function's top (line 280) alongside the other globals for consistency and clarity.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
temp_monitor.logis excluded by!**/*.log
📒 Files selected for processing (24)
.env.example.github/workflows/ci.yml.gitignoreAGENTS.MDCLAUDE.mdDockerfileLICENSEREADME.mdapi_models.pydeployment/systemd/temp-monitor-compose.servicedeployment/systemd/temp-monitor.servicedocker-compose.ymldocs/PI4_DEPLOYMENT.mdgenerate_token.pyrequirements.txtstart_production.shtemp_monitor.pytest_api_models.pytest_periodic_updates.pytest_webhook.pytest_webhook_api.pythoughts/tasks/issue-24-pydantic-validation/2025-12-31-research.mdwebhook_service.pywsgi.py
💤 Files with no reviewable changes (1)
- generate_token.py
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Use the@require_tokendecorator to protect API endpoints that require bearer token authentication
Add logging statements for all significant operations using theloggingmodule
Wrap hardware operations (Sense HAT interactions) in try-except blocks with appropriate error handling and logging
Usejsonify()for all JSON API responses in Flask endpoints
Load environment variables usingos.getenv('VARIABLE_NAME', default)with appropriate defaults for configuration
Use a background daemon thread pattern with configurable sampling intervals for continuous sensor monitoring without blocking the main application
Implement temperature compensation using the formula:comp_temp = raw_temp - ((cpu_temp - raw_temp) * factor)with a calibration factor of 0.7
Remove outliers by taking readings and removing the highest and lowest values before averaging for sensor data accuracy
Automatically create log directories if they don't exist and provide clear error messages if logging initialization fails
Bearer tokens must be 64-character hex strings generated usingsecrets.token_hex(32)for cryptographic security
RequireAuthorization: Bearer <token>header format for all protected API endpoints
For new API endpoints, add@require_tokendecorator, return responses withjsonify(), log access attempts, and update README.md documentation
Use global variables for sensor data (current_temp, current_humidity, last_updated) thread-safe due to Python GIL, with updates from background daemon thread
Implement graceful degradation for hardware sensor failures (e.g., use raw temperature if CPU temperature unavailable)
The web dashboard (/) is intentionally public with no authentication; API endpoints (/api/*) require bearer token authentication unless explicitly documented as public
Never assume hardware is available; handle Sense HAT initialization failures gracefully with informative error messages
Files:
test_api_models.pytest_webhook.pytest_periodic_updates.pyapi_models.pywsgi.pywebhook_service.pytemp_monitor.pytest_webhook_api.py
CLAUDE.md
📄 CodeRabbit inference engine (CLAUDE.md)
CLAUDE.md: Update CLAUDE.md when making architectural changes, adding features, or modifying core functionality
Update this CLAUDE.md documentation whenever line numbers change, functions are added/modified, or architectural changes are made to keep AI assistants informed
Files:
CLAUDE.md
README.md
📄 CodeRabbit inference engine (CLAUDE.md)
Document all API endpoints in README.md with their purpose, authentication requirements, request/response format, and example curl commands
Files:
README.md
.gitignore
📄 CodeRabbit inference engine (CLAUDE.md)
.gitignore: Store sensitive information (bearer tokens, credentials) in.envfile which must be gitignored and never committed
Include.envin.gitignoreto prevent accidental commits of environment variables and secrets
Files:
.gitignore
requirements.txt
📄 CodeRabbit inference engine (CLAUDE.md)
Consider hardware compatibility and resource constraints when adding dependencies; this application runs on Raspberry Pi Zero with limited memory
Files:
requirements.txt
🧠 Learnings (24)
📓 Common learnings
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Test all API endpoints with actual hardware when possible; manual testing should verify hardware LED display shows temperature, auto-refresh works, and token validation works
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/temp_monitor.py : Store BEARER_TOKEN in .env file (never committed to git) and load at application startup; auto-generate token if .env is missing
Applied to files:
.env.exampleDockerfileCLAUDE.mdREADME.mdAGENTS.MDtemp_monitor.py
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/temp_monitor.py : Use environment variables (loaded via python-dotenv) for configurable paths: LOG_FILE, LOGO_PATH, FAVICON_PATH, BEARER_TOKEN
Applied to files:
.env.exampleDockerfileCLAUDE.mdREADME.mdAGENTS.MDtemp_monitor.py
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Consider deployment context: the application runs on Raspberry Pi in a headless environment; avoid cloud-only features or assumptions
Applied to files:
docs/PI4_DEPLOYMENT.md
📚 Learning: 2025-12-07T22:09:12.211Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Applies to requirements.txt : Consider hardware compatibility and resource constraints when adding dependencies; this application runs on Raspberry Pi Zero with limited memory
Applied to files:
docs/PI4_DEPLOYMENT.mdrequirements.txt
📚 Learning: 2025-12-07T22:09:12.211Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Test all API endpoints with actual hardware when possible; manual testing should verify hardware LED display shows temperature, auto-refresh works, and token validation works
Applied to files:
docs/PI4_DEPLOYMENT.mdCLAUDE.mdREADME.mdtest_periodic_updates.pyAGENTS.MDtest_webhook_api.py
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/CLAUDE.md : Update CLAUDE.md when making architectural changes, adding new configuration options, or modifying core algorithms
Applied to files:
CLAUDE.md.gitignoreAGENTS.MD
📚 Learning: 2025-12-07T22:09:12.211Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Applies to CLAUDE.md : Update CLAUDE.md when making architectural changes, adding features, or modifying core functionality
Applied to files:
CLAUDE.md.gitignoreAGENTS.MD
📚 Learning: 2025-12-07T22:09:12.211Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Applies to CLAUDE.md : Update this CLAUDE.md documentation whenever line numbers change, functions are added/modified, or architectural changes are made to keep AI assistants informed
Applied to files:
CLAUDE.md.gitignoreAGENTS.MD
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/temp_monitor.py : Protect API endpoints with require_token decorator for bearer token authentication, except for public routes like web dashboard and favicon
Applied to files:
CLAUDE.mdREADME.mdtemp_monitor.py
📚 Learning: 2025-12-07T22:09:12.211Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Applies to **/*.py : For new API endpoints, add `require_token` decorator, return responses with `jsonify()`, log access attempts, and update README.md documentation
Applied to files:
CLAUDE.mdREADME.mdtemp_monitor.pyrequirements.txt
📚 Learning: 2025-12-07T22:09:12.211Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Applies to **/*.py : The web dashboard (`/`) is intentionally public with no authentication; API endpoints (`/api/*`) require bearer token authentication unless explicitly documented as public
Applied to files:
CLAUDE.mdREADME.mdtemp_monitor.py
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/temp_monitor.py : Use Flask decorator pattern (app.route, require_token) for defining API endpoints and middleware
Applied to files:
CLAUDE.mdREADME.mdAGENTS.MDtemp_monitor.py
📚 Learning: 2026-01-01T09:42:31.833Z
Learnt from: fakebizprez
Repo: freightCognition/temp_monitor PR: 23
File: .claude/flask-restx-api/.claude-plugin/plugin.json:21-21
Timestamp: 2026-01-01T09:42:31.833Z
Learning: Files under `.claude/**/.claude-plugin/` directories are Claude Code plugin metadata files that define generic, reusable development skills/plugins. Their repository URLs should point to the plugin's own repository, not to the repository where they are used.
Applied to files:
.gitignore
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/.gitignore : Include Python cache files (__pycache__/, *.pyc, *.pyo, *.pyd) in .gitignore
Applied to files:
.gitignore
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/temp_monitor.py : Use consistent JSON API response format with fields: temperature_c, temperature_f, humidity, timestamp
Applied to files:
temp_monitor.py
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/temp_monitor.py : Return JSON responses using Flask's jsonify() function for all API endpoints
Applied to files:
temp_monitor.py
📚 Learning: 2025-12-07T22:09:12.211Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Applies to **/*.py : Use the `require_token` decorator to protect API endpoints that require bearer token authentication
Applied to files:
temp_monitor.py
📚 Learning: 2025-12-07T22:09:12.211Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Applies to **/*.py : Require `Authorization: Bearer <token>` header format for all protected API endpoints
Applied to files:
temp_monitor.py
📚 Learning: 2025-12-07T22:09:12.211Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Applies to **/*.py : Use global variables for sensor data (current_temp, current_humidity, last_updated) thread-safe due to Python GIL, with updates from background daemon thread
Applied to files:
temp_monitor.py
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/temp_monitor.py : Use daemon threads for background monitoring tasks (sensor data collection) with appropriate sampling intervals (default 60 seconds)
Applied to files:
temp_monitor.py
📚 Learning: 2025-12-07T22:09:12.211Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Applies to **/*.py : Use a background daemon thread pattern with configurable sampling intervals for continuous sensor monitoring without blocking the main application
Applied to files:
temp_monitor.py
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/generate_token.py : Use Bearer token format of 64-character hex strings (32 bytes) generated via secrets.token_hex(32) for cryptographically secure tokens
Applied to files:
temp_monitor.py
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Avoid adding heavy dependencies to requirements.txt; prioritize lightweight packages suitable for resource-constrained Raspberry Pi Zero hardware
Applied to files:
requirements.txt
🧬 Code graph analysis (7)
test_api_models.py (1)
api_models.py (2)
validate_webhook_config(123-145)validate_thresholds(148-166)
test_periodic_updates.py (2)
temp_monitor.py (1)
get(474-510)test_webhook.py (1)
main(580-600)
api_models.py (1)
temp_monitor.py (1)
get(474-510)
wsgi.py (1)
temp_monitor.py (1)
start_sensor_thread(750-777)
webhook_service.py (1)
temp_monitor.py (4)
post(615-640)post(651-662)post(673-684)get(474-510)
temp_monitor.py (3)
webhook_service.py (7)
WebhookService(37-413)WebhookConfig(19-25)AlertThresholds(29-34)check_and_alert(179-324)send_status_update(326-372)set_webhook_config(70-74)set_alert_thresholds(76-80)api_models.py (2)
validate_thresholds(148-166)validate_webhook_config(123-145)sense_hat.py (2)
get_humidity(17-18)show_message(23-24)
test_webhook_api.py (2)
webhook_service.py (3)
WebhookService(37-413)WebhookConfig(19-25)AlertThresholds(29-34)temp_monitor.py (2)
put(518-603)get(474-510)
🪛 actionlint (1.7.9)
.github/workflows/ci.yml
72-72: shellcheck reported issue in this script: SC2129:style:23:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 32-32: [UnorderedKey] The ALERT_TEMP_MAX_C key should go before the ALERT_TEMP_MIN_C key
(UnorderedKey)
[warning] 37-37: [UnorderedKey] The ALERT_HUMIDITY_MAX key should go before the ALERT_HUMIDITY_MIN key
(UnorderedKey)
🪛 GitHub Actions: CI
temp_monitor.py
[error] 144-144: IndentationError: unexpected indent
🪛 Gitleaks (8.30.0)
README.md
[high] 151-151: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
[high] 173-174: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
🪛 LanguageTool
thoughts/tasks/issue-24-pydantic-validation/2025-12-31-research.md
[style] ~150-~150: ‘without warning’ might be wordy. Consider a shorter alternative.
Context: ... humidity at 100% - Silent adjustment without warning #### Pattern E: Try/Except Error Handl...
(EN_WORDINESS_PREMIUM_WITHOUT_WARNING)
CLAUDE.md
[uncategorized] ~30-~30: Did you mean the communication tool “Slack” (= proper noun, capitalized)?
Context: ...nd_status_update()(periodic reports),send_slack_message()` (generic Slack formatting) ...
(ON_SKYPE)
README.md
[uncategorized] ~101-~101: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...------| | ALERT_TEMP_MIN_C | 15.0 | Low temperature alert (Celsius) | | ALERT_TEMP_MAX_C ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~102-~102: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...sius) | | ALERT_TEMP_MAX_C | 27.0 | High temperature alert (Celsius) | | `ALERT_HUMIDITY_MIN...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~291-~291: Consider using a less common alternative to make your writing sound more unique and professional.
Context: ...ontributing Contributions are welcome! Please feel free to submit a Pull Request. ## License [MI...
(FEEL_FREE_TO_STYLE_ME)
AGENTS.MD
[uncategorized] ~30-~30: Did you mean the communication tool “Slack” (= proper noun, capitalized)?
Context: ...nd_status_update()(periodic reports),send_slack_message()` (generic Slack formatting) ...
(ON_SKYPE)
🪛 markdownlint-cli2 (0.18.1)
docs/PI4_DEPLOYMENT.md
381-381: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
thoughts/tasks/issue-24-pydantic-validation/2025-12-31-research.md
290-290: Strong style
Expected: asterisk; Actual: underscore
(MD050, strong-style)
290-290: Strong style
Expected: asterisk; Actual: underscore
(MD050, strong-style)
303-303: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
324-324: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
381-381: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
627-627: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
CLAUDE.md
19-19: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
25-25: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
32-32: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
38-38: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
78-78: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
83-83: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
88-88: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
94-94: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
193-193: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
197-197: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
201-201: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
206-206: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
README.md
260-260: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
AGENTS.MD
19-19: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
25-25: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
32-32: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
38-38: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
78-78: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
83-83: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
88-88: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
94-94: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
193-193: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
197-197: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
201-201: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
206-206: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🪛 Ruff (0.14.10)
test_webhook.py
1-1: Found useless expression. Either assign it to a variable or remove it.
(B018)
1-1: Undefined name f
(F821)
1-1: Shebang should be at the beginning of the file
(EXE005)
test_periodic_updates.py
26-26: Avoid equality comparisons to False; use not status_update_enabled: for false checks
Replace with not status_update_enabled
(E712)
37-37: Avoid equality comparisons to True; use status_update_enabled: for truth checks
Replace with status_update_enabled
(E712)
44-44: Avoid equality comparisons to True; use send_on_startup: for truth checks
Replace with send_on_startup
(E712)
78-78: Avoid equality comparisons to True; use should_send_update: for truth checks
Replace with should_send_update
(E712)
87-87: Avoid equality comparisons to True; use should_send_update: for truth checks
Replace with should_send_update
(E712)
96-96: Avoid equality comparisons to False; use not should_send_update: for false checks
Replace with not should_send_update
(E712)
105-105: Avoid equality comparisons to True; use should_send_update: for truth checks
Replace with should_send_update
(E712)
118-118: Avoid equality comparisons to True; use should_send_on_first_loop: for truth checks
Replace with should_send_on_first_loop
(E712)
124-124: Avoid equality comparisons to False; use not should_send_on_first_loop: for false checks
Replace with not should_send_on_first_loop
(E712)
158-158: Avoid equality comparisons to True; use should_send_status: for truth checks
Replace with should_send_status
(E712)
162-162: Avoid equality comparisons to False; use not can_send_alert: for false checks
Replace with not can_send_alert
(E712)
231-231: Consider moving this statement to an else block
(TRY300)
236-236: Do not catch blind exception: Exception
(BLE001)
wsgi.py
31-31: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
webhook_service.py
66-66: Do not catch blind exception: Exception
(BLE001)
116-116: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
118-118: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
203-203: f-string without any placeholders
Remove extraneous f prefix
(F541)
237-237: f-string without any placeholders
Remove extraneous f prefix
(F541)
268-268: f-string without any placeholders
Remove extraneous f prefix
(F541)
299-299: f-string without any placeholders
Remove extraneous f prefix
(F541)
397-397: String contains ambiguous ℹ (INFORMATION SOURCE). Did you mean i (LATIN SMALL LETTER I)?
(RUF001)
temp_monitor.py
144-144: Unexpected indentation
(invalid-syntax)
149-149: Expected a statement
(invalid-syntax)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Greptile Review
🔇 Additional comments (39)
LICENSE (1)
1-21: LGTM!Standard MIT License with clear copyright notice.
.gitignore (1)
2-15: LGTM!Environment variables, Python cache files, and development artifacts are properly gitignored. The .env file coverage prevents accidental commits of sensitive information per coding guidelines.
requirements.txt (1)
2-2: Dependencies align well with hardware constraints.The new dependencies—flask-restx for API development, requests for webhooks, waitress for lightweight production serving, and psutil for monitoring—are all appropriate lightweight choices for Raspberry Pi Zero. Flask is a simple and expressive framework, and psutil is a great cross-platform module for system information, and waitress is the standard choice for resource-constrained deployments compared to heavier alternatives.
Also applies to: 4-7
AGENTS.MD (1)
1-236: Excellent comprehensive documentation!This file provides thorough guidance on the project's architecture, API structure, deployment options, and testing strategy. The organization is logical and will be very helpful for both AI assistants and human developers working with the codebase.
.github/workflows/ci.yml (1)
55-63: Excellent security practices for the deploy job!The conditional deployment guards and detailed comments clearly explain the security model, ensuring that untrusted code from pull requests cannot execute in the production environment. This follows CI/CD security best practices.
CLAUDE.md (1)
1-236: Well-structured documentation with content overlap.This file provides comprehensive guidance similar to AGENTS.MD. The content is excellent and well-organized. Note that maintaining two nearly identical documentation files (CLAUDE.md and AGENTS.MD) may create maintenance burden, though this duplication may be intentional for different AI tooling contexts.
README.md (2)
1-30: Excellent comprehensive README structure!The updated README provides a professional overview with clear features, organized table of contents, and comprehensive documentation. This will greatly help new users understand and deploy the system.
114-183: Excellent API documentation!The API Reference section thoroughly documents all endpoints with their purpose, authentication requirements, request/response format, and example curl commands. This follows the project's coding guidelines for README documentation.
As per coding guidelines: all API endpoints are well-documented with examples.
webhook_service.py (2)
18-25: LGTM! Well-structured dataclass for webhook configuration.The
WebhookConfigdataclass provides sensible defaults and clear documentation. Consider adding__post_init__validation in a future iteration to enforce constraints (e.g.,retry_count >= 1,timeout > 0) as noted in the research document for Issue #24.
326-372: LGTM! Well-structured status update with optional CPU temperature.The method correctly handles optional
cpu_tempand builds a comprehensive status message with Fahrenheit conversion.thoughts/tasks/issue-24-pydantic-validation/2025-12-31-research.md (1)
1-12: LGTM! Comprehensive research documentation.The research document provides excellent context for the validation improvements, comparing Pydantic vs Flask-RESTX approaches with clear trade-offs. The decision to use Flask-RESTX aligns well with the existing stack.
Dockerfile (2)
42-43: LGTM! Production-ready Waitress configuration.The configuration with single thread, 120s channel timeout, and explicit host binding is appropriate for the Pi 4's resource constraints. The
--callflag correctly invokes the WSGI application factory pattern.
29-29: LGTM! Updated COPY includes all required modules.The addition of
webhook_service.py,sense_hat.py,api_models.py, andwsgi.pyensures the production container has all necessary application files.start_production.sh (1)
52-59: LGTM! Consistent Waitress configuration with additional connection limit.The script properly uses
--call wsgi:appmatching the Dockerfile. The--connection-limit=50is a good addition for resource-constrained Pi 4 environments. Note this differs slightly from the Dockerfile (which lacks--connection-limit); consider aligning them for consistency.test_api_models.py (3)
12-91: LGTM! Comprehensive test coverage for webhook config validation.Excellent coverage of boundary values (min/max), empty configs, None values, and invalid ranges. The tests properly verify both the boolean result and error message content.
176-181: Good edge case: negative temperature support for freezer monitoring.This test correctly validates that the threshold validators work with negative temperatures, which is important for server room monitoring in cold environments or freezer monitoring use cases.
93-194: Thevalidate_thresholdsfunction does not validate absolute humidity ranges (0-100%).The implementation only validates that min < max relationships; it does not enforce that humidity values fall within 0-100% or that temperatures are within any specific range. Testing for out-of-range values would not be meaningful unless such validation is first added to the function itself.
Likely an incorrect or invalid review comment.
docs/PI4_DEPLOYMENT.md (1)
1-31: LGTM! Excellent production deployment documentation.This guide provides comprehensive coverage of hardware requirements, deployment options, and operational procedures. The memory footprint estimates and performance tuning sections align well with the Pi 4's resource constraints. Based on learnings, this appropriately considers the headless Raspberry Pi deployment context.
.env.example (4)
1-4: LGTM - Bearer token configuration is clear and secure.The token generation instructions using
secrets.token_hex(32)align with the coding guidelines requirement for 64-character hex strings. The clear labeling as REQUIRED helps prevent misconfiguration.Based on learnings: The implementation correctly stores BEARER_TOKEN in .env (never committed) with clear generation instructions.
15-26: Webhook configuration looks good.The default values for webhook retry configuration (retry_count=3, retry_delay=5, timeout=10) match the WebhookConfig defaults from the codebase, ensuring consistency.
28-37: Alert threshold defaults are appropriate for server room monitoring.The temperature range (15-27°C / 59-80.6°F) and humidity range (30-70%) are sensible defaults for server room environmental monitoring.
Note: The static analysis warnings about key ordering (UnorderedKey) can be safely ignored. Grouping min/max pairs together is more intuitive than strict alphabetical ordering.
39-56: Excellent documentation for periodic status updates.The configuration includes helpful examples of common interval values and clearly documents the constraint that intervals cannot be less than 60 seconds (the sampling interval). Defaulting to disabled (
false) is appropriate for most use cases.test_webhook.py (1)
15-604: Excellent test coverage and organization.The test suite is well-structured with clear separation of concerns across 8 test classes. The comprehensive coverage includes:
- Message formatting and payload structure
- Alert scenarios (temperature/humidity high/low)
- Status updates and system events
- Webhook disabled behavior
- Threshold detection and cooldown logic
- Configuration management
The use of mocking to isolate webhook behavior without making actual HTTP requests is appropriate.
test_webhook_api.py (3)
15-44: Good test setup with proper hardware mocking and cleanup.The test setup correctly:
- Mocks the
sense_hatmodule before importing to avoid hardware dependencies- Retrieves the bearer token from the environment with a fallback for testing
- Preserves and restores the original webhook_service state in tearDown
This aligns with the learning that tests should avoid actual hardware dependencies.
45-333: Comprehensive API integration tests with excellent security coverage.The test suite provides thorough coverage including:
- Bug fix validation: Line 45-82 specifically tests the AttributeError fix when
webhook_serviceis None- Input validation: Missing URL, invalid thresholds (min >= max)
- Security: Authentication (401), invalid tokens (403), URL masking to prevent token exposure
- CRUD operations: Create, read, update webhook configurations
The URL masking test (lines 282-333) is particularly important as it verifies that sensitive Slack webhook tokens are not exposed in API responses.
Based on learnings: These integration tests complement the manual testing recommendation for API endpoints, providing automated verification of token validation.
335-360: LGTM - Well-structured test runner.The main entry point provides clear test output with visual feedback and proper exit codes for CI/CD integration.
test_periodic_updates.py (4)
1-11: LGTM on test file structure!The test file is well-organized and self-contained for validating periodic status update functionality without requiring Flask or hardware dependencies.
55-108: LGTM!The timing logic tests comprehensively cover all edge cases including first update, interval elapsed, pre-interval blocking, and exact boundary conditions.
111-127: LGTM!Startup behavior tests correctly validate both enabled and disabled scenarios.
130-168: LGTM!Good test demonstrating that periodic status updates operate independently from the alert cooldown mechanism.
api_models.py (3)
1-77: LGTM!Well-structured Flask-RESTX models with clear field descriptions and examples for Swagger documentation. The separation of input and output models provides flexibility for API evolution.
79-120: LGTM!Response models are well-organized with appropriate separation between input and output schemas.
123-145: LGTM!Validation logic correctly enforces range constraints for webhook configuration fields. The pattern handles partial updates properly by only validating fields that are present.
temp_monitor.py (6)
1-27: LGTM on imports and environment loading!Good graceful handling of the optional
psutildependency with try/except. Environment loading follows the coding guidelines.
58-78: LGTM!Flask-RESTX API is well-configured with Swagger documentation at
/docs. The bearer token authorization is correctly defined for per-endpoint application rather than global enforcement, allowing the Swagger UI to remain accessible.
105-138: LGTM on webhook service initialization!Environment-driven configuration with appropriate defaults and clear logging. Good handling of the edge case where status updates are enabled but webhook service is not configured.
466-604: LGTM on webhook config resource!Well-structured REST resource with proper:
- Bearer token authentication via
@require_token- Input validation using helpers from
api_models- URL masking for security
- Error correlation IDs for debugging
Note: This depends on
generate_error_id()which has a syntax error that must be fixed first.
690-747: LGTM on health and metrics endpoints!Appropriate public endpoints for monitoring and load balancer health checks. Good graceful degradation when
psutilis unavailable.
801-811: LGTM!Clean main block with proper error handling and logging.
| @webhooks_ns.route('/enable') | ||
| class WebhookEnableResource(Resource): | ||
| """Enable webhook notifications""" | ||
|
|
||
| @webhooks_ns.doc(security='bearer') | ||
| @webhooks_ns.marshal_with(message_response) | ||
| @webhooks_ns.response(400, 'Webhook not configured', error_response) | ||
| @require_token | ||
| def post(self): | ||
| """Enable webhook notifications""" | ||
| if not webhook_service or not webhook_service.webhook_config: | ||
| webhooks_ns.abort(400, 'Webhook not configured') | ||
|
|
||
| webhook_service.webhook_config.enabled = True | ||
| logging.info("Webhook notifications enabled") | ||
|
|
||
| return { | ||
| 'message': 'Webhook notifications enabled', | ||
| 'enabled': True | ||
| } | ||
|
|
||
|
|
||
| @webhooks_ns.route('/disable') | ||
| class WebhookDisableResource(Resource): | ||
| """Disable webhook notifications""" | ||
|
|
||
| @webhooks_ns.doc(security='bearer') | ||
| @webhooks_ns.marshal_with(message_response) | ||
| @webhooks_ns.response(400, 'Webhook not configured', error_response) | ||
| @require_token | ||
| def post(self): | ||
| """Disable webhook notifications""" | ||
| if not webhook_service or not webhook_service.webhook_config: | ||
| webhooks_ns.abort(400, 'Webhook not configured') | ||
|
|
||
| webhook_service.webhook_config.enabled = False | ||
| logging.info("Webhook notifications disabled") | ||
|
|
||
| return { | ||
| 'message': 'Webhook notifications disabled', | ||
| 'enabled': False | ||
| } |
There was a problem hiding this comment.
Potential thread-safety concern with direct attribute modification.
Lines 656 and 678 directly modify webhook_service.webhook_config.enabled without using the service's lock. The WebhookService uses self._lock in methods like set_webhook_config for thread safety. Direct attribute modification could race with _send_webhook which reads enabled.
Consider using a method in WebhookService to safely toggle the enabled state.
🔎 Suggested approach
Add a thread-safe method to WebhookService:
def set_enabled(self, enabled: bool):
"""Thread-safe enable/disable of webhook notifications"""
with self._lock:
if self.webhook_config:
self.webhook_config.enabled = enabled
logging.info(f"Webhook notifications {'enabled' if enabled else 'disabled'}")Then use it in the endpoints:
webhook_service.set_enabled(True) # or False🤖 Prompt for AI Agents
In temp_monitor.py around lines 643 to 684, the endpoints directly set
webhook_service.webhook_config.enabled (lines ~656 and ~678) which bypasses
WebhookService's lock and risks races with _send_webhook; add a thread-safe
WebhookService.set_enabled(self, enabled: bool) that acquires self._lock,
verifies self.webhook_config is present, sets webhook_config.enabled and logs
the change, and then update the enable/disable endpoints to call
webhook_service.set_enabled(True) / set_enabled(False) (keeping the existing
pre-check or letting set_enabled raise/abort if no config) and return the same
response.
| # Give the thread a moment to get initial readings | ||
| time.sleep(2) |
There was a problem hiding this comment.
Duplicate sleep - start_sensor_thread() already waits 2 seconds.
Per the relevant code snippet from temp_monitor.py (lines 769-770), start_sensor_thread() already includes time.sleep(2) before returning. This results in a 4-second total delay on startup.
🔎 Proposed fix
try:
logger.info("Initializing sensor thread for production deployment...")
sensor_thread = start_sensor_thread()
-
- # Give the thread a moment to get initial readings
- time.sleep(2)
-
logger.info("Sensor thread started successfully")
except Exception as e:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Give the thread a moment to get initial readings | |
| time.sleep(2) | |
| try: | |
| logger.info("Initializing sensor thread for production deployment...") | |
| sensor_thread = start_sensor_thread() | |
| logger.info("Sensor thread started successfully") | |
| except Exception as e: |
🤖 Prompt for AI Agents
In wsgi.py around lines 26-27, there is a redundant time.sleep(2) call that
duplicates the 2s delay already performed by start_sensor_thread() in
temp_monitor.py; remove the sleep from wsgi.py so startup only waits once (let
start_sensor_thread() handle the initial delay) and update any brief comment to
reflect that the sensor thread handles its own warm-up delay.
- Changed the Git clone URL to point to the correct repository for the temp_monitor project.
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
|
Looks like there are a few issues preventing this PR from being merged!
If you'd like me to help, just leave a comment, like Feel free to include any additional details that might help me get this PR into a better state. You can manage your notification settings |
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
temp_monitor.py (3)
656-656: Thread-safety concern with direct attribute modification.Lines 656 and 678 directly modify
webhook_service.webhook_config.enabledwithout acquiring the service's internal lock. This could race with_send_webhookwhich reads theenabledflag under lock.As suggested in a previous review, add a thread-safe method to
WebhookService:def set_enabled(self, enabled: bool): with self._lock: if self.webhook_config: self.webhook_config.enabled = enabledThen replace the direct assignments with
webhook_service.set_enabled(True/False).Also applies to: 678-678
780-791: Critical: Lock created on each call provides no protection.Both functions create a new
threading.Lock()on each invocation. Since each thread gets a different lock instance, there's no actual synchronization. This was flagged in a previous review.🔎 Proposed fix
+# Module-level lock for counter synchronization +_counter_lock = threading.Lock() + def increment_request_counter(): """Middleware-like function to track requests""" global request_counter - with threading.Lock(): + with _counter_lock: request_counter += 1 def increment_alert_counter(): """Increment webhook alert counter""" global webhook_alert_counter - with threading.Lock(): + with _counter_lock: webhook_alert_counter += 1
140-145: Moveimport randomto top of file with other imports.The
import randomstatement on line 143 is inside the function body. While syntactically valid (lazy import), it's unconventional and was previously flagged. Move it to the imports section for consistency.Note: The static analysis warning about cryptographic use (S311) can be safely ignored here since this is for error correlation IDs, not security-sensitive operations.
🔎 Proposed fix
Add to imports at top of file (around line 8):
import functools import random import signalThen update the function:
def generate_error_id(): """Generate a correlation ID for error tracking in logs and responses""" timestamp = int(time.time() * 1000) - import random suffix = format(random.randint(0, 65535), '04x') return f"{timestamp}_{suffix}"
🧹 Nitpick comments (9)
README.md (1)
256-264: Add language specification to the code block.The temperature compensation formula code block would benefit from a language specification for proper syntax highlighting.
🔎 Proposed fix
-``` +```python compensated_temp = raw_temp - ((cpu_temp - raw_temp) * factor)</details> </blockquote></details> <details> <summary>temp_monitor.py (8)</summary><blockquote> `117-122`: **Consider caching environment variable reads.** Each threshold line calls `os.getenv()` twice - once for the conditional check and once for the value. While not a bug, this is slightly inefficient. <details> <summary>🔎 Suggested refactor</summary> ```diff - alert_thresholds = AlertThresholds( - temp_min_c=float(os.getenv('ALERT_TEMP_MIN_C', '15.0')) if os.getenv('ALERT_TEMP_MIN_C') else None, - temp_max_c=float(os.getenv('ALERT_TEMP_MAX_C', '27.0')) if os.getenv('ALERT_TEMP_MAX_C') else None, - humidity_min=float(os.getenv('ALERT_HUMIDITY_MIN', '30.0')) if os.getenv('ALERT_HUMIDITY_MIN') else None, - humidity_max=float(os.getenv('ALERT_HUMIDITY_MAX', '70.0')) if os.getenv('ALERT_HUMIDITY_MAX') else None - ) + def parse_optional_float(env_var: str) -> float | None: + value = os.getenv(env_var) + return float(value) if value else None + + alert_thresholds = AlertThresholds( + temp_min_c=parse_optional_float('ALERT_TEMP_MIN_C'), + temp_max_c=parse_optional_float('ALERT_TEMP_MAX_C'), + humidity_min=parse_optional_float('ALERT_HUMIDITY_MIN'), + humidity_max=parse_optional_float('ALERT_HUMIDITY_MAX') + )
302-303: Uselogging.exceptioninstead oflogging.errorfor full traceback.When logging exceptions,
logging.exceptionautomatically includes the traceback, which is valuable for debugging.🔎 Proposed fix
except Exception as webhook_error: - logging.error(f"Error sending webhook alert: {webhook_error}") + logging.exception(f"Error sending webhook alert: {webhook_error}")
328-329: Uselogging.exceptionfor status update errors as well.🔎 Proposed fix
except Exception as update_error: - logging.error(f"Error sending periodic status update: {update_error}") + logging.exception(f"Error sending periodic status update: {update_error}")
305-332: Moveglobal last_status_updatedeclaration to function level.The
globaldeclaration on line 307 is inside the while loop's try block. While it works, it's unconventional and reduces readability. Move it to line 279 with the other globals.🔎 Proposed fix
def update_sensor_data(): """Background thread function to update sensor data periodically""" - global current_temp, current_humidity, last_updated + global current_temp, current_humidity, last_updated, last_status_update while True: try: # ... existing code ... # Send periodic status updates if enabled if status_update_enabled and webhook_service: - global last_status_update current_time = time.time()
600-603: Remove unused exception variables.The caught exceptions
eare logged vialogging.exception()which already captures the exception, making the variable unnecessary.🔎 Proposed fix
- except Exception as e: + except Exception: error_id = generate_error_id() logging.exception(f"Error updating webhook config [error_id: {error_id}]") return {'error': 'Failed to update webhook configuration', 'error_id': error_id}, 500- except Exception as e: + except Exception: error_id = generate_error_id() logging.exception(f"Error sending test webhook [error_id: {error_id}]") webhooks_ns.abort(500, 'Failed to send test webhook')Also applies to: 637-640
524-534: Simplify dictionary access patterns.The
if 'key' in data and data['key']pattern can be simplified usingdata.get('key')for cleaner code.🔎 Example refactor
- if 'webhook' in data and data['webhook']: - is_valid, error_msg = validate_webhook_config(data['webhook']) + webhook_data = data.get('webhook') + if webhook_data: + is_valid, error_msg = validate_webhook_config(webhook_data)
701-701: Remove unused exception variables in health/metrics endpoints.Same pattern as above - the exception variables are not used.
🔎 Proposed fix
- except Exception as e: + except Exception: error_id = generate_error_id()Also applies to: 737-737, 744-744
809-811: Uselogging.exceptionfor startup failure.🔎 Proposed fix
except Exception as e: - logging.error(f"Failed to start service: {e}") + logging.exception(f"Failed to start service: {e}") raise
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.mdtemp_monitor.pytest_webhook.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Use the@require_tokendecorator to protect API endpoints that require bearer token authentication
Add logging statements for all significant operations using theloggingmodule
Wrap hardware operations (Sense HAT interactions) in try-except blocks with appropriate error handling and logging
Usejsonify()for all JSON API responses in Flask endpoints
Load environment variables usingos.getenv('VARIABLE_NAME', default)with appropriate defaults for configuration
Use a background daemon thread pattern with configurable sampling intervals for continuous sensor monitoring without blocking the main application
Implement temperature compensation using the formula:comp_temp = raw_temp - ((cpu_temp - raw_temp) * factor)with a calibration factor of 0.7
Remove outliers by taking readings and removing the highest and lowest values before averaging for sensor data accuracy
Automatically create log directories if they don't exist and provide clear error messages if logging initialization fails
Bearer tokens must be 64-character hex strings generated usingsecrets.token_hex(32)for cryptographic security
RequireAuthorization: Bearer <token>header format for all protected API endpoints
For new API endpoints, add@require_tokendecorator, return responses withjsonify(), log access attempts, and update README.md documentation
Use global variables for sensor data (current_temp, current_humidity, last_updated) thread-safe due to Python GIL, with updates from background daemon thread
Implement graceful degradation for hardware sensor failures (e.g., use raw temperature if CPU temperature unavailable)
The web dashboard (/) is intentionally public with no authentication; API endpoints (/api/*) require bearer token authentication unless explicitly documented as public
Never assume hardware is available; handle Sense HAT initialization failures gracefully with informative error messages
Files:
test_webhook.pytemp_monitor.py
README.md
📄 CodeRabbit inference engine (CLAUDE.md)
Document all API endpoints in README.md with their purpose, authentication requirements, request/response format, and example curl commands
Files:
README.md
🧠 Learnings (15)
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/temp_monitor.py : Protect API endpoints with require_token decorator for bearer token authentication, except for public routes like web dashboard and favicon
Applied to files:
temp_monitor.pyREADME.md
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/temp_monitor.py : Use Flask decorator pattern (app.route, require_token) for defining API endpoints and middleware
Applied to files:
temp_monitor.pyREADME.md
📚 Learning: 2025-12-07T22:09:12.211Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Applies to **/*.py : For new API endpoints, add `require_token` decorator, return responses with `jsonify()`, log access attempts, and update README.md documentation
Applied to files:
temp_monitor.pyREADME.md
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/temp_monitor.py : Use consistent JSON API response format with fields: temperature_c, temperature_f, humidity, timestamp
Applied to files:
temp_monitor.py
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/temp_monitor.py : Return JSON responses using Flask's jsonify() function for all API endpoints
Applied to files:
temp_monitor.py
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/temp_monitor.py : Store BEARER_TOKEN in .env file (never committed to git) and load at application startup; auto-generate token if .env is missing
Applied to files:
temp_monitor.pyREADME.md
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/temp_monitor.py : Use environment variables (loaded via python-dotenv) for configurable paths: LOG_FILE, LOGO_PATH, FAVICON_PATH, BEARER_TOKEN
Applied to files:
temp_monitor.pyREADME.md
📚 Learning: 2025-12-07T22:09:12.211Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Applies to **/*.py : Use the `require_token` decorator to protect API endpoints that require bearer token authentication
Applied to files:
temp_monitor.py
📚 Learning: 2025-12-07T22:09:12.211Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Applies to **/*.py : The web dashboard (`/`) is intentionally public with no authentication; API endpoints (`/api/*`) require bearer token authentication unless explicitly documented as public
Applied to files:
temp_monitor.pyREADME.md
📚 Learning: 2025-12-07T22:09:12.211Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Applies to **/*.py : Require `Authorization: Bearer <token>` header format for all protected API endpoints
Applied to files:
temp_monitor.py
📚 Learning: 2025-12-07T22:09:12.211Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Applies to **/*.py : Use global variables for sensor data (current_temp, current_humidity, last_updated) thread-safe due to Python GIL, with updates from background daemon thread
Applied to files:
temp_monitor.py
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/temp_monitor.py : Use daemon threads for background monitoring tasks (sensor data collection) with appropriate sampling intervals (default 60 seconds)
Applied to files:
temp_monitor.py
📚 Learning: 2025-12-07T22:09:12.211Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Applies to **/*.py : Use a background daemon thread pattern with configurable sampling intervals for continuous sensor monitoring without blocking the main application
Applied to files:
temp_monitor.py
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/generate_token.py : Use Bearer token format of 64-character hex strings (32 bytes) generated via secrets.token_hex(32) for cryptographically secure tokens
Applied to files:
temp_monitor.py
📚 Learning: 2025-12-07T22:09:12.211Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Test all API endpoints with actual hardware when possible; manual testing should verify hardware LED display shows temperature, auto-refresh works, and token validation works
Applied to files:
README.md
🧬 Code graph analysis (2)
test_webhook.py (1)
webhook_service.py (9)
WebhookService(37-413)WebhookConfig(19-25)AlertThresholds(29-34)send_slack_message(151-177)check_and_alert(179-324)send_status_update(326-372)send_system_event(374-413)_can_send_alert(128-144)_mark_alert_sent(146-149)
temp_monitor.py (2)
webhook_service.py (7)
WebhookService(37-413)WebhookConfig(19-25)AlertThresholds(29-34)check_and_alert(179-324)send_status_update(326-372)set_webhook_config(70-74)set_alert_thresholds(76-80)api_models.py (2)
validate_thresholds(148-166)validate_webhook_config(123-145)
🪛 Gitleaks (8.30.0)
README.md
[high] 151-151: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
[high] 173-174: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
🪛 LanguageTool
README.md
[uncategorized] ~101-~101: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...------| | ALERT_TEMP_MIN_C | 15.0 | Low temperature alert (Celsius) | | ALERT_TEMP_MAX_C ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~102-~102: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...sius) | | ALERT_TEMP_MAX_C | 27.0 | High temperature alert (Celsius) | | `ALERT_HUMIDITY_MIN...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~291-~291: Consider using a less common alternative to make your writing sound more unique and professional.
Context: ...ontributing Contributions are welcome! Please feel free to submit a Pull Request. ## License [MI...
(FEEL_FREE_TO_STYLE_ME)
🪛 markdownlint-cli2 (0.18.1)
README.md
260-260: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.14.10)
temp_monitor.py
144-144: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
183-183: Do not catch blind exception: Exception
(BLE001)
302-302: Do not catch blind exception: Exception
(BLE001)
303-303: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
328-328: Do not catch blind exception: Exception
(BLE001)
329-329: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
525-525: Unnecessary key check before dictionary access
Replace with dict.get
(RUF019)
531-531: Unnecessary key check before dictionary access
Replace with dict.get
(RUF019)
537-537: Unnecessary key check before dictionary access
Replace with dict.get
(RUF019)
549-549: Unnecessary key check before dictionary access
Replace with dict.get
(RUF019)
567-567: Unnecessary key check before dictionary access
Replace with dict.get
(RUF019)
600-600: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
637-637: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
701-701: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
737-737: Local variable psutil_error is assigned to but never used
Remove assignment to unused variable psutil_error
(F841)
744-744: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
774-774: Avoid specifying long messages outside the exception class
(TRY003)
808-808: Possible binding to all interfaces
(S104)
810-810: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
🔇 Additional comments (16)
test_webhook.py (1)
1-604: Excellent test coverage and implementation!This test suite is comprehensive and well-structured. It covers:
- Slack message payload structure and formatting (basic messages, colors, fields)
- All alert types with correct payload validation (temperature/humidity high/low)
- Status updates with optional CPU temperature handling
- System events with proper severity mapping
- Webhook disabled behavior across all message types
- Threshold detection logic and edge cases
- Cooldown mechanisms between alerts
- Configuration management with defaults and custom values
The use of mocking is appropriate and isolates the webhook service from external dependencies. The test organization is logical, and assertions are precise and thorough.
README.md (3)
1-118: Excellent documentation structure and clarity!The updated README provides comprehensive coverage of:
- Clear feature list highlighting all major capabilities
- Accurate hardware requirements (Pi 4 with appropriate specs)
- Well-organized Quick Start with development, production, and Docker options
- Thorough configuration documentation with environment variables
The Bearer token generation example correctly follows the coding guideline using
secrets.token_hex(32).
114-208: Comprehensive API documentation with clear examples!The API Reference section effectively documents:
- Clear separation between public and protected endpoints
- Proper authentication requirements with Bearer token header format
- Realistic example requests and responses
- All webhook management endpoints with descriptions
This aligns well with the coding guidelines requiring documentation of all API endpoints with authentication requirements, request/response formats, and example curl commands.
Note: Static analysis flagged "YOUR_TOKEN" in curl examples as a potential security issue, but these are clearly placeholder values for documentation purposes.
209-295: Comprehensive deployment and troubleshooting documentation!The deployment section provides multiple production-ready options (Docker, systemd, Waitress), complete with configuration guidance. The troubleshooting table addresses common issues with clear solutions, and the dependency list helps users understand the project's requirements.
The temperature compensation formula documentation matches the coding guideline implementation.
temp_monitor.py (12)
1-26: LGTM - Imports and environment setup.Imports are well-organized with proper optional dependency handling for
psutil. The graceful fallback pattern aligns with the coding guideline for optional dependencies.
28-46: LGTM - Logging configuration follows guidelines.Proper directory creation with error handling and clear failure messages as per coding guidelines.
48-54: LGTM - Sense HAT initialization with proper error handling.Hardware initialization follows guidelines with try-except and logging. Re-raising is appropriate here since the application requires Sense HAT to function.
58-78: LGTM - Flask-RESTX API with proper authorization configuration.Good setup with Swagger documentation at
/docsand bearer token authorization scheme. Individual endpoint protection via decorators is the right approach.
148-158: LGTM - Bearer token validation with clear error messaging.Proper enforcement of required token with helpful generation instructions. Follows the guideline for 64-character hex tokens via
secrets.token_hex(32).
160-185: LGTM - Secure webhook URL masking.Good security practice to prevent exposure of sensitive path components and tokens in API responses and logs.
187-205: LGTM - Token authentication decorator.Properly implements bearer token validation with appropriate HTTP status codes (401/403) and logs access attempts per coding guidelines.
207-276: LGTM - Sensor data collection with proper compensation and outlier removal.Implementation correctly follows coding guidelines:
- Temperature compensation formula with 0.7 factor
- Outlier removal (highest/lowest values)
- Graceful degradation when CPU temp unavailable
345-463: LGTM - Web and API routes properly configured.
- Public dashboard at
/per guidelines- All
/api/*endpoints protected with@require_token- JSON responses via
jsonify()- Consistent response format with temperature_c, temperature_f, humidity, timestamp
690-747: Verify public access for health/metrics is intentional.The
/healthand/metricsendpoints are not protected with@require_token. This is typically correct for load balancer health checks and monitoring systems (Prometheus, etc.), but confirm this is intentional as these endpoints expose operational data.If authentication is needed, add
@require_tokendecorator to both endpoints.
750-778: LGTM - Sensor thread management with duplicate start protection.Good implementation with:
- Check for already-running thread
- Daemon thread pattern per guidelines
- Startup verification with clear error
807-808: Binding to all interfaces (0.0.0.0) flagged by static analysis.This is typically intentional for development and container deployments. If this is a concern, consider adding a comment explaining the rationale, or make the host configurable via environment variable.
app.run(host=os.getenv('FLASK_HOST', '0.0.0.0'), port=int(os.getenv('FLASK_PORT', 8080)))
|
baz review |
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
- Added URL validation logic in `validate_webhook_config` to ensure URLs are non-empty strings and have valid schemes and hosts. - Updated `temp_monitor.py` to handle cases where the URL is missing or null during webhook configuration updates. - Expanded unit tests in `test_api_models.py` to cover various URL validation scenarios, including valid, empty, and improperly formatted URLs. - Added integration tests in `test_webhook_api.py` to verify API behavior when creating webhook configurations with invalid URLs.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
temp_monitor.py (3)
293-303: Bug: Alert counter increments even when webhook sends fail.The
check_and_alertmethod returns entries withFalsevalues when webhook sends fail. The current checkif alerts_sent:is truthy whenever the dict has any keys, regardless of whether any alert was successfully sent. This inflates thewebhook_alert_countermetric.This issue was previously identified in a past review comment.
🔎 Proposed fix
# Check thresholds and send alerts via webhook if webhook_service: try: alerts_sent = webhook_service.check_and_alert( current_temp, current_humidity, last_updated ) - if alerts_sent: + if alerts_sent and any(alerts_sent.values()): increment_alert_counter() logging.info(f"Webhook alerts sent: {list(alerts_sent.keys())}")
656-656: Thread-safety concern: Direct attribute modification bypasses lock.Lines 656 and 678 directly modify
webhook_service.webhook_config.enabledwithout using the service's lock. TheWebhookServiceusesself._lockfor thread-safe operations. Direct attribute modification could race with_send_webhookwhich readsenabled.This issue was previously identified in a past review comment.
🔎 Suggested approach from prior review
Add a thread-safe method to
WebhookService:def set_enabled(self, enabled: bool): """Thread-safe enable/disable of webhook notifications""" with self._lock: if self.webhook_config: self.webhook_config.enabled = enabled logging.info(f"Webhook notifications {'enabled' if enabled else 'disabled'}")Then use it in the endpoints:
webhook_service.set_enabled(True) # or FalseAlso applies to: 678-678
780-791: Bug: Lock is created on each call, providing no synchronization.Both
increment_request_counterandincrement_alert_countercreate a newthreading.Lock()on each invocation. A lock only provides protection when the same lock instance is used by all threads. This defeats the purpose of synchronization.This issue was previously identified in a past review comment.
🔎 Proposed fix
+# Module-level lock for counter synchronization +_counter_lock = threading.Lock() + def increment_request_counter(): """Middleware-like function to track requests""" global request_counter - with threading.Lock(): + with _counter_lock: request_counter += 1 def increment_alert_counter(): """Increment webhook alert counter""" global webhook_alert_counter - with threading.Lock(): + with _counter_lock: webhook_alert_counter += 1
🧹 Nitpick comments (4)
api_models.py (1)
160-177: Cross-field validation is correct, but individual threshold ranges are not enforced.The function correctly validates that
temp_min_c < temp_max_candhumidity_min < humidity_max. However, the individual field ranges defined in the model (e.g., temp between -50 and 100, humidity between 0 and 100) are only OpenAPI documentation hints—they aren't enforced at runtime. This could allow values liketemp_min_c=200.If strict range enforcement is desired, consider adding individual field range checks here.
🔎 Optional: Add individual range validation
def validate_thresholds(thresholds: dict) -> tuple: """ Validate threshold relationships (cross-field validation). Args: thresholds: Dictionary with threshold values Returns: Tuple of (is_valid: bool, error_message: str) """ + # Validate individual ranges + for key in ('temp_min_c', 'temp_max_c'): + if thresholds.get(key) is not None: + if not (-50 <= thresholds[key] <= 100): + return False, f'{key} must be between -50 and 100' + + for key in ('humidity_min', 'humidity_max'): + if thresholds.get(key) is not None: + if not (0 <= thresholds[key] <= 100): + return False, f'{key} must be between 0 and 100' + if thresholds.get('temp_min_c') is not None and thresholds.get('temp_max_c') is not None: if thresholds['temp_min_c'] >= thresholds['temp_max_c']: return False, 'temp_min_c must be less than temp_max_c'temp_monitor.py (3)
140-145: Import inside function is valid but consider moving to module level.While importing
randominsidegenerate_error_id()is valid Python, it's unconventional. Additionally, per static analysis (S311), standard pseudo-random generators aren't suitable for cryptographic purposes. However, for error ID generation (not security-critical), this is acceptable.Consider moving the import to the module level for consistency with other imports.
160-185: URL masking logic is correct but duplicatesWebhookService._mask_url.This function correctly masks webhook URLs by returning only the scheme and host. However, nearly identical logic exists in
webhook_service.py(_mask_urlmethod at lines 47-66). Consider consolidating into a shared utility to avoid maintenance burden.
302-303: Uselogging.exceptionfor better error diagnostics.Per static analysis (TRY400), replace
logging.errorwithlogging.exceptionwhen catching exceptions. This automatically includes the traceback, which is valuable for debugging webhook and status update failures.🔎 Proposed fix
except Exception as webhook_error: - logging.error(f"Error sending webhook alert: {webhook_error}") + logging.exception(f"Error sending webhook alert: {webhook_error}")except Exception as update_error: - logging.error(f"Error sending periodic status update: {update_error}") + logging.exception(f"Error sending periodic status update: {update_error}")Also applies to: 328-329
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
temp_monitor.logis excluded by!**/*.log
📒 Files selected for processing (5)
api_models.pydeployment/systemd/temp-monitor.servicetemp_monitor.pytest_api_models.pytest_webhook_api.py
🚧 Files skipped from review as they are similar to previous changes (2)
- deployment/systemd/temp-monitor.service
- test_api_models.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Use the@require_tokendecorator to protect API endpoints that require bearer token authentication
Add logging statements for all significant operations using theloggingmodule
Wrap hardware operations (Sense HAT interactions) in try-except blocks with appropriate error handling and logging
Usejsonify()for all JSON API responses in Flask endpoints
Load environment variables usingos.getenv('VARIABLE_NAME', default)with appropriate defaults for configuration
Use a background daemon thread pattern with configurable sampling intervals for continuous sensor monitoring without blocking the main application
Implement temperature compensation using the formula:comp_temp = raw_temp - ((cpu_temp - raw_temp) * factor)with a calibration factor of 0.7
Remove outliers by taking readings and removing the highest and lowest values before averaging for sensor data accuracy
Automatically create log directories if they don't exist and provide clear error messages if logging initialization fails
Bearer tokens must be 64-character hex strings generated usingsecrets.token_hex(32)for cryptographic security
RequireAuthorization: Bearer <token>header format for all protected API endpoints
For new API endpoints, add@require_tokendecorator, return responses withjsonify(), log access attempts, and update README.md documentation
Use global variables for sensor data (current_temp, current_humidity, last_updated) thread-safe due to Python GIL, with updates from background daemon thread
Implement graceful degradation for hardware sensor failures (e.g., use raw temperature if CPU temperature unavailable)
The web dashboard (/) is intentionally public with no authentication; API endpoints (/api/*) require bearer token authentication unless explicitly documented as public
Never assume hardware is available; handle Sense HAT initialization failures gracefully with informative error messages
Files:
api_models.pytest_webhook_api.pytemp_monitor.py
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/temp_monitor.py : Use Flask decorator pattern (app.route, require_token) for defining API endpoints and middleware
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Test all API endpoints with actual hardware when possible; manual testing should verify hardware LED display shows temperature, auto-refresh works, and token validation works
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/temp_monitor.py : Protect API endpoints with require_token decorator for bearer token authentication, except for public routes like web dashboard and favicon
📚 Learning: 2025-12-07T22:09:12.211Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Applies to **/*.py : For new API endpoints, add `require_token` decorator, return responses with `jsonify()`, log access attempts, and update README.md documentation
Applied to files:
test_webhook_api.pytemp_monitor.py
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/temp_monitor.py : Protect API endpoints with require_token decorator for bearer token authentication, except for public routes like web dashboard and favicon
Applied to files:
temp_monitor.py
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/temp_monitor.py : Use Flask decorator pattern (app.route, require_token) for defining API endpoints and middleware
Applied to files:
temp_monitor.py
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/temp_monitor.py : Use consistent JSON API response format with fields: temperature_c, temperature_f, humidity, timestamp
Applied to files:
temp_monitor.py
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/temp_monitor.py : Return JSON responses using Flask's jsonify() function for all API endpoints
Applied to files:
temp_monitor.py
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/temp_monitor.py : Use environment variables (loaded via python-dotenv) for configurable paths: LOG_FILE, LOGO_PATH, FAVICON_PATH, BEARER_TOKEN
Applied to files:
temp_monitor.py
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/temp_monitor.py : Store BEARER_TOKEN in .env file (never committed to git) and load at application startup; auto-generate token if .env is missing
Applied to files:
temp_monitor.py
📚 Learning: 2025-12-07T22:09:12.211Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Applies to **/*.py : Use the `require_token` decorator to protect API endpoints that require bearer token authentication
Applied to files:
temp_monitor.py
📚 Learning: 2025-12-07T22:09:12.211Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Applies to **/*.py : The web dashboard (`/`) is intentionally public with no authentication; API endpoints (`/api/*`) require bearer token authentication unless explicitly documented as public
Applied to files:
temp_monitor.py
📚 Learning: 2025-12-07T22:09:12.211Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Applies to **/*.py : Require `Authorization: Bearer <token>` header format for all protected API endpoints
Applied to files:
temp_monitor.py
📚 Learning: 2025-12-07T22:09:12.211Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Applies to **/*.py : Use global variables for sensor data (current_temp, current_humidity, last_updated) thread-safe due to Python GIL, with updates from background daemon thread
Applied to files:
temp_monitor.py
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/temp_monitor.py : Use daemon threads for background monitoring tasks (sensor data collection) with appropriate sampling intervals (default 60 seconds)
Applied to files:
temp_monitor.py
📚 Learning: 2025-12-07T22:09:12.211Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T22:09:12.211Z
Learning: Applies to **/*.py : Use a background daemon thread pattern with configurable sampling intervals for continuous sensor monitoring without blocking the main application
Applied to files:
temp_monitor.py
📚 Learning: 2025-11-27T09:21:56.117Z
Learnt from: CR
Repo: freightCognition/temp_monitor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T09:21:56.117Z
Learning: Applies to **/generate_token.py : Use Bearer token format of 64-character hex strings (32 bytes) generated via secrets.token_hex(32) for cryptographically secure tokens
Applied to files:
temp_monitor.py
🧬 Code graph analysis (3)
api_models.py (1)
temp_monitor.py (1)
get(474-510)
test_webhook_api.py (2)
webhook_service.py (3)
WebhookService(37-413)WebhookConfig(19-25)AlertThresholds(29-34)temp_monitor.py (2)
put(518-603)get(474-510)
temp_monitor.py (3)
webhook_service.py (4)
WebhookConfig(19-25)AlertThresholds(29-34)check_and_alert(179-324)send_status_update(326-372)api_models.py (2)
validate_thresholds(160-178)validate_webhook_config(124-157)sense_hat.py (2)
get_humidity(17-18)show_message(23-24)
🪛 Ruff (0.14.10)
temp_monitor.py
144-144: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
183-183: Do not catch blind exception: Exception
(BLE001)
302-302: Do not catch blind exception: Exception
(BLE001)
303-303: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
328-328: Do not catch blind exception: Exception
(BLE001)
329-329: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
525-525: Unnecessary key check before dictionary access
Replace with dict.get
(RUF019)
531-531: Unnecessary key check before dictionary access
Replace with dict.get
(RUF019)
537-537: Unnecessary key check before dictionary access
Replace with dict.get
(RUF019)
549-549: Unnecessary key check before dictionary access
Replace with dict.get
(RUF019)
567-567: Unnecessary key check before dictionary access
Replace with dict.get
(RUF019)
600-600: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
637-637: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
701-701: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
737-737: Local variable psutil_error is assigned to but never used
Remove assignment to unused variable psutil_error
(F841)
744-744: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
774-774: Avoid specifying long messages outside the exception class
(TRY003)
808-808: Possible binding to all interfaces
(S104)
810-810: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
🔇 Additional comments (12)
test_webhook_api.py (6)
1-21: LGTM! Proper test setup with hardware mocking.The test file correctly mocks the
sense_hatmodule before importingtemp_monitor, which prevents hardware-related failures during testing. The imports and module structure are appropriate for Flask integration testing.
26-43: Test isolation and cleanup handled correctly.The
setUpandtearDownmethods properly save and restore thewebhook_servicestate, ensuring tests don't affect each other. The auth header setup is appropriate.
83-180: Good coverage of URL validation edge cases.These tests comprehensively validate the new URL validation logic for missing, empty, null, and malformed URLs. The assertions correctly verify both the HTTP 400 status codes and the presence of appropriate error messages.
181-216: Important partial update test.This test validates the critical behavior where partial updates without a URL should succeed when an existing config already has a URL. This is essential for the PUT endpoint's usability.
392-442: Thorough security test for URL masking.This test properly validates that sensitive webhook tokens in the URL path are not exposed in API responses. The explicit assertions checking that token components (
T12345,B67890, etc.) are NOT present in the response add strong security validation.
445-470: LGTM!Standard test runner implementation with appropriate exit code handling.
api_models.py (3)
1-12: LGTM! Clear module structure and namespace setup.The module docstring clearly describes the purpose, and the webhooks namespace is properly configured with a descriptive name for Swagger documentation.
14-78: Models well-defined with documentation hints.The input models are properly structured with descriptive fields and examples. Note that Flask-RESTX
min/maxattributes onfields.Integerandfields.Floatare primarily for OpenAPI documentation; actual enforcement is handled by the separatevalidate_webhook_configandvalidate_thresholdsfunctions.
124-157: URL validation logic looks correct.The
validate_webhook_configfunction properly validates:
- URL is a non-empty string with valid scheme and host (using
urlparse)- Numeric fields are within their defined ranges
- Allows
NoneURL for partial updates with existing configThis aligns with the PR objectives for stricter webhook URL validation.
temp_monitor.py (3)
536-545: URL validation correctly prevents empty URL creation.The validation at lines 536-545 properly checks that a URL is required when no existing webhook config exists. Combined with
validate_webhook_configwhich validates URL format, this addresses the concern about creating configs with empty/null URLs.
707-747: Verify/metricsendpoint should be public.The
/metricsendpoint exposes detailed application and system metrics without authentication. While/healthbeing public is appropriate for load balancers,/metricsincludes sensitive operational data (request counts, webhook alerts, memory usage, CPU temps).Per coding guidelines, API endpoints under
/api/*require bearer token authentication. These endpoints are intentionally not under/api/, but consider if public metrics exposure aligns with your security requirements.
750-777: LGTM! Defensive sensor thread startup.The
start_sensor_threadfunction properly handles the case where the thread is already running and verifies the thread started successfully. This is good defensive programming for production deployment.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
- Added new log entries for webhook service initialization, periodic status updates, and alert threshold updates. - Implemented a dedicated lock for counters to ensure thread safety when incrementing request and alert counters.
|
baz review |
Co-authored-by: baz-reviewer[bot] <174234987+baz-reviewer[bot]@users.noreply.github.com>
|
Skipped: This PR does not contain any of your configured labels: ( |
1 participant
Summary by CodeRabbit
New Features
Deployment
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.