PowerShell / PSArm

PSArm is a PowerShell module that provides a PowerShell-embedded domain-specific language (DSL) for Azure Resource Manager (ARM) templates
MIT License
77 stars 19 forks source link

Build-PSArmTemplate Alias for Publish-PSArmTemplate #135

Open JustinGrote opened 3 years ago

JustinGrote commented 3 years ago

Publish to most powershell people means "do a deployment" e.g. Publish-Module. If the command isn't renamed directly, it should at least be aliased to Build-PSArmTemplate to be more instinctive and follow the typical lexicon:


❯ get-verb build,publish | fl

Verb        : Publish
AliasPrefix : pb
Group       : Data
Description : Makes a resource available to others

Verb        : Build
AliasPrefix : bd
Group       : Lifecycle
Description : Creates an artifact (usually a binary or document) out of some set of input files (usually source code
              or declarative documents)
rjmholt commented 3 years ago

In Windows PowerShell 5.1, I think these are our options:

> get-verb |sort -Property Verb

Verb        Group
----        -----
Add         Common
Approve     Lifecycle
Assert      Lifecycle
Backup      Data
Block       Security
Checkpoint  Data
Clear       Common
Close       Common
Compare     Data
Complete    Lifecycle
Compress    Data
Confirm     Lifecycle
Connect     Communications
Convert     Data
ConvertFrom Data
ConvertTo   Data
Copy        Common
Debug       Diagnostic
Deny        Lifecycle
Disable     Lifecycle
Disconnect  Communications
Dismount    Data
Edit        Data
Enable      Lifecycle
Enter       Common
Exit        Common
Expand      Data
Export      Data
Find        Common
Format      Common
Get         Common
Grant       Security
Group       Data
Hide        Common
Import      Data
Initialize  Data
Install     Lifecycle
Invoke      Lifecycle
Join        Common
Limit       Data
Lock        Common
Measure     Diagnostic
Merge       Data
Mount       Data
Move        Common
New         Common
Open        Common
Optimize    Common
Out         Data
Ping        Diagnostic
Pop         Common
Protect     Security
Publish     Data
Push        Common
Read        Communications
Receive     Communications
Redo        Common
Register    Lifecycle
Remove      Common
Rename      Common
Repair      Diagnostic
Request     Lifecycle
Reset       Common
Resize      Common
Resolve     Diagnostic
Restart     Lifecycle
Restore     Data
Resume      Lifecycle
Revoke      Security
Save        Data
Search      Common
Select      Common
Send        Communications
Set         Common
Show        Common
Skip        Common
Split       Common
Start       Lifecycle
Step        Common
Stop        Lifecycle
Submit      Lifecycle
Suspend     Lifecycle
Switch      Common
Sync        Data
Test        Diagnostic
Trace       Diagnostic
Unblock     Security
Undo        Common
Uninstall   Lifecycle
Unlock      Common
Unprotect   Security
Unpublish   Data
Unregister  Lifecycle
Update      Data
Use         Other
Wait        Lifecycle
Watch       Common
Write       Communications
rjmholt commented 3 years ago

I'd say my top candidates in order are:

JustinGrote commented 3 years ago

I think I would opt for New over Publish, or even Invoke-BuildPSArmTemplate if you want to get really silly.

I'm mostly thinking of existing conventions that people are used to for these verbs (that's why they exist after all)

My vote is new for 5.1 compatibilty, with Build available as an alias for 7:

New-PSArmTemplate -Path Myscript.arm.ps1 -OutFile MyOutputFolder

It lines up nicely with New-AzDeployment as well

rjmholt commented 3 years ago

Invoke is more transparent about the fact that we're actually running something user-provided under the hood

rjmholt commented 3 years ago

Anyway, I'll let this discussion play out without me now that I've added my 2 cents — I know how verb discussions go.

kilasuit commented 3 years ago

ConvertTo & ConvertFrom would be my choices though that said we don't usually output a file from those commands

JustinGrote commented 3 years ago

@kilasuit I agree, a lot of these are tricky because some "de-facto" conventions have evolved that the action being done here doesn't quite fit into the column. For instance the Convert verbs are almost exclusively used by piping and output an object.

rjmholt commented 3 years ago

Just found out that Build actually isn't valid in PS 7.0 unfortunately (VerbsLifecycle.Build doesn't compile against the 7.0 SDK) — it seemed like the most appropriate choice to me, but not sure if this rules it out entirely

iSazonov commented 3 years ago

Just found out that Build actually isn't valid in PS 7.0 unfortunately (VerbsLifecycle.Build doesn't compile against the 7.0 SDK) — it seemed like the most appropriate choice to me, but not sure if this rules it out entirely

I an very wonder, it presents in 7.*. Bug in SDK?

bjompen commented 3 years ago

While I agree Build would be best yet undoable if ps5 compatibility is the case, I would also argue that according to the descriptions "export" would be most fitting.

Encapsulates the primary input into a persistent data store, such as a file, or into an interchange format sounds to me, non native, like it fits pretty well.

JustinGrote commented 3 years ago

Export is my leading 5.1-compatible verb vote, since it is typically used with a path in other situations. The Build alias could be conditional on a 7.1 version check.

rdbartram commented 3 years ago

Convert, Export, merge or new.

the publish keyword really isn’t right in my opinion.

invoke could be an option but the noun afterwards would have to change to invoke-psarmtemplateconversion or invoke-psarm templatebuild. Saying invoke-psarmtemplate would imply to me we are actually running/deploying the template. if you were just to say that to me, that’s what I would think.

convert-psarmtojson or something like that would be my preference and is most clear to someone not knowing the semantics of what is going on

rdbartram commented 3 years ago

I guess the end goal is to avoid this all together right and have the new-azresourcegroupdeployment command accept psarm.ps1 files natively

JustinGrote commented 3 years ago

@rdbartram that would be ideal of course once it gets to a rc status I was planning to raise an issue or PR. There's precedent for it since they now accept Bicep files (even if all they do is just run bicep and then submit the json).

kilasuit commented 3 years ago

I don't think this is needed as least not what is requested in the issue title and for proper downlevel support & proper usability

Aliases should be short, and should ideally never need to be a full cmdlet/function name aliasing from one long name to another. We don't need or want the Enable-AzureRmAlias pain again if it can be avoided.

A shorter alias though of something along the lines of ipsab short for Invoke-PSArmTemplateBuildI think makes more sense overall

Jaykul commented 3 years ago
Verb Group Description Why / Why not
New Common Creates a resource You're not creating a resource
Write Communications Adds information to a target This isn't about communication
Out Data Sends data out of the environment We are not sending data out
Export Data Encapsulates the primary input into a persistent data store, such as a file, or into an interchange format Well, this is ... sort-of what we're doing
Build Lifecycle Creates an artifact (usually a binary or document) out of some set of input files (usually source code or declarative documents) Yes, this is what we're doing, exactly.

You should never use Invoke-<something>ActualVerb or Invoke-ActualVerb<something> just because you feel like the right verb isn't "allowed". Save Invoke for when you're calling something else, like Invoke-Item or Invoke-Sql or Invoke-WebRequest or Invoke-WSManAction` ...

In this case, Build-PSArmTemplate seems obvious to me. Or Build-ArmTemplate if you think the right noun for "Build" is it's output (Build-CSProj vs Build-Executable seems right to me).

If you can't bear to see a warning on PowerShell 5, then I think it should be Export-ArmTemplate with a Build-PSArmTemplate alias (but for the love of scripting, don't make the alias conditional).

StartAutomating commented 3 years ago

Given that:

  1. Build is not an approved verb for <7
  2. This command does not publish the template, it converts your PSArm template into an ARM template

I'd like to suggest / request that the verb be "ConvertTo" or "Convert"

Note: ConvertTo will imply eventually building a ConvertFrom

JustinGrote commented 3 years ago

While ConvertTo and ConvertFrom generally mean changing data types and outputting them as objects to output rather than to file, it's a reasonable consideration over Export/Build I think.

StartAutomating commented 3 years ago

While ConvertTo and ConvertFrom generally mean changing data types and outputting them as objects to output rather than to file, it's a reasonable consideration over Export/Build I think.

Thanks for the vote of support!

This is also the beautiful part:

It is converting. Specifically: Converting from a ScriptBlock containing PSArm commands to JSON.

Though, perhaps, just perhaps, it should be able to output an object as well 😄

Additionally, ConvertTo could be huge in helping adoption.

ConvertTo-PSArmTemplate could generate a PSArm template from an existing template / template object, thus letting every template under the sun quickly and cheerfully convert to PSArm.

When implemented right, these two verbs are really valuable.