-
Notifications
You must be signed in to change notification settings - Fork 51
Feature: Bulk Task Support #318
base: master
Are you sure you want to change the base?
Conversation
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.
Do you know if R plays nice with concurrency? With the python SDK we had the benefit of the GIL which simplified a lot of concurrency stuff
|
||
staticHtml <- "<h1>Errors:</h1>" | ||
for (i in 1:length(failTasks)) { | ||
if (failTasks[i] == 1) { |
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.
Shouldn't there be an else somewhere?
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.
Or closue earlier
|
||
if (!is.null(cloudCombine)) { | ||
assign("cloudCombine", cloudCombine, .doAzureBatchGlobals) | ||
containerSettings$imageName <- "brianlovedocker/doazureparallel-merge-dockerfile:0.12.1" |
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.
I think I've heard some of the pro's to having this be here, but the docker image should likely be moved to a more official sounding host, if if not hosted in the official Microsoft Docker / ACR
threads = 1 | ||
){ | ||
size <- length(tasks) | ||
self$originalTaskCollection <- tasks |
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.
Why make the extra reference/clone.
#300