Add async get_raw_access_token() method to AsyncPipedream #264
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the WalkthroughTwo new asynchronous properties Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
…cking OAuth token fetching Co-authored-by: jverce <5479513+jverce@users.noreply.github.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/pipedream/pipedream.py(1 hunks)tests/custom/test_client.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/pipedream/pipedream.py (1)
src/pipedream/core/client_wrapper.py (1)
_get_token(43-47)
tests/custom/test_client.py (1)
src/pipedream/pipedream.py (5)
AsyncPipedream(67-133)Pipedream(16-64)raw_access_token(60-64)raw_access_token(111-120)async_raw_access_token(123-133)
🪛 Ruff (0.14.8)
tests/custom/test_client.py
16-16: Possible hardcoded password assigned to argument: "access_token"
(S106)
17-17: Possible hardcoded password assigned to: "raw_access_token"
(S105)
22-22: Possible hardcoded password assigned to argument: "access_token"
(S106)
23-23: Possible hardcoded password assigned to: "raw_access_token"
(S105)
28-28: Possible hardcoded password assigned to argument: "access_token"
(S106)
30-30: Possible hardcoded password assigned to: "token"
(S105)
37-37: Possible hardcoded password assigned to argument: "client_secret"
(S106)
49-49: Possible hardcoded password assigned to: "token"
(S105)
🔇 Additional comments (6)
tests/custom/test_client.py (4)
1-6: LGTM!The imports are appropriate for the new test cases. AsyncMock is correctly imported for mocking async token providers in the OAuth test.
14-17: LGTM!Test correctly verifies synchronous token access for the Pipedream client.
20-23: LGTM!Test correctly verifies the synchronous raw_access_token property on AsyncPipedream with a static token.
26-30: LGTM!Test correctly verifies the new async_raw_access_token property with a static token. The async/await pattern is properly used.
src/pipedream/pipedream.py (2)
113-120: LGTM!The updated docstring appropriately warns about potential blocking behavior when using OAuth and directs async application developers to the new
async_raw_access_tokenproperty.
122-133: The implementation is complete and correct.The async property properly initializes
_async_tokenfor OAuth flows (AsyncOAuthTokenProvider.get_token) inAsyncClient.__init__, while static tokens safely fall back to the synchronousraw_access_tokenproperty (which is non-blocking for string tokens). The test assertions attests/custom/test_client.py:42confirming_async_tokenis set for OAuth are valid. No naming inconsistency exists—the implementation correctly adds theasync_raw_access_tokenproperty as intended.
| async def test_async_pipedream_async_raw_access_token_with_oauth() -> None: | ||
| """Test AsyncPipedream async method with OAuth flow.""" | ||
| client = AsyncPipedream( | ||
| client_id="test-client-id", | ||
| client_secret="test-client-secret", | ||
| project_id="test-project", | ||
| ) | ||
|
|
||
| # The client should have _async_token set when using OAuth | ||
| assert client._client_wrapper._async_token is not None | ||
|
|
||
| # Mock the async token provider | ||
| client._client_wrapper._async_token = AsyncMock(return_value="mocked-oauth-token") | ||
|
|
||
| # Test the async method | ||
| token = await client.async_raw_access_token | ||
| assert token == "mocked-oauth-token" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for _async_token initialization and usage
echo "=== Searching for _async_token initialization ==="
rg -nP --type=py '_async_token\s*='
echo -e "\n=== Searching for _async_token references ==="
rg -nP --type=py '_async_token' -C3
echo -e "\n=== Locating AsyncClientWrapper class ==="
ast-grep --pattern 'class AsyncClientWrapper { $$$ }'
echo -e "\n=== Searching for AsyncClientWrapper in Python files ==="
rg -nP --type=py 'class AsyncClientWrapper'Repository: PipedreamHQ/pipedream-sdk-python
Length of output: 2919
🏁 Script executed:
#!/bin/bash
echo "=== Checking AsyncPipedream constructor ==="
rg -nP --type=py 'class AsyncPipedream' -A 30
echo -e "\n=== Checking how _async_token is set during initialization ==="
rg -nP --type=py 'async_token' -B2 -A2 | head -60Repository: PipedreamHQ/pipedream-sdk-python
Length of output: 4973
Remove the assertion on line 42 to avoid testing private implementation details.
The assertion assert client._client_wrapper._async_token is not None directly accesses a private attribute (_async_token), creating tight coupling to implementation details. Since the test verifies the actual behavior through the public API at line 48 (token = await client.async_raw_access_token), the assertion is redundant. If OAuth initialization fails, the behavioral test will catch it when attempting to retrieve the token. Removing line 42 makes the test cleaner and less dependent on internal state.
Note: Line 45 (mocking _async_token) is acceptable since there is no public setter and mocking is necessary to test the async token provider behavior.
🧰 Tools
🪛 Ruff (0.14.8)
37-37: Possible hardcoded password assigned to argument: "client_secret"
(S106)
49-49: Possible hardcoded password assigned to: "token"
(S105)
🤖 Prompt for AI Agents
In tests/custom/test_client.py around lines 33 to 49, remove the assertion that
accesses the private attribute (the line asserting
client._client_wrapper._async_token is not None) to avoid testing private
implementation details; simply delete that assert line and keep the mock of
_async_token and the behavioral await/verify of client.async_raw_access_token
as-is.
Description
raw_access_tokenperforms blocking network calls when using OAuth authentication, which blocks the event loop in async applications (FastAPI, Django ASGI, etc.).Changes
AsyncPipedream: Addedasync_raw_access_tokenasync property for non-blocking token retrievalraw_access_tokenproperty preserved for backwards compatibilityUsage
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.