Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions dashscope/api_entities/api_request_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ def get_batch_binary_data(self) -> bytes: # type: ignore[return]

def _only_parameters(self) -> str:
obj = {"model": self.model, "parameters": self.parameters, "input": {}}
if "raw_input" in self.parameters:
obj["input"] = self.parameters.pop("raw_input")
Comment on lines 160 to +162

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Mutating self.parameters in-place inside _only_parameters() introduces side effects. If the request is retried, logged, or reused, raw_input will be permanently missing from self.parameters. Copying self.parameters before popping avoids this side effect.

Suggested change
obj = {"model": self.model, "parameters": self.parameters, "input": {}}
if "raw_input" in self.parameters:
obj["input"] = self.parameters.pop("raw_input")
parameters = self.parameters.copy()
obj = {"model": self.model, "parameters": parameters, "input": {}}
if "raw_input" in parameters:
obj["input"] = parameters.pop("raw_input")

if self.task is not None:
obj["task"] = self.task
if self.task_group is not None:
Expand Down
7 changes: 7 additions & 0 deletions dashscope/audio/asr/transcription.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,14 +167,20 @@ def wait(
task: Union[str, TranscriptionResponse], # type: ignore[override]
api_key: str = None,
workspace: str = None,
wait_timeout: int = -1,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Note that Transcription.call() delegates to BaseAsyncApi.call(), which currently does not pass **kwargs or wait_timeout to cls.wait(). Consequently, passing wait_timeout to Transcription.call() will have no effect. To fix this, BaseAsyncApi.call() in dashscope/client/base_api.py needs to be updated to pass **kwargs or wait_timeout to cls.wait().

**kwargs,
) -> TranscriptionResponse:
"""Poll task until the final results of transcription is obtained.

Args:
task (Union[str, TranscriptionResponse]): The task_id or
response including task_id returned from async_call().
api_key (str, optional): The api_key. Defaults to None.
workspace (str): The dashscope workspace id.
wait_timeout (int, optional): The timeout for waiting.
Defaults to -1.That means no timeout.
If set to a value > 0, the task does not complete
within this time, a timeout error response will be returned.

Returns:
TranscriptionResponse: The result of batch transcription.
Expand All @@ -183,6 +189,7 @@ def wait(
task,
api_key=api_key,
workspace=workspace,
wait_timeout=wait_timeout,
**kwargs,
)
return TranscriptionResponse.from_api_response(response)
Expand Down
46 changes: 46 additions & 0 deletions dashscope/client/base_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ async def wait(
task: Union[str, DashScopeAPIResponse],
api_key: str = None,
workspace: str = None,
wait_timeout: int = -1,
**kwargs,
) -> DashScopeAPIResponse:
"""Wait for async task completion and return task result.
Expand All @@ -199,6 +200,12 @@ async def wait(
task (Union[str, DashScopeAPIResponse]): The task_id, or
async_call response.
api_key (str, optional): The api_key. Defaults to None.
workspace (str, optional): The dashscope workspace id.
wait_timeout (int, optional): The maximum seconds to wait
for the task to complete. Default is -1, which means no
timeout. When set to a value > 0, if the task does not
complete within this time, a timeout error response will
be returned.

Returns:
DashScopeAPIResponse: The async task information.
Expand All @@ -208,6 +215,7 @@ async def wait(
max_wait_seconds = 5
increment_steps = 3
step = 0
start_time = time.time()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Use time.monotonic() instead of time.time() to measure elapsed time. time.monotonic() is unaffected by system clock updates or adjustments, making it much more reliable for timeout calculations.

Suggested change
start_time = time.time()
start_time = time.monotonic()

while True:
step += 1
# we start by querying once every second, and double
Expand All @@ -217,6 +225,21 @@ async def wait(
# (server side return immediately when ready)
if wait_seconds < max_wait_seconds and step % increment_steps == 0:
wait_seconds = min(wait_seconds * 2, max_wait_seconds)
if 0 < wait_timeout <= (time.time() - start_time):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Use time.monotonic() instead of time.time() to measure elapsed time consistently with start_time.

Suggested change
if 0 < wait_timeout <= (time.time() - start_time):
if 0 < wait_timeout <= (time.monotonic() - start_time):

logger.warning(
"Wait task: %s timeout after %s seconds.",
task_id,
wait_timeout,
)
return DashScopeAPIResponse(
request_id=task_id,
status_code=HTTPStatus.REQUEST_TIMEOUT,
code="WaitTaskTimeout",
message=(
f"Wait task: {task_id} timeout after "
f"{wait_timeout} seconds."
),
)
rsp = await cls._get(
task_id,
api_key,
Expand Down Expand Up @@ -767,6 +790,7 @@ def wait(
task: Union[str, DashScopeAPIResponse],
api_key: str = None,
workspace: str = None,
wait_timeout: int = -1,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

While wait_timeout is now accepted by BaseAsyncApi.wait(), note that BaseAsyncApi.call() (at line 618) does not pass **kwargs or wait_timeout to cls.wait(). This means any wait_timeout passed to synchronous call() methods (such as Transcription.call(..., wait_timeout=60)) will be silently ignored. Please update BaseAsyncApi.call() to pass wait_timeout or **kwargs to cls.wait() so that the timeout is respected in synchronous calls.

**kwargs,
) -> DashScopeAPIResponse:
"""Wait for async task completion and return task result.
Expand All @@ -775,6 +799,12 @@ def wait(
task (Union[str, DashScopeAPIResponse]): The task_id, or
async_call response.
api_key (str, optional): The api_key. Defaults to None.
workspace (str, optional): The dashscope workspace id.
wait_timeout (int, optional): The maximum seconds to wait
for the task to complete. Default is -1, which means no
timeout. When set to a value > 0, if the task does not
complete within this time, a timeout error response will
be returned.

Returns:
DashScopeAPIResponse: The async task information.
Expand All @@ -784,6 +814,7 @@ def wait(
max_wait_seconds = 5
increment_steps = 3
step = 0
start_time = time.time()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Use time.monotonic() instead of time.time() to measure elapsed time. time.monotonic() is unaffected by system clock updates or adjustments, making it much more reliable for timeout calculations.

Suggested change
start_time = time.time()
start_time = time.monotonic()

while True:
step += 1
# we start by querying once every second, and double
Expand All @@ -794,6 +825,21 @@ def wait(
# (server side return immediately when ready)
if wait_seconds < max_wait_seconds and step % increment_steps == 0:
wait_seconds = min(wait_seconds * 2, max_wait_seconds)
if 0 < wait_timeout <= (time.time() - start_time):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Use time.monotonic() instead of time.time() to measure elapsed time consistently with start_time.

Suggested change
if 0 < wait_timeout <= (time.time() - start_time):
if 0 < wait_timeout <= (time.monotonic() - start_time):

logger.warning(
"Wait task: %s timeout after %s seconds.",
task_id,
wait_timeout,
)
return DashScopeAPIResponse(
request_id=task_id,
status_code=HTTPStatus.REQUEST_TIMEOUT,
code="WaitTaskTimeout",
message=(
f"Wait task: {task_id} timeout after "
f"{wait_timeout} seconds."
),
)
rsp = cls._get(task_id, api_key, workspace=workspace, **kwargs)
if rsp.status_code == HTTPStatus.OK:
if rsp.output is None:
Expand Down
Loading