-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[AppConfig]Fix key vault client initialization #15826
Conversation
@avanigupta Can you help to look at this pr? Thanks. |
Hey @shenmuxiaosen, can you remove the new test recordings since the functionality (service calls) didn't really change in this PR? So we have no reason to update yaml files. |
AppConfig |
Yeah, we don't need to record for AAD testcase and it is intented to be a LiveScenairoTest. Also there are some credentials in it which failed the build checks. However, I do think we should keep updating recordings for scenariotests as documentated here. Whoever runs the tests should update the latest records regardless of whether they change the underlayer http requests. In reply to: 723386455 [](ancestors = 723386455) |
with self.assertRaisesRegex(CLIError, "Operation returned an invalid status 'Unauthorized'"): | ||
# Before assigning data reader role, read operation should fail with AAD auth. | ||
# The exception really depends on the which identity is used to run this testcase. | ||
with self.assertRaisesRegex(CLIError, "Operation returned an invalid status '(?:Unauthorized|Forbidden)'"): |
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.
There was a bug ( fixed now ) in get_login_credentials()
functionality because of which @fengzhou-msft must have gotten Unauthorized error when he updated the test here.
We should always get 'Forbidden' error if the get_login_credentials()
functionality returns the correct access token from AAD.
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.
Will fix in a new pr.
Description
Testing Guide
History Notes
[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change.
[Component Name 2] az command b: Add some customer-facing feature.
This checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.