feat: hardcoded resource cmds #203

k3llymariee · 2024-04-23T23:28:11Z

Adds a hardcoded implementation of the NewResourceCmd and NewOperationCmd functions that we plan to generate in the future.

Calling help:

$ go run main.go teams create --help
Create a team. To learn more, read [Creating a team](https://docs.launchdarkly.com/home/teams/creating).

### Expanding the teams response
LaunchDarkly supports four fields for expanding the "Create team" response. By default, these fields are **not** included in the response.

To expand the response, append the `expand` query parameter and add a comma-separated list with any of the following fields:

* `members` includes the total count of members that belong to the team.
* `roles` includes a paginated list of the custom roles that you have assigned to the team.
* `projects` includes a paginated list of the projects that the team has any write access to.
* `maintainers` includes a paginated list of the maintainers that you have assigned to the team.

For example, `expand=members,roles` includes the `members` and `roles` fields in the response.

Usage:
  ldcli teams create [flags]

Flags:
  -d, --data string     Input data in JSON
  -e, --expand string   A comma-separated list of properties that can reveal additional information in the response. Supported fields are explained above.
  -h, --help            help for create

Global Flags:
      --access-token string   LaunchDarkly API token with write-level access
      --base-uri string       LaunchDarkly base URI (default "https://app.launchdarkly.com")

Calling the command w/required flags:

$ go run main.go teams create --data '{"key": "my-team-key", "name": "My Team Name"}'
would be making a request to /api/v2/teams here, with args: map[data:map[key:my-team-key name:My Team Name] expand:]

k3llymariee · 2024-04-23T23:31:39Z

cmd/resource_cmds.go

+		"A team is a group of members in your LaunchDarkly account.",
+		"A team can have maintainers who are able to add and remove team members. It also can have custom roles assigned to it that allows shared access to those roles for all team members. To learn more, read [Teams](https://docs.launchdarkly.com/home/teams).\n\nThe Teams API allows you to create, read, update, and delete a team.\n\nSeveral of the endpoints in the Teams API require one or more member IDs. The member ID is returned as part of the [List account members](/tag/Account-members#operation/getMembers) response. It is the `_id` field of each element in the `items` array.",


this is just a snippet from the teams tag description, which comes from here

I stripped out the header manually, hoping that the format holds true for the other resources (looking like mayybe it is? Will check with Molly).

k3llymariee · 2024-04-23T23:32:39Z

cmd/resource_cmds_test.go

+
+		require.NoError(t, err)
+		s := string(output)
+		assert.Contains(t, s, "would be making a request to /api/v2/teams here, with args: map[data:map[key:team-key name:Team Name] expand:]\n")


haven't mocked out a client yet since we're not actually making any requests. will update this test with a mock client when that's a thing

k3llymariee · 2024-04-23T23:33:02Z

cmd/resources/resources.go

+		Use:   resourceName,
+		Short: shortDescription,
+		Long:  longDescription,
+		//TODO: add tracking here


actually not sure if we wanna add tracking when just the resource cmd is run? maybe we do?

I've been adding them as PersistentPreRun. That means they won't run when just the resource command runs. But they'll run when subcommands on the resource run. This is what that looks like on my branch/PR

yup there's a comment in there - will make sure to add it in there in a subsequent PR!

k3llymariee · 2024-04-23T23:36:33Z

cmd/resource_cmds.go

+	gen_TeamsResourceCmd := resources.NewResourceCmd(
+		rootCmd,
+		"teams",
+		"A team is a group of members in your LaunchDarkly account.",


This is the first sentence from the "Teams" tag description (after the headers).

Our previously established format is "Make requests (list, create, etc.) on {resource name}" - not sure which is better in this context 🤔

k3llymariee · 2024-04-23T23:46:13Z

cmd/root.go

@@ -129,6 +134,7 @@ func Execute(analyticsTracker analytics.Tracker, version string) {
 		FlagsClient:        flags.NewClient(version),
 		MembersClient:      members.NewClient(version),
 		ProjectsClient:     projects.NewClient(version),
+		GenericClient:      &http.Client{Timeout: time.Second * 3},


i guess this isn't required yet but wanted to get feedback on where we'd be passing it in

This seems fine until we can remove the other clients.

dbolson

This looks great

dbolson · 2024-04-24T15:33:03Z

cmd/resources/resources.go

+		}
+	}
+
+	fmt.Fprintf(cmd.OutOrStdout(), fmt.Sprintf("would be making a request to %s here, with args: %s\n", op.Path, paramVals))


Looks like we can remove the fmt.Sprintf since Fprintf acts the same.

dbolson · 2024-04-24T15:34:01Z

cmd/resources/resources.go

+	}
+
+	cmd := &cobra.Command{
+		Args:  validators.Validate(), // tbd on this validator


If we need different validators depending on the operation, could we add it as a field on OperationData?

I'm thinking this is ok for now? Since we're pretty much just validating the required flags, and our existing validator already does that right?

Yeah, that makes sense.

dbolson · 2024-04-24T15:35:41Z

cmd/root.go

@@ -129,6 +134,7 @@ func Execute(analyticsTracker analytics.Tracker, version string) {
 		FlagsClient:        flags.NewClient(version),
 		MembersClient:      members.NewClient(version),
 		ProjectsClient:     projects.NewClient(version),
+		GenericClient:      &http.Client{Timeout: time.Second * 3},


This seems fine until we can remove the other clients.

dbolson · 2024-04-24T16:08:59Z

cmd/resources/resources.go

+	}
+
+	for _, p := range op.Params {
+		shorthand := fmt.Sprintf(p.Name[0:1]) // todo: how do we handle potential dupes


It seems like a low chance, if it's even possible. "params" comes from here. I duplicated the line and reran the generator, getting this error

Operations must have unique `name` + `in` parameters. Repeats of `in:query` + `name:expand`.

oh i meant duplicate shorthands - e.g. limit and list would both be l (not that list is a thing)

we can see if that actually ends up being an issue once we get the rest in there maybe.

Oh, that's interesting. It looks like it will panic with

panic: unable to redefine 'e' shorthand in "get" flagset: it's already used for "expand" flag

I suppose we could iterate through all the existing flags to check if it's already been set and then set it to something else (e2?), or we just don't set the shorthand.

ya or add them to a map and check before adding it. was thinking it could be the first two letters if the one letter was already taken (but even then that's not a guaranteed unique value)

e ex exp expa expan expand

sunnyguduru

Once PersistentPreRun is added to NewResourceCmd it looks good to me.

k3llymariee added 7 commits April 23, 2024 10:02

add barebones cmds for resources and operations

c69118c

wip: add flags

af384fc

add more flag types

e66bddd

remove cmd

72312c2

remove comments

2ccea7d

add (bad) tests, fix output

e871844

fix teams short/long description

109ceeb

k3llymariee commented Apr 23, 2024

View reviewed changes

change back rootCmd name

1d5d925

k3llymariee commented Apr 23, 2024

View reviewed changes

k3llymariee marked this pull request as ready for review April 23, 2024 23:49

k3llymariee requested review from dbolson and sunnyguduru April 23, 2024 23:49

dbolson approved these changes Apr 24, 2024

View reviewed changes

pr feedback

98c6925

sunnyguduru suggested changes Apr 24, 2024

View reviewed changes

sunnyguduru approved these changes Apr 24, 2024

View reviewed changes

k3llymariee merged commit b8dc52a into main Apr 24, 2024
2 checks passed

k3llymariee deleted the kelly/sc-241150/hardcoded-resource-cmds branch April 24, 2024 20:26

github-actions bot mentioned this pull request Apr 23, 2024

chore(main): release 0.9.0 #198

Merged

This was referenced Apr 24, 2024

feat: track quick-start events #205

Closed

feat: track cli setup started event #215

Merged

fix: remove get projects limit #220

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: hardcoded resource cmds #203

feat: hardcoded resource cmds #203

k3llymariee commented Apr 23, 2024 •

edited

Loading

k3llymariee Apr 23, 2024

k3llymariee Apr 23, 2024

k3llymariee Apr 23, 2024

sunnyguduru Apr 24, 2024

k3llymariee Apr 24, 2024 •

edited

Loading

k3llymariee Apr 23, 2024

k3llymariee Apr 23, 2024

dbolson Apr 24, 2024

dbolson left a comment

dbolson Apr 24, 2024

dbolson Apr 24, 2024

k3llymariee Apr 24, 2024

dbolson Apr 24, 2024

dbolson Apr 24, 2024

dbolson Apr 24, 2024

k3llymariee Apr 24, 2024 •

edited

Loading

k3llymariee Apr 24, 2024

dbolson Apr 24, 2024

k3llymariee Apr 24, 2024

dbolson Apr 24, 2024

sunnyguduru left a comment

		"A team is a group of members in your LaunchDarkly account.",
		"A team can have maintainers who are able to add and remove team members. It also can have custom roles assigned to it that allows shared access to those roles for all team members. To learn more, read [Teams](https://docs.launchdarkly.com/home/teams).\n\nThe Teams API allows you to create, read, update, and delete a team.\n\nSeveral of the endpoints in the Teams API require one or more member IDs. The member ID is returned as part of the [List account members](/tag/Account-members#operation/getMembers) response. It is the `_id` field of each element in the `items` array.",

feat: hardcoded resource cmds #203

feat: hardcoded resource cmds #203

Conversation

k3llymariee commented Apr 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k3llymariee Apr 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbolson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k3llymariee Apr 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunnyguduru left a comment

Choose a reason for hiding this comment

k3llymariee commented Apr 23, 2024 •

edited

Loading

k3llymariee Apr 24, 2024 •

edited

Loading

k3llymariee Apr 24, 2024 •

edited

Loading