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

Refactor client package and remove Execute funcs #133

Open
jwreagor opened this issue Mar 1, 2018 · 1 comment
Open

Refactor client package and remove Execute funcs #133

jwreagor opened this issue Mar 1, 2018 · 1 comment
Milestone

Comments

@jwreagor
Copy link
Contributor

jwreagor commented Mar 1, 2018

I want to discuss refactoring the client package because it's been a thorn in my eye since I merged together both triton-go and manta-go. I'd like to nail down a few key areas of a future implementation before we actually begin touching code.

Problem

We currently have 4 (and maybe a 5th soon) Client.ExecuteRequest* functions. These functions essentially combine all the behavior around preparing then sending our backend API requests to CloudAPI, Manta, and TSG, using the net/http client package (even if this is relatively straight forward).

Work has gone into combining these functions and drawing out their similarities. Today the task is much easier, and there is little that is unique about each besides...

  1. The backend endpoint URL used (input).
  2. What is returned by the functions themselves, whether a *http.Response or raw body (output).

Each function follows a rather common pattern.

  1. Accept inputs of HTTP Method, Path, Body, Query Parameters.
  2. Marshal Body into JSON.
  3. Combine and encode Query parameters into Endpoint URL and Path.
  4. Generate a new http.Request object.
  5. Attach Headers into http.Request.
  6. HTTP request signing, attach Authorization and Date for authentication.
  7. Optionally override any headers that have been generated.
  8. Send client request.
  9. Return successful Body or http.Response.
  10. Client.DecodeError from within Body if request was not successful.

Work has gone into simplifying these and now is a great time to move to a new abstraction.

Requirements

  • Combine all Client.ExecuteRequest* functions into a single abstraction.
  • Provide a way to send client requests to varying backend endpoints.
  • Provide a way to share common request behavior, setting Headers and Authentication.
  • Provide a way for us to instrument at any layer of the request cycle.
  • Return a single common struct that all upstack API functions can utilize.
  • Maintain all existing public API contracts.

Thoughts

I think we should explore a layered/middleware type implementation that allows us to inject behavior at any point in the request generation and response handling pattern above. This should allow us to break apart and share common behavior used across every API request while giving us future flexibility to swap out behavior for things like authentication or encoding.

While we can definitely learn from the AWS SDK we also do not need everything they have (yet).

/cc @kwilczynski @stack72

@jwreagor jwreagor added this to the 1.x milestone Mar 1, 2018
@jwreagor
Copy link
Contributor Author

jwreagor commented Mar 2, 2018

An example was mentioned off thread that lends credence to developing better support for instrumenting, specifically with Instana.

@kwilczynski kwilczynski self-assigned this Mar 19, 2018
@kwilczynski kwilczynski removed their assignment Aug 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants