canonical / pebble

Take control of your internal daemons!
https://canonical-pebble.readthedocs-hosted.com/
GNU General Public License v3.0
143 stars 54 forks source link

feat(client): add `Requester` interface #308

Closed anpep closed 11 months ago

anpep commented 11 months ago

The Requester interface aims to replace the set of raw, do, doSync and doAsync internal methods from the Client and abstract them out to an interface that can be plugged into the Client and used by internal client API implementations and clients on external applications extending Pebble.

[!IMPORTANT] This draft PR only focuses on the interface and a reference implementation. For this reason, no existing code from the client was removed or refactored, so right now there are a lot of redundancies and a huge room for refactoring the requester implementation.

I'd like to gather feedback on the Requester interface itself.

Also, I'd appreciate some feedback on the handling of maintenance and warningCount/warningTimestamp.

Example usage for a synchronous API

// Change fetches information about a Change given its ID.
func (client *Client) Change(id string) (*Change, error) {
    var chgd changeAndData
        ctx := context.Background()
    _, err := client.Requester.RequestSync(ctx, "GET", "/v1/changes/"+id, &RequestOptions{
        Output: &chgd,
    })
    if err != nil {
        return nil, err
    }
    chgd.Change.data = chgd.Data
    return &chgd.Change, nil
}

Example usage for an asynchronous API

// Exec starts a command with the given options, returning a value
// representing the process.
func (client *Client) Exec(opts *ExecOptions) (*ExecProcess, error) {
    /* ... */

    var body bytes.Buffer
    err := json.NewEncoder(&body).Encode(&payload)
    if err != nil {
        return nil, fmt.Errorf("cannot encode JSON payload: %w", err)
    }
    info, err := client.Requester.RequestAsync(ctx, "POST", "/v1/exec", &RequestOptions{
        Headers: map[string]string{"Content-Type": "application/json"},
        Body: &body,
    })
    if err != nil {
        return nil, err
    }

    var result execResult
    if err = json.Unmarshal(info.Result, &result); err != nil {
        return nil, fmt.Errorf("cannot unmarshal JSON response: %w", err)
    }

    changeID := info.ChangeID
    taskID := result.TaskID

    /* ... */
}
niemeyer commented 11 months ago

Dropped the Polish tag as polishing is generally making things behave the same way but better. This is a major API change.

flotter commented 11 months ago

@anpep I have taken of this PR as per our agreement, and following conversations with Gustavo I have made some changes and pushed it into a branch I own. I will close this PR for now to reduce the traffic on the Pebble repo.