Add public SodaCloud.execute_query seam#2734
Conversation
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a public SodaCloud.execute_query(query_json_dict, request_log_name) method as a supported seam for issuing CQRS “query” requests to Soda Cloud, while keeping the existing private _execute_query as a backwards-compatible alias for internal callers.
Changes:
- Added public
SodaCloud.execute_query(...)that delegates to the existing CQRS request helper. - Converted
_execute_query(...)into a thin alias that forwards toexecute_query(...). - Added a unit test covering that
execute_queryPOSTs to the/api/queryendpoint and injects the auth token into the request body.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| soda-tests/tests/unit/test_soda_cloud.py | Adds a unit test for the new execute_query public seam and its HTTP request shape. |
| soda-core/src/soda_core/common/soda_cloud.py | Introduces execute_query as a public method and keeps _execute_query as a compatibility alias. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return self._execute_cqrs_request( | ||
| request_type="query", request_log_name=request_log_name, request_body=query_json_dict, is_retry=True | ||
| ) |
There was a problem hiding this comment.
Fixed: execute_query now passes query_json_dict.copy(), so the token injected by _execute_cqrs_request no longer mutates the caller's dict. Added a regression assertion that the input dict has no token after the call.
Note: #2734 was auto-closed by a branch rename (aligning the branch name with soda-extensions#414 for joint CI) — the change now lives in #2738.
| called = mock_post.call_args | ||
| assert called.kwargs["url"].endswith("/api/query") | ||
| assert called.kwargs["json"]["type"] == "someQueryType" | ||
| assert called.kwargs["json"]["token"] == "some_token" |
There was a problem hiding this comment.
Done: the seam test now uses assert_called_once_with(url=..., headers=..., json=...) to validate the full request (url/headers/full json body incl. token), matching test_fetch_contract and the other tests in this file. (Carried into #2738.)
- execute_query: copy the caller's dict so the downstream token injection no longer mutates caller input (Copilot review on #2734); strengthen the seam test to assert the full request and that the input dict is left untouched. - Add SodaCloud.execute_dataset_query(): builds the {type, dataset} envelope, issues via execute_query, robustly parses the body, and maps the shared Cloud failures (datasource/dataset not-found, non-200, missing/non-JSON bodies) to typed exceptions. _parse_json_body() makes non-JSON gateway error pages surface as a clean SodaCloudException instead of a raw JSONDecodeError. - Refactor fetch_contract_for_dataset and fetch_data_source_configuration_for_dataset onto the new helper. - Add execute_dataset_query unit tests (happy path, error matrix, non-JSON). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Promote the existing private
_execute_queryto a public, feature-neutralSodaCloud.execute_query(query_json_dict, request_log_name), so callers can issue CQRS queries to Soda Cloud through a supported, stable method instead of reaching into a private one._execute_querybecomes a thin backwards-compatible alias (existing internal callers unchanged).Test Plan
test_soda_cloud.py— new test assertsexecute_queryPOSTs to/api/query, injects the auth token, and passes the query body through; all pre-existing tests green.🤖 Generated with Claude Code