fix(depends): allow a CPU-only dependency resolve, opt-in - #1708
Conversation
🤖 Internal: Discord sync markerAuto-managed by the Discord notification workflow. Stores the linked Discord message ID and forum thread ID. Do not edit or delete. |
github-actions
Bot
added
module:server
📝 WalkthroughWalkthroughDependency resolution now supports an environment-controlled CPU-only mode that adds CPU PyTorch wheel directives and adjusts non-macOS ONNX Runtime exclusions. ChangesCPU-only dependency resolution
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nodes/src/nodes/tool_google_workspace/gmail/IInstance.py`:
- Line 149: The Gmail connection check in the IInstance implementation currently
probes users.getProfile unconditionally, which fails for send-only
configurations. Update the _check_connection_impl call and its probe logic to
use an API permitted by the configured access tier, or return an unknown
diagnostic for gmail.send-only access while preserving the existing profile
probe for broader scopes.
In `@nodes/src/nodes/tool_google_workspace/IInstance.py`:
- Around line 103-114: Update the API probe block in execute() so
out['checked'].append('api') occurs before calling probe(service). Preserve the
existing exception handling and error/errorReason extraction, ensuring failed
probes still record the API check while reporting the raised diagnostic.
In `@packages/server/engine-lib/rocketlib-python/lib/depends.py`:
- Around line 691-704: The ensure_constraints() cache key currently hashes only
requirement files, so changes to _cpu_only() can reuse stale CPU/GPU
constraints. Include the normalized CPU-only state in that key, and add
regression coverage for both GPU-to-CPU and CPU-to-GPU transitions while
preserving the existing cache behavior for unchanged inputs.
- Around line 699-704: Update the CPU-only dependency output in the code
generating this block so PyTorch packages resolve exclusively from the CPU index
rather than competing with PyPI candidates under unsafe-best-match. Use the
existing ROCKETRIDE_CPU_ONLY_DEPS path and explicitly constrain torch and
related PyTorch packages to the CPU index, while preserving the current non-CPU
dependency behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c97b7df2-2ccd-401a-be92-2b58a1c0d17d
📒 Files selected for processing (4)
nodes/src/nodes/tool_google_workspace/IInstance.pynodes/src/nodes/tool_google_workspace/drive/IInstance.pynodes/src/nodes/tool_google_workspace/gmail/IInstance.pypackages/server/engine-lib/rocketlib-python/lib/depends.py
| def check_connection(self, args: dict) -> dict: | ||
| """Check Gmail connection status and whether granted OAuth scopes cover the configured access tier. Read-only.""" | ||
| return self._check_connection_impl() | ||
| return self._check_connection_impl(probe=lambda s: execute(s.users().getProfile(userId='me'))) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'IInstance.py' . || true
echo "== target file excerpt =="
file="$(fd 'IInstance.py' . | head -n1 || true)"
if [ -n "${file:-}" ]; then
wc -l "$file"
sed -n '1,220p' "$file" | cat -n
fi
echo "== search for Gmail probe/send requirements =="
rg -n "gmail|getProfile|_require_send|connection_ok|probe=lambda|require_send" . --glob '*.py' || trueRepository: rocketride-org/rocketride-server
Length of output: 37844
🌐 Web query:
Gmail API users.getProfile required OAuth scopes gmail.send gmail.compose gmail.modify gmail.readonly gmail.metadata
💡 Result:
The Gmail API method users.getProfile requires at least one of the following OAuth 2.0 scopes [1][2]: - https://mail.google.com/ - https://www.googleapis.com/auth/gmail.modify - https://www.googleapis.com/auth/gmail.compose - https://www.googleapis.com/auth/gmail.readonly - https://www.googleapis.com/auth/gmail.metadata You do not need to request all of these scopes simultaneously. To adhere to the principle of least privilege, you should select the narrowest scope that meets your application's requirements [3]. If your application only needs to read profile information (such as the user's email address and message counts), the gmail.metadata or gmail.readonly scope is typically sufficient [1][3].
Citations:
- 1: https://developers.google.com/workspace/gmail/api/reference/rest/v1/users/getProfile
- 2: https://developers.google.com/gmail/api/reference/rest/v1/users/getProfile
- 3: https://developers.google.com/workspace/gmail/api/auth/scopes
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='nodes/src/nodes/tool_google_workspace/gmail/IInstance.py'
echo "== target excerpt 70-155 =="
sed -n '70,155p' "$file" | cat -n
echo "== target excerpt 590-630 =="
sed -n '590,630p' "$file" | cat -n
echo "== implementation helpers in parent/base if applicable =="
base='nodes/src/nodes/tool_google_workspace/IInstance.py'
sed -n '70,145p' "$base" | cat -n
echo "== google access scope definitions =="
sed -n '120,175p' nodes/src/nodes/core/google_access.py | cat -nRepository: rocketride-org/rocketride-server
Length of output: 14351
Do not probe Gmail with users.getProfile for send-only connections.
users.getProfile requires mail.google.com, gmail.modify, gmail.compose, gmail.readonly, or gmail.metadata; gmail.send alone is not accepted. Since _require_send only checks for gmail.send or full access, a send-only config can satisfy the required scopes for operations but still report connection_ok: false from the diagnostics probe. Use a probe allowed by the configured access tier, or return unknown for send-only checks instead of running this unsupported API call.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@nodes/src/nodes/tool_google_workspace/gmail/IInstance.py` at line 149, The
Gmail connection check in the IInstance implementation currently probes
users.getProfile unconditionally, which fails for send-only configurations.
Update the _check_connection_impl call and its probe logic to use an API
permitted by the configured access tier, or return an unknown diagnostic for
gmail.send-only access while preserving the existing profile probe for broader
scopes.
| try: | ||
| probe(service) | ||
| out['checked'].append('api') | ||
| except Exception as exc: | ||
| out['connection_ok'] = False | ||
| out['error'] = str(exc)[:200] | ||
| # Google distinguishes accessNotConfigured (API disabled) from | ||
| # forbidden (permission) from rateLimitExceeded, and that word | ||
| # alone names the fix. Losing it costs the reader the diagnosis. | ||
| reason = getattr(exc, 'reason', None) or getattr(exc, '_get_reason', lambda: None)() | ||
| if reason: | ||
| out['errorReason'] = str(reason).strip()[:120] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'IInstance\.py$' . || true
echo "== file exists? =="
if [ -f nodes/src/nodes/tool_google_workspace/IInstance.py ]; then
wc -l nodes/src/nodes/tool_google_workspace/IInstance.py
echo "== relevant section =="
sed -n '1,180p' nodes/src/nodes/tool_google_workspace/IInstance.py | cat -n
fi
echo "== searches for google_client and probe =="
rg -n "google_client|execute\(|probe\(|checked|errorReason|_get_reason|reason" nodes/src/nodes/tool_google_workspace IInstance.py . --glob '*.py' || trueRepository: rocketride-org/rocketride-server
Length of output: 50389
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== google_client execute implementation =="
sed -n '380,435p' nodes/src/nodes/tool_google_workspace/google_client.py | cat -n
echo "== google_client imports and _is_rate_limit_403 imports =="
sed -n '1,80p' nodes/src/nodes/tool_google_workspace/google_client.py | cat -n
echo "== Python probe-except behavior from source =="
python3 - <<'PY'
import ast
from pathlib import Path
p = Path('nodes/src/nodes/tool_google_workspace/google_client.py')
tree = ast.parse(p.read_text())
for node in tree.body:
if isinstance(node, ast.FunctionDef) and node.name == 'execute':
print(node.name, node.lineno, 'to', node.end_lineno)
for stmt in node.body:
print(' ', ast.get_source_segment(p.read_text(), stmt))
PY
echo "== behavioral probe: current source pattern =="
python3 - <<'PY'
def current_probe_impl():
class Original(Exception):
reason = 'apiDisabled'
def _get_reason(self):
return 'neverCalled'
def probe(service):
raise Original
out = {}
try:
probe(service={})
out['checked'].append('api')
except Exception as exc:
out['connection_ok'] = False
out['error'] = str(exc)[:200]
reason = getattr(exc, 'reason', None) or getattr(exc, '_get_reason', lambda: None)()
if reason:
out['errorReason'] = str(reason).strip()[:120]
return out
def proposed_probe_impl():
class Original(Exception):
reason = 'apiDisabled'
def _get_reason(self):
return 'neverCalled'
def probe(service):
raise Original
exc = None
try:
raise RuntimeError('probe ran')
except Exception as e:
exc = e
out = {}
try:
out['checked'].append('api')
raise exc
except Exception as exc2:
out['connection_ok'] = False
out['error'] = str(exc2)[:200]
cause = getattr(exc2, '__cause__', None) or exc2
# simulate current cause being the original wrapper is gone from exc2
# by keeping exc as the preserved exc.__cause__ is None / exc2 itself here.
reason = getattr(cause, 'reason', None)
if not reason:
get_reason = getattr(cause, '_get_reason', None)
reason = get_reason() if callable(get_reason) else None
if reason:
out['errorReason'] = str(reason).strip()[:120]
return out
print('current', current_probe_impl())
# Build a wrapper around the exact original so exc.__cause__ is the Original exception.
orig = Original = type('Original', (Exception,), {'reason': 'apiDisabled', '_get_reason': lambda self: 'neverCalled'})
try:
try:
raise orig
except Exception as inner:
raise ValueError('wrapper') from inner
except Exception as exc:
print('wrapper cause present:', exc.__cause__ is not None and getattr(exc.__cause__, 'reason', None))
PYRepository: rocketride-org/rocketride-server
Length of output: 9089
Record the API probe result before checking it fails.
execute() wraps Google API HttpErrors in ValueError with the original exception available as __cause__, but also includes the diagnostic detail like accessNotConfigured/rate-limit reasons in the raised message. If probe(service) raises, checked currently stays at ['client', 'scopes'], making a failed live request look unprobed. Call probe only after appending "api" to checked.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@nodes/src/nodes/tool_google_workspace/IInstance.py` around lines 103 - 114,
Update the API probe block in execute() so out['checked'].append('api') occurs
before calling probe(service). Preserve the existing exception handling and
error/errorReason extraction, ensuring failed probes still record the API check
while reporting the raised diagnostic.
The cloud ALB carries 6.6 GB of CUDA it cannot execute — nvidia 4.3G, torch 1.7G, triton 639M, which is 80% of its 8.2 GB site-packages. The pod runs on an m7i-flex.large with no GPU capacity and no /dev/nvidia*, and it holds no models at all: inference goes out to the H100 behind model.rocketride.dev. That runtime is downloaded and written on every pod start and has never once been executed. It is the root of the DiskPressure incidents (#1697). Adds ROCKETRIDE_CPU_ONLY_DEPS. When set, the compile input gains `--extra-index-url https://download.pytorch.org/whl/cpu`, so torch resolves to +cpu (~200 MB) and the nvidia-* packages are never selected. THREE things had to line up, and I only knew about two when I filed the issue: 1. torch — no node declares it; it arrives transitively through gliner / transformers / accelerate. PyPI's default wheel is the CUDA build. 2. anonymize declares `onnxruntime-gpu; platform_system != 'Darwin'`. 3. THIS FILE excluded plain `onnxruntime` on every non-Darwin host, which would have defeated a fix to (2) — the CPU build could not be installed even if something asked for it. Found while implementing, not while diagnosing. (3) is the interesting one: "not a Mac" was being used as a proxy for "has CUDA", and that is wrong on every Linux box without a GPU. The exclusion now honours the flag, and on a CPU-only host it excludes the GPU build instead — same clobbering logic, correct direction. Opt-in, NOT auto-detected. Inferring "no GPU visible, therefore CPU" would silently downgrade a host whose driver merely has not loaded yet, and a silent downgrade of an inference machine is far worse than an oversized install. The cloud sets the flag; self-hosted users with a GPU are untouched, which is the whole point — this must not become a hardcoded CPU default in node requirements. VERIFICATION, stated precisely: * PROVEN — the resolve. `uv pip compile` over the four nodes that pull torch, targeting x86_64-manylinux2014/cp312: 119 packages / 13 nvidia-* with the default index, 105 / 0 with this exact flag string. No conflicts. * PROVEN — the emitted line is byte-identical to the flag I resolved with. * NOT PROVEN — that _combine_requirements() runs correctly here. The module imports engine dependencies I do not have locally, so both branches of the flag are unexercised; I read the diff rather than ran it. * NOT PROVEN — that the four nodes still work on torch+cpu. torch+cpu raises when CUDA is actually requested, not at import, so a green resolve is not evidence. This needs a functional run before it is enabled anywhere. Do not set the flag in the cloud until that functional run is done. The measured saving (8.2 GB -> ~1.8 GB) is worth having, but not worth guessing at. Refs #1697.
24b8f58 to
ae0b3f0
Compare
The functional verification this was waiting on — doneThe PR body says a clean dependency resolve is not proof, because Environment: fresh venv, the four nodes' combined requirements installed through
Executed, not imported: The gliner line is the one that matters most — Also checked in the node sources rather than assumed: none of the four hardcodes a device. No One honest caveat on my own test: the first run reported a Separately: this branch had a duplicate of #1707 on it, now removedThe branch carried That mattered: today I found and fixed three test failures that change causes, including a real defect (drive gained an Rebased onto develop dropping that commit. This PR is now one commit, one file: |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/server/engine-lib/rocketlib-python/lib/depends.py (2)
677-704: 🎯 Functional Correctness | 🟠 MajorInclude CPU-only mode in the constraints cache key.
_cpu_only()changes the generated requirements, butensure_constraints()hashes onlyreq_files. TogglingROCKETRIDE_CPU_ONLY_DEPScan therefore reuse stale CPU/GPU constraints and install the wrong dependency set. Include the normalized CPU-only state in the cache key and cover both mode transitions with a regression test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server/engine-lib/rocketlib-python/lib/depends.py` around lines 677 - 704, The constraints cache key in ensure_constraints must incorporate the normalized state returned by _cpu_only(), since that state changes generated requirements. Update the hash inputs so CPU-only and GPU/default modes produce distinct cache entries, and add a regression test covering both transitions to ensure stale constraints are not reused.
691-704: 🎯 Functional Correctness | 🟠 MajorMake CPU PyTorch resolution exclusive.
The new
--extra-index-urldirective still competes with PyPI because_install_dry_run()usesunsafe-best-match. A regular CUDAtorchcandidate can therefore win and reintroducenvidia-*packages. Use an index strategy or explicit package-to-index directives that force PyTorch packages to the CPU index, then verify clean resolutions in both modes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server/engine-lib/rocketlib-python/lib/depends.py` around lines 691 - 704, The CPU-only branch in _install_dry_run must force PyTorch packages to resolve exclusively from the CPU index rather than merely adding it as an extra source. Update the directive emitted by the _cpu_only() path to use the supported index strategy or explicit torch package-to-index rules, while preserving normal PyPI resolution for unrelated packages and verifying clean CPU-only and default resolutions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/server/engine-lib/rocketlib-python/lib/depends.py`:
- Around line 810-820: Update the onnxruntime-gpu exclusion branch in
_write_excludes_file to remain gated by platform.system() != 'Darwin', matching
the documented non-Darwin scope; preserve the existing CPU-only behavior on
non-Darwin hosts.
---
Duplicate comments:
In `@packages/server/engine-lib/rocketlib-python/lib/depends.py`:
- Around line 677-704: The constraints cache key in ensure_constraints must
incorporate the normalized state returned by _cpu_only(), since that state
changes generated requirements. Update the hash inputs so CPU-only and
GPU/default modes produce distinct cache entries, and add a regression test
covering both transitions to ensure stale constraints are not reused.
- Around line 691-704: The CPU-only branch in _install_dry_run must force
PyTorch packages to resolve exclusively from the CPU index rather than merely
adding it as an extra source. Update the directive emitted by the _cpu_only()
path to use the supported index strategy or explicit torch package-to-index
rules, while preserving normal PyPI resolution for unrelated packages and
verifying clean CPU-only and default resolutions.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f5a66e1e-71e1-4288-bc83-1490113aa57c
📒 Files selected for processing (1)
packages/server/engine-lib/rocketlib-python/lib/depends.py
| # The onnxruntime exclusion keys on OS, which is not the question being | ||
| # asked. "Not a Mac" was used as a proxy for "has CUDA", and it is wrong on | ||
| # every Linux box without a GPU — including the cloud ALB, where it forces | ||
| # the GPU build and blocks the CPU one from ever being installed. Excluding | ||
| # it there would defeat the CPU-only resolve above, so honour that flag. | ||
| if platform.system() != 'Darwin' and not _cpu_only(): | ||
| excludes += 'onnxruntime\n' | ||
| elif _cpu_only(): | ||
| # Mirror image: on a CPU-only host the GPU build is the one that must | ||
| # not win, for the same clobbering reason the original comment gives. | ||
| excludes += 'onnxruntime-gpu\n' |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and inspect the relevant function and surrounding documentation/tests.
sed -n '760,840p' packages/server/engine-lib/rocketlib-python/lib/depends.py
printf '\n--- _cpu_only definition/usages ---\n'
rg -n "def _cpu_only|_cpu_only\(|cpu_only|onnxruntime-gpu|excludes" packages/server/engine-lib/rocketlib-python/lib/depends.py
printf '\n--- related docs/comments ---\n'
rg -n -C 3 "CUDA|cpu|Darwin|onnxruntime-gpu|onnxruntime" packages/server/engine-lib/rocketlib-python/lib/depends.py packages/server/engine-lib -g '*.py' -g '*.md' | head -n 200Repository: rocketride-org/rocketride-server
Length of output: 15816
Keep the CPU-only exclusion OS-gated as documented.
_write_excludes_file() says the onnxruntime exclusions apply to non-Darwin hosts, but elif _cpu_only() excludes onnxruntime-gpu on Darwin too. If macOS CPU-only hosts may require the GPU build, either gate this branch with platform.system() != 'Darwin' or update the docstring/comments to make macOS behavior an explicit contract.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 820-820: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(excludes_path, 'w', encoding='utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/server/engine-lib/rocketlib-python/lib/depends.py` around lines 810
- 820, Update the onnxruntime-gpu exclusion branch in _write_excludes_file to
remain gated by platform.system() != 'Darwin', matching the documented
non-Darwin scope; preserve the existing CPU-only behavior on non-Darwin hosts.
1 participant
The cloud ALB carries 6.6 GB of CUDA it cannot execute —
nvidia4.3G,torch1.7G,triton639M: 80% of its 8.2 GB site-packages. The pod is anm7i-flex.largewith no GPU capacity and no/dev/nvidia*, and it holds no models — inference goes out to the H100. That runtime is downloaded and written on every pod start and has never once run. It is the root of the DiskPressure incidents (#1697).Adds
ROCKETRIDE_CPU_ONLY_DEPS. When set, the compile input gains--extra-index-url https://download.pytorch.org/whl/cpu, sotorchresolves to+cpu(~200 MB) and thenvidia-*packages are never selected.Three things had to line up — I knew about two
torch— no node declares it; it arrives transitively via gliner / transformers / accelerate, and PyPIs default wheel is the CUDA build.anonymizedeclaresonnxruntime-gpu; platform_system != Darwin.onnxruntimeon every non-Darwin host — which would have defeated a fix to (2) outright. The CPU build could not be installed even if something asked for it.(3) I found while implementing, not while diagnosing, and it is the one worth reviewing:
"not a Mac"was being used as a proxy for"has CUDA", which is wrong on every Linux box without a GPU. The exclusion now honours the flag, and on a CPU-only host excludes the GPU build instead — same clobbering logic, correct direction.Opt-in, deliberately not auto-detected
Inferring "no GPU visible, therefore CPU" would silently downgrade a host whose driver simply has not loaded yet. A silent downgrade of an inference machine is far worse than an oversized install. The cloud sets the flag; self-hosted users with a GPU are untouched — which is the point, and why this is not a hardcoded CPU default in node requirements.
Verification — what is proven and what is not
Proven
uv pip compileover the four torch-pulling nodes, targetingx86_64-manylinux2014/cp312 — 119 packages / 13nvidia-*on the default index, 105 / 0 with this exact flag. No conflicts.Not proven
_combine_requirements()runs correctly here. The module imports engine dependencies I dont have locally, so both branches of the flag are unexercised — I read the diff rather than ran it.torch+cpu.torch+cpuraises when CUDA is actually requested, not at import, so a green resolve is not evidence.Do not set the flag in the cloud until that functional run is done. The saving (8.2 GB → ~1.8 GB, node 84% → ~15-20%) is worth having, and not worth guessing at.
Refs #1697.
Summary by CodeRabbit