-
Notifications
You must be signed in to change notification settings - Fork 59
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
Implement --service-name
as swap-in replacement for --service-id
#495
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've commented to help highlight the key changes...
// RegisterServiceNameFlag defines a --service-name flag that will attempt to | ||
// acquire the Service ID associated with the given service name. | ||
// | ||
// See: cmd.OptionalServiceNameID.Parse() | ||
func (b Base) RegisterServiceNameFlag(action kingpin.Action, dst *string) { | ||
b.CmdClause.Flag("service-name", "The name of the service").Action(action).StringVar(dst) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's the new abstraction for creating a --service-name
flag.
// OptionalServiceNameID represents a mapping between a Fastly service name and | ||
// its ID. | ||
type OptionalServiceNameID struct { | ||
OptionalString | ||
} | ||
|
||
// Parse returns a service ID based off the given service name. | ||
func (sv *OptionalServiceNameID) Parse(client api.Interface) (serviceID string, err error) { | ||
services, err := client.ListServices(&fastly.ListServicesInput{}) | ||
if err != nil { | ||
return serviceID, fmt.Errorf("error listing services: %w", err) | ||
} | ||
for _, s := range services { | ||
if s.Name == sv.Value { | ||
return s.ID, nil | ||
} | ||
} | ||
return serviceID, errors.New("error matching service name with available services") | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is where we implement the logic for mapping a service name to an ID.
@@ -39,6 +40,7 @@ func NewCreateCommand(parent cmd.Registerer, globals *config.Data, data manifest | |||
c.manifest = data | |||
c.CmdClause = parent.Command("create", "Create a backend on a Fastly service version").Alias("add") | |||
c.RegisterServiceIDFlag(&c.manifest.Flag.ServiceID) | |||
c.RegisterServiceNameFlag(c.serviceName.Set, &c.serviceName.Value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is an example of using the new --service-name
abstraction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a change I've been looking forward to and it was easy to review thanks to the comments 👍
ship it!
Fixes #481