Skip to content

feat: Vllm url support v1 #4010

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.

Sign up for GitHub

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

Merged
zhanweizhang7 merged 1 commit into from
Sep 9, 2025
Merged

feat: Vllm url support v1 #4010

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 5 additions & 2 deletions apps/chat/serializers/chat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,12 @@ def generate_prompt(self, instance: dict, with_valid=True):
q = prompt.replace("{userInput}", message)
messages[-1]['content'] = q

model_exist = QuerySet(Model).filter(workspace_id=workspace_id, id=model_id).exists()
model_exist = QuerySet(Model).filter(workspace_id=workspace_id,
id=model_id,
model_type = "LLM"
).exists()
if not model_exist:
raise Exception(_("model does not exists"))
raise Exception(_("Model does not exists or is not an LLM model"))

def process():
model = get_model_instance_by_model_workspace_id(model_id=model_id, workspace_id=workspace_id)
Copy link
Copy Markdown
Contributor Author

@shaohuzhang1 shaohuzhang1 Sep 9, 2025

Choose a reason for hiding this comment

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

This code snippet contains some minor issues and can be optimized:

Issues/Improvements:

  1. The if condition checks both existence of the model in the database (QuerySet(Model).filter(...).exists()) and its type (model.model_type == "LLM"). However, it should only check if the model exists before checking its type. This reduces unnecessary computations.

Optimization Suggestions:
2. Use a more efficient way to filter queries using dictionary unpacking where appropriate.
3. If you decide to keep the dual check (both existence and type), ensure that they are done sequentially. Checking the type after already knowing there's one could potentially lead to slower execution time because it still involves retrieving a record from the database even if it doesn't match the desired conditions.

Modified Code:

def generate_prompt(self, instance: dict, with_valid=True):
    q = prompt.replace("{userInput}", message)
    messages[-1]['content'] = q

    # Check if the model exists first
    model_existence_query = QuerySet(Model).filter(
        workspace_id=workspace_id,
        id=model_id
    ).exists()

    if not model_existence_query:
        raise Exception(_("Model does not exist"))

    # If we reach here, the model must exist now
    # You might want to add this line if necessary:
    model = get_model_instance_by_model_workspace_id(model_id=model_id, workspace_id=workspace_id)

    def process():
        pass

In the modified version, I removed the unnecessary check for model type after assuming the model exists based on its ID existence. Make sure to adjust logic according to your application requirements regarding handling such cases differently.

Expand Down
3 changes: 2 additions & 1 deletion apps/models_provider/impl/vllm_model_provider/model/reranker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,11 @@ def is_cache_model():

@staticmethod
def new_instance(model_type, model_name, model_credential: Dict[str, object], **model_kwargs):
r_url = model_credential.get('api_url')[:-3] if model_credential.get('api_url').endswith('/v1') else model_credential.get('api_url')
return VllmBgeReranker(
model=model_name,
api_key=model_credential.get('api_key'),
api_url=model_credential.get('api_url'),
api_url=r_url,
params=model_kwargs,
**model_kwargs
)
Copy link
Copy Markdown
Contributor Author

@shaohuzhang1 shaohuzhang1 Sep 9, 2025

Choose a reason for hiding this comment

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

The provided code for is_cache_model() and new_instance methods seems to be complete without identifying any significant irregularities or potential issues. However, I can suggest some optimizations:

  1. Variable Naming: For clarity and consistency, consider renaming r_url to something like _processed_api_url.
  2. Error Handling: Consider adding error handling to manage cases where 'api_url' might not be present in the model_credential dictionary.
  3. Parameter Consistency: Ensure that all dictionaries passed into the method have consistent keys.

Here's an optimized version of the code with these suggestions:

def is_cache_model():
    # Implement logic if needed

@staticmethod
def new_instance(model_type, model_name, model_credential: Dict[str, object], **model_kwargs):
    _processed_api_url = (
        model_credential.get('api_url')[:-4]
        if model_credential.get('api_url').endswith('/v1/')
        else model_credential.get('api_url', "")
    )

    try:
        return VllmBgeReranker(
            model=model_name,
            api_key=model_credential.get('api_key'),
            api_url=_processed_api_url,
            params=model_kwargs,
            **model_kwargs
        )
    except AttributeError as e:
        print(f"Error while setting up the model: {e}")

These improvements make the function more robust and easier to understand.

Expand Down
4 changes: 3 additions & 1 deletion apps/models_provider/impl/vllm_model_provider/model/whisper_sst.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ def check_auth(self):
self.speech_to_text(audio_file)

def speech_to_text(self, audio_file):
base_url = f"{self.api_url}/v1"

base_url = self.api_url if self.api_url.endswith('v1') else f"{self.api_url}/v1"

try:
client = OpenAI(
api_key=self.api_key,
Copy link
Copy Markdown
Contributor Author

@shaohuzhang1 shaohuzhang1 Sep 9, 2025

Choose a reason for hiding this comment

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

The provided code snippet appears to be part of a class that checks authentication and performs speech-to-text conversion using the OpenAI API. Here are some observations, potential issues, and recommendations:

Observations

  1. Conditional Check: The code includes a conditional check to ensure the base URL ends with /v1.

  2. API Key Handling: There is no explicit error handling for invalid API keys.

  3. Logging/Output: No logging statements are visible, which could make it difficult to debug or monitor the execution flow.

  4. Thread Safety: If this method is used concurrently within multiple threads or processes, additional thread safety measures (like locking) might be required.

  5. Exception Handling in client Creation: It's not clear how exceptions will be handled when creating the OpenAI client instance. A try-except block should be implemented around the client creation line to catch and handle potential errors like network issues or incorrect API key formats.

  6. Code Length: While short, it lacks specific comments explaining each step, making it harder for other developers to understand.

Additional Recommendations

  1. Comments: Add detailed comments to explain the purpose of each section of the code. For example, describing what variables store what and why certain conditions are checked.

    # Construct the base URL ensuring it ends with /v1
base_url = self.api_url if self.api_url.endswith('/v1') else f"{self.api_url}/v1"

# Handle the case where self.api_url does not end with '/v1'

2. **Error Handling and Logging**: Implement comprehensive error handling for both file operations and API requests. Consider logging details such as timestamps, HTTP status codes, and exception messages for better diagnostics.

   ```python
   from openai import OpenAI, AuthenticationError, ApiConnectionError

   try:
       client = OpenAI(api_key=self.api_key)
       
       # Process response...
   
   except(AuthenticationError as e):
       print(f"Authentication failed: {e}")
   
   except(ApiConnectionError as e):
       print(f"Network problem encountered: {e}")

Additionally, you can write logs using Python's built-in logging module or a third-party library if needed.

  1. Concurrency Consideration: If this method might be called from multiple parts of your application simultaneously, consider implementing mutexes or locks to protect shared resources between different processes or threads.

  2. Documentation: Ensure that any functions or classes defined in this code have appropriate docstrings explaining their usage, parameters, and return values. This helps fellow developers quickly grasp the functionalities without running into ambiguity.

By addressing these points, you can enhance the robustness, maintainability, and debuggability of your code.

Expand Down
Loading