Skip to content

funasr support payload.input and bug fix in transcription#142

Closed
songguocola wants to merge 2 commits into
dashscope:mainfrom
songguocola:dev/audio_0618_2
Closed

funasr support payload.input and bug fix in transcription#142
songguocola wants to merge 2 commits into
dashscope:mainfrom
songguocola:dev/audio_0618_2

Conversation

@songguocola

Copy link
Copy Markdown
Contributor

No description provided.

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request introduces a wait_timeout parameter to the asynchronous and synchronous wait methods in base_api.py and transcription.py to handle task timeouts, and updates _only_parameters in api_request_data.py to extract raw_input from parameters. The review feedback highlights that mutating self.parameters in-place introduces side effects and recommends copying it first. Additionally, it is suggested to use time.monotonic() instead of time.time() for robust elapsed time measurements, and to update BaseAsyncApi.call() to pass wait_timeout so that synchronous calls respect the timeout.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

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.

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().

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

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")

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()

# (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):

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()

# (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):

@songguocola songguocola deleted the dev/audio_0618_2 branch June 16, 2026 08:11
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