feat: Centralize RunPod HTTP authentication for all client types#133
Conversation
Centralizes HTTP client creation for RunPod load-balanced endpoints to prevent manual Authorization header code duplication and ensure consistent authentication: 1. Create centralized HTTP utility function (src/tetra_rp/core/utils/http.py) - New function: get_authenticated_httpx_client() - Automatically adds Bearer token Authorization header if RUNPOD_API_KEY set - Provides consistent timeout handling (default 30s, customizable) - Follows existing GraphQL/REST client authentication pattern 2. Fix critical authentication bug in LoadBalancerSlsStub._execute_via_user_route() - Previously: Missing Authorization header (401 errors on user routes) - Now: Uses centralized utility for proper authentication - Enables direct HTTP calls to user-defined routes with auth 3. Refactor two methods to use centralized utility - LoadBalancerSlsStub._execute_function() - removes 7+ lines of manual auth code - LoadBalancerSlsResource._check_ping_endpoint() - simplifies auth setup 4. Add comprehensive unit tests (tests/unit/core/utils/test_http.py) - Tests API key presence/absence handling - Tests custom and default timeout configuration - Tests edge cases (empty key, zero timeout) - All 7 tests pass with 100% coverage Results: - Single source of truth for HTTP authentication (centralized utility) - Fixes 401 Unauthorized errors on load-balanced endpoints - Eliminates repetitive manual auth code across 3+ locations - Easier to maintain and update authentication patterns in future - All 499 unit tests pass - Code coverage: 64% (exceeds 35% requirement)
…tication Extends the centralized HTTP authentication pattern to all RunPod API calls: 1. Add get_authenticated_requests_session() for synchronous requests - Creates requests.Session with automatic Bearer token Authorization header - Follows same pattern as async get_authenticated_httpx_client() - Single source of truth for sync HTTP authentication 2. Refactor template.py to use centralized utility - Removes manual Authorization header setup (line 86) - Now uses get_authenticated_requests_session() for all template updates - Improves error handling with raise_for_status() - Token parameter marked deprecated; uses RUNPOD_API_KEY env var 3. Add comprehensive tests for sync utility (4 tests) - Tests API key presence/absence handling - Tests empty API key edge case - Tests Session object validation - All tests pass with proper cleanup Benefits: - True single source of truth for all RunPod HTTP authentication (sync + async) - Consistent patterns across entire codebase - Easier future auth changes across all HTTP client types - Eliminates manual auth header code in template.py - All 503 unit tests pass - Code coverage: 64% (exceeds 35% requirement) Note: requests.Session doesn't support default timeouts; timeout should be specified per request (e.g., session.post(url, json=data, timeout=30.0))
deanq
changed the base branch from
main
to
deanq/ae-1102-load-balancer-sls-resource
There was a problem hiding this comment.
Pull request overview
This PR centralizes HTTP authentication for RunPod API calls by introducing two utility functions that automatically inject Bearer token headers. The change eliminates code duplication across synchronous (requests) and asynchronous (httpx) HTTP clients and fixes a critical 401 Unauthorized bug in load-balanced endpoint user routes.
Key changes:
- Created centralized authentication utilities for both sync and async HTTP clients
- Refactored existing code to use these utilities instead of manual header setup
- Fixed missing authentication in load-balanced endpoint user route execution
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tetra_rp/core/utils/http.py | New utility module providing centralized authentication for httpx and requests clients |
| src/tetra_rp/core/resources/template.py | Refactored to use centralized auth session instead of manual header construction |
| src/tetra_rp/core/resources/load_balancer_sls_resource.py | Updated to use authenticated httpx client utility for health checks |
| src/tetra_rp/stubs/load_balancer_sls.py | Updated both execute methods to use authenticated httpx client |
| tests/unit/core/utils/test_http.py | Comprehensive test coverage for new authentication utilities |
| tests/unit/test_load_balancer_sls_resource.py | Updated test mocks to patch httpx client at correct import location |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| def update_system_dependencies( | ||
| template_id, token, system_dependencies, base_entry_cmd=None | ||
| template_id, system_dependencies, base_entry_cmd=None, token=None |
There was a problem hiding this comment.
The function signature introduces a breaking change by reordering parameters. The token parameter was previously in position 2 but is now in position 4 (after base_entry_cmd). This will break any existing calls that use positional arguments like update_system_dependencies(template_id, token, deps). Consider deprecating this function and creating a new one, or keeping the original parameter order while adding a deprecation warning for the token parameter.
| response = requests.post(url, json=payload, headers=headers) | ||
|
|
||
| # Use centralized auth utility instead of manual header setup | ||
| # Note: token parameter is deprecated; uses RUNPOD_API_KEY environment variable |
There was a problem hiding this comment.
The comment states that the token parameter is deprecated, but there is no deprecation warning raised when it's used. Consider adding a warnings.warn() call if the token parameter is provided to properly communicate the deprecation to users.
| patch( | ||
| "tetra_rp.core.resources.load_balancer_sls_resource.httpx.AsyncClient" | ||
| ) as mock_client, | ||
| "tetra_rp.core.resources.load_balancer_sls_resource.get_authenticated_httpx_client", |
There was a problem hiding this comment.
Inconsistent patching approach: this test patches get_authenticated_httpx_client directly while the two tests above patch httpx.AsyncClient. This inconsistency makes the test suite harder to maintain. All three tests should use the same patching strategy for consistency.
deanq
merged commit 8b97197
into
deanq/ae-1102-load-balancer-sls-resource
Prerequisite: #131
Summary
Standardizes HTTP authentication across the entire codebase by creating centralized utilities for both synchronous and asynchronous RunPod API calls. Eliminates manual Authorization header duplication and fixes a critical 401 Unauthorized bug in load-balanced endpoints.
What's Changed
1. Synchronous HTTP Utility
get_authenticated_requests_session()insrc/tetra_rp/core/utils/http.pyRUNPOD_API_KEY2. Asynchronous HTTP Utility
get_authenticated_httpx_client()insrc/tetra_rp/core/utils/http.pyRUNPOD_API_KEY3. Bug Fixes
LoadBalancerSlsStub._execute_via_user_route()by adding missing Authorization headerLoadBalancerSlsStub._execute_function()andLoadBalancerSlsResource._check_ping_endpoint()4. Template Refactor
template.pyto use centralizedget_authenticated_requests_session()raise_for_status()Benefits
Files Modified
src/tetra_rp/core/utils/http.py- New centralized utilities (2 functions)src/tetra_rp/core/resources/template.py- Refactored to use sync utilitysrc/tetra_rp/core/resources/load_balancer_sls_resource.py- Uses async utilitysrc/tetra_rp/stubs/load_balancer_sls.py- Uses async utility for user routestests/unit/core/utils/test_http.py- New comprehensive teststests/unit/test_load_balancer_sls_resource.py- Updated mocks for httpx