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

Implement --service-name as swap-in replacement for --service-id #495

Merged
merged 4 commits into from
Jan 4, 2022

Conversation

Integralist
Copy link
Collaborator

Fixes #481

@Integralist Integralist added the enhancement New feature or request label Dec 20, 2021
@Integralist Integralist requested review from a team and kailan and removed request for a team December 20, 2021 18:05
Copy link
Collaborator Author

@Integralist Integralist left a 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...

Comment on lines +34 to +40
// 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)
}
Copy link
Collaborator Author

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.

Comment on lines +106 to +125
// 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")
}

Copy link
Collaborator Author

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)
Copy link
Collaborator Author

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.

Copy link
Member

@kailan kailan left a 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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow use of --service-name as an swap-in replacement for -s / --service-id in calls that require is
2 participants