Update Import-AzureRMAutomationRunbook command to support Python Runbooks #5000

rdbartram · 2017-11-16T14:12:51Z

Description

This checklist is used to make sure that common guidelines for a pull request are followed. You can find a more complete discussion of PowerShell cmdlet best practices here.

I have read the contribution guidelines.
If changes were made to any cmdlet, the XML help was regenerated using the platyPSHelp module.
If any large changes are made to a service, they are reflected in the respective change log.

General Guidelines

Title of the pull request is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.
The pull request does not introduce breaking changes (unless a major version change occurs in the assembly and module).

Testing Guidelines

Pull request includes test coverage for the included changes.
PowerShell scripts used in tests should do any necessary setup as part of the test or suite setup, and should not use hard-coded values for locations or existing resources.

Cmdlet Signature Guidelines

New cmdlets that make changes or have side effects should implement ShouldProcess and have SupportShouldProcess=true specified in the cmdlet attribute. You can find more information on ShouldProcess here.
Cmdlet specifies OutputType attribute if any output is produced - if the cmdlet produces no output, it should implement a PassThru parameter.

Cmdlet Parameter Guidelines

Parameter types should not expose types from the management library - complex parameter types should be defined in the module.
Complex parameter types are discouraged - a parameter type should be simple types as often as possible. If complex types are used, they should be shallow and easily creatable from a constructor or another cmdlet.
Cmdlet parameter sets should be mutually exclusive - each parameter set must have at least one mandatory parameter not in other parameter sets.

azuresdkci · 2017-11-16T14:16:04Z

Can one of the admins verify this patch?

cormacpayne · 2017-11-16T16:25:13Z

@rdbartram Hey Ryan, thanks for opening this PR!

@vrdmr Hey Varad, would you mind reviewing this PR?

cormacpayne · 2017-11-16T16:25:25Z

@azuresdkci add to whitelist

vrdmr · 2017-11-16T22:07:08Z

Adding @Diastro to review this.

Diastro · 2017-11-16T22:29:08Z

src/ResourceManager/Automation/Commands.Automation/Cmdlet/ImportAzureAutomationRunbook.cs

@@ -66,6 +66,7 @@ public class ImportAzureAutomationRunbook : AzureAutomationBaseCmdlet
            Constants.RunbookType.PowerShellWorkflow,
            Constants.RunbookType.GraphicalPowerShellWorkflow,
            Constants.RunbookType.Graph,
+            Constants.RunbookType.Python,


Might be a good idea to call this Python2 (the backend already makes the difference between Py2 and Py3).

Hi Diastro,I thought the same, my only concern was there is no reference to the powershell version, Core or otherwise. So I assumed when the day came that Python 3 was supported, Python 2 would be deprecated. I can change it to Python2 before you accept the change. What does everyone else think?

@Diastro gentle ping

I think we should also look into test coverage around this so that we will know immediately when Python 2 support is deprecated and we can move to Python 3.

@cormacpayne are we doing integration testing with this module? How would you recommend detecting whether the API has deprecated Python2 or not?

@rdbartram we have some live tests we run in Azure Automation, so we could add test coverage there to know when the call no longer works. We also should add a scenario test that runs during our regular CI to make sure this functionality is working correctly. @vrdmr @Diastro it doesn't look like we have any tests around the Import-AzureRmAutomationRunbook cmdlet, are we able to add one for this functionality?

@rdbartram Sorry for the delay this slipped through my notifications.
I don't think we will deprecate Python2 if Python3 becomes available (we almost threat the as different languages to some extent). For now I think we can leave it as it (the variable name is really just an internal implementation detail at this point and can be refactored later). @vrdmr can comment on the test question.

@cormacpayne I just synced with @vrdmr and you can open a task against @najams he's the owner for this area of our RP. We are waiting for our SDK to be migrated from Hyak to Swagger which will happen in December (this is when we will add test for this).
In the mean time can we let this go in please?

cormacpayne · 2017-11-27T16:59:12Z

@rdbartram Hey Ryan, would you mind updating the Automation change log (underneath the ## Current Release header) with a snippet about the change you are making in this PR?

… python version 2 is supported version

cormacpayne · 2017-11-29T16:37:25Z

@vrdmr @Diastro any additional comments from your side?

Diastro · 2017-11-29T19:41:18Z

src/ResourceManager/Automation/Commands.Automation/help/Import-AzureRMAutomationRunbook.md

@@ -23,7 +23,7 @@ Import-AzureRmAutomationRunbook [-Path] <String> [-Description <String>] [-Name
 ## DESCRIPTION
 The **Import-AzureRmAutomationRunbook** cmdlet imports an Azure Automation runbook. Specify the
 path to a wps_2 script (.ps1 ) file to import for wps_2 and wps_2 Workflow runbooks, or to a
-graphical runbook (.graphrunbook) file for graphical runbooks. The name of the file becomes the
+graphical runbook (.graphrunbook) file for graphical runbooks. (.py) files are required when importing python2 scripts The name of the file becomes the


Sorry for being picky, punctuation seems weird here. Also, I believe the extension requirement is true for other runbook type too no? Also can you remove the extra space from (.ps1 ) please.

Suggestion :
"Specify the path to a wps_2 script (.ps1) file to import for wps_2 and wps_2 Workflow runbooks, or to a graphical runbook (.graphrunbook) file for graphical runbooks, or to a python 2 script (.py) file for python 2 runbooks... "

Thoughts?

Diastro · 2017-11-30T03:52:34Z

src/ResourceManager/Automation/Commands.Automation/help/Import-AzureRMAutomationRunbook.md

-definition that matches the name of the file.
+The **Import-AzureRmAutomationRunbook** cmdlet imports an Azure Automation runbook. Specify the 
+path to a wps_2 script (.ps1) file to import for wps_2 and wps_2 Workflow runbooks, 
+or to a graphical runbook (.graphrunbook) file for graphical runbooks, or to a python 2 script (.py) file for python 2 runbooks. 


Can you remove the 'or' before the 'to a graphical runbook' please? Sorry I might have added it in my suggestion but 2 'or' doesnt make sens.
Singing off.

Thanks!

cormacpayne · 2017-11-30T16:24:40Z

On-demand: https://azuresdkci.westus2.cloudapp.azure.com/job/powershell-demand/217/

cormacpayne · 2017-11-30T16:25:08Z

Adding test for this functionality will be handled by @vrdmr or @Diastro in the next milestone

Ryan Bartram added 2 commits November 16, 2017 15:06

Update Import-AzureRMAutomationRunbook command to support Python scripts

1c98506

Updated Help file

8939bf3

azuresdkci added the needs-review label Nov 16, 2017

azuresdkci assigned markcowl Nov 16, 2017

markcowl requested a review from vrdmr November 16, 2017 18:23

markcowl assigned vrdmr Nov 16, 2017

markcowl added the Automation label Nov 16, 2017

vrdmr requested a review from Diastro November 16, 2017 22:07

Diastro previously approved these changes Nov 16, 2017

View reviewed changes

maddieclayton unassigned vrdmr Nov 20, 2017

cormacpayne assigned vrdmr and Diastro and unassigned markcowl Nov 21, 2017

cormacpayne added needs-revision and removed Automation needs-review labels Nov 27, 2017

cormacpayne assigned rdbartram and unassigned vrdmr and Diastro Nov 27, 2017

Update to ChangeLog. Aswell as update Runbook enum to make apparently…

4beae67

… python version 2 is supported version

rdbartram dismissed Diastro’s stale review via 4beae67 November 29, 2017 11:34

cormacpayne previously approved these changes Nov 29, 2017

View reviewed changes

cormacpayne added needs-review and removed needs-revision labels Nov 29, 2017

cormacpayne assigned vrdmr and Diastro and unassigned rdbartram Nov 29, 2017

Diastro reviewed Nov 29, 2017

View reviewed changes

altered description of import-azurermautomationrunbook

b491e74

rdbartram dismissed cormacpayne’s stale review via b491e74 November 29, 2017 20:50

Diastro reviewed Nov 30, 2017

View reviewed changes

Diastro previously approved these changes Nov 30, 2017

View reviewed changes

Update to help to improve grammer

faf706c

rdbartram dismissed Diastro’s stale review via faf706c November 30, 2017 08:30

cormacpayne approved these changes Nov 30, 2017

View reviewed changes

cormacpayne unassigned vrdmr and Diastro Nov 30, 2017

cormacpayne added Waiting for On Demand and removed needs-review labels Nov 30, 2017

Merge branch 'preview' into preview

06add2c

cormacpayne merged commit 4c68486 into Azure:preview Dec 1, 2017

Update Import-AzureRMAutomationRunbook command to support Python Runbooks #5000

Update Import-AzureRMAutomationRunbook command to support Python Runbooks #5000

Uh oh!

Conversation

rdbartram commented Nov 16, 2017 • edited by azuresdkci Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

azuresdkci commented Nov 16, 2017

Uh oh!

cormacpayne commented Nov 16, 2017

Uh oh!

cormacpayne commented Nov 16, 2017

Uh oh!

vrdmr commented Nov 16, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cormacpayne commented Nov 27, 2017

Uh oh!

cormacpayne commented Nov 29, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cormacpayne commented Nov 30, 2017

Uh oh!

cormacpayne commented Nov 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

rdbartram commented Nov 16, 2017 •

edited by azuresdkci

Loading

cormacpayne commented Nov 30, 2017 •

edited

Loading