Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add suport for dashscope openai client #2013

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

zhengwj533
Copy link
Contributor

@zhengwj533 zhengwj533 commented Feb 27, 2025

Support the openai online services provided by Alibaba.

Behavior:

  • Init dashscope client in openai. py.
  • Use dashscope client to execute task if modle name has a prefix like 'dashscope/' in openai. py

Important

Add support for Dashscope OpenAI clients in openai.py, handling models with 'dashscope/' prefix.

  • Behavior:
    • Initialize Dashscope clients in openai.py if DASHSCOPE_API_KEY and DASHSCOPE_API_BASE are set.
    • Use Dashscope client for models with 'dashscope/' prefix in _get_client_and_model() and _get_async_client_and_model().
  • Error Handling:
    • Raise ValueError if 'dashscope/' prefix is used without Dashscope credentials in _get_client_and_model() and _get_async_client_and_model().

This description was created by Ellipsis for 89b5375. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 89b5375 in 46 seconds

More details
  • Looked at 62 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 drafted comments based on config settings.
1. py/core/providers/llm/openai.py:16
  • Draft comment:
    Initialize dashscope_client and async_dashscope_client to None along with the other client attributes. Without this, referencing them in the any() check may trigger an AttributeError when the env vars are not set.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. py/core/providers/llm/openai.py:131
  • Draft comment:
    Dashscope client initialization looks correct. Consider refactoring common client setup logic and updating documentation for DASHSCOPE_API_KEY and DASHSCOPE_API_BASE.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
3. py/core/providers/llm/openai.py:206
  • Draft comment:
    Dashscope model prefix extraction is implemented; consider aligning substring handling (e.g., trimming whitespace) for consistency with other providers.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
4. py/core/providers/llm/openai.py:265
  • Draft comment:
    Async dashscope client handling is consistent with sync logic. No issues found, but consider adding inline comments for clarity.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None

Workflow ID: wflow_rEfqPbypzri8D8lM


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

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