-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat: Add app_id support to workflow #181
Conversation
WalkthroughThe changes update the Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant SyncClient as WorkflowsRunsClient
participant AsyncClient as AsyncWorkflowsRunsClient
participant Util as remove_none_values
participant Server
Note over User: Synchronous flow
User->>SyncClient: create(workflow_id, parameters, bot_id, app_id)
SyncClient->>Util: remove_none_values(payload)
Util-->>SyncClient: cleansed payload
SyncClient->>Server: send request with cleansed payload
Note over User: Asynchronous flow
User->>AsyncClient: create(workflow_id, parameters, bot_id, app_id)
AsyncClient->>Util: remove_none_values(payload)
Util-->>AsyncClient: cleansed payload
AsyncClient->>Server: send async request with cleansed payload
Possibly related PRs
Poem
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 0
🧹 Nitpick comments (1)
cozepy/workflows/runs/__init__.py (1)
183-184
: Enhance the docstring for app_id parameter.The docstring for
app_id
is incomplete. Please provide more details about:
- What types of workflow executions require an app_id
- Whether it's optional or required for specific scenarios
- Any constraints or format requirements for the app_id value
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cozepy/workflows/runs/__init__.py
(10 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test (Python 3.8 on macOS)
🔇 Additional comments (4)
cozepy/workflows/runs/__init__.py (4)
9-9
: LGTM!The import of
remove_none_values
is correctly placed and consistently used throughout the code to clean up request payloads.
166-166
: LGTM!The implementation of
app_id
support is consistent across methods and includes proper payload cleaning usingremove_none_values
.Also applies to: 193-193, 197-197, 205-205, 228-228, 236-236
317-317
: Enhance the docstring for app_id parameter.The docstring for
app_id
in the async methods is incomplete. Please provide more details about:
- What types of workflow executions require an app_id
- Whether it's optional or required for specific scenarios
- Any constraints or format requirements for the app_id value
Also applies to: 353-353
300-300
: LGTM!The implementation of
app_id
support in async methods mirrors the synchronous implementation, maintaining consistency across the codebase.Also applies to: 327-327, 331-331, 339-339, 362-362, 370-370
dd640e7
to
f730696
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
@@ Coverage Diff @@
## main #181 +/- ##
=======================================
Coverage 90.02% 90.02%
=======================================
Files 65 65
Lines 5845 5845
=======================================
Hits 5262 5262
Misses 583 583
|
f730696
to
5ce228b
Compare
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: 0
🧹 Nitpick comments (3)
examples/workflow_stream.py (1)
22-24
: Consider adding error handling for JSON parsing.While the default empty object fallback is good, invalid JSON in COZE_PARAMETERS would raise a JSONDecodeError.
-parameters = json.loads(os.getenv("COZE_PARAMETERS") or "{}") +try: + parameters = json.loads(os.getenv("COZE_PARAMETERS") or "{}") +except json.JSONDecodeError: + print("Warning: Invalid JSON in COZE_PARAMETERS, using empty object") + parameters = {}cozepy/workflows/runs/__init__.py (2)
166-166
: Add more details to the app_id parameter documentation.The current documentation is incomplete. Consider adding information about when app_id is required and its purpose.
- :param app_id: The app_id is required for some workflow executions, + :param app_id: The application ID required for workflow executions that interact with specific + application resources or require application-level context. This is typically needed for workflows + that access application-specific data or integrate with application-specific services.Also applies to: 183-183
219-219
: Add more details to the stream method's app_id documentation.The app_id documentation in the stream methods should be as detailed as the create methods.
- :param app_id: The app_id is required for some workflow executions, + :param app_id: The application ID required for workflow executions that interact with specific + application resources or require application-level context. This is typically needed for workflows + that access application-specific data or integrate with application-specific services.Also applies to: 353-353
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cozepy/workflows/runs/__init__.py
(10 hunks)examples/workflow_async.py
(1 hunks)examples/workflow_stream.py
(3 hunks)tests/test_workflows.py
(0 hunks)
💤 Files with no reviewable changes (1)
- tests/test_workflows.py
✅ Files skipped from review due to trivial changes (1)
- examples/workflow_async.py
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test (Python 3.8 on Windows)
🔇 Additional comments (5)
examples/workflow_stream.py (2)
5-5
: LGTM!The json import is correctly added to support JSON parsing of parameters.
50-50
: LGTM!The parameters are correctly passed to the stream method.
cozepy/workflows/runs/__init__.py (3)
9-9
: LGTM!The remove_none_values utility is correctly imported for payload sanitization.
197-197
: LGTM!The remove_none_values utility is correctly used to sanitize request payloads, ensuring no None values are sent to the API.
Also applies to: 236-237, 370-371
193-194
: LGTM!The app_id parameter is consistently added to all relevant request payloads across both synchronous and asynchronous implementations.
Also applies to: 227-229, 327-328, 362-363
Summary by CodeRabbit