feat: Allow plugins to be downloaded using a user team_name context
#162
Conversation
managedplugin/download.go
Outdated
| @@ -92,7 +92,7 @@ func getURLLocation(ctx context.Context, org string, name string, version string | |||
| return "", fmt.Errorf("failed to find plugin %s/%s version %s", org, name, version) | |||
| } | |||
|
|
|||
| func DownloadPluginFromHub(ctx context.Context, authToken, localPath, team, name, version string, typ PluginType) error { | |||
| func DownloadPluginFromHub(ctx context.Context, authToken, teamName, localPath, pluginTeam, name, version string, typ PluginType) error { | |||
There was a problem hiding this comment.
I wonder if it's time to use a struct here so it's not so easy to get the order of all the string parameters wrong
There was a problem hiding this comment.
I would love that but how far to take it? This pattern is everywhere and technically the methods are public.
There was a problem hiding this comment.
Technically public yes, but we're already breaking the signature with this change by adding a parameter, right? May as well make it a struct
Maybe something like func DownloadPluginFromHub(ctx context.Context, opts HubDownloadOptions) with
type HubDownloadOptions struct {
AuthToken string
TeamName string
LocalPath string
PluginTeam string
PluginKind string
PluginName string
PluginVersion string
}
There was a problem hiding this comment.
Good suggestion - added in 2bdb221
| aj := "application/json" | ||
|
|
||
| switch { | ||
| case ops.TeamName != "": |
There was a problem hiding this comment.
nit: Personally I would have used if/else here (or even just if with return), as it's a switch with a single case :)
There was a problem hiding this comment.
I agree but leaned towards reviewer has final say
There was a problem hiding this comment.
Any reason to prefer a switch given it is a single case + default @disq ?
There was a problem hiding this comment.
Subjectively it looks more symmetrical and clearer than an if block or an if else statement if it's the last thing a fn does.
Adding the ability to pass the
team_nameto the download. This will be passed in by the CLI as either theteam_namein the user's saved configuration (i.e.cloudquery switchconfig) or theteam_nameassociated with the API key being used.Ref: https://github.com/cloudquery/cloudquery-issues/issues/835