refactor: sync remove ignore_header_params behavior from sdk-gen - #423
Closed
chyroc wants to merge 1 commit into
Closed
refactor: sync remove ignore_header_params behavior from sdk-gen#423chyroc wants to merge 1 commit into
chyroc wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe changes add a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #423 +/- ##
=======================================
Coverage 91.75% 91.75%
=======================================
Files 105 105
Lines 9542 9548 +6
=======================================
+ Hits 8755 8761 +6
Misses 787 787
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cozepy/datasets/__init__.py`:
- Around line 252-259: Add a docstring entry for the new required parameter
agw_js_conv in both the synchronous and asynchronous process methods so
generated API docs are accurate: update the docstrings for the process(...)
method and the async process(...) method to include a ":param agw_js_conv:
<short description>" line (describe that it is the AGW JS conversation
token/string used for X processing or routing), and if present add a ":type
agw_js_conv: str" line to match existing style; ensure the parameter name
matches exactly ("agw_js_conv") and place the entry alongside the other :param
lines for dataset_id and document_ids.
In `@tests/test_datasets.py`:
- Line 176: The tests call coze.datasets.process(..., agw_js_conv="str") but
never assert that the outbound request contains the Agw-Js-Conv header; update
the test helper mock_process_datasets to return the route (or response) object
and in both test functions (the one using res =
coze.datasets.process(dataset_id=dataset_id, document_ids=[document_id],
agw_js_conv="str") and the other at line ~250) add assertions that the
route/request headers include "Agw-Js-Conv" with value "str" (e.g., check
route.request.headers.get("Agw-Js-Conv") == "str" or equivalent), ensuring you
reference the returned route object from mock_process_datasets when making the
assertion.
| dataset_id, document_id, mock_logid = mock_process_datasets(respx_mock) | ||
|
|
||
| res = coze.datasets.process(dataset_id=dataset_id, document_ids=[document_id]) | ||
| res = coze.datasets.process(dataset_id=dataset_id, document_ids=[document_id], agw_js_conv="str") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's find and examine the test file
rg -n -C5 'test_(sync|async)_datasets_process|agw_js_conv' tests/test_datasets.py | head -100Repository: coze-dev/coze-py
Length of output: 1235
🏁 Script executed:
#!/bin/bash
# Check the mock_process_datasets function and respx setup
rg -n -B5 -A20 'def mock_process_datasets' tests/test_datasets.pyRepository: coze-dev/coze-py
Length of output: 937
🏁 Script executed:
#!/bin/bash
# Check imports and test class decorators
head -50 tests/test_datasets.pyRepository: coze-dev/coze-py
Length of output: 1587
🏁 Script executed:
#!/bin/bash
# Check if there are any existing header assertions using respx in the test file
rg -n 'respx_mock\.calls|\.request\.headers|\.headers\[' tests/test_datasets.pyRepository: coze-dev/coze-py
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Check if respx is used with pytest decorator
rg -n '@pytest.mark.respx|respx' tests/test_datasets.py | head -20Repository: coze-dev/coze-py
Length of output: 1141
🏁 Script executed:
#!/bin/bash
# Check the entire test class structure to see test decorators
sed -n '108,180p' tests/test_datasets.pyRepository: coze-dev/coze-py
Length of output: 2561
🏁 Script executed:
#!/bin/bash
# Check if respx_mock.calls pattern is used anywhere in the test suite
rg -r 'calls\[-?[0-9]\]\.request|\.request\.headers' --type pyRepository: coze-dev/coze-py
Length of output: 456
🏁 Script executed:
#!/bin/bash
# Check if respx_mock.calls pattern is used anywhere in the test suite
rg 'calls\[-?[0-9]\]\.request|\.request\.headers' --type pyRepository: coze-dev/coze-py
Length of output: 926
🏁 Script executed:
#!/bin/bash
# Check how headers are verified in existing tests
rg -B2 -A2 'calls\[0\]\.request\.headers|route\.calls' tests/test_connectors.py | head -40Repository: coze-dev/coze-py
Length of output: 942
🏁 Script executed:
#!/bin/bash
# Check respx API documentation and how to access request headers
# First, let's see if respx provides .calls attribute on the mock object
rg -B5 -A5 'respx_mock\.' tests/test_workflows_chat.py | head -50Repository: coze-dev/coze-py
Length of output: 1209
🏁 Script executed:
#!/bin/bash
# Get the full context of how route.calls is used
rg -B5 -A10 'route\.calls\[0\]\.request' tests/test_workflows_chat.py | head -60Repository: coze-dev/coze-py
Length of output: 733
🏁 Script executed:
#!/bin/bash
# Check how mock_process_datasets is used and whether respx_mock.calls is available
# Let's look for documentation or examples of respx.calls in tests
rg 'respx_mock\.calls|respx_mock\.calls\[-?[0-9]\]' --type pyRepository: coze-dev/coze-py
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Check if respx_mock itself has a calls attribute or if we need to store route
# Let's look at the complete mock_process_datasets implementation
sed -n '79,105p' tests/test_datasets.pyRepository: coze-dev/coze-py
Length of output: 945
🏁 Script executed:
#!/bin/bash
# Let's check how other tests that need to inspect requests handle it
# Look at test_workflows_chat.py to see how route is captured
sed -n '45,85p' tests/test_workflows_chat.pyRepository: coze-dev/coze-py
Length of output: 1419
🏁 Script executed:
#!/bin/bash
# Check if respx_mock object itself has .calls attribute
# by searching for any usage of respx_mock.calls
rg 'respx_mock\.calls' --type py -A2Repository: coze-dev/coze-py
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Let's verify the respx API by checking what respx returns
# and by looking at the pattern used in test_datasets_documents.py
rg -B10 -A10 'route\.calls\[0\]\.request' tests/test_datasets_documents.py | head -60Repository: coze-dev/coze-py
Length of output: 942
Add assertions for the new Agw-Js-Conv header behavior in the process tests.
Lines 176 and 250 update the call signature with agw_js_conv="str", but the tests don't verify that this header is actually sent in the outbound request.
Update mock_process_datasets to return the route object, then add header assertions to both test functions:
Suggested test assertions
def mock_process_datasets(respx_mock):
dataset_id = random_hex(10)
document_id = random_hex(10)
logid = random_hex(10)
- respx_mock.post(f"/v1/datasets/{dataset_id}/process").mock(
+ route = respx_mock.post(f"/v1/datasets/{dataset_id}/process").mock(
httpx.Response(...)
)
- return dataset_id, document_id, logid
+ return dataset_id, document_id, logid, route def test_sync_datasets_process(self, respx_mock):
coze = Coze(auth=TokenAuth(token="token"))
- dataset_id, document_id, mock_logid = mock_process_datasets(respx_mock)
+ dataset_id, document_id, mock_logid, route = mock_process_datasets(respx_mock)
res = coze.datasets.process(dataset_id=dataset_id, document_ids=[document_id], agw_js_conv="str")
assert res
assert res.response.logid == mock_logid
+ assert route.calls[0].request.headers["Agw-Js-Conv"] == "str" async def test_async_datasets_process(self, respx_mock):
coze = AsyncCoze(auth=AsyncTokenAuth(token="token"))
- dataset_id, document_id, mock_logid = mock_process_datasets(respx_mock)
+ dataset_id, document_id, mock_logid, route = mock_process_datasets(respx_mock)
res = await coze.datasets.process(dataset_id=dataset_id, document_ids=[document_id], agw_js_conv="str")
assert res
assert res.response.logid == mock_logid
+ assert route.calls[0].request.headers["Agw-Js-Conv"] == "str"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_datasets.py` at line 176, The tests call
coze.datasets.process(..., agw_js_conv="str") but never assert that the outbound
request contains the Agw-Js-Conv header; update the test helper
mock_process_datasets to return the route (or response) object and in both test
functions (the one using res = coze.datasets.process(dataset_id=dataset_id,
document_ids=[document_id], agw_js_conv="str") and the other at line ~250) add
assertions that the route/request headers include "Agw-Js-Conv" with value "str"
(e.g., check route.request.headers.get("Agw-Js-Conv") == "str" or equivalent),
ensuring you reference the returned route object from mock_process_datasets when
making the assertion.
Collaborator
Author
|
Closing per request. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ignore_header_params-based behavior in generated Python SDK output.datasets.processnow follows swagger header definition and requiresagw_js_convin both sync/async clients.tests/test_datasets.pycall sites to passagw_js_conv="str".Behavior Changes
Coze.datasets.process(...)now requires keyword argagw_js_conv.AsyncCoze.datasets.process(...)now requires keyword argagw_js_conv.Agw-Js-Convfrom the explicit method argument.Validation
./scripts/genpy.sh --output-sdk exist-repo/coze-py --ci-checkSummary by CodeRabbit
agw_js_convparameter to the dataset processing methods, enabling users to specify custom header values for dataset operations in both synchronous and asynchronous workflows.