Skip to content

Conversation

@mshsheikh
Copy link
Contributor

This PR adds defensive checks and clearer error messages to the multi provider implementation.

Summary of changes

  1. Validate model name in get_model and reject None or empty values.
  2. Normalize empty model parts in _get_prefix_and_model_name so inputs like "openai/" are handled as missing model name.
  3. Replace raw delete on provider mapping with a check and UserError to avoid KeyError.
  4. Catch ImportError for optional litellm provider import and raise a helpful UserError that explains how to resolve the issue.
  5. Validate set_mapping inputs and shallow copy the provided mapping to prevent external mutation.
  6. Small docstring and annotation fixes.
This PR adds defensive checks and clearer error messages to the multi provider implementation. ### Summary of changes 1. Validate model name in get_model and reject None or empty values. 2. Normalize empty model parts in _get_prefix_and_model_name so inputs like "openai/" are handled as missing model name. 3. Replace raw delete on provider mapping with a check and UserError to avoid KeyError. 4. Catch ImportError for optional litellm provider import and raise a helpful UserError that explains how to resolve the issue. 5. Validate set_mapping inputs and shallow copy the provided mapping to prevent external mutation.
@seratch
Copy link
Member

seratch commented Nov 6, 2025

Thanks for sending this. However, the type hints for the arguments are clear, so I don't see the necessity to have these additional validation for obviously wrong data. If we do this for many parts of this SDK, it will make the internals unnecessarily complex.

@seratch seratch closed this Nov 6, 2025
@mshsheikh
Copy link
Contributor Author

Got it, thanks for the clarification, @seratch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants