{HDInsight} Support creating cluster with ADLSGen1/2 storage in national clouds#12310
Conversation
| HdInsightManagementClient.VirtualMachines.RestartHosts(resourceGroupName, clusterName, hosts); | ||
| } | ||
|
|
||
| private void ResetClusterIdentity(ClusterCreateParametersExtended createParams, string cloudAadAuthority, string cloudDataLakeAudience) |
There was a problem hiding this comment.
Nit - I would drop the word cloud. E.g cloudAadAuthority -> aadAuthority
There was a problem hiding this comment.
Nit - I would drop the word cloud. E.g
cloudAadAuthority->aadAuthority
Changed.
|
|
||
| string aadTenantIdWithUrl; | ||
| clusterIdentity.TryGetValue("clusterIdentity.aadTenantId", out aadTenantIdWithUrl); | ||
| string newAadTenantIdWithUrl = aadTenantIdWithUrl?.Replace("https://login.windows.net/", cloudAadAuthority); |
There was a problem hiding this comment.
Nit - nice to extract a constant for "https://login.windows.net/"
There was a problem hiding this comment.
Nit - nice to extract a constant for "https://login.windows.net/"
Fixed.
| { | ||
| return; | ||
| } | ||
| clusterIdentity.SetProperty("clusterIdentity.resourceUri", cloudDataLakeAudience); |
There was a problem hiding this comment.
Nit - you can just say clusterIdentity["clusterIdentity.resourceUri"] = cloudDataLakeAudience; for simplicity. BTW, what if clusterIdentity["clusterIdentity.resourceUri"] is not set originally?
There was a problem hiding this comment.
Nit - you can just say
clusterIdentity["clusterIdentity.resourceUri"] = cloudDataLakeAudience;for simplicity. BTW, what ifclusterIdentity["clusterIdentity.resourceUri"]is not set originally?
Fixed. If it is not set originally, then our code will add the key/value pair, I think it is ok.
| { | ||
| return; | ||
| } | ||
| clusterIdentity.SetProperty("clusterIdentity.resourceUri", cloudDataLakeAudience); |
There was a problem hiding this comment.
Just throw a question out of curiosity. Is there such a scenario that clusterIdentity is not null but cloudAadAuthority /cloudDataLakeAudience is null? I am not familiar with clusterIdentity, so if my question is wrong, please correct it.
There was a problem hiding this comment.
Just throw a question out of curiosity. Is there such a scenario that
clusterIdentityis not null butcloudAadAuthority/cloudDataLakeAudienceis null? I am not familiar withclusterIdentity, so if my question is wrong, please correct it.
Yes, cloudDataLakeAudience may be null. A dictionary can accept a null as value. From the code side, it is ok.
…nal clouds (Azure#12310) * Support creating cluster with Adlsgen1/2 storage in national clouds * Update ChangeLog.md * Refactor the ResetClusterIdentity function Co-authored-by: Zhenyu Zhou <zhezhou@microsoft.com>
5 participants
Description
Support creating cluster with ADLSGen1/2 storage in national clouds
Checklist
CONTRIBUTING.mdChangeLog.mdfile(s) has been updated:ChangeLog.mdfile can be found atsrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md## Upcoming Releaseheader -- no new version header should be added