Proposal: Enrich `draft info` with variable defaults

davidgamero commented 1 year ago

Is your feature request related to a problem? Please describe. When consuming draft as a wrapped binary, the available variable defaults for VERSION and BUILDERVERSION are not exposed non-interactively. Exposing these in the info command would make draft more extensible in this pattern.

This would be particularly useful for the aks-devx-tools vscode extension since it is pioneering our binary consumption pattern

The goal of this PR is to remove the need for static files like languages.ts that are sensitive to becoming out-of-date with each incremental draft version, and would require significant extra work to keep in sync with 3rd party draft templates.

Exposing the following information would make the draft binary more extensible:

Default values for BuilderVariables in the draft.yaml
A formatted/capitalized display name for a selected language/deployType/addon

Mention what platform you want to support the new feature Mention what platform you would like the feature request to live in (az-extension, oss draft, vscode) and select the proper label.

Describe the solution you'd like

Draft info output

The current info command prints the following format in stdout:

{
  "supported_languages": [
    "javascript",
    ...
  ],
 "supported_deployment_types": [
    "helm",
    ...
  ]
}

I propose enriching this information with the following additional sub fields for each language:

{
  "supported_languages": [
    {
      "name": "javascript"
      "display_name": "JavaScript",
      "exampleValues": {
        "VERSION": [
          "14.0",
          "16.0"
        ]
      }
    }
    ..
  ],
 "supported_deployment_types": [
    "helm"
    ..
  ]
}

draft.yml format changes

In order to better support standardized consumption, I propose changing the value field on the variableDefault type from a string to an array of strings. For the CLI, the first default in the array would be used, but allowing additional defaults can be echoed in the info command to provide examples (ex: providing several recommended versions for a language dockerfile).

# Current abbreviated example for a language draft.yaml
language: swift
variables:
  - name: "VERSION"
    description: "the version of swift used by the application"
variableDefaults:
  - name: "VERSION"
    value: 5.5

# Proposed abbreviated changes for a language draft.yaml
language: swift
displayName: Swift # Add a display name for the selected resource (language/deploymentType/addon)
variables:
  - name: "VERSION"
    description: "the version of swift used by the application"
    exampleValues: ["5.5","5.4"] # New field

variableDefaults:
  - name: "VERSION"
     value: "5.5"

Describe alternatives you've considered Alternatives include parsing the raw draft templates in their yaml formats, or creating a second source of truth for this information.

Since one of the core value-adds of draft is its single-binary install that leverages embedded file systems and its focus on extensibility, these alternatives are less in line with the overall project vision.

Additional context

update integration-info test to ensure our json output maintains a strong contract

jaiveerk commented 1 year ago

Another reason why I like the use of an array for defaults for language versions is because it implies users are free to use languages beyond just what the CLI would pre-populate.

Potentially dumb question, but are users allow allowed to put in minor versions that may not be specified in the variable defaults? Or are the versions that would come out with the defaults for each language the only ones we support?

davidgamero commented 1 year ago

Another reason why I like the use of an array for defaults for language versions is because it implies users are free to use languages beyond just what the CLI would pre-populate.

Potentially dumb question, but are users allow allowed to put in minor versions that may not be specified in the variable defaults? Or are the versions that would come out with the defaults for each language the only ones we support?

Thanks for the review The defaults are just what is prepopulated in the cli, you can specify any string for rhe builder variables.

qpetraroia commented 1 year ago

The goal of this PR is to remove the need for static files like languages.ts that are sensitive to becoming out-of-date with each incremental draft version, and would require significant extra work to keep in sync with 3rd party draft templates.

I agree. I like this, thanks @davidgamero !

davidgamero commented 1 year ago

another possibility is to add them as suggested values onto the variables themselves in the draft.yaml, which would keep the single-default behaviour

davidgamero commented 1 year ago

After exploring the implementation of this original proposal, the inclusion of multiple default values when only the first one is ever used seems at best confusing for users.

In order to better separate the purpose of these new values, I have the following revision in which the defaultValues optional field is added to the variables in draft.yaml, which separates this feature from the default values entirely, which is more in line with its goal.


# Revised proposed abbreviated changes for a language draft.yaml
language: swift
displayName: Swift # Add a display name for the selected resource (language/deploymentType/addon)
variables:
  - name: "VERSION"
    description: "the version of swift used by the application"
    exampleValues: ["5.5","5.4"] #  New optional field that is used to populate draft info, and which could be used in the cli for suggestions in the future.
variableDefaults:
  - name: "VERSION"
     value: 5.5

# Current abbreviated example for a language draft.yaml
language: swift
variables:
  - name: "VERSION"
    description: "the version of swift used by the application"
variableDefaults:
  - name: "VERSION"
    value: 5.5

Tatsinnit commented 1 year ago

☕️💡 Thank you for the ping and adding me to this: Here are couple of high level ideas to take care. Please feel free to skim over and I am adding them just for any existing vs if these changes might add any breaking scenario.

Looks good (have one key suggestions) and I will leave the info specific subject matter to you, but few things to keep in mind to avoid any breaking change or hard to get adoption will be.

☕️💡🧑‍🔧 Key observation: Default vs suggested / exmaple values clarity is better - it clearly separates the usage and its context.

Suggestions: I think we should keep defaultValue as 1:1 map, unless I am missing a usage scenario and provide some for of, suggestedValues or validValues as separate for suers to know what are the valid values they can supply.
- Reason: I think we should avoid a classic pitfalls of adding many value option as defaultValue which kind of mis-represent the understanding for default word and its understanding when user tries to use it.

Minor:

☕️ Idea only - Regarding the command if this is existing command - we should make sure any existing contracts will not be broken?
☕️ Please note, might be worth to add e-2-e scenario of usage in the empty Additional context subtopic. Might be add what end user for this scenario will expect, also if this has more usage beyond the vscode consumption then worth a mention which could latter be part of documentation.

Thanks heaps,

jaiveerk commented 1 year ago

After exploring the implementation of this original proposal, the inclusion of multiple default values when only the first one is ever used seems at best confusing for users.

In order to better separate the purpose of these new values, I have the following revision in which the defaultValues optional field is added to the variables in draft.yaml, which separates this feature from the default values entirely, which is more in line with its goal.
# Revised proposed abbreviated changes for a language draft.yaml
language: swift
displayName: Swift # Add a display name for the selected resource (language/deploymentType/addon)
variables:
  - name: "VERSION"
    description: "the version of swift used by the application"
    exampleValues: ["5.5","5.4"] #  New optional field that is used to populate draft info, and which could be used in the cli for suggestions in the future.
variableDefaults:
  - name: "VERSION"
     value: 5.5

# Current abbreviated example for a language draft.yaml
language: swift
variables:
  - name: "VERSION"
    description: "the version of swift used by the application"
variableDefaults:
  - name: "VERSION"
    value: 5.5

what if we included defaults in the same place as description/example values? Feel like it would be easier to parse and for users to add new ones

davidgamero commented 1 year ago

Default vs suggested / exmaple values clarity is better - it clearly separates the usage and its context.

I completely agree, and we will use this approach.

☕️ Idea only - Regarding the command if this is existing command - we should make sure any existing contracts will not be broken?

I will make sure to document all the changes in the changelog/readme

☕️ Please note, might be worth to add e-2-e scenario of usage in the empty Additional context subtopic. Might be add what end user for this scenario will expect, also if this has more usage beyond the vscode consumption then worth a mention which could latter be part of documentation.

Good point. I have an integration test for the info command that validates it using a json schema using ajv-cli. I can update that for this PR to ensure we have a strong contract

davidgamero commented 1 year ago

@jaiveerk

what if we included defaults in the same place as description/example values? Feel like it would be easier to parse and for users to add new ones

this is a good point, and we should consider adding this to the backlog to see if we can redesign to get this working

Azure / draft

Proposal: Enrich `draft info` with variable defaults #164

Draft info output

draft.yml format changes