-
Notifications
You must be signed in to change notification settings - Fork 5
docs(samples): Adding first sample code + tests #22
Conversation
Here is the summary of changes. You are about to add 5 region tags.
This comment is generated by snippet-bot.
|
runnable.script.text = "echo Hello world! This is task ${BATCH_TASK_INDEX}. This job has a total of ${BATCH_TASK_COUNT} tasks." | ||
# You can also run a script from a file. Just remember, that needs to be a script that's | ||
# already on the VM that will be running the job. | ||
# runnable.script.path = '/tmp/test.sh' |
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.
Can we use echo
or something available out of the box so that running this would always do something visible and make the sample useful out of the box?
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'm afraid that won't work for the runnable.script.path
version. It has to be a path to a file, and the file needs to be present on the running machine. This is useful only when we'd use a custom machine image I think, that has some file where we need it.
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.
Does /usr/bin/echo
not work? It should be there on nearly any Linux system.
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 don't think I understand. It does run echo Hello world! This is task ${BATCH_TASK_INDEX}. This job has a total of ${BATCH_TASK_COUNT} tasks.
.
The commented out part about script path is an alternate version, exclusive with running a script by providing it as a text.
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.
As a user, I'm still confused why this is even an option and how it differs from the regular invocation. If all it does is read the test and execute it in the default shell, I might as well write /bin/sh /tmp/test.sh
. So why does this exist and under what circumstances may I want to use it?
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 agree that it is redundant, however it was not my idea to create it that way :D I just want to present to the end user that it's an option, it's up to them if they want to use it or not.
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.
Mostly corrections to comments, and one unresolved item that we may need input from the product team on.
Tip for the future: copy your comments into Google Docs and let it nitpick your English.
runnable.script.text = "echo Hello world! This is task ${BATCH_TASK_INDEX}. This job has a total of ${BATCH_TASK_COUNT} tasks." | ||
# You can also run a script from a file. Just remember, that needs to be a script that's | ||
# already on the VM that will be running the job. | ||
# runnable.script.path = '/tmp/test.sh' |
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.
As a user, I'm still confused why this is even an option and how it differs from the regular invocation. If all it does is read the test and execute it in the default shell, I might as well write /bin/sh /tmp/test.sh
. So why does this exist and under what circumstances may I want to use it?
811a763
to
fb658dc
Compare
I've requested clarification from the Batch PM on why text and path exist as separate entities. Waiting on their response. |
Actually, let's not block the merge on that. We can always handle that particular clarification later. |
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: