Skip to content

Mismatch in client construction patterns #397

@jclem

Description

@jclem

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 = myHTTPClient

This 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions