This repository has been archived by the owner on Oct 12, 2023. It is now read-only.
Feature/nationalcloud #239
Merged
Conversation
This file contains 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
paselem
reviewed
Mar 26, 2018
R/utility.R
Outdated
|
|
||
| baseUrl <- paste0(accountName, azureDomain) | ||
| baseUrl <- paste0(parsedValue[1], "//", parsedValue[3]) |
There was a problem hiding this comment.
nitpick: Since you removed the named varailbes for the array indexes, can you add a comment or something to say what parsedValue[1] and parsedValue[3] are?
paselem
reviewed
Mar 26, 2018
README.md
Outdated
| @@ -74,10 +74,12 @@ This information can be found in the Azure Portal inside your Batch Account: | |||
| For your Azure Storage Account, we need to get: | |||
| - Storage Account Name | |||
| - Storage Account Access Key | |||
| - Storage Account Endpoint Suffix | |||
There was a problem hiding this comment.
nitpick: Do we need this? By default we use public Azure. I think that is correct for this readme. I would have another document explaining how to use national clouds and not make the getting started more complicated.
paselem
suggested changes
Mar 26, 2018
brnleehng
suggested changes
Mar 26, 2018
| @@ -417,20 +418,21 @@ setHttpTraffic <- function(value = FALSE) { | |||
| # Creating read-only SAS token blob resource file urls | |||
There was a problem hiding this comment.
We should validate the endpointSuffix if they supply it
There was a problem hiding this comment.
Just realized we don't need user to provide endpointSuffix, we can automatically discover it based on batch account url, update the code to do auto-discovery.
There was a problem hiding this comment.
Customer might create their own DNS name for the endpoint suffix so we can't always assume it's our default one.
|
R/doAzureParallel.R:431:1: style: Lines should not be more than 120 characters. rAzureBatch::createBlobUrl(storageCredentials$name, id, "install_cran.R", sasToken, storageCredentials$endpointSuffix)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~R/doAzureParallel.R:433:1: style: Lines should not be more than 120 characters. rAzureBatch::createBlobUrl(storageCredentials$name, id, "install_bioconductor.R", sasToken, storageCredentials$endpointSuffix)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~R/utility.R:175:5: style: Commented code should be removed. # parsedValue[1] = "https"
^~~~~~~~~~~~~~~~~~~~~~~~R/utility.R:176:5: style: Commented code should be removed. # parsedValue[2] = ""
^~~~~~~~~~~~~~~~~~~R/utility.R:177:5: style: Commented code should be removed. # parsedValue[3] = "accountname.blob.core.windows.net"
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~R/utility.R:178:5: style: Commented code should be removed. # parsedValue[4] = "outputs?se=2017-07-31&sr=c&st=2017-07-12"
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
paselem
reviewed
Apr 4, 2018
docs/34-national-clouds.md
Outdated
| @@ -0,0 +1,30 @@ | |||
| # Configuration for national clouds | |||
|
|
|||
| By default, doAzureParallel is configured to run in public Azure cloud, if you want to run workload in national clouds, you can configure endpoint suffix for storage account in cluster config, it tells doAzureParallel which national cloud environment the storage account resides. | |||
There was a problem hiding this comment.
Update to:
doAzureParallel is configured to run in public Azure cloud by default. To run workloads in national clouds, configure endpoint suffix for storage account in the cluster config which tells doAzureParallel which national cloud environment the storage account resides.
paselem
reviewed
Apr 4, 2018
docs/34-national-clouds.md
Outdated
|
|
||
| By default, doAzureParallel is configured to run in public Azure cloud, if you want to run workload in national clouds, you can configure endpoint suffix for storage account in cluster config, it tells doAzureParallel which national cloud environment the storage account resides. | ||
|
|
||
| EndpointSuffix is the last part of the Connection string shown in the Storage Account Access keys blade from Azure portal. The possible values usually are: |
There was a problem hiding this comment.
'connection' not 'Connection'
paselem
reviewed
Apr 4, 2018
docs/34-national-clouds.md
Outdated
| Azure US government cloud: core.usgovcloudapi.net | ||
| Azure German cloud: core.cloudapi.de | ||
|
|
||
| The value may be different if a DNS redirect is used, so better to double check its value on Storage Account Access keys blade. |
paselem
reviewed
Apr 4, 2018
docs/34-national-clouds.md
Outdated
| "storageAccount": { | ||
| "name": <Azure Storage Account Name>, | ||
| "key": <Azure Storage Account Key>, | ||
| "endpointSuffix": <Azure Storage Account Endpoint Suffix> |
paselem
approved these changes
Apr 4, 2018
brnleehng
approved these changes
Apr 4, 2018
brnleehng
added a commit
that referenced
this pull request
Jul 13, 2018
* Update README.md (#164) * remove unused validator file (#167) * initial docs for docker users (#166) * initial docs for docker users * Fixes plus PR feedback * Feature/configfromobj support programmatically created credentials and cluster configs (#168) * support programmatically generated credentials/cluster config * update docs for programmatically generated config * reformat code * styling fixes * combine credentials cluster config methods * fix set credentials issue and test input * do not create az_config.json * update messages * move githubAuthenticationToken from cluster config to credentials * updated docs (#172) * Implemented progress bar with verbose details (#181) * Implemented a more detailed verbose progress bar * Fixed infix operator spacing * Added newline for 'Tasks have completed' message * Changing UI of progress bar (#183) * Redone the progress bar * Added string utilities * Feature/longrunjob, long running job improvement, add deleteJob and terminateJob (#174) * add documentation and sample for long running job * update sample file name * update long running job doc and test * update metadata code * add errorHandling to job metadata * add deleteJob to delete both job defintion and job result * styling fix * save foreach wait setting to metadata * implement retry logic in getjobresult * add terminateJob * handle various corner cases * regenerate document * add job state in getJob * do not fail getJobResult if getMetadata failed for backward compatibility * add deleteJob option to foreach, by default it is true for wait=TRUE job * styling fix * update version and changelog * address review feedback * add setJobAutoDelete function * rename jobAutoDelete to autoDeleteJob to workaround R bugs and update docs * update faq * fix styling issues * more styling fix * roll back manual update to DESCRIPTION * add namespace to api call * Feature/docker registry auth (#182) * initial changes to support auth * Temp changes to pull install scripts from local branch * Updated docs to expose dockerAuth field * Minor tweaks to doc language * revert change to setup scripts back to master * fix linter line too long error * add links to cluster configuration docs * remove whitespace * PR feedback * write and read docker password from disk * Fixed based on recent feedback (#185) * Updated DESCRIPTION's reference rAzureBatch to v0.5.4 (#184) * address issue where empty docker auth credentials are used to create … (#190) * address issue where empty docker auth credentials are used to create the cluster * remove unnecessary null check * Collapsing pool package installation on start task command line (#191) * Collasping the R package installations for pool installation * Renamed script variable * Fixed pool installation test * Fixed length test * Improve R console UI experience (#193) * Improve UI experience * Added verbose mode to deleteJob and deleteStorageContainer * Refactor print method * Fixed order of package installation (#196) * Fix/add task perf (#195) * Added task id range * Removed upload blob methods * Removed upload blob * Fixed trailing whitespace * Discarded job id on merge task id name * Adding chunk logic for argsList * Added check for args containing data sets * Removed container name for docker run command for all tasks * Added test for hasDataSet * Fix travis yml * Adding before_install for R * Removed before install, added github package of nycflights13 * fix link to generate config doc (#199) * Feature/asynccluster (#197) * support for async cluster creation * fix lintr bot errors * remove test files * use private function for duplicate code * update change log * Feature/asynccluster1 (#200) * show node status in getCluster * workaround error * minor fixes * add getClusterList api * add metadata to pool indicating pool is created by doAzureParallel * add test for getClusterList(), add 'other' state for nodes for getCluster() * Update documentation on using private docker registries (#201) * update documentation for private docker registry * update docs to be less confusing * In order correction (#202) * Update long_running_job.R (#206) renamed a misnamed azure options variable * Created an issue template (#207) * Typos in README.md (#210) * list cluster should use paste() instead of + (#213) * use paste() instead of + * use paste0 * Feature/getjobresultlocal (#204) * Get job result locally * Get job result locally * preserve task sequence in getjobresult * keep task result sequence * keep task result in sequence and as a flat list * fix lintr error * fix typo in error message * delete cluster after test is done * add retry to getJobResultLocal, resolve xml2 issue * fix typo, lintr and missing variable * allow local RProfile libraries to be loaded by default (#209) * allow local session info to be loaded * update line lengths * Bundling worker scripts into zip (#212) * Added zip file extraction * Fixed cluster setup * Added cluster script and quiet zip function * Replaced url name with correct zip file name * Removed startup folder name * Added apt-get install on job prep * Fixed branch names * Reverted changes (#227) * Upgraded description for fix resize cluster (#225) * Update sample to only use the first 6 files (#228) The file format change over the year so only use the first 6 so the sample is consistent. This also has the added benefit that the sample runs a bit faster so users can get a feel for the tooling a bit more easily. * Added optional retry count flag (#235) * Added job retry count flag * Renamed maxTaskRetryCount * Added cluster config for caret example (#237) * Added cluster config for caret * Reverted changes for installation * Reverted fit model * Changed to low priority * Added cluster submission output (#236) * Finish output for cluster * Added resource files print info * Fixed ordering * Renamed Nodes to Scale * Fixed typo * Feature/nationalcloud (#239) * support national cloud * fix hardcoded domain name in createOutputFile * update rAzureBatch version etc * auto discovery of storage account endpoint suffix * styling fix * fix test failure * add back endpointSuffix for storage account * add storage account endpoint suffix to downloadBlob call * update docs * improve error handling for create cluster (#241) * improve error handling for create cluster * remove extra space * Fixed argument validation (#244) * Fixed incorrect variable name (#243) * Reverted variable name (#245) * Improvement on merge task performance (#223) * Added doParallel support * Renamed txt file * Fixed lintr * Restructured merger script * Removed some error handling cases * Fixed syntax * Renamed error handling test * Added accumulator * Using filter on function * Proper filtering of tasks * Fixed merge naming * Added error handling for worker, separate merge task function * Added buckets * Added addSubMergeTask * Added merge sub task functions * Fixing file names * Fixed sorting order for merger * Added space * Merger in R * Clean up merger worker script * Added mergeSize option * By default one bucket * Removed merge size flag * Fixed test * Fixed lint code * Fixed more lintr issues * Fixed lintr * Fixed the added comments * Fixed the if statement * Add list combine function validation * Removed verification * Fixed lintr * Mapping of job results (#248) * handle addjob error 403 (#251) * Sample - Add a sample for using SAS resource files (#253) * resubmit sas resource files example * fixed typos and grammar * remove unnecessary github reference * fix sample link to sas resource files (#254) * Feature/pkgmgmtdoc (#231) * merge package management doc * merge packagement docs * address review feedback (#232) * address review feedback * add reference to github and bioconductor packages in worker * update package management sample * update package management doc (#233) * address review feedback * add reference to github and bioconductor packages in worker * update package management sample * update package management doc * remove cluster.json * remove package installation * Feature/getstarted (#255) * get started script * add account_setup.sh * fix typo * fix typo * support shared key * fix typos * retrieve batch/storage account keys * fix bug * typo * fix if logic * fix typo * bug * retrieve batch primary key * storage list_keys * storage keys * storage key exceptin * storage key * storage key * storage key * storage key * storage key * storage key * storage key * storage key * storage key\ * storage key * storage key * storage key * storage key * storage key * storage key * storage key * storage key * storage key * storage key * storage key * storage key * storage key * storage key * storage key * storage key * storage key * storage key * storage key * storage key\ * storage key * storage key * storage key * delete resource group * print * print * print * exit from delete rg * delete resource group * aad auth * resource group name * add docs for get started script * update python script location * update doc * update doc * update doc * fix credential setting names * fix credential setting names * address review feedback * Updated version v0.6.3 (#256) * Enable AAD and VNet Support (#252) * Added config property for service principal * Fixed property names for service principal * Added makeBatchClient and makeStorageClient functions * Added ARM calls * Fixed configuration * Added more working features for AAD auth * Switched sas token generator * Added storage client generate sas tokens into doAzureParallel * basic foreach loop works * Long running jobs with AAD validated * Removed credentials output * Added docs for vnets * Fixed network configurations for doazp * Replaced rAzureBatch namespace with batchClient * Fixed url auth for storage * Changed cluster parameter naming * Renamed helper file and fixed lintr * Wrong file name * Removed new line * Removed lowerCamelCase for R6 classes * Fixed sas token * Fixed createBlobUrl * Fixed configuration endpoint suffix * Fixed endpoint suffix for download merge results * Added environment parameter in setCredentials * Added default for endpoint suffix for svp * Changed default credentials file to shared key * Updated docs for sharedkey/serviceprincipal * Updated documentation * Broken test for doc example * Fixed lintr for docs * Updated version to 0.7.0 * Fixed get workers count function (#261) * Fixing change of named parameter in makeCluster (#259) Parameter name looks to have changed. Fixing. There are still some issues that I will open a separate ticket for. * Fixed resource files docs (#262) * Added change log details (#258) * Fix/getstartdoc (#263) * fix AAD credential config field names * fix json format * add sharedKey to credentials related code and doc (#266) * Fix/storage management (#267) * Removed rAzureBatch from storage api calls * Fixed quota documentation * Added job and core quota limits * Replaced deprecated function (#269) * Fixed output (#270) * Feature/custom package (#272) * Added custom package script * Added feature custom download * Fixed typo * Fixed directory for installation * Fixed full folder directory * Add dependencies and fix pattern * Fix pattern not found * Added repo * Switching to devtools * Fixing devtools install with directory * Fix in for merger.R * Working cluster custom packages * Removed printed statements * Working on custom docs * Custom packages sample docs * Fixed typo in azure files typo * Fixed typos based on PR * Documentation rewrite (#273) * Renamed operations * Fixing docs * Removed stuff from README.md * Fixed links for TOC * Added descriptions for TOC * Major renaming of files * Added TOC to main README.md * Added low pri link * Added link to vm priority * Fix broken links * Added Notable features * Clarifying comment on DSVM (#274) * Tests/r (#275) * Added install R * Added devtools build * devtools build and test * Clean up rscript * Added line breaks * Added devtools('.') * Added testthat package * Added roxygen * Replaced rscript * Test live * Added environment variables to test * Fixed test * Removed * Fixed tests * makeClusters to makeCluster * Error handling to stop exit * Added params to setup function * Removed pool names * Get / Get Cluster List * Added utility R source * Fixed tests * Fixed remove error handling with combine test * Forgot \ lines * Switched to R6 sas client (#276) * Switched to R6 sas client * Added storage endpoint suffix * Fixed package strip name (#278) * Fix: Updated MaxTasksPerNode documentation (#279) * Added comment on maxTasksPerNode * Added bolded note * After comments * Fix: Updating Persistent Storage Documentation (#283) * Switched to R6 client * Replaced createContainer function * CHANGELOG for v0.7.1 (#285) * Added 0.7.1 changelog version (#286)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Support for national cloud, require rAzureBatch version 0.5.7