Skip to content

Conversation

@mnriem
Copy link
Contributor

@mnriem mnriem commented Oct 23, 2020

No description provided.

@ghost ghost added the azure-spring All azure-spring related issues label Oct 23, 2020
Copy link
Member

@vcolin7 vcolin7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most things look good to me. I think it would be very helpful if these PRs had a brief description of what changes were made to look out for them, especially when there's another unmerged PR this is based on.

String clientId = System.getProperty("azure.keyvault.clientId");
String clientSecret = System.getProperty("azure.keyvault.clientSecret");
keyVault = new KeyVaultClient(keyVaultUri, tenantId, clientId, clientSecret);
keyVaultClient = new KeyVaultClient(keyVaultUri, tenantId, clientId, clientSecret);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
keyVaultClient = new KeyVaultClient(keyVaultUri, tenantId, clientId, clientSecret);
keyVaultClient = new KeyVaultClient(keyVaultUrl, tenantId, clientId, clientSecret);

All occurrences, really :)

Copy link

@chenrujun chenrujun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@chenrujun chenrujun merged commit ce4db65 into Azure:master Oct 26, 2020
@chenrujun
Copy link

chenrujun commented Oct 26, 2020

Hi, @mnriem ,

I merged this PR.
Please create another PR to resolve all the unresolved conversations in the last 3 PRs:

cc: @vcolin7 , FYI.

@mnriem mnriem deleted the end-to-end-tls-ssl-3 branch October 26, 2020 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

azure-spring All azure-spring related issues KeyVault

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants