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

[sdk] add optional display name and tag fields to project templates #14587

Merged
merged 3 commits into from
Nov 18, 2023

Conversation

dschaller
Copy link
Member

Description

Add support for two new fields to a project template block:

  • DisplayName which is used to store a user friendly template name
  • Tags which is used to store additional template metadata that is author-defined (e.g. team, environment, etc.)

This information will be used in the near-term within the console but longer term could also be used within the CLI.

Checklist

  • I have run make tidy to update any new dependencies
  • I have run make lint to verify my code passes the lint check
  • I have formatted my code using gofumpt
  • I have added tests that prove my fix is effective or that my feature works

Since this change only adds optional fields I didn't think tests would be worthwhile. I can add tests if it's thought they would be valuable.

  • I have run make changelog and committed the changelog/pending/<file> documenting my change
  • Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version

@dschaller dschaller requested review from a team, blampe and komalali and removed request for a team November 15, 2023 22:36
@pulumi-bot
Copy link
Contributor

pulumi-bot commented Nov 15, 2023

Changelog

[uncommitted] (2023-11-18)

Features

  • [sdk/go] add optional display name and tag fields to project templates
    #14587

// Description is an optional description of the template.
Description string `json:"description,omitempty" yaml:"description,omitempty"`
// Quickstart contains optional text to be displayed after template creation.
Quickstart string `json:"quickstart,omitempty" yaml:"quickstart,omitempty"`
// Config is an optional template config.
Config map[string]ProjectTemplateConfigValue `json:"config,omitempty" yaml:"config,omitempty"`
// Important indicates the template is important and should be listed by default.
Copy link
Member Author

Choose a reason for hiding this comment

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

quick note on this change - it looks like this used to be the case but is no longer true since templatesToOptionArrayAndMap is always called with showAll as true

@Frassle
Copy link
Member

Frassle commented Nov 16, 2023

This information will be used in the near-term within the console but longer term could also be used within the CLI.

What's the exceptions for how these will work with the CLI?

DisplayName I guess would be used when listing the template in "new", but not clear what "tags" would be used for? Should that be used for the default value of stack tags? If its only for the service maybe it should be named more explicitly that it's only used by the cloud console?

@dschaller
Copy link
Member Author

Yeah, display name could be used for listing out templates from the new command. Tags could also be used there acting as filters based on user input (e.g. new --tag env=production) or could be shown in the new list template subcommand.

@Frassle
Copy link
Member

Frassle commented Nov 16, 2023

I suspect we'll get a few confusions over tags getting confused with stack tags but not sure there's much to help there.

@Frassle Frassle requested a review from a team November 16, 2023 09:16
@dschaller
Copy link
Member Author

Makes sense how that could potentially be confusing since these tags are unique to the template and not necessarily the stack. I think there are a few options:

  • Rename this field to something like TemplateTags which would help clarify where these are applied
  • We could also propagate these tags to the stack upon creation / offer them as tag options during creation

What do you think @Frassle?

@cnunciato
Copy link
Member

cnunciato commented Nov 17, 2023

Agree the presence of tags could be confusing (are these stack tags? resource tags? actually they’re neither…), but templateTags seems redundant as a child of template.

How about labels? Still means basically the same thing, is sufficiently short to serve the CLI use case, and doesn’t overload a term we’re already using elsewhere. Also brings to mind GitHub labels, which users may find familiar and which serve a similar purpose (grouping, filtering).

categories could be another option — though might be a little more cumbersome to use at the command line.

I do think tags could be fine though. It’s probably worth pointing out that the other two kinds of tags we support (stack and resource) aren’t the same thing, either, so while introducing a third kind of tag might seem potentially confusing, at least it’d be consistent. I’ve definitely warmed up to this idea the more I’ve thought about it. 😊

@MitchellGerdisch
Copy link

How about something like metadata instead of tags or templateTags since it seems like it'll be a bit of a catch-all to transmit information to the NPW feature?

@cnunciato
Copy link
Member

cnunciato commented Nov 17, 2023

How about something like metadata

I like this too — it’s clear and leaves room for other kinds of metadata, beyond just tags/labels. Although it’s somewhat less clear to me how it’d work at the CLI.

@AaronFriel
Copy link
Member

If I read the change here correctly, it'll be present under template:

template:
  tags: ...

That's pretty clear to me that this is for the template, and not anything else.

@dschaller
Copy link
Member Author

dschaller commented Nov 17, 2023

I can swap to metadata if it'll be clearer - definitely don't want to cause confusion with overloading tags even more. My only concern with metadata is that it might be too broad where it might not be clear to user's how this metadata will translate to the template but it's likely not a huge issue.

Updated :)

@dschaller dschaller added this pull request to the merge queue Nov 17, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 17, 2023
Copy link
Member

@Frassle Frassle left a comment

Choose a reason for hiding this comment

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

That CI error isn't due to your change but due to template updating and CI not handling it.

But kinda handy this didn't merge because I noticed while trying this out that "sdk/go/common/workspace/project.json" needs updating otherwise these templates refuse to load!

I think a couple of entries like:

"displayName": {
    "description":"Optional name to use as the display name of the template.",
    "type":[
        "string",
        "null"
    ]
},

and

"metadata":{
    "description":"Metadata is a map of key/value pairs to associate with the template.",
    "type":[
        "object",
        "null"
    ],
    "additionalProperties":{
        "type": "string"
    }

In the template/properties block covers it.

@dschaller
Copy link
Member Author

thanks for catching that @Frassle! will update and push

@Frassle Frassle added this pull request to the merge queue Nov 18, 2023
Merged via the queue into master with commit 85a9c8b Nov 18, 2023
44 checks passed
@Frassle Frassle deleted the dschaller/projectTemplateFields branch November 18, 2023 19:43
github-merge-queue bot pushed a commit that referenced this pull request Nov 30, 2023
### Features

- [cli/config] Include config values from ESC in `pulumi config`
  [#14560](#14560)

- [cli/config] Add commands for managing stack environments
  [#14628](#14628)

- [cli/config] Add a command to create an ESC environment from stack
config
  [#14634](#14634)

- [sdk/go] add optional display name and tag fields to project templates
  [#14587](#14587)

- [cli/plugin] Load policy packs in parallel on startup to reduce
startup time
  [#14495](#14495)

- [sdkgen/{go,nodejs,python}] Resource methods with plain: true outputs
can now return plain values without an Output wrapper. In particular,
this feature enables resource methods to serve as explicit provider
factories by returning preconfigured explicit providers.
  [#13592](#13592)


### Bug Fixes

- [auto/go] Fix a datarace in cloning git repos.
  [#14643](#14643)

- [auto/go] Fixes event stream lag on windows runtime
  [#14659](#14659)

- [engine] Engine now correctly handles any resource name.
  [#14107](#14107)

- [engine] Fix a panic in cancellation.
  [#14612](#14612)

- [engine] Fix root directory passed to langauge plugins when starting
pulumi in a subfolder.
  [#14684](#14684)

- [sdkgen] Schemas now validate that 'urn' and 'id' are not used as
resource output properties.
  [#14637](#14637)

- [sdkgen] Fixes marshalling the "plain" flag from object or resource
properties
  [#14648](#14648)

- [programgen/nodejs] Fix generated readFile function so that it
includes the encoding and returns a string
  [#14633](#14633)

- [sdkgen/{dotnet,go,nodejs,python}] No longer writing out name and
project from alias definitions into SDKs, only type
  [#14625](#14625)

- [sdk/go] Fix optional handling on nested props
  [#14629](#14629)

- [sdkgen/go] Fixes plain and optional properties for generated types
for Go SDKs using generics
  [#14616](#14616)

- [sdkgen/go] Generate non-plain type variants for types used as inputs
inside unions
  [#14679](#14679)

- [sdk/python] Introduces RuntimeError when we detect a cycle upon
adding dependencies to the graph. Additionally adds
"PULUMI_ERROR_ON_DEPENDENCY_CYCLES" as a new environment variable to
control this behavior. Set to `False` to return to the previous
behavior, which could potentially re-introduce infinite hangs for some
programs.
  [#14597](#14597)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants