Skip to content

Make buildRequest method public. - #331

Merged
jmcarp merged 1 commit into
mainfrom
jmcarp/build-request-public
Oct 14, 2025
Merged

Make buildRequest method public.#331
jmcarp merged 1 commit into
mainfrom
jmcarp/build-request-public

Conversation

@jmcarp

@jmcarp jmcarp commented Oct 14, 2025

Copy link
Copy Markdown
Contributor

Generally, users don't need to call the client's buildRequest method, since it's used internally by generated public methods. However, users may occasionally want to invoke buildRequest directly, analogous to oxide api in the CLI. For example, they may want to use an API endpoint that's not yet supported in the generated client. This patch makes the buildRequest method public by renaming it to BuildRequest.

@jmcarp
jmcarp requested a review from a team as a code owner October 14, 2025 14:38

@sudomateo sudomateo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No objections from me. I like the escape hatch `oxide api functionality.

Comment thread oxide/lib.go Outdated
Comment on lines +279 to +280
// buildRequest creates an HTTP request to interact with the Oxide API.
func (c *Client) buildRequest(ctx context.Context, body io.Reader, method, uri string, params, queries map[string]string) (*http.Request, error) {
// BuildRequest creates an HTTP request to interact with the Oxide API.
func (c *Client) BuildRequest(ctx context.Context, body io.Reader, method, uri string, params, queries map[string]string) (*http.Request, error) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest a new BuildRequest public method that wraps the existing privatebuildRequest. This way we can provide a nicer public API to users where the arguments are defined in a struct instead of spread around several parameters, like:

type Request struct {
	Method string
	URL    string
	Body   io.Reader
	Params map[string]string
	Query  map[string]string
}

func (c *Client) BuildRequest(ctx context.Context, req Request) (*http.Request, error) {
	buildRequest(...)
	// ...
}

We could also go one step further and make the HTTP request and return the *http.Response to users directly. This seems to be a pretty common set of steps after buildRequest is called internally:

	// Create the request
	req, err := c.buildRequest(...)
	if err != nil {
		return nil, fmt.Errorf("error building request: %v", err)
	}

	// Send the request.
	resp, err := c.client.Do(req)
	if err != nil {
		return nil, fmt.Errorf("error sending request: %v", err)
	}
	defer resp.Body.Close()

	// Create and return an HTTPError when an error response code is received.
	if err := NewHTTPError(resp); err != nil {
		return nil, err
	}

	// Decode the body from the response.
	if resp.Body == nil {
		return nil, errors.New("request returned an empty body in the response")
	}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated, WDYT? I think we can leave error checking and parsing the body to the caller.

@sudomateo sudomateo Oct 14, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for moving the discussion here! I like the updated recommendation personally. It just needs some Go doc comments but I trust you both to finalize the implementation. I have to give a conference talk soon then travel home so ping if needed!

@jmcarp
jmcarp force-pushed the jmcarp/build-request-public branch 3 times, most recently from 85ad78b to 3d2fbeb Compare October 14, 2025 15:57
Most of the time, sdk users will interact with the api using generated client
methods. However, in some cases it may be useful to interact with the api more
directly, analogous to `oxide api` in the cli. For example, users may want to
invoke an api method that isn't yet supported in the sdk. This patch adds a new
public method called MakeRequest that allows users to send custom requests to
the api.
@jmcarp
jmcarp force-pushed the jmcarp/build-request-public branch from 3d2fbeb to 946c00e Compare October 14, 2025 16:01

@lgfa29 lgfa29 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating it!

@jmcarp
jmcarp merged commit bd541f4 into main Oct 14, 2025
1 check passed
@jmcarp
jmcarp deleted the jmcarp/build-request-public branch October 14, 2025 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants