-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add startTask config item to Azure #1
Add startTask config item to Azure #1
Conversation
Signed-off-by: Adam Talbot <12817534+adamrtalbot@users.noreply.github.com>
|
||
static AzStartTaskOpts parseStartTask(Map startTaskOpts) { | ||
final newStartTaskOpts = new AzStartTaskOpts() | ||
newStartTaskOpts.script = startTaskOpts.script ?: '' |
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.
newStartTaskOpts.script = startTaskOpts.script ?: '' | |
newStartTaskOpts.script = startTaskOpts.script |
null
is better
// !opts.startTask | ||
// !opts.startTaskPrivileged |
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.
No need to commit commented code
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.
Still doesn't work yet. This is to remind me to add a test back in for the new class.
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.
👍
Signed-off-by: Adam Talbot <12817534+adamrtalbot@users.noreply.github.com>
Signed-off-by: Adam Talbot <12817534+adamrtalbot@users.noreply.github.com>
Signed-off-by: Adam Talbot <12817534+adamrtalbot@users.noreply.github.com>
this.startTask = opts.startTask | ||
this.startTaskPrivileged = opts.startTaskPrivileged ?: false | ||
this.maxVmCount = opts.maxVmCount as Integer ?: vmCount * 3 | ||
this.startTask = getStartTask( opts.startTask as Map ) |
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.
@pditommaso why does this not work? I always get an empty AzStartTaskOpts class.
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.
Hard to say. I'd suggest you work in branch of the main repo instead of your own fork, otherwise I cannot contribute changes back
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.
Alright, pushed to the original dev branch
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.
Guess what, the problem was I was using nextflow
instead of my dev branch 🤦
b53386c
into
azure_batch_configurable_start_task
Adds startTask config set to Azure config options