Users/zhaodongwang/remove default credentail#26
Merged
tpellissier-msft merged 2 commits intomainfrom Nov 3, 2025
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR removes the optional credential default behavior in favor of requiring explicit credential provision. The SDK no longer falls back to DefaultAzureCredential when no credential is provided.
- Changed
credentialparameter from optional to required inDataverseClientandAuthManager - Added runtime type checking for credential in
AuthManager - Updated documentation to reflect the mandatory credential requirement
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/dataverse_sdk/client.py | Changed credential parameter from Optional[TokenCredential] with default None to required TokenCredential |
| src/dataverse_sdk/auth.py | Removed DefaultAzureCredential fallback and added explicit isinstance check; changed credential parameter to required |
| README.md | Updated Auth section and examples to show explicit credential creation instead of defaulting behavior |
Comments suppressed due to low confidence (1)
src/dataverse_sdk/client.py:36
- The docstring's
Raisessection is incomplete. It should document thatTypeErrorcan be raised fromAuthManager.__init__()when an invalid credential is provided (based on the isinstance check in auth.py line 18-21).
Raises
------
ValueError
If ``base_url`` is missing or empty after trimming.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tpellissier-msft
approved these changes
Nov 3, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request updates the authentication approach for the Dataverse SDK to require an explicit Azure Identity credential, rather than defaulting to
DefaultAzureCredentialwhen none is provided. This change improves clarity and control over authentication, ensuring that developers must always specify the credential they wish to use.Authentication changes:
azure.core.credentials.TokenCredentialcredential for authentication; automatic fallback toDefaultAzureCredentialhas been removed. (README.md,src/dataverse_sdk/auth.py,src/dataverse_sdk/client.py) [1] [2] [3] [4]AuthManagerclass constructor now enforces that the credential argument implementsTokenCredential, raising aTypeErrorotherwise.Documentation updates:
README.mdand example code have been updated to show explicit credential usage, removing examples that previously relied on automatic credential selection.Code cleanup:
Optionaltyping related to the credential argument, reflecting the new requirement for explicit credentials. [1] [2]