search

Azure / doAzureParallel

A R package that allows users to submit parallel workloads in Azure
MIT License
107 stars 50 forks source link

minor fixes in 'Pushing output files' example #358

Open zerweck opened 5 years ago

zerweck commented 5 years ago

The sharedKeys object is not necessary to select the storageAccount object. Also, the storageAccountName is already an object in this script, but was referred as a string.

brnleehng commented 5 years ago

Hi @zerweck

Are you using an older version of the credentials configuration file?

Thanks, Brian

zerweck commented 5 years ago

Hi @brnleehng No, I just ran the 'Getting Started script' 2 days ago from the Azure Shell. The JSON that the Batch Explorer gives me also has the structure for Shared Key authentication like this:

{
  "batchAccount": {
    "name": "asd",
    "key": "xxx",
    "url": "https://asd.batch.azure.com"
  },
  "storageAccount": {
    "name": "asd",
    "key": "xxx",
    "endpointSuffix": "core.windows.net"
  },
  "githubAuthenticationToken": {}
}

Since this file is parsed via fromJSON in the given example, no list object sharedKeys is needed to subselect the storage account parameters.

zerweck commented 4 years ago

If have now identified the following two sources for the version of the credentials.json without the sharedKeys object:

  1. https://github.com/Azure/doAzureParallel/blob/9e9b4942f40e957140ca415a917963193b5e4dc9/account_setup.py#L533-L546
  2. Selecting doAzureParallel in the Batch Explorer "Credentials and code samples for this Batch account" window (current Version 2.2.0-stable.368)

The only place where it is created with the sharedKeys object is doAzureParallel::generateCredentialsConfig. I am now wondering if the version without sharedKeys is outdated or the generateCredentialsConfig is non-standard.

brnleehng commented 4 years ago

This appears the set up script is outdated. The config file should include sharedKeys property to support our AAD credentials.

I'll take a look on adding the fix.

Thanks, Brian