-
-
Notifications
You must be signed in to change notification settings - Fork 199
graphql #984
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
base: master
Are you sure you want to change the base?
graphql #984
Conversation
docs(graphql): add strictness section to GraphQL docs
…scription cleanup - **example/graphql/server.py**: Added a loop to drain the global subscription queue `q` during `reset_db()` to prevent leftover subscriptions from persisting between test runs. - **tavern/_plugins/graphql/client.py**: Updated `__aexit__` to explicitly close all active GraphQL subscriptions (`s.aclose()`) before closing the underlying stack, and adjusted the early‑return logic when the event loop is already running. This guarantees that subscription resources are released correctly after each test.
- Deleted the `AsyncExitStack` import from `tavern/_plugins/graphql/client.py`. - Removed the `stack` attribute from `GraphQLClient` and its initialization. - Eliminated calls to `stack.__aenter__()` in `__enter__` and `stack.aclose()` in `__exit__`. - Updated related comments to reflect the removal.
Update GraphQLClient to correctly manage cases where an asyncio event loop is already running. Cleanup tasks are scheduled as async tasks instead of using run_until_complete, and subscription creation respects the current loop state.
- Updated `example/graphql/pyproject.toml` - Added a `[tool.pytest.ini_options]` section with `norecursedirs = ["tavern_graphql_example"]` - Configures pytest to ignore the `tavern_graphql_example` directory during test discovery.
| - name: create user by operation_name | ||
| graphql_request: | ||
| url: "{graphql_server_url}/graphql" | ||
| query: !include multiple_operations.graphql |
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.
that will also work if I generate the grapql operations dynamically, right? standard Tavern way
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.
You mean if you have a fixture that generates them or something?
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.
yes, a fixture, or generated from the SDL
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.
As long as it generates a string then it should be fine? I can't tell from the docs how to easily do that though
- Added `to_close` list on `GraphQLClient` to track async generators that require explicit closing. - Initialized `self.to_close` in the client constructor. - Updated `_close_subscriptions` to also call `aclose()` on all generators stored in `to_close`. - Replaced direct `ws_client.subscribe_async` usage with a `subscribe_async_wrapper` that: - Manages the WebSocket session via `async with`. - Stores the subscription generator in `to_close`. - Yields results from the generator. - This change fixes the warning from the gql library about subscriptions not being closed correctly.
32b1d74 to
5093a5a
Compare
…op management - Replaced `ThreadPoolExecutor` with direct `threading.Thread` for managing the event loop thread in `GraphQLClient`. - Enabled event loop debugging by calling `self._loop.set_debug(True)`. - Initiated the event loop thread in `__enter__` by starting `self._async_thread`. - Modified `__exit__` to: - Ensure `asyncio.run_coroutine_threadsafe` results are awaited via `.result()` before proceeding. - Replace `self._loop.run_forever()` with `self._async_thread.join()` for synchronization. - Improved subscription management in `make_request` by awaiting task results using `.result()`. This simplifies threading logic, improves readability, and ensures proper synchronization between threads.
…client - Added `_shutdown_event` to manage graceful thread shutdown in `client.py`. - Updated event loop to check the shutdown flag and stop appropriately. - Ensured proper cleanup of subscriptions with timeout handling and logging. - Adjusted threading model to join threads safely, preventing indefinite blocking in `client.py`. - Modified `response.py` to use `asyncio.run_coroutine_threadsafe` for handling subscription responses.
|
any chance to add a test and example for upload? |
…e hints - Removed `get_file_arguments` from `tavern/_plugins/rest/files.py` and added detailed type hints to `_parse_file_mapping` and `_parse_file_list`. - Implemented `get_file_arguments` in `tavern/_plugins/rest/request.py` with proper imports of `_parse_file_mapping` and `_parse_file_list`. - Updated import statements in `request.py` to include `Optional`, `Union` and the internal parsing helpers. - Adjusted unit tests in `tests/unit/test_request.py` to import `get_file_arguments` from the new location. - Minor formatting changes to maintain code style after refactor.
…prove file handling
- Added `FileSendSpec` as a `NamedTuple` in `tavern/_plugins/rest/files.py` to standardize file descriptor structures.
- Updated functions `guess_filespec`, `_parse_file_mapping`, and `_parse_file_list` to return `FileSendSpec` instead of tuple-based file specifications.
- Modified `tavern/_plugins/rest/request.py` to adapt to `FileSendSpec` use, updating how file content type and encoding are inferred and handled.
- Enhanced clarity in log messages for cases where content type or encoding is not inferred.
- Updated tests in `tests/unit/test_request.py` to reflect the integration of `FileSendSpec` for multipart/form-data uploads.
feat(graphql): add support for file uploads in GraphQL requests
- Introduced `get_file_arguments` function in `tavern/_plugins/graphql/request.py` for handling file inputs in GraphQL requests.
- Used `_parse_file_list` from `tavern._plugins.rest.files` to format file specifications for GraphQL requests.
- Integrated file handling with `FileVar` from `gql` to facilitate file streaming and attachments.
- Updated `_format_graphql_request` to handle `files` in the request specification and pass them properly as variables.
- Adjusted import statements in `tavern/_plugins/graphql/request.py` to include necessary modules for file handling.
feat(graphql): implement and test file upload functionality for GraphQL requests
- Added `TestGraphQLFileUploads` class in `tests/unit/plugins/graphql/test_graphql_request.py` to cover file upload scenarios, including single file, multiple files, and files with metadata.
- Modified `_parse_file_list` in `tavern/_plugins/rest/files.py` to include an optional `require_filename` argument, enabling support for unnamed files in GraphQL uploads.
- Updated file parsing logic in `tavern/_plugins/graphql/request.py` to handle `FileVar` properly when filenames are not provided.
- Added `files` schema definition in `tavern/_core/schema/tests.jsonschema.yaml` to validate the new `files` parameter in GraphQL requests.
Update lockfile
refactor(files): move file handling to common
fix mocks
feat(graphql): enhance file handling and support in GraphQL requests
- Added `has_files` parameter to `GraphQLClient` to handle file upload scenarios.
- Updated content type setting in `tavern/_plugins/graphql/request.py` based on file presence (`multipart/form-data` for file uploads, `application/json` otherwise).
- Incorporated `deep_dict_merge` for merging custom headers with default ones.
- Refactored `_parse_file_list` in `tavern/_core/files.py` to support `resolved_file_path` and optional `require_form_field_name` for flexibility in parsing file arguments.
- Changed tests in `tests/unit/test_request.py` and `tests/unit/plugins/graphql/test_graphql_request.py` to align with updated file handling and response validation.
- Enhanced integration of `FileVar` with file path and metadata for better compatibility with the `gql` library.
feat(graphql): add file upload support and improve middleware logging
- Added `create_post_from_file` mutation in `example/graphql/tavern_graphql_example/server.py` to allow creating posts using file uploads.
- Enabled multipart uploads in `GraphQLRouter` instantiation by setting `multipart_uploads_enabled=True`.
- Introduced a custom middleware in `server.py` to log request and response bodies asynchronously.
- Updated logging level to `DEBUG` for more detailed server logs.
- Registered a scalar override for `UploadFile` to integrate file uploads using the `Upload` scalar type from Strawberry.
- Modified `tavern/_plugins/graphql/request.py` to adjust file handling logic and remove hardcoding of headers.
- Added a new test file `example/graphql/tests/test_files.tavern.yaml` to test file uploads, post creation, and content retrieval.
- Included a test data file `example/graphql/testdata/test_file_content.txt` for verifying file uploads in tests.
feat(graphql): support file uploads as variable‑name‑to‑path mapping
- Updated example test to use a variable name (`myFileName`) and a mapping entry for the file path.
- Changed JSON schema `files` from an array to an object, documenting it as a mapping of GraphQL variable names to file paths.
- Simplified core file handling in `tavern/_core/files.py` by removing the `require_form_field_name` flag and always enforcing that a form field name is provided.
- Refactored GraphQL request plugin:
- Imported `guess_filespec` directly.
- Modified `get_file_arguments` to accept a dict of `{variable_name: file_path}`.
- Built `FileVar` objects per variable, defaulting the form field name to the variable name when absent.
- Returned the mapping of variable names to `FileVar` objects (no longer wrapped in a `"files"` key).
- Adjusted request processing to merge these file arguments into the GraphQL variables.
add file list spec but its not that useful
typo
Add exmple to docs for long form files
refactor(graphql): update file handling and tests for improved clarity and consistency
- Changed `get_file_arguments` in `tavern/_plugins/graphql/request.py` to expect a dictionary as input instead of supporting both lists and dictionaries, simplifying file argument handling.
- Removed logic for processing file lists, requiring all file arguments to be mappings of variable names to file paths or long-form specifications.
- Updated tests in `tests/unit/plugins/graphql/test_graphql_request.py` to reflect changes in file handling:
- Adjusted test cases to use dictionaries for file arguments.
- Refactored assertions to check individual file variables instead of lists.
- Enhanced error handling to raise explicit exceptions for invalid input types in `get_file_arguments`.
- Updated JSON schema and validation (`tests.jsonschema.yaml` and `jsonschema.py`) to enforce that `files` must be a dictionary.
- Improved exception message in `tavern/_core/files.py` to provide type information for invalid file specifications.
fix test
Fix file spec for sending to requests
…Client - Added `TransportKey` class to serve as hashable keys for identifying unique HTTP transports in `tavern/_plugins/graphql/client.py`. - Introduced `_http_transport_cache` attribute in `GraphQLClient` to cache and reuse HTTP transports for requests with the same configuration. - Modified `make_request` method to retrieve or cache `AIOHTTPTransport` instances based on `TransportKey`. - Ensured proper cleanup of cached transports by closing them during the client shutdown process. - Added comprehensive tests in `tests/unit/plugins/graphql/test_graphql_client.py`: - Verified caching behavior based on URL, headers, and timeout. - Tested proper equality, hashability, and string representation of `TransportKey`. - Confirmed correct transport reuse and creation across various request scenarios. - Improved logging to clearly differentiate between transport creation and reuse.
…aching GraphQL clients and transports - Renamed `TransportKey` to `ClientCacheKey` across `tavern/_plugins/graphql/client.py` and corresponding tests. - Updated `ClientCacheKey` to cache both GraphQL clients and HTTP transports, replacing the previous `_http_transport_cache` with `_gql_client_cache` and `_transport_cache`. - Refactored `make_request` method to manage GraphQL client creation and reuse using `ClientCacheKey`. - Modified tests in `tests/unit/plugins/graphql/test_graphql_client.py` to reflect changes: - Renamed all `TransportKey` test cases to align with `ClientCacheKey`. - Added checks for both client and transport caching behavior. - Improved docstrings and comments to clarify changes and ensure consistency in naming conventions.
- Added `await asyncio.gather(*(s.close() for s in self._transport_cache.values()))` inside the coroutine that closes subscriptions, ensuring transport closures are awaited. - Removed the previous synchronous loop that called `transport.close()` after the subscription shutdown, consolidating all cleanup into the async task. - Guarantees transports are closed within the event loop and respects the same timeout handling as subscription closures.
…tion - Added a new test case in `example/graphql/tests/test_files.tavern.yaml` to demonstrate a `createPostFromFile` mutation using a file upload in long form. - Updated `docs/source/graphql.md` with detailed examples for file uploads: - Using single and multiple files - Setting custom `content_type` for files - Expanded documentation to clarify GraphQL file upload handling with step-by-step instructions and YAML examples.
- Removed custom threading and `asyncio.new_event_loop` initialization in `tavern/_plugins/graphql/client.py` and replaced it with a reusable `ThreadedAsyncLoop` class. - Introduced `ThreadedAsyncLoop` in `tavern/_core/asyncio.py` to handle asynchronous operations in a separate thread safely and consistently. - Updated `client.py` to use `ThreadedAsyncLoop` for managing coroutine execution and subscription cleanup. - Added timeout handling via `ThreadedAsyncLoop.run_coroutine` for operations such as subscription responses and client shutdown. - Improved error handling for timeouts during subscription response processing with clear logging messages. - Simplified code structure by consolidating repetitive threading logic in a dedicated utility class. - Removed redundant imports and variables such as `threading.Event` and `threading.Thread` in `client.py`. - Refined context management (`__enter__` and `__exit__`) for `GraphQLClient` to integrate seamlessly with `ThreadedAsyncLoop`.
✅ Actions performedReview triggered.
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis pull request introduces comprehensive GraphQL integration testing support to the Tavern framework. It adds a complete GraphQL plugin with client, request/response handlers, subscription support, an example GraphQL test server, documentation, integration tests, and updates core file handling to support GraphQL-specific workflows alongside existing HTTP, MQTT, and gRPC backends. Changes
Sequence DiagramssequenceDiagram
participant User as Tavern User
participant Framework as Tavern Framework
participant Client as GraphQL Client
participant Server as GraphQL Server
participant Cache as Client Cache
User->>Framework: Provide graphql_request
Framework->>Framework: Format request (preserve query)
Framework->>Client: make_request(url, query, variables, headers)
alt Cache Hit
Client->>Cache: Check ClientCacheKey
Cache->>Client: Return cached gql Client
else Cache Miss
Client->>Cache: Create new Client & Transport
Cache->>Client: Store in cache
end
Client->>Server: HTTP POST with GraphQL query
Server->>Server: Parse & execute query
Server->>Client: Return ExecutionResult
Client->>Framework: GraphQLResponseLike
Framework->>Framework: Validate response (data/errors)
Framework->>User: Test result
sequenceDiagram
participant User as Tavern User
participant Framework as Tavern Framework
participant Client as GraphQL Client
participant EventLoop as Event Loop Thread
participant Server as GraphQL Server
participant Transport as WebSocket Transport
User->>Framework: Provide subscription graphql_request
Framework->>Client: start_subscription(url, query, operation_name)
Client->>EventLoop: Queue subscription setup
EventLoop->>Transport: Create WebSocket Transport
EventLoop->>Server: Upgrade to WebSocket
EventLoop->>EventLoop: Create async generator
EventLoop->>Client: Store subscription generator
loop Get next message
User->>Framework: Poll subscription
Framework->>Client: get_next_message(operation_name, timeout)
Client->>EventLoop: anext(generator, timeout)
EventLoop->>Server: Await next message
Server->>EventLoop: Publish next result
EventLoop->>Client: Return message
Client->>Framework: _SubResponse
Framework->>User: Subscription data
end
User->>Framework: Test complete
Framework->>Client: Exit context manager
Client->>EventLoop: Signal shutdown
EventLoop->>Transport: Close WebSocket
EventLoop->>Client: Join thread
sequenceDiagram
participant Test as Integration Test
participant DockerCompose as Docker Compose
participant Server as FastAPI/Strawberry Server
participant Database as SQLite Database
Test->>DockerCompose: docker-compose up
DockerCompose->>Server: Start container
Server->>Database: Create tables (User, Post)
Server->>Server: Setup authentication middleware
Server->>Server: Setup subscription queue
Test->>Server: POST /reset
Server->>Database: Clear tables
Server->>Server: Reset publish queue
Test->>Server: POST /graphql_authenticated
Note over Test,Server: GraphQL query with auth header
Server->>Server: Validate Bearer token
Server->>Database: Query/mutate data
Server->>Test: Return result
Test->>Test: Validate response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Comment |
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.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/test_request.py (1)
461-472: Inconsistent use of FileSendSpec accessor.Line 471 uses tuple indexing (
file[0]) instead of the named tuple attribute (file.filename), which is inconsistent with the refactoring applied elsewhere in this file (lines 424, 444). This suggests incomplete migration to theFileSendSpecnamed tuple.🔎 Proposed fix
file = file_spec["files"]["file1"] - assert file[0] == os.path.basename(tfile.name) + assert file.filename == os.path.basename(tfile.name)
🧹 Nitpick comments (19)
tavern/_core/dict_util.py (1)
30-46: Clarify where error-ignoring behaviour is controlled.The docstring mentions "dangerously_ignore_string_format_errors is False" at line 42, but this parameter is not part of
_check_and_format_values' signature—it's controlled by the caller (format_keys). The current wording could mislead readers into thinking this function handles that logic directly. Consider revising to make clear this behaviour is controlled externally.🔎 Suggested docstring refinement
- Raises: - MissingFormatError: If a required format variable is not found in - box_vars (and dangerously_ignore_string_format_errors is False) + Raises: + MissingFormatError: If a required format variable is not found in + box_varsAlternatively, if you want to document the broader behaviour:
"""Checks and formats a string with the given variables. Parses the input string to identify format placeholders and verifies that - all required variables exist in the provided box_vars. Performs string - formatting after validation, optionally ignoring missing format variables. + all required variables exist in the provided box_vars. Raises an error if + a variable is not found. Performs string formatting after validation.example/graphql/README.md (1)
1-25: Well-documented example.The README provides a clear overview of the GraphQL example server features and test coverage. Consider adding specific commands to run the tests (e.g.,
pytestortavern-ciinvocation examples) to help users get started quickly.example/graphql/tests/conftest.py (1)
10-10: Add timeout to prevent test hangs.The
requests.postcall lacks a timeout parameter, which could cause tests to hang indefinitely if the server becomes unresponsive. This is particularly important for autouse fixtures that run after every test.🔎 Proposed fix
- requests.post("http://localhost:5010/reset") + requests.post("http://localhost:5010/reset", timeout=10)example/graphql/tests/test_errors.tavern.yaml (1)
45-81: Document the reason for xfail markers.Two tests are marked with
_xfail, indicating expected failures. Whilst this is acceptable for tracking known issues, consider adding comments explaining why these tests are expected to fail and whether there are plans to address them. This helps maintain test suite clarity and prevents these from being forgotten.Additionally, consider creating GitHub issues to track fixing these xfail cases if they represent real bugs or missing functionality.
Would you like me to generate GitHub issue templates for tracking these expected failures?
example/graphql/tests/test_subscriptions.tavern.yaml (1)
196-338: Track the skipped concurrent subscription test.This test is marked as
skipwith a FIXME comment indicating issues with delayed subscription messages (line 202). Whilst skipping is appropriate for known issues, ensure this is tracked in your issue tracker so it doesn't get forgotten.Would you like me to help create a GitHub issue to track this failing test case, or generate a script to investigate the timing issue with concurrent subscriptions?
example/graphql/pyproject.toml (1)
7-15: Consider version constraints for dependencies.Most dependencies lack version constraints (except SQLAlchemy). Whilst this may be intentional for an example project to demonstrate compatibility with the latest versions, consider adding at least upper-bound constraints to prevent breakage from major version updates. For instance:
dependencies = [ "tavern[graphql]", "uvicorn[standard]>=0.27,<1", "fastapi>=0.109,<1", ... ]This is particularly important for rapidly evolving libraries like FastAPI and Strawberry GraphQL.
example/graphql/tests/test_auth.tavern.yaml (1)
63-66: TODO noted for future enhancement.The comment correctly identifies that HTTP-layer authentication failures (non-200 status) differ from GraphQL errors. The suggestion to allow
status_codechecking in GraphQL responses would be valuable for comprehensive transport-layer testing.Would you like me to open an issue to track this status_code checking enhancement?
tavern/_plugins/graphql/response.py (2)
257-257: Prefer%-style formatting in logger calls for lazy evaluation.Using f-strings in logging calls evaluates the string even if the log level is disabled. Use
%-style formatting for deferred evaluation.🔧 Proposed fix
- logger.info(f"response: {response}") + logger.info("response: %s", response)
122-127: Consider documenting the thread-safety pattern for subscription handling.The code uses
asyncio.run_coroutine_threadsafewithself.session._loopto handle subscriptions from a synchronous context. This pattern is correct but accessing the private_loopattribute couples this class to the internal implementation ofGraphQLClient.Consider exposing a public method on
GraphQLClientto run coroutines, e.g.,session.run_async(coro), to encapsulate the event loop access.pyproject.toml (1)
70-74: Consider adding version constraints foraiohttpandwebsockets.Unlike
gql>=4.0.0, theaiohttpandwebsocketspackages lack version pins. This could lead to unexpected breaking changes when these dependencies release major versions.The proposed
aiohttp>=3,<4constraint is appropriate given the current stable version (3.13.2). However, if usingwebsockets>=12,<14, be aware this excludes the current stable version (15.0.1, released March 2025); considerwebsockets>=12,<16orwebsockets>=15,<16instead.🔧 Proposed fix
graphql = [ - "aiohttp", - "websockets", + "aiohttp>=3,<4", + "websockets>=12,<16", "gql>=4.0.0", ]tests/unit/plugins/graphql/test_graphql_client.py (1)
119-130: Unreachableyieldstatement after exception.The
yieldon line 122 is unreachable since the exception is raised before it. While this pattern works for testing exception propagation, consider using a more explicit approach that doesn't leave dead code.🔎 Alternative approach using async iterator
- # Create a mock async generator that raises an exception - async def error_async_gen(): - raise Exception("Test error") - yield + # Create a mock async iterator that raises an exception + class ErrorAsyncIter: + def __aiter__(self): + return self + async def __anext__(self): + raise Exception("Test error") - client._subscriptions["error_op"] = error_async_gen() + client._subscriptions["error_op"] = ErrorAsyncIter()example/graphql/tavern_graphql_example/server.py (1)
241-243: Consider usinglogging.exceptionfor better exception context.When logging exceptions in the except block,
logging.exceptionautomatically includes the traceback, which is more useful for debugging.🔎 Proposed fix
except Exception as e: - logging.error(f"Authentication failed: {e}") + logging.exception("Authentication failed") raisetavern/_plugins/rest/response.py (1)
87-117: Multipletype:ignorecomments suggest type refinement opportunity.The
type:ignoreannotations on lines 87, 99, and 117 indicate thatrequests.Responsedoesn't fully satisfy theResponseLikeprotocol orMappingtype expectations. This works at runtime but could be cleaned up by:
- Ensuring
ResponseLikeis compatible withrequests.Response- Adding a thin wrapper or type adapter
This is minor and can be addressed in a follow-up if desired.
tavern/_plugins/graphql/tavernhook.py (1)
25-41: Consider documenting or marking the unusedsessionparameter as intentionally ignored.The
sessionparameter is part of the plugin interface contract defined intavern/_core/plugins.pyand is always passed by the framework, but it is not referenced in this implementation. While this is consistent across other plugins (REST, MQTT, gRPC), adding clarification would improve code readability. Either add a brief comment explaining that it is required by the plugin interface, or prefix it with an underscore to indicate it is intentionally unused.tavern/_plugins/common/response.py (1)
30-54: Add a comment explaining thesessionparameter.The
sessionparameter is accepted but not used within CommonResponse itself; it's passed through for subclass compatibility (e.g. GraphQLResponse stores and uses it). A brief comment would clarify this intent and address the ARG002 linter warning.tavern/_plugins/graphql/request.py (1)
180-185: Subscription detection may miss queries with leading comments.GraphQL allows operation definitions to be preceded by comments (e.g.
# Subscription\nsubscription { ... }), which the current simplestartswith("subscription")check would miss. For typical test scenarios this is unlikely to cause issues, but consider using a GraphQL parser (from graphql-core) if handling arbitrary query strings becomes necessary.tavern/_plugins/graphql/client.py (3)
84-86: Use explicitis Nonecheck instead of truthiness check.The condition
if not self.resultrelies on truthiness, which could be ambiguous. WhilstExecutionResultandTransportQueryErrorobjects should typically be truthy, it's clearer and more defensive to explicitly checkif self.result is None.🔎 Proposed fix
- if not self.result: + if self.result is None: raise ValueError("No GraphQL result to return")
292-318: Consider usinglogging.exceptionfor better error context.The subscription logic is well-structured. However, line 316 uses
logging.errorwithin an exception handler. Usinglogging.exceptionwould automatically include the stack trace, aiding debugging.🔎 Proposed fix
except Exception as e: - logger.error(f"Failed to start subscription: {e}") + logger.exception(f"Failed to start subscription: {e}") raiseBased on static analysis hints.
345-359: Uselogging.exceptionin exception handlers for better error context.Lines 350, 355, and 358 use
logging.errorwithin exception handlers. Usinglogging.exceptionwould automatically include stack traces, which aids debugging without requiringexc_info=True.🔎 Proposed fix
except StopAsyncIteration: - logger.error( + logger.exception( f"got unexpected StopAsyncIteration from subscription {op_name}" ) return None except TimeoutError as e: - logger.error(f"Timeout getting next message from subscription: {e}") + logger.exception(f"Timeout getting next message from subscription: {e}") raise except Exception as e: - logger.error(f"Error getting next message from subscription: {e}") + logger.exception(f"Error getting next message from subscription: {e}") raiseBased on static analysis hints.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
example/mqtt/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (53)
.github/workflows/main.yml.pre-commit-config.yamldocs/source/conf.pydocs/source/graphql.mddocs/source/http.mddocs/source/index.mdexample/graphql/.dockerignoreexample/graphql/Dockerfileexample/graphql/README.mdexample/graphql/docker-compose.yamlexample/graphql/pyproject.tomlexample/graphql/tavern_graphql_example/server.pyexample/graphql/testdata/test_file_content.txtexample/graphql/tests/conftest.pyexample/graphql/tests/graphql_config.yamlexample/graphql/tests/multiple_operations.graphqlexample/graphql/tests/test_auth.tavern.yamlexample/graphql/tests/test_basic_query.tavern.yamlexample/graphql/tests/test_errors.tavern.yamlexample/graphql/tests/test_files.tavern.yamlexample/graphql/tests/test_multiple_operations.tavern.yamlexample/graphql/tests/test_subscriptions.tavern.yamlexample/graphql/tests/test_variables.tavern.yamlpyproject.tomlscripts/smoke.bashtavern/_core/dict_util.pytavern/_core/files.pytavern/_core/loader.pytavern/_core/pytest/config.pytavern/_core/pytest/error.pytavern/_core/pytest/util.pytavern/_core/schema/extensions.pytavern/_core/schema/jsonschema.pytavern/_core/schema/tests.jsonschema.yamltavern/_plugins/common/response.pytavern/_plugins/graphql/__init__.pytavern/_plugins/graphql/client.pytavern/_plugins/graphql/jsonschema.yamltavern/_plugins/graphql/request.pytavern/_plugins/graphql/response.pytavern/_plugins/graphql/tavernhook.pytavern/_plugins/rest/request.pytavern/_plugins/rest/response.pytavern/response.pytests/unit/conftest.pytests/unit/plugins/graphql/test_graphql_client.pytests/unit/plugins/graphql/test_graphql_request.pytests/unit/plugins/graphql/test_graphql_response.pytests/unit/plugins/graphql/test_graphql_tavernhook.pytests/unit/response/test_rest.pytests/unit/test_request.pytox-integration.initox.ini
💤 Files with no reviewable changes (1)
- docs/source/conf.py
🧰 Additional context used
🧬 Code graph analysis (14)
tests/unit/plugins/graphql/test_graphql_tavernhook.py (2)
tavern/_plugins/graphql/client.py (1)
GraphQLClient(93-359)tavern/_plugins/graphql/tavernhook.py (1)
get_expected_from_request(25-41)
tavern/_core/loader.py (1)
tavern/_core/exceptions.py (1)
BadSchemaError(23-24)
tavern/_core/pytest/error.py (2)
tavern/_plugins/graphql/request.py (1)
_format_graphql_request(58-82)tavern/_core/dict_util.py (1)
format_keys(134-202)
example/graphql/tests/conftest.py (1)
example/graphql/tavern_graphql_example/server.py (2)
reset_db(289-294)post(78-82)
tavern/_plugins/rest/request.py (4)
tavern/_core/files.py (3)
_parse_file_list(149-173)_parse_file_mapping(130-146)guess_filespec(75-127)tavern/_core/pytest/config.py (1)
TestConfig(21-70)tavern/_plugins/graphql/request.py (1)
get_file_arguments(21-55)tavern/_core/exceptions.py (1)
BadSchemaError(23-24)
tavern/_plugins/graphql/request.py (4)
tavern/_core/dict_util.py (2)
deep_dict_merge(235-257)format_keys(134-202)tavern/request.py (1)
BaseRequest(12-32)tavern/_plugins/graphql/client.py (3)
GraphQLClient(93-359)GraphQLResponseLike(71-90)text(78-86)tavern/_core/exceptions.py (3)
BadSchemaError(23-24)MissingKeysError(51-52)TavernException(7-20)
tests/unit/plugins/graphql/test_graphql_response.py (2)
tavern/_plugins/graphql/client.py (1)
text(78-86)tavern/_plugins/graphql/response.py (1)
_validate_graphql_response_structure(65-90)
tests/unit/conftest.py (2)
tavern/_core/pytest/config.py (4)
TestConfig(21-70)TavernInternalConfig(13-17)backends(55-70)copy(39-41)tavern/_core/strict_util.py (2)
StrictLevel(87-132)all_on(127-128)
tavern/_core/schema/jsonschema.py (1)
tavern/_core/schema/extensions.py (1)
validate_file_spec(397-444)
tavern/_core/files.py (2)
tavern/_core/pytest/config.py (1)
TestConfig(21-70)tavern/_core/dict_util.py (1)
format_keys(134-202)
tests/unit/plugins/graphql/test_graphql_request.py (3)
tavern/_plugins/graphql/client.py (2)
GraphQLResponseLike(71-90)text(78-86)tavern/_plugins/graphql/request.py (3)
get_file_arguments(21-55)request_vars(121-131)run(133-178)tavern/_core/exceptions.py (2)
MissingKeysError(51-52)TavernException(7-20)
tavern/_plugins/graphql/client.py (1)
tavern/_plugins/common/response.py (3)
json(24-24)ResponseLike(16-24)text(22-22)
tests/unit/test_request.py (1)
tavern/_core/files.py (1)
FileSendSpec(66-72)
tavern/_plugins/graphql/tavernhook.py (4)
tavern/_core/dict_util.py (1)
format_keys(134-202)tavern/_core/pytest/config.py (1)
TestConfig(21-70)tavern/_plugins/graphql/request.py (1)
GraphQLRequest(85-185)tavern/_plugins/graphql/response.py (1)
GraphQLResponse(20-307)
🪛 LanguageTool
docs/source/index.md
[uncategorized] ~5-~5: Possible missing comma found.
Context: ...ports Python 3.4 and up. At the time of writing we test against Python 3.11. Python 2 i...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~21-~21: Would you like to use the Oxford spelling “organize”? The spelling ‘organise’ is also correct.
Context: ... anchors, whilst using pytest.mark to organise your tests. Your tests should become mo...
(OXFORD_SPELLING_Z_NOT_S)
docs/source/graphql.md
[uncategorized] ~283-~283: Possible missing comma found.
Context: ...L responses follow the standard GraphQL format with data and/or errors at the top ...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~363-~363: ‘Future Plans’ might be wordy. Consider a shorter alternative.
Context: ...esponses (defer/stream directives) ### Future Plans - Improved error message formatting - ...
(EN_WORDINESS_PREMIUM_FUTURE_PLANS)
docs/source/http.md
[duplication] ~430-~430: Possible typo: you repeated a word.
Context: ...To specify the same file multiple times with with a different form field name, use a list...
(ENGLISH_WORD_REPEAT_RULE)
🪛 Ruff (0.14.10)
tavern/_plugins/common/response.py
32-32: Unused method argument: session
(ARG002)
53-53: Avoid specifying long messages outside the exception class
(TRY003)
tavern/_core/loader.py
173-175: Avoid specifying long messages outside the exception class
(TRY003)
example/graphql/tests/conftest.py
10-10: Probable use of requests call without timeout
(S113)
tavern/_plugins/rest/request.py
57-59: Avoid specifying long messages outside the exception class
(TRY003)
tavern/_plugins/graphql/request.py
44-46: Avoid specifying long messages outside the exception class
(TRY003)
106-108: Avoid specifying long messages outside the exception class
(TRY003)
111-113: Avoid specifying long messages outside the exception class
(TRY003)
116-118: Avoid specifying long messages outside the exception class
(TRY003)
171-171: Consider moving this statement to an else block
(TRY300)
178-178: Avoid specifying long messages outside the exception class
(TRY003)
example/graphql/tavern_graphql_example/server.py
70-70: Create your own exception
(TRY002)
70-70: Avoid specifying long messages outside the exception class
(TRY003)
81-81: Create your own exception
(TRY002)
81-81: Avoid specifying long messages outside the exception class
(TRY003)
98-98: Create your own exception
(TRY002)
98-98: Avoid specifying long messages outside the exception class
(TRY003)
105-105: Unused method argument: info
(ARG002)
114-114: Unused method argument: info
(ARG002)
123-123: Unused method argument: info
(ARG002)
137-137: Unused method argument: info
(ARG002)
141-141: Create your own exception
(TRY002)
141-141: Avoid specifying long messages outside the exception class
(TRY003)
242-242: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
312-312: Possible binding to all interfaces
(S104)
tavern/_plugins/graphql/response.py
42-44: Avoid specifying long messages outside the exception class
(TRY003)
51-53: Avoid specifying long messages outside the exception class
(TRY003)
57-59: Avoid specifying long messages outside the exception class
(TRY003)
130-133: Avoid specifying long messages outside the exception class
(TRY003)
177-177: Redundant exception object included in logging.exception call
(TRY401)
tests/unit/plugins/graphql/test_graphql_client.py
121-121: Create your own exception
(TRY002)
121-121: Avoid specifying long messages outside the exception class
(TRY003)
tavern/_plugins/graphql/client.py
85-85: Avoid specifying long messages outside the exception class
(TRY003)
265-265: Avoid specifying long messages outside the exception class
(TRY003)
268-270: Avoid specifying long messages outside the exception class
(TRY003)
316-316: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
338-340: Avoid specifying long messages outside the exception class
(TRY003)
350-352: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
355-355: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
358-358: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
tavern/_plugins/graphql/tavernhook.py
28-28: Unused function argument: session
(ARG001)
🔇 Additional comments (66)
tavern/_core/dict_util.py (1)
77-93: Docstrings successfully enhance code clarity.Both
_check_and_format_valuesand_attempt_find_includenow have clear, well-structured docstrings that document intent, parameters, exceptions, and return values. This significantly improves maintainability of these internal helpers. The documentation for_attempt_find_includeis accurate and well-written.tavern/_core/pytest/error.py (2)
1-1: LGTM!The
copymodule import is necessary for deep-copying stages when handling GraphQL requests.
160-175: Excellent implementation of GraphQL-specific formatting!The conditional logic correctly handles GraphQL requests by:
- Deep-copying the stage to avoid mutating the original
- Pre-formatting the GraphQL request to wrap the query field in
FormattedString, preventing format placeholder conflicts with GraphQL curly braces- Applying standard formatting to the modified stage, which correctly skips already-formatted fields
The lazy import pattern on line 163 is appropriate for plugin code, ensuring the GraphQL plugin is only loaded when actually needed.
tavern/_core/loader.py (1)
293-294: Type value is intentionally different from other sentinels.Unlike other
TypeSentinelsubclasses that assign actual type objects toallowed_types(e.g.,int,float,str),AnythingSentinelassigns the string literal"<anything>". Whilst this is semantically appropriate for representing "any type", it does create a type inconsistency in the class hierarchy.This appears to be an intentional design choice and works correctly with the
__str__method (line 198).example/graphql/testdata/test_file_content.txt (1)
1-1: LGTM!Simple test data file for GraphQL file upload tests. The content is appropriate for its purpose.
tavern/_plugins/graphql/__init__.py (1)
1-1: LGTM!Appropriate module-level docstring for the GraphQL plugin package.
tavern/_core/pytest/util.py (2)
43-47: LGTM!The CLI option for GraphQL backend follows the established pattern for other backends and correctly defaults to "gql".
101-105: LGTM!The ini option for GraphQL backend is consistent with the CLI option and follows the same pattern as other backend configurations.
tox-integration.ini (4)
2-2: LGTM!GraphQL environment correctly added to the envlist alongside existing integration test environments.
5-6: LGTM!Addition of dependency_groups aligns with modern dependency management practices.
18-18: LGTM!The changedir mapping for graphql correctly points to the example/graphql directory.
32-32: LGTM!The graphql extras configuration follows the established pattern for other backends.
example/graphql/tests/graphql_config.yaml (1)
1-11: LGTM!The configuration structure is clean and provides sensible defaults for GraphQL integration tests. The variables are well-organised and clearly named.
scripts/smoke.bash (3)
5-10: LGTM!The fallback mechanism for pre-commit command detection is well-implemented using
command -vfor portability. This provides good flexibility for different development environments.
13-14: LGTM!Correctly uses the PRE_COMMIT_CMD variable to invoke pre-commit hooks consistently.
23-23: LGTM!Addition of py3-graphql to the integration test suite aligns with the PR objectives for GraphQL support.
.pre-commit-config.yaml (1)
3-30: LGTM!The pre-commit hook version updates (actionlint, ruff, mypy, uv) are appropriate routine maintenance.
docs/source/graphql.md (1)
359-359: Verify WebSocket/subscription support status.The documentation states "GraphQL subscriptions over WebSocket are not supported yet", but the PR objectives mention "Add graphql with basic subscriptions". Please clarify the current status of subscription support.
docs/source/index.md (1)
1-44: LGTM!The documentation updates appropriately add GraphQL to the integration testing documentation index.
tox.ini (1)
9-9: LGTM!Adding
graphqlto the extras list appropriately enables GraphQL testing in the tox environment, consistent with existing integrations.example/graphql/.dockerignore (1)
1-3: LGTM!The
.dockerignoreconfiguration appropriately restricts the Docker build context to only include necessary project files (pyproject.toml and the tavern_graphql_example directory).tavern/_core/schema/extensions.py (1)
427-427: LGTM!The typo correction in the error message ("uplaod" → "upload") improves clarity.
.github/workflows/main.yml (1)
81-82: LGTM!The addition of the
py3-graphqlintegration test matrix entry appropriately extends CI coverage to include GraphQL testing, consistent with the existing integration test patterns.tests/unit/response/test_rest.py (1)
384-384: LGTM! Logger path correctly updated.The logger patch path has been updated to reflect the REST response's reliance on the common response infrastructure. This change aligns with the broader refactoring mentioned in the PR objectives.
example/graphql/docker-compose.yaml (1)
1-8: LGTM! Docker Compose configuration is appropriate.The configuration correctly defines the GraphQL example server with the expected port mapping (5010:5010), aligning with the Dockerfile and test infrastructure.
tavern/_core/schema/jsonschema.py (1)
197-197: LGTM! GraphQL file validation properly integrated.The addition of GraphQL file validation follows the established pattern for REST file uploads, ensuring consistent validation across different request types.
example/graphql/Dockerfile (1)
1-12: LGTM! Dockerfile appropriately configured for the example service.The Dockerfile uses a slim base image, installs dependencies via uv, and correctly exposes the GraphQL server on port 5010, aligning with the docker-compose configuration and test infrastructure.
tavern/_plugins/graphql/jsonschema.yaml (1)
1-28: LGTM! GraphQL schema is well-defined.The JSON Schema correctly defines the GraphQL test configuration with appropriate types, defaults, and constraints. The 30-second default timeout is reasonable for GraphQL operations.
tavern/response.py (1)
135-137: LGTM! GraphQL response validation properly integrated.The implementation correctly mirrors the MQTT response validation pattern, ensuring consistent handling of external validation functions across different response types.
example/graphql/tests/multiple_operations.graphql (1)
1-41: LGTM! GraphQL operations are well-formed.The document contains properly structured GraphQL operations with correct variable declarations and field selections, supporting operation_name-based test execution as intended.
tests/unit/plugins/graphql/test_graphql_response.py (1)
1-93: LGTM!The test coverage for
GraphQLResponseis comprehensive and well-structured. The tests appropriately cover initialisation defaults, custom status codes, string representation, and response format validation. The use of Mock objects and clear assertions makes the tests maintainable and easy to understand.tests/unit/test_request.py (2)
15-15: LGTM!The import changes correctly reflect the refactoring of
FileSendSpecto the core module and the relocation ofget_file_arguments.Also applies to: 20-20
495-517: LGTM!The explicit construction of
FileSendSpecinstances correctly uses the named tuple structure with all required fields in the proper order.example/graphql/tests/test_errors.tavern.yaml (1)
1-44: LGTM!The error handling tests are well-structured and verify appropriate error messages for non-existent users and invalid queries. The test structure follows Tavern conventions correctly.
example/graphql/tests/test_subscriptions.tavern.yaml (2)
1-75: LGTM!The subscription test correctly demonstrates the expected workflow: create a user, subscribe to updates, perform an update, and verify both the subscription notification and mutation response. The structure properly uses
operation_nameto identify the subscription.
340-612: LGTM!The invalid subscription scenarios and complex multi-operation tests are well-structured. The error handling test (lines 407-498) particularly demonstrates good practice by verifying that subscriptions continue working after an error occurs in an unrelated mutation.
example/graphql/pyproject.toml (2)
17-18: LGTM!The entry point definition correctly maps the
gqlidentifier to the Tavern GraphQL plugin hook, enabling plugin discovery by the Tavern framework.
20-33: LGTM!The pytest and setuptools configurations are appropriate. Excluding
tavern_graphql_examplefrom test discovery whilst including it in the package distribution is the correct approach for an example server.example/graphql/tests/test_variables.tavern.yaml (1)
1-57: LGTM!The variable substitution tests are well-structured and correctly demonstrate GraphQL variable usage. The appropriate use of
!anyintfor generated IDs and!int "{user_id}"for formatted variables shows good understanding of Tavern's validation patterns.tests/unit/plugins/graphql/test_graphql_tavernhook.py (1)
1-50: LGTM!The unit tests provide good coverage of the Tavern GraphQL plugin hook's key functionality:
- Plugin property verification
- Response block transformation
- Handling of missing responses
- Variable substitution
The tests are clear, focused, and follow appropriate testing patterns.
example/graphql/tests/test_multiple_operations.tavern.yaml (1)
1-40: LGTM!The first test block correctly demonstrates multiple operations with
operation_nameselection, including creating a user and then retrieving by ID. Variable chaining viasaveblocks is properly implemented.example/graphql/tests/test_files.tavern.yaml (1)
1-109: LGTM!This test file provides good coverage for GraphQL file uploads, demonstrating both short-form (line 49) and long-form with explicit
content_type(lines 74-76) file specifications. The test flow correctly chains user creation with post creation and verification.pyproject.toml (2)
271-278: LGTM!The workspace configuration correctly adds the new GraphQL example package to both members and sources, consistent with the existing patterns for mqtt, grpc, and http examples.
108-109: No issue found. pytest-asyncio version 1.3.0 exists on PyPI (released 10 November 2025) and is the current latest version. The specified constraintpytest-asyncio>=1.3.0is correct.Likely an incorrect or invalid review comment.
example/graphql/tests/test_basic_query.tavern.yaml (1)
1-52: LGTM!The test demonstrates proper GraphQL query testing with inline mutations and queries, including creating a user and retrieving by ID with variable substitution. Response matching with
!anyint,!anystr, and!intvalidators is appropriate.tavern/_plugins/rest/request.py (2)
32-64: LGTM!The new
get_file_argumentsfunction cleanly handles file argument parsing for both dict and list formats, with proper error handling for invalid types. The integration with_parse_file_mappingand_parse_file_listfromtavern._core.filesprovides consistent file handling across REST and GraphQL plugins.
136-180: LGTM!The refactored file_body handling using
FileSendSpecattributes (content_type,content_encoding) is cleaner than positional tuple access. The added debug logging for missing inferences (lines 160-163, 177-180) aids troubleshooting.tavern/_core/schema/tests.jsonschema.yaml (3)
295-337: LGTM!The
graphql_requestschema definition is well-structured with appropriate required fields (url,query) and optional fields (variables,files,headers,operation_name). The permissivevariablestype handling aligns with GraphQL's flexibility for variable types.
338-373: LGTM!The
graphql_responseschema correctly defines the expected structure includingdata,errors, subscription-specific fields (subscription,timeout), and verification/save blocks. TheadditionalProperties: falseensures strict validation.
513-522: LGTM!The stage integration for GraphQL follows the established pattern from other protocols (MQTT, gRPC), supporting both single response and array form with
minItems: 1constraint.tavern/_plugins/graphql/response.py (1)
65-90:_validate_graphql_response_structureis not integrated into the verification flow.This method is tested in unit tests but is not called by
verify()or_check_sync_response(). If the validation is needed as part of the response verification process, integrate it into_check_sync_response()before accessingresponse.result.dataorresponse.result.errors. Otherwise, consider removing it if it serves no purpose beyond unit testing.tavern/_plugins/graphql/tavernhook.py (1)
44-47: Schema loading at module level looks good.The schema is loaded once at module import time using
SafeLoader, which is the secure approach for loading YAML files. This is appropriate for plugin registration.tests/unit/plugins/graphql/test_graphql_client.py (1)
223-464: Comprehensive caching tests look good.The
TestClientCachingclass thoroughly covers the caching behaviour including:
- New client creation on first request
- Client reuse for identical URL and headers
- Separate clients for different URLs or headers
- Proper cleanup on context manager exit
The tests correctly validate cache population and transport/client creation counts.
tavern/_core/files.py (2)
66-73: Good introduction ofFileSendSpecNamedTuple.The typed
FileSendSpecprovides a clean interface for file upload specifications. Thecontent_encoding: Optional[str | dict]type is intentional since it can be either a raw encoding string or a headers dictionary (as seen in line 125 where{"Content-Encoding": encoding}is appended).Consider adding a brief docstring clarifying when each type is used.
75-127: Refactoredguess_filespecis well-structured.The function now:
- Resolves file paths using
format_keysbefore file operations- Correctly uses
resolved_file_pathconsistently for filename extraction, file opening, and MIME type guessing- Returns a well-typed tuple with
FileSendSpec, optional form field name, and resolved pathThe logic flow is clear and handles all edge cases (missing content type, encoding, etc.).
tavern/_plugins/graphql/request.py (2)
21-55: File argument handling is well-implemented.The
get_file_argumentsfunction correctly:
- Uses
ExitStackfor resource management (though the files aren't actually kept open)- Validates for duplicate form field names
- Builds
FileVarobjects with appropriate metadataThe duplicate name check provides good error messaging for users.
133-178: Request execution flow is well-structured.The
runmethod correctly:
- Handles both subscription and query/mutation flows
- Merges file arguments into variables when files are present
- Sets appropriate Content-Type header for non-file requests
- Catches
TransportQueryErrorseparately from general exceptionsThe header merging order (defaults first, then user headers) allows user overrides.
tavern/_plugins/common/response.py (2)
16-25: Good use of Protocol for response abstraction.The
ResponseLikeProtocol provides a clean interface that both REST and GraphQL responses can implement, enabling shared verification logic without tight coupling to specific response types.
86-124: Block validation logic is well-designed.The
_validate_blockmethod correctly:
- Handles the
$extblock check and raises an appropriate exception- Performs case-insensitive header comparison
- Maps "data" blockname to "json" strictness option
- Uses the configured strictness level for validation
tests/unit/plugins/graphql/test_graphql_request.py (2)
11-101: Comprehensive test coverage forGraphQLRequest.The
TestGraphQLRequestclass provides excellent coverage:
- Initialisation with valid/invalid requests
- Default value handling for optional fields
- Success and failure paths for the
runmethod- Proper header merging verification
103-297: Thorough file upload testing.The
TestGraphQLFileUploadsclass effectively tests:
- Single and multiple file uploads
- Content type specification
- Empty file arguments
- Combined files and variables
- Proper
FileVartype verificationThe use of
tmp_pathfor creating test files is the correct pytest approach.tavern/_plugins/rest/response.py (1)
17-18: Good refactoring to useCommonResponsebase class.The inheritance change reduces code duplication and centralises common response handling logic. This aligns well with the new GraphQL support that also uses
CommonResponse.tavern/_plugins/graphql/client.py (4)
1-26: LGTM!The imports and type definitions are well-organised and appropriate for a GraphQL client implementation with async support.
28-68: LGTM! Cache key implementation is correct.The
ClientCacheKeyimplementation properly ensures hashability by converting headers to a sorted tuple, and implements all necessary methods for use as a dictionary key. This caching mechanism addresses the connection pooling concern raised in previous reviews.
146-182: LGTM! Cleanup logic is well-structured.The context manager implementation properly handles resource cleanup with appropriate timeouts and error logging. Once the event loop busy-waiting issue (lines 135-142) is addressed, this shutdown mechanism will work more efficiently.
183-248: LGTM! Request handling with proper client caching.The implementation correctly caches GraphQL clients and transports per endpoint, which enables HTTP connection pooling and addresses the connection reuse concern from previous reviews. The error handling and file upload support are also properly implemented.
- Removed use of `threading.Event` for signaling shutdown in `tavern/_core/asyncio.py`. - Replaced `run_until_complete` with `run_forever` for loop execution in `ThreadedAsyncLoop.run`. - Updated `__exit__` to use `call_soon_threadsafe` for thread-safe loop stopping. - Ensured proper cleanup by verifying and explicitly closing the event loop if not already closed. - Simplified and clarified code structure for better maintainability.
- Added `global_db_session.commit()` after delete statements in `reset_db` function in `example/graphql/tavern_graphql_example/server.py` to ensure changes are persisted. - Ensures proper database state consistency during reset operation.
Make file extension handling case‑insensitive by lower‑casing the suffix. Read GraphQL include files with explicit UTF‑8 encoding to avoid Unicode issues on non‑default encodings.t l
extend include handling to accept .json and .graphql files and update the error message to list all supported extensions (yaml, yml, json, graphql).
- Added `usersFromCsv` query to `example/graphql/tavern_graphql_example/server.py` to allow fetching users by names from a CSV upload. - Updated schema to handle CSV file uploads using the `Upload` type. - Implemented logic to read and parse the CSV file and match user data from the database. - Created `example/graphql/testdata/test_users.csv` test data file for CSV-based user queries. - Enhanced `example/graphql/tests/test_files.tavern.yaml` to include integration tests for querying users using a CSV file.
Add graphql with basic subscriptions.
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.