[Identity] Throwing if required parameters are not provided#16846
Merged
6 commits merged intoAzure:mainfrom Aug 20, 2021
Merged
[Identity] Throwing if required parameters are not provided#168466 commits merged intoAzure:mainfrom
6 commits merged intoAzure:mainfrom
Conversation
ghost
added
the
KarishmaGhiya
approved these changes
Aug 10, 2021
Member
KarishmaGhiya
left a comment
KarishmaGhiya
left a comment
There was a problem hiding this comment.
Do you think this will affect any related tests in tests for other credentials like DefaultAzureCred or ManagedIdentityCred - will you have to update the error messages in those tests too? @sadasant
Contributor
Author
|
@KarishmaGhiya I’ll check what other credentials are affected! Thank you! |
Contributor
Author
|
@KarishmaGhiya DefaultAzureCred nor ManagedIdentityCredential are really affected, since they don’t have required parameters. I’ll keep it simple in this PR. |
|
Hello @sadasant! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit
openapi-sdkautomation bot
pushed a commit
to AzureSDKAutomation/azure-sdk-for-js
that referenced
this pull request
Jan 5, 2022
Fix flipped order in Face generated models (Azure#16846) * Fix flipped order in Face generated models The recent change changing a nested parameter from optional to required caused the AutoRest generated model to have a reversed order of two string arguments. Expanding the nesting for affected models to avoid breaking changes. * Fix malformatted examples
This pull request was closed.
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
2 participants
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.
Since I moved most of the processing to a common place (
nodeCommon.ts), some of the requirements per credential were obscured.ClientSecretCredentialandClientCertificateCredentialin particular where not throwing early if the parameters weren’t provided (which of course is possible if users bypass the parameter types, or if they use JavaScript). I’ve also added tests to confirm this behavior in the future.