Repository navigation
fix: health check 503, bind to 127.0.0.1, standardize dependencies - #64
Conversation
📝 WalkthroughWalkthroughThis pull request updates FastAPI and Uvicorn version constraints, changes uvicorn host binds from 0.0.0.0 to 127.0.0.1 in several Makefiles, and modifies multiple /health endpoints to return HTTP 503 with Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
e3c2d06 to
10f043c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
agents/langgraph/react_with_database_memory/Makefile (1)
20-25:⚠️ Potential issue | 🟠 Major
make envcan fail on a fresh checkout after droppinginit.Line 25 still unconditionally runs
source .env; if.envis missing,make envnow fails. This is a regression from the previousenv: initbehavior.Proposed fix (keep standardization, avoid first-run failure)
env: ## Create venv, load .env, and install dependencies `@echo` "==> Creating virtual environment and installing dependencies..." && \ rm -rf .venv && \ uv sync --python 3.12 && \ + if [ ! -f .env ]; then cp .env.example .env; fi && \ echo "==> Load .env variables..." && \ source .venv/bin/activate && set -a && source .env && set +a && \ echo "" && \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@agents/langgraph/react_with_database_memory/Makefile` around lines 20 - 25, The env Makefile target unconditionally runs "source .env" which causes make to fail on a fresh checkout if .env is missing; update the env recipe (the env target) to only source .env when the file exists (e.g., guard with a file-existence check before running source .env) while preserving the set -a / set +a behavior so environment variables are exported when .env is present and the recipe still succeeds when it's absent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@agents/google/adk/main.py`:
- Around line 414-418: Split liveness and readiness checks so liveness never
fails on startup: keep the current readiness logic that inspects runner
(initialized = runner is not None) and returns JSONResponse(status_code=503,
content=body) when not initialized on a dedicated readiness endpoint (e.g.,
/ready or keep /health as readiness), and add a separate liveness endpoint
(e.g., /live) that always returns a 200 OK body (or returns {"status":"alive"})
regardless of runner state; update the route handlers in main.py to use the
initialized variable and JSONResponse for readiness only and ensure the liveness
handler always returns a simple 200 response so probes do not trigger restart
loops.
---
Outside diff comments:
In `@agents/langgraph/react_with_database_memory/Makefile`:
- Around line 20-25: The env Makefile target unconditionally runs "source .env"
which causes make to fail on a fresh checkout if .env is missing; update the env
recipe (the env target) to only source .env when the file exists (e.g., guard
with a file-existence check before running source .env) while preserving the set
-a / set +a behavior so environment variables are exported when .env is present
and the recipe still succeeds when it's absent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 31e7c543-5bc9-45d3-9620-b44539ad2feb
📒 Files selected for processing (23)
agents/autogen/mcp_agent/mcp_automl_template/pyproject.tomlagents/autogen/mcp_agent/pyproject.tomlagents/crewai/websearch_agent/Makefileagents/crewai/websearch_agent/main.pyagents/google/adk/Makefileagents/google/adk/main.pyagents/google/adk/pyproject.tomlagents/langgraph/agentic_rag/Makefileagents/langgraph/agentic_rag/main.pyagents/langgraph/agentic_rag/pyproject.tomlagents/langgraph/human_in_the_loop/Makefileagents/langgraph/human_in_the_loop/main.pyagents/langgraph/react_agent/Makefileagents/langgraph/react_agent/main.pyagents/langgraph/react_agent/pyproject.tomlagents/langgraph/react_with_database_memory/Makefileagents/langgraph/react_with_database_memory/main.pyagents/langgraph/react_with_database_memory/pyproject.tomlagents/llamaindex/websearch_agent/Makefileagents/llamaindex/websearch_agent/main.pyagents/vanilla_python/openai_responses_agent/Makefileagents/vanilla_python/openai_responses_agent/main.pyagents/vanilla_python/openai_responses_agent/pyproject.toml
- Return HTTP 503 from /health when agent is uninitialized so K8s readiness probes correctly gate traffic (8 agents, audit #5) - Bind local dev servers to 127.0.0.1 to prevent DNS rebinding attacks; container Dockerfiles remain at 0.0.0.0 (audit #36) - Standardize FastAPI to >=0.135.1 across all agents (audit #33) - Bump uvicorn in mcp_automl_template to >=0.41.0 (audit #34) - Remove env:init dependency in react_with_database_memory (audit red-hat-data-services#46) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
10f043c to
3e252c7
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
agents/langgraph/react_with_database_memory/Makefile (1)
20-25:⚠️ Potential issue | 🟠 Major
envtarget can fail on fresh setup after removinginitdependencyLine 25 unconditionally runs
source .env, but Line 20 no longer depends oninit. On a fresh clone without.env,make envwill fail before completing setup.Suggested fix
-env: ## Create venv, load .env, and install dependencies +env: init ## Create venv, load .env, and install dependencies `@echo` "==> Creating virtual environment and installing dependencies..." && \ rm -rf .venv && \ uv sync --python 3.12 && \ echo "==> Load .env variables..." && \ source .venv/bin/activate && set -a && source .env && set +a && \Or, if you want to keep
envindependent, add a guard/copy step before sourcing.env.As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@agents/langgraph/react_with_database_memory/Makefile` around lines 20 - 25, The Makefile env target unconditionally sources .env which breaks on a fresh clone since env no longer depends on init; update the env target (symbol: env) to guard against missing .env by either adding the init dependency back or inserting a conditional/guard before the source .env step (e.g., check for .env existence and copy a .env.example or skip sourcing if absent), ensuring the rest of the commands (uv sync, .venv activation) still run when .env is not present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@agents/langgraph/react_with_database_memory/Makefile`:
- Around line 20-25: The Makefile env target unconditionally sources .env which
breaks on a fresh clone since env no longer depends on init; update the env
target (symbol: env) to guard against missing .env by either adding the init
dependency back or inserting a conditional/guard before the source .env step
(e.g., check for .env existence and copy a .env.example or skip sourcing if
absent), ensuring the rest of the commands (uv sync, .venv activation) still run
when .env is not present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 31f3a765-7fe4-4f3a-9a27-d94dcc29f5c9
📒 Files selected for processing (23)
agents/autogen/mcp_agent/mcp_automl_template/pyproject.tomlagents/autogen/mcp_agent/pyproject.tomlagents/crewai/websearch_agent/Makefileagents/crewai/websearch_agent/main.pyagents/google/adk/Makefileagents/google/adk/main.pyagents/google/adk/pyproject.tomlagents/langgraph/agentic_rag/Makefileagents/langgraph/agentic_rag/main.pyagents/langgraph/agentic_rag/pyproject.tomlagents/langgraph/human_in_the_loop/Makefileagents/langgraph/human_in_the_loop/main.pyagents/langgraph/react_agent/Makefileagents/langgraph/react_agent/main.pyagents/langgraph/react_agent/pyproject.tomlagents/langgraph/react_with_database_memory/Makefileagents/langgraph/react_with_database_memory/main.pyagents/langgraph/react_with_database_memory/pyproject.tomlagents/llamaindex/websearch_agent/Makefileagents/llamaindex/websearch_agent/main.pyagents/vanilla_python/openai_responses_agent/Makefileagents/vanilla_python/openai_responses_agent/main.pyagents/vanilla_python/openai_responses_agent/pyproject.toml
✅ Files skipped from review due to trivial changes (9)
- agents/vanilla_python/openai_responses_agent/pyproject.toml
- agents/autogen/mcp_agent/mcp_automl_template/pyproject.toml
- agents/langgraph/react_with_database_memory/pyproject.toml
- agents/autogen/mcp_agent/pyproject.toml
- agents/langgraph/react_agent/pyproject.toml
- agents/google/adk/pyproject.toml
- agents/langgraph/human_in_the_loop/Makefile
- agents/langgraph/agentic_rag/pyproject.toml
- agents/langgraph/human_in_the_loop/main.py
🚧 Files skipped from review as they are similar to previous changes (12)
- agents/google/adk/main.py
- agents/langgraph/agentic_rag/main.py
- agents/crewai/websearch_agent/Makefile
- agents/vanilla_python/openai_responses_agent/Makefile
- agents/google/adk/Makefile
- agents/langgraph/react_agent/Makefile
- agents/langgraph/react_with_database_memory/main.py
- agents/vanilla_python/openai_responses_agent/main.py
- agents/llamaindex/websearch_agent/Makefile
- agents/llamaindex/websearch_agent/main.py
- agents/crewai/websearch_agent/main.py
- agents/langgraph/agentic_rag/Makefile
|
Looks good @tarun-etikala |
Summary
--host 127.0.0.1inrun-app/run-app-freshtargets. Container Dockerfiles remain at0.0.0.0for K8s networking.>=0.135.1— 6 agents bumped from>=0.129.0or>=0.132.0.>=0.41.0— from>=0.30.0.env: initdependency inreact_with_database_memory/Makefile— aligns with all other agents whereenvis standalone.Files changed (23 + 1 doc)
main.pyJSONResponseimport, return 503 when not initializedMakefile--host 0.0.0.0→--host 127.0.0.1(run-app + run-app-fresh)pyproject.toml>=0.129.0/>=0.132.0→>=0.135.1pyproject.toml>=0.30.0→>=0.41.0(mcp_automl_template)Makefileenv: init→env:(react_with_database_memory)mdTest plan
curl localhost:8000/healthbefore agent init returns HTTP 503 with{"status": "not_ready", ...}curl localhost:8000/healthafter agent init returns HTTP 200 with{"status": "healthy", ...}make run-appbinds to127.0.0.1(not reachable from other hosts)