Skip to content

Comments

feat: add StageTask and StageFunction -> StageTask#19

Closed
gouzil wants to merge 34 commits intoAgentEra:mainfrom
gouzil:feat/add_stage_task
Closed

feat: add StageTask and StageFunction -> StageTask#19
gouzil wants to merge 34 commits intoAgentEra:mainfrom
gouzil:feat/add_stage_task

Conversation

@gouzil
Copy link
Contributor

@gouzil gouzil commented Feb 27, 2025

feat

  • 将原有的 StageFunction 改为 StageTask
  • StageTask 增加取消
  • StageFunction 替换为较为常见的允许连续多次 __call__, 并且不需要 reset

NOTE

@gouzil gouzil requested a review from Copilot February 27, 2025 06:33
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This PR refactors the stage task API by renaming StageFunction to StageTask, adds cancellation support, and improves support for continuous call chaining. Key changes include updates in test cases (reducing sleep durations and removing skip markers), modifications in the StageResponse and StageDispatch logic for handling callbacks and auto-close behavior, and reorganization of StageFunction into distinct StageTask and StageFunction classes.

Reviewed Changes

File Description
tests/test_hybrid/test_Stage_EventEmitter.py Removed skip markers and added a new timeout test for StageEventEmitter.
tests/test_api/test_Stage_with.py Adjusted sleep durations and added tests for task decorators and callbacks.
agently_stage/StageResponse.py Refactored callback handling using dataclasses and TaskResult instead of dictionaries.
agently_stage/StageDispatch.py Enhanced auto-close and shutdown functionality with improved logging and exception handling.
agently_stage/StageFunction.py Refactored StageFunction into StageTask and StageFunction with added cancel support.
tests/test_api/test_EventEmitter.py Modified sleep duration in stress test to avoid long wait times.
tests/test_api/test_Listener.py Removed tests for StageListener as part of the refactoring.

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

self._cancel = False
return self

def cancel(self):
Copy link

Copilot AI Feb 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the cancel() method of StageTask, the _cancel attribute is set to False instead of True, which may prevent proper cancellation behavior. Consider setting _cancel to True to mark the task as canceled.

Copilot uses AI. Check for mistakes.
@gouzil gouzil requested review from Maplemx and Copilot February 27, 2025 06:39
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This PR introduces the new StageTask class to replace the legacy StageFunction with enhanced behavior including support for repeated calls and task cancellation. Key changes include the renaming of StageFunction to StageTask, the addition of a cancel mechanism for StageTask, and updates to tests and internal stage dispatch and response handling to accommodate these changes.

Reviewed Changes

File Description
tests/test_hybrid/test_Stage_EventEmitter.py Removed skipped markers and added a new timeout test for StageEventEmitter behavior.
tests/test_api/test_Stage_with.py Adjusted sleep durations and added tests for both stage function and task decorators.
agently_stage/StageDispatch.py Introduced tracemalloc startup, reworked shutdown handling, and adjusted active task counting.
agently_stage/StageResponse.py Refactored callback handling with structured data classes and enhanced result encapsulation.
agently_stage/StageFunction.py Added the new StageTask with cancellation support and retained StageFunction for backward compatibility.
tests/test_api/test_EventEmitter.py Reduced sleep time in stress test to speed up execution.

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

agently_stage/StageFunction.py:71

  • [nitpick] Having both StageTask and StageFunction classes in the same module may cause confusion about which one should be used. Consider deprecating or removing StageFunction if StageTask is intended as its full replacement.
class StageFunction:

Comment on lines 256 to 260
if self._dispatch_env._is_daemon:
with self._dispatch_env._active_tasks_lock:
self._dispatch_env._active_tasks -= 1
raise ValueError("func must be a coroutine function or coroutine")

Copy link

Copilot AI Feb 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decrementing the active_tasks counter in the error branch may lead to an underflow if the counter was not incremented for this branch. Please review the logic to ensure the counter remains consistent.

Suggested change
if self._dispatch_env._is_daemon:
with self._dispatch_env._active_tasks_lock:
self._dispatch_env._active_tasks -= 1
raise ValueError("func must be a coroutine function or coroutine")
raise ValueError("func must be a coroutine function or coroutine")
if self._dispatch_env._is_daemon:
with self._dispatch_env._active_tasks_lock:
self._dispatch_env._active_tasks += 1

Copilot uses AI. Check for mistakes.
@gouzil gouzil closed this Apr 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant