Skip to content
Merged
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
12 changes: 12 additions & 0 deletions dashscope/aigc/generation.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,12 @@ def call(
to_merge_incremental_output = True
parameters['incremental_output'] = True

# Pass incremental_to_full flag via headers user-agent
if 'headers' not in parameters:
parameters['headers'] = {}
flag = '1' if to_merge_incremental_output else '0'
parameters['headers']['user-agent'] = f'incremental_to_full/{flag}'
Comment on lines +155 to +156
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The current implementation overwrites the user-agent header. This could lead to loss of information if a user-agent was already present. It's better to append to the existing user-agent. The suggested change ensures the new user-agent part is appended correctly, handling cases where the user-agent does not exist.

        flag = '1' if to_merge_incremental_output else '0'
        new_ua_part = f'incremental_to_full/{flag}'
        existing_ua = parameters['headers'].get('user-agent', '')
        parameters['headers']['user-agent'] = f'{existing_ua}; {new_ua_part}' if existing_ua else new_ua_part


response = super().call(model=model,
task_group=task_group,
task=Generation.task,
Expand Down Expand Up @@ -354,6 +360,12 @@ async def call(
to_merge_incremental_output = True
parameters['incremental_output'] = True

# Pass incremental_to_full flag via headers user-agent
if 'headers' not in parameters:
parameters['headers'] = {}
flag = '1' if to_merge_incremental_output else '0'
parameters['headers']['user-agent'] = f'incremental_to_full/{flag}'
Comment on lines +366 to +367
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Similar to the synchronous call method, the user-agent header is being overwritten. This can lead to loss of information if a user-agent was already present. The user-agent should be appended to, not replaced.

        flag = '1' if to_merge_incremental_output else '0'
        new_ua_part = f'incremental_to_full/{flag}'
        existing_ua = parameters['headers'].get('user-agent', '')
        parameters['headers']['user-agent'] = f'{existing_ua}; {new_ua_part}' if existing_ua else new_ua_part


response = await super().call(model=model,
task_group=task_group,
task=Generation.task,
Expand Down
15 changes: 15 additions & 0 deletions dashscope/aigc/multimodal_conversation.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,12 @@ def call(
to_merge_incremental_output = True
kwargs['incremental_output'] = True

# Pass incremental_to_full flag via headers user-agent
if 'headers' not in kwargs:
kwargs['headers'] = {}
flag = '1' if to_merge_incremental_output else '0'
kwargs['headers']['user-agent'] = f'incremental_to_full/{flag}'
Comment on lines +126 to +127
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The user-agent header is being overwritten, which could discard existing user-agent information. It should be appended to instead. This is also inconsistent with the aio_call method in the same file which attempts to append.

        flag = '1' if to_merge_incremental_output else '0'
        new_ua_part = f'incremental_to_full/{flag}'
        existing_ua = kwargs['headers'].get('user-agent', '')
        kwargs['headers']['user-agent'] = f'{existing_ua}; {new_ua_part}' if existing_ua else new_ua_part


response = super().call(model=model,
task_group=task_group,
task=MultiModalConversation.task,
Expand Down Expand Up @@ -287,6 +293,15 @@ async def call(
to_merge_incremental_output = True
kwargs['incremental_output'] = True

# Pass incremental_to_full flag via headers user-agent
if 'headers' not in kwargs:
kwargs['headers'] = {}
flag = '1' if to_merge_incremental_output else '0'
kwargs['headers']['user-agent'] = (
kwargs['headers'].get('user-agent', '') +
f'; incremental_to_full/{flag}'
)
Comment on lines +300 to +303
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The current logic for appending to the user-agent has a few issues. If user-agent is not already in the headers, it will prepend a semicolon, resulting in an invalid format like ; incremental_to_full/0. Additionally, it doesn't add a space after the semicolon, which is inconsistent with the convention in base_request.py.

        new_ua_part = f'incremental_to_full/{flag}'
        existing_ua = kwargs['headers'].get('user-agent', '')
        kwargs['headers']['user-agent'] = f'{existing_ua}; {new_ua_part}' if existing_ua else new_ua_part


response = await super().call(model=model,
task_group=task_group,
task=AioMultiModalConversation.task,
Expand Down
7 changes: 5 additions & 2 deletions dashscope/api_entities/aiohttp_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ def __init__(self,
async_request: bool = False,
query: bool = False,
timeout: int = DEFAULT_REQUEST_TIMEOUT_SECONDS,
task_id: str = None) -> None:
task_id: str = None,
user_agent: str = '') -> None:
"""HttpSSERequest, processing http server sent event stream.

Args:
Expand All @@ -33,9 +34,11 @@ def __init__(self,
stream (bool, optional): Is stream request. Defaults to True.
timeout (int, optional): Total request timeout.
Defaults to DEFAULT_REQUEST_TIMEOUT_SECONDS.
user_agent (str, optional): Additional user agent string to
append. Defaults to ''.
"""

super().__init__()
super().__init__(user_agent=user_agent)
self.url = url
self.async_request = async_request
self.headers = {
Expand Down
18 changes: 13 additions & 5 deletions dashscope/api_entities/api_request_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,15 @@ def _get_protocol_params(kwargs):
base_address = kwargs.pop('base_address', None)
flattened_output = kwargs.pop('flattened_output', False)
extra_url_parameters = kwargs.pop('extra_url_parameters', None)

# Extract user-agent from headers if present
user_agent = ''
if headers and 'user-agent' in headers:
user_agent = headers.pop('user-agent')

return (api_protocol, ws_stream_mode, is_binary_input, http_method, stream,
async_request, query, headers, request_timeout, form, resources,
base_address, flattened_output, extra_url_parameters)
base_address, flattened_output, extra_url_parameters, user_agent)


def _build_api_request(model: str,
Expand All @@ -46,8 +52,8 @@ def _build_api_request(model: str,
**kwargs):
(api_protocol, ws_stream_mode, is_binary_input, http_method, stream,
async_request, query, headers, request_timeout, form, resources,
base_address, flattened_output,
extra_url_parameters) = _get_protocol_params(kwargs)
base_address, flattened_output, extra_url_parameters,
user_agent) = _get_protocol_params(kwargs)
task_id = kwargs.pop('task_id', None)
enable_encryption = kwargs.pop('enable_encryption', False)
encryption = None
Expand Down Expand Up @@ -87,7 +93,8 @@ def _build_api_request(model: str,
timeout=request_timeout,
task_id=task_id,
flattened_output=flattened_output,
encryption=encryption)
encryption=encryption,
user_agent=user_agent)
elif api_protocol == ApiProtocol.WEBSOCKET:
if base_address is not None:
websocket_url = base_address
Expand All @@ -101,7 +108,8 @@ def _build_api_request(model: str,
is_binary_input=is_binary_input,
timeout=request_timeout,
flattened_output=flattened_output,
pre_task_id=pre_task_id)
pre_task_id=pre_task_id,
user_agent=user_agent)
else:
raise UnsupportedApiProtocol(
'Unsupported protocol: %s, support [http, https, websocket]' %
Expand Down
7 changes: 6 additions & 1 deletion dashscope/api_entities/base_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@


class BaseRequest(ABC):
def __init__(self) -> None:
def __init__(self, user_agent: str = '') -> None:
try:
platform_info = platform.platform()
except Exception:
Expand All @@ -26,6 +26,11 @@ def __init__(self) -> None:
platform_info,
processor_info,
)

# Append user_agent if provided and not empty
if user_agent:
ua += '; ' + user_agent

self.headers = {'user-agent': ua}
disable_data_inspection = os.environ.get(
DASHSCOPE_DISABLE_DATA_INSPECTION_ENV, 'true')
Expand Down
7 changes: 5 additions & 2 deletions dashscope/api_entities/http_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ def __init__(self,
timeout: int = DEFAULT_REQUEST_TIMEOUT_SECONDS,
task_id: str = None,
flattened_output: bool = False,
encryption: Optional[Encryption] = None) -> None:
encryption: Optional[Encryption] = None,
user_agent: str = '') -> None:
"""HttpSSERequest, processing http server sent event stream.

Args:
Expand All @@ -43,9 +44,11 @@ def __init__(self,
stream (bool, optional): Is stream request. Defaults to True.
timeout (int, optional): Total request timeout.
Defaults to DEFAULT_REQUEST_TIMEOUT_SECONDS.
user_agent (str, optional): Additional user agent string to
append. Defaults to ''.
"""

super().__init__()
super().__init__(user_agent=user_agent)
self.url = url
self.flattened_output = flattened_output
self.async_request = async_request
Expand Down
3 changes: 2 additions & 1 deletion dashscope/api_entities/websocket_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,9 @@ def __init__(
timeout: int = DEFAULT_REQUEST_TIMEOUT_SECONDS,
flattened_output: bool = False,
pre_task_id=None,
user_agent: str = '',
) -> None:
super().__init__()
super().__init__(user_agent=user_agent)
"""HttpRequest.

Args:
Expand Down