-
Notifications
You must be signed in to change notification settings - Fork 6
feat: AE-1741 manifest management via gql for flash client #144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
AE-1741 |
- Add 13 unit tests covering basic operations, error handling, retry logic, and concurrency - Fix race condition by adding asyncio.Lock for manifest updates - Replace generic exception handling with specific GraphQL exception types - Add runtime validation to get_flash_build() API - Standardize error messages and add deprecation warnings - Update documentation with GraphQL migration guide
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR migrates manifest management from REST API to GraphQL for the Flash client, enabling direct interaction with RunPod's GraphQL API for build and manifest operations.
Changes:
- Introduces GraphQL mutations (
updateFlashBuildManifest) and queries (get_flash_build) for manifest management - Refactors
StateManagerClientto useRunpodGraphQLClientinstead of httpx-based REST calls - Updates
get_flash_buildAPI signature to accept build ID strings directly instead of dictionary inputs
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/runtime/test_state_manager_client.py | Comprehensive test suite for GraphQL-based StateManagerClient covering basic operations, error handling, retry logic, and concurrency |
| src/tetra_rp/runtime/state_manager_client.py | Migration from REST/httpx to GraphQL-based manifest persistence with locking for concurrency safety |
| src/tetra_rp/runtime/exceptions.py | New GraphQL-specific exception hierarchy (GraphQLError, GraphQLMutationError, GraphQLQueryError) |
| src/tetra_rp/core/resources/app.py | Updated get_build call to use new string-based API signature |
| src/tetra_rp/core/api/runpod.py | Added update_build_manifest mutation and updated get_flash_build to accept build_id as string |
| docs/Cross_Endpoint_Routing.md | Updated documentation to reflect GraphQL-based architecture and migration guide |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Document manifest field change in get_flash_build docstring - Remove unused _client attribute from StateManagerClient - Use calculated backoff values in retry tests for maintainability
- Remove unused api_key, base_url, and timeout parameters - Remove self.api_key storage and validation (redundant with RunpodGraphQLClient) - Remove unused os import and DEFAULT_REQUEST_TIMEOUT import - Update all tests to use simplified constructor - Increase test coverage from 93% to 95%
…nager methods - Update module docstring: HTTP client -> GraphQL client - Fix get_persisted_manifest return type docs (never returns None) - Remove unused context manager methods (close, __aenter__, __aexit__)
Uh oh!
There was an error while loading. Please reload this page.