Skip to content
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 Python Script task #7242

Merged
merged 24 commits into from
May 18, 2018
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
"loc.friendlyName": "Python Script",
"loc.helpMarkDown": "",
"loc.description": "Run a Python script.",
"loc.instanceNameFormat": "Run Python script",
"loc.group.displayName.advanced": "Advanced",
"loc.input.label.targetType": "Type",
"loc.input.help.targetType": "Target script type: File path or Inline",
"loc.input.label.filePath": "Script Path",
"loc.input.help.filePath": "Path of the script to execute. Must be a fully qualified path or relative to $(System.DefaultWorkingDirectory).",
"loc.input.label.script": "Script",
"loc.input.help.script": "The Python script to run",
"loc.input.label.arguments": "Arguments",
"loc.input.help.arguments": "Arguments passed to the script execution, available through `sys.argv`.",
"loc.input.label.pythonInterpreter": "Python interpreter",
"loc.input.help.pythonInterpreter": "Absolute path to the Python interpreter to use. If not specified, the task will use the interpreter in PATH.",
"loc.input.label.workingDirectory": "Working directory",
"loc.input.label.failOnStderr": "Fail on standard error",
"loc.input.help.failOnStderr": "If this is true, this task will fail if any text are written to the stderr stream.",
"loc.messages.NotAFile": "The given path was not to a file: '%s'."
}
349 changes: 349 additions & 0 deletions Tasks/PythonScriptV0/ThirdPartyNotice.txt

Large diffs are not rendered by default.

Binary file added Tasks/PythonScriptV0/icon.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
22 changes: 22 additions & 0 deletions Tasks/PythonScriptV0/main.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import * as path from 'path';
import * as task from 'vsts-task-lib/task';
import { pythonScript } from './pythonscript';

(async () => {
try {
task.setResourcePath(path.join(__dirname, 'task.json'));
await pythonScript({
targetType: task.getInput('targetType'),
filePath: task.getPathInput('filePath'),
script: task.getInput('script'),
arguments: task.getInput('arguments'),
pythonInterpreter: task.getInput('pythonInterpreter'), // string instead of path: a path will default to the agent's sources directory
workingDirectory: task.getPathInput('workingDirectory'),
failOnStderr: task.getBoolInput('failOnStderr')
});
task.setResult(task.TaskResult.Succeeded, "");
} catch (error) {
task.error(error.message);
task.setResult(task.TaskResult.Failed, error.message);
}
})();
91 changes: 91 additions & 0 deletions Tasks/PythonScriptV0/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 25 additions & 0 deletions Tasks/PythonScriptV0/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{
"name": "python-script",
"version": "1.0.0",
"description": "Create and activate a Conda environment.",
"main": "pythonscript.js",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"repository": {
"type": "git",
"url": "git+https://github.com/microsoft/vsts-tasks.git"
},
"author": "Microsoft Corporation",
"license": "MIT",
"bugs": {
"url": "https://github.com/microsoft/vsts-tasks/issues"
},
"homepage": "https://github.com/microsoft/vsts-tasks#readme",
"dependencies": {
"@types/node": "^6.0.101",
"@types/q": "^1.5.0",
"@types/uuid": "^3.4.3",
"vsts-task-lib": "^2.4.0"
}
}
69 changes: 69 additions & 0 deletions Tasks/PythonScriptV0/pythonscript.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import * as fs from 'fs';
import * as path from 'path';

import * as task from 'vsts-task-lib/task';
import * as toolRunner from 'vsts-task-lib/toolrunner';

import * as uuidV4 from 'uuid/v4';

interface TaskParameters {
targetType: string,
filePath?: string,
script?: string,
arguments?: string,
pythonInterpreter?: string,
workingDirectory?: string,
failOnStderr?: boolean
}

export async function pythonScript(parameters: Readonly<TaskParameters>): Promise<void> {
// Get the script to run
const scriptPath = await (async () => {
if (parameters.targetType.toLowerCase() === 'filepath') { // Run script file
// Required if `targetType` is 'filepath':
const filePath = parameters.filePath!;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand it, the trailing bang doesn't actually perform a null check. I think it would be an improvement to have an error message that states 'filePath is required'.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right -- it's just a compile-time assertion. I can change it to a runtime check.


if (!fs.statSync(filePath).isFile()) {
throw new Error(task.loc('NotAFile', filePath));
}
return filePath;
} else { // Run inline script
// Required if `targetType` is 'script':
const script = parameters.script!;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parameters.script! => Same as above.


// Write the script to disk
task.assertAgent('2.115.0');
const tempDirectory = task.getVariable('agent.tempDirectory');
task.checkPath(tempDirectory, `${tempDirectory} (agent.tempDirectory)`);
const filePath = path.join(tempDirectory, `${uuidV4()}.py`);
await fs.writeFileSync(
filePath,
script,
{ encoding: 'utf8' });

return filePath;
}
})();

// Create the tool runner
const pythonPath = parameters.pythonInterpreter || task.which('python');
const python = task.tool(pythonPath).arg(scriptPath);

// Calling `line` with a falsy argument returns `undefined`, so can't chain this call
if (parameters.arguments) {
python.line(parameters.arguments);
}

// Run the script
// Use `any` to work around what I suspect are bugs with `IExecOptions`'s type annotations:
// - optional fields need to be typed as optional
// - `errStream` and `outStream` should be `NodeJs.WritableStream`, not `NodeJS.Writable`
await python.exec(<any>{
cwd: parameters.workingDirectory,
failOnStdErr: parameters.failOnStderr,
// Direct all output to stdout, otherwise the output may appear out-of-order since Node buffers its own stdout but not stderr
errStream: process.stdout,
outStream: process.stdout,
ignoreReturnCode: false
});
}
112 changes: 112 additions & 0 deletions Tasks/PythonScriptV0/task.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
{
"id": "6392F95F-7E76-4A18-B3C7-7F078D2F7700",
"name": "PythonScript",
"friendlyName": "Python Script",
"description": "Run a Python script.",
"helpMarkDown": "",
"category": "Utility",
"visibility": [
"Build",
"Release"
],
"runsOn": [
"Agent",
"DeploymentGroup"
],
"author": "Microsoft Corporation",
"version": {
"Major": 0,
"Minor": 135,
"Patch": 0
},
"preview": true,
"demands": [],
"instanceNameFormat": "Run Python script",
"groups": [
{
"name": "advanced",
"displayName": "Advanced",
"isExpanded": false
}
],
"inputs": [
{
"name": "targetType",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In YAML, targetType might be more intuitive if named scriptSource.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but this is consistent with the other script tasks.

"type": "radio",
"label": "Type",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make this label "Type" match the "name" field?

"required": true,
"defaultValue": "filePath",
"helpMarkDown": "Target script type: File path or Inline",
"options": {
"filePath": "File path",
"inline": "Inline"
}
},
{
"name": "filePath",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rename this input from "filePath" to "scriptPath" so that the designer and YAML views match?

"type": "filePath",
"label": "Script Path",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sentence-cased as "Script path"

"visibleRule": "targetType = filePath",
"required": true,
"defaultValue": "",
"helpMarkDown": "Path of the script to execute. Must be a fully qualified path or relative to $(System.DefaultWorkingDirectory)."
},
{
"name": "script",
"type": "multiLine",
"label": "Script",
"visibleRule": "targetType = inline",
"required": true,
"defaultValue": "",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could set the default inline script to the following so that it succeeds faster without modification:

# Write your Python script here.\n\n# Add variables marked secret on the Variables tab to pass secret variables to this script.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Synced offline, won't fix for now. We'll hold off on doing secret variables for this task.

"properties": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't suppose that by some miracle there might be a 'show whitespace' property on this, is there? (probably not...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that I know of ...

"resizable": "true",
"rows": "10",
"maxLength": "5000"
},
"helpMarkDown": "The Python script to run"
},
{
"name": "arguments",
"type": "string",
"label": "Arguments",
"required": false,
"defaultValue": "",
"helpMarkDown": "Arguments passed to the script execution, available through `sys.argv`."
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Bash task only lets you pass arguments when you're running a script from a file. I don't see any reason for that restriction (in Python at least), so this task lets you pass arguments to inline scripts as well.

},
{
"name": "pythonInterpreter",
"type": "string",
"label": "Python interpreter",
"defaultValue": "",
"required": false,
"helpMarkDown": "Absolute path to the Python interpreter to use. If not specified, the task will use the interpreter in PATH.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could append:
"The Use Python Version task can be used to set the version of Python in PATH."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about I add this as a "tip" in the docs page? That will keep the help text here brief.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like David's suggestion. Many users don't look at docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Copy link
Contributor Author

@brcrista brcrista May 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

"groupName": "advanced"
},
{
"name": "workingDirectory",
"type": "filePath",
"label": "Working directory",
"defaultValue": "",
"required": false,
"groupName": "advanced"
},
{
"name": "failOnStderr",
"type": "boolean",
"label": "Fail on standard error",
"defaultValue": "false",
"required": false,
"helpMarkDown": "If this is true, this task will fail if any text are written to the stderr stream.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is written to...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"any texts are written to"

"groupName": "advanced"
}
],
"execution": {
"Node": {
"target": "main.js",
"argumentFormat": ""
}
},
"messages": {
"NotAFile": "The given path was not to a file: '%s'."
}
}
Loading