-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
feat: prioritize api_key over tenant_id for more Azure AD token provider #8318
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -195,7 +195,8 @@ def set_client( # noqa: PLR0915 | |||
organization = get_secret_str(organization_env_name) | |||
litellm_params["organization"] = organization | |||
azure_ad_token_provider: Optional[Callable[[], str]] = None | |||
if litellm_params.get("tenant_id"): | |||
# If we have api_key, then we have higher priority | |||
if not api_key and litellm_params.get("tenant_id"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this logic is getting quite complicated, can we please refactor into a smaller function and add testing for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which section are you referring to (from which line to which line)?
Hi ,@krrishdholakia
this logic is getting quite complicated, can we please refactor into a smaller function and add testing for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What kind of test is expected? Do we any example that I can refer to?
prioritize api_key over tenant_id for more Azure AD token provider
Relevant issues
Type
🆕 New Feature
🐛 Bug Fix
🧹 Refactoring
📖 Documentation
🚄 Infrastructure
✅ Test
Changes
[REQUIRED] Testing - Attach a screenshot of any new tests passing locally
If UI changes, send a screenshot/GIF of working UI fixes