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

allow a way of creating client from known environment variables #166

Closed
halkyon opened this issue Dec 10, 2023 · 1 comment
Closed

allow a way of creating client from known environment variables #166

halkyon opened this issue Dec 10, 2023 · 1 comment
Labels
enhancement Improvements to the current code that are not breaking current functionality

Comments

@halkyon
Copy link

halkyon commented Dec 10, 2023

At the moment you have to specify token and host when creating the client. Since it seems OXIDE_HOST and OXIDE_TOKEN was already used for the Terraform provider and Oxide CLI, you could possibly have something like:

client, err := oxide.NewClientFromEnvironmentCredentials()
if err != nil {
  panic(err)
}
@karencfv karencfv added the enhancement Improvements to the current code that are not breaking current functionality label Dec 10, 2023
sudomateo added a commit to sudomateo/oxidecomputer-oxide.go that referenced this issue Dec 22, 2023
Previously, there were two exported functions to create an Oxide API
client.

- `func NewClient(token, userAgent, host string) (*Client, error)`
- `func NewClientFromEnv(userAgent string) (*Client, error)`

These two functions required a user agent to be passed as an argument,
which should be an optional argument instead. Additionally, having two
separate functions means a user must switch functions when they wish to
switch to or from using an environment to configure the client.

The code always created a custom `http.RoundTripper` to be used as the
transport for the underlying HTTP client. The only function of this
custom tranport was to add HTTP headers to the request.

This patch updates the SDK to export a single function to create an
Oxide API client.

- `func NewClientWithOpts(opts ...Opt) (*Client, error)`

This new function uses the functional options pattern to accept optional
configuration for the client, taking inspiration from the popular
[Docker Go SDK](https://github.com/moby/moby/blob/46f7ab808b9504d735d600e259ca0723f76fb164/client/client.go#L182).
Now, a user can not only build an API client from their environment, but
also override the user agent or HTTP client used.

The custom transport was removed and the logic to set HTTP headers for
the request was moved into the `buildRequest` method where it will have
access to the user agent string and the request to set the headers.

With these changes, it's possible for a user to create a client without
a host or token, but that's something that I'd like to discuss as part
of this patch since I'm not aware of what the desired behavior is for
this client.

Closes: oxidecomputer#165, oxidecomputer#166
sudomateo added a commit to sudomateo/oxidecomputer-oxide.go that referenced this issue Dec 22, 2023
Previously, there were two exported functions to create an Oxide API
client.

- `func NewClient(token, userAgent, host string) (*Client, error)`
- `func NewClientFromEnv(userAgent string) (*Client, error)`

These two functions required a user agent to be passed as an argument,
which should be an optional argument instead. Additionally, having two
separate functions means a user must switch functions when they wish to
switch to or from using an environment to configure the client.

The code always created a custom `http.RoundTripper` to be used as the
transport for the underlying HTTP client. The only function of this
custom tranport was to add HTTP headers to the request.

This patch updates the SDK to export a single function to create an
Oxide API client.

- `func NewClientWithOpts(opts ...Opt) (*Client, error)`

This new function uses the functional options pattern to accept optional
configuration for the client, taking inspiration from the popular
[Docker Go SDK](https://github.com/moby/moby/blob/46f7ab808b9504d735d600e259ca0723f76fb164/client/client.go#L182).
Now, a user can not only build an API client from their environment, but
also override the user agent or HTTP client used.

The custom transport was removed and the logic to set HTTP headers for
the request was moved into the `buildRequest` method where it will have
access to the user agent string and the request to set the headers.

With these changes, it's possible for a user to create a client without
a host or token, but that's something that I'd like to discuss as part
of this patch since I'm not aware of what the desired behavior is for
this client.

Closes: oxidecomputer#165, oxidecomputer#166
sudomateo added a commit to sudomateo/oxidecomputer-oxide.go that referenced this issue Jan 17, 2024
Previously, there were two exported functions to create an Oxide API
client.

- `func NewClient(token, userAgent, host string) (*Client, error)`
- `func NewClientFromEnv(userAgent string) (*Client, error)`

These two functions required a user agent to be passed as an argument,
which should be an optional argument instead. Additionally, having two
separate functions means a user must switch functions when they wish to
switch to or from using an environment to configure the client.

The code always created a custom `http.RoundTripper` to be used as the
transport for the underlying HTTP client. The only function of this
custom tranport was to add HTTP headers to the request.

This patch updates the SDK to export a single function to create an
Oxide API client.

- `func NewClient(cfg *Config) (*Client, error)`

This new function uses a new `Config` type to accept optional
configuration for the client. Now, a user can not only build an API
client from their environment, but also override the user agent or HTTP
client used.

The custom transport was removed and the logic to set HTTP headers for
the request was moved into the `buildRequest` method where it will have
access to the user agent string and the request to set the headers.

Closes: oxidecomputer#165, oxidecomputer#166
sudomateo added a commit to sudomateo/oxidecomputer-oxide.go that referenced this issue Jan 17, 2024
Previously, there were two exported functions to create an Oxide API
client.

- `func NewClient(token, userAgent, host string) (*Client, error)`
- `func NewClientFromEnv(userAgent string) (*Client, error)`

These two functions required a user agent to be passed as an argument,
which should be an optional argument instead. Additionally, having two
separate functions means a user must switch functions when they wish to
switch to or from using an environment to configure the client.

The code always created a custom `http.RoundTripper` to be used as the
transport for the underlying HTTP client. The only function of this
custom tranport was to add HTTP headers to the request.

This patch updates the SDK to export a single function to create an
Oxide API client.

- `func NewClient(cfg *Config) (*Client, error)`

This new function uses a new `Config` type to accept optional
configuration for the client. Now, a user can not only build an API
client from their environment, but also override the user agent or HTTP
client used.

The custom transport was removed and the logic to set HTTP headers for
the request was moved into the `buildRequest` method where it will have
access to the user agent string and the request to set the headers.

Closes: oxidecomputer#165, oxidecomputer#166
sudomateo added a commit to sudomateo/oxidecomputer-oxide.go that referenced this issue Jan 17, 2024
Previously, there were two exported functions to create an Oxide API
client.

- `func NewClient(token, userAgent, host string) (*Client, error)`
- `func NewClientFromEnv(userAgent string) (*Client, error)`

These two functions required a user agent to be passed as an argument,
which should be an optional argument instead. Additionally, having two
separate functions means a user must switch functions when they wish to
switch to or from using an environment to configure the client.

The code always created a custom `http.RoundTripper` to be used as the
transport for the underlying HTTP client. The only function of this
custom tranport was to add HTTP headers to the request.

This patch updates the SDK to export a single function to create an
Oxide API client.

- `func NewClient(cfg *Config) (*Client, error)`

This new function uses a new `Config` type to accept optional
configuration for the client. Now, a user can not only build an API
client from their environment, but also override the user agent or HTTP
client used.

The custom transport was removed and the logic to set HTTP headers for
the request was moved into the `buildRequest` method where it will have
access to the user agent string and the request to set the headers.

Closes: oxidecomputer#165, oxidecomputer#166
sudomateo added a commit to sudomateo/oxidecomputer-oxide.go that referenced this issue Jan 19, 2024
Previously, there were two exported functions to create an Oxide API
client.

- `func NewClient(token, userAgent, host string) (*Client, error)`
- `func NewClientFromEnv(userAgent string) (*Client, error)`

These two functions required a user agent to be passed as an argument,
which should be an optional argument instead. Additionally, having two
separate functions means a user must switch functions when they wish to
switch to or from using an environment to configure the client.

The code always created a custom `http.RoundTripper` to be used as the
transport for the underlying HTTP client. The only function of this
custom tranport was to add HTTP headers to the request.

This patch updates the SDK to export a single function to create an
Oxide API client.

- `func NewClient(cfg *Config) (*Client, error)`

This new function uses a new `Config` type to accept optional
configuration for the client. Now, a user can not only build an API
client from their environment, but also override the user agent or HTTP
client used.

The custom transport was removed and the logic to set HTTP headers for
the request was moved into the `buildRequest` method where it will have
access to the user agent string and the request to set the headers.

Closes: oxidecomputer#165, oxidecomputer#166
sudomateo added a commit to sudomateo/oxidecomputer-oxide.go that referenced this issue Jan 19, 2024
Previously, there were two exported functions to create an Oxide API
client.

- `func NewClient(token, userAgent, host string) (*Client, error)`
- `func NewClientFromEnv(userAgent string) (*Client, error)`

These two functions required a user agent to be passed as an argument,
which should be an optional argument instead. Additionally, having two
separate functions means a user must switch functions when they wish to
switch to or from using an environment to configure the client.

The code always created a custom `http.RoundTripper` to be used as the
transport for the underlying HTTP client. The only function of this
custom tranport was to add HTTP headers to the request.

This patch updates the SDK to export a single function to create an
Oxide API client.

- `func NewClient(cfg *Config) (*Client, error)`

This new function uses a new `Config` type to accept optional
configuration for the client. Now, a user can not only build an API
client from their environment, but also override the user agent or HTTP
client used.

The custom transport was removed and the logic to set HTTP headers for
the request was moved into the `buildRequest` method where it will have
access to the user agent string and the request to set the headers.

Closes: oxidecomputer#165, oxidecomputer#166
karencfv pushed a commit that referenced this issue Jan 22, 2024
* oxide: refactor exported client API

Previously, there were two exported functions to create an Oxide API
client.

- `func NewClient(token, userAgent, host string) (*Client, error)`
- `func NewClientFromEnv(userAgent string) (*Client, error)`

These two functions required a user agent to be passed as an argument,
which should be an optional argument instead. Additionally, having two
separate functions means a user must switch functions when they wish to
switch to or from using an environment to configure the client.

The code always created a custom `http.RoundTripper` to be used as the
transport for the underlying HTTP client. The only function of this
custom tranport was to add HTTP headers to the request.

This patch updates the SDK to export a single function to create an
Oxide API client.

- `func NewClient(cfg *Config) (*Client, error)`

This new function uses a new `Config` type to accept optional
configuration for the client. Now, a user can not only build an API
client from their environment, but also override the user agent or HTTP
client used.

The custom transport was removed and the logic to set HTTP headers for
the request was moved into the `buildRequest` method where it will have
access to the user agent string and the request to set the headers.

Closes: #165, #166

* oxide: address review feedback

* oxide: correct authorization header and update tests

When the authorization header was moved to the `buildRequest` function,
the `Bearer` part was left out. That meant the total header was:
Authorization: TOKEN

Instead of:
Authorization: Bearer TOKEN

This commit fixes that.
@karencfv
Copy link
Collaborator

Fixed by #180

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements to the current code that are not breaking current functionality
Projects
None yet
Development

No branches or pull requests

2 participants