-
Notifications
You must be signed in to change notification settings - Fork 27
Open
Description
Different packages in the SDK use different methods of creating clients. This can lead to confusion, and can also easily lead to misconfigured clients that panic.
The organizations package has no constructor, and one creates a client like so:
myClient := organizations.Client{
APIKey: myKey,
HTTPClient: myHTTPClient,
}However, the usermanagement package uses a constructor, and additional options have to be set afterwards:
myClient := usermanagement.NewClient(apiKey)
myClient.HTPClient = myHTTPClientThis is confusing, and can lead to panics when one doesn't carefully read the SDK source code:
myOrgClient := organizations.Client{
APIKey: myKey,
HTTPClient: myHTTPClient,
}
myUserClient := usermanagement.Client{
APIKey: myKey,
HTTPClient: myHTTPClient,
}
// I didn't see `usermanagement.NewClient`, and assumed it worked like `organizations.Client`,
// so the following line panics, since `myUserClient.JSONEncode` is `nil`:
user, err := myUserClient.CreateUser(ctx, opts)I think a better API for both would be the functional option pattern, where I would create clients like so:
myOrgClient := organizations.NewClient(
myKey,
organizations.WithHTTPClient(myHTTPClient),
)
myUserClient := usermanagement.NewClient(
myKey,
usermanagement.WithHTTPClient(myHTTPClient),
)This avoids the need for the somewhat awkward sync.Once in organizations.Client, and makes it easy to ensure that default fields are always set while giving users the ability to override them.
corysullivan, timc13 and viqueen
Metadata
Metadata
Assignees
Labels
No labels