Fail fast on missing inference credentials - #1163
Conversation
Check endpoint credentials before full mode starts Isaac Sim so configuration errors avoid GPU startup and unclean simulator teardown. Signed-off-by: Qian Lin <qianl@nvidia.com>
Greptile SummaryThe PR centralizes endpoint credential resolution and adds a credential preflight before full-mode Isaac Sim startup.
Confidence Score: 4/5The PR appears safe to merge, with a non-blocking robustness issue when the CLI runs under optimized Python. The normal CLI path correctly validates credentials before simulator startup, but optimized execution strips the assertion and allows the expensive startup to proceed before authentication fails downstream. Files Needing Attention: isaaclab_arena/agentic_environment_generation/inference_backend.py Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Start full-mode CLI] --> B[Resolve endpoint and API key]
B --> C{Credential present?}
C -- No --> D[Exit before simulator startup]
C -- Yes --> E[Enter SimulationAppContext]
E --> F[Construct inference backend]
F --> G[Generate and build environment]
Reviews (1): Last reviewed commit: "Fail fast on missing inference credentia..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| assert resolved_api_key, ( | ||
| f"API key required for the {inference_endpoint.name!r} inference endpoint: set " | ||
| f"{inference_endpoint.api_key_env_var} or pass api_key. Select another endpoint with " | ||
| f"{INFERENCE_ENDPOINT_ENV_VAR}." | ||
| ) |
There was a problem hiding this comment.
If the CLI runs under python -O or with PYTHONOPTIMIZE enabled, Python removes this assertion, so full mode enters SimulationAppContext with no API key and fails during downstream client authentication after Isaac Sim has started.
| assert resolved_api_key, ( | |
| f"API key required for the {inference_endpoint.name!r} inference endpoint: set " | |
| f"{inference_endpoint.api_key_env_var} or pass api_key. Select another endpoint with " | |
| f"{INFERENCE_ENDPOINT_ENV_VAR}." | |
| ) | |
| if not resolved_api_key: | |
| raise AssertionError( | |
| f"API key required for the {inference_endpoint.name!r} inference endpoint: set " | |
| f"{inference_endpoint.api_key_env_var} or pass api_key. Select another endpoint with " | |
| f"{INFERENCE_ENDPOINT_ENV_VAR}." | |
| ) |
Knowledge Base Used:
![arena-review-bot[bot]](https://avatars.githubusercontent.com/u/195794629?s=60&v=4){kind=small}
| return 0 | ||
|
|
||
| # Validate inference endpoint credentials before full mode starts Isaac Sim. | ||
| require_inference_api_key(args_cli.inference_endpoint) |
There was a problem hiding this comment.
🟡 Could full mode just resolve before starting Isaac Sim?
A missing key is only one of the resolve-time failures that currently pays for a full Isaac Sim startup — five lines below, an invalid agent spec also bails out with return 1 after the sim is already up. Since --mode resolve runs resolve_env_spec with no sim at all, could full mode do the same and enter SimulationAppContext only once it has a spec path? That would cover every resolve failure and this preflight (and the new helper's second caller) could go away:
env_graph_spec_path = resolve_env_spec(args_cli)
if env_graph_spec_path is None:
return 1
with SimulationAppContext(args_cli):
build_env_and_run_policy(env_graph_spec_path, args_cli)
return 0Or is there an import-ordering reason the resolve step has to run inside the context here?
🤖 Isaac Lab-Arena Review BotSummaryExtracts the endpoint + API-key resolution out of Design, Boundaries & Scope
Findings🟡 Warning: Test CoverageThree new unit tests on VerdictMinor fixes needed |
1 participant
Summary
Validate inference credentials before simulation
Detailed description
Fix for v0.3 VDR