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

Split out interfaces for Retrier vs HTTP Client #19

Closed
rShetty opened this issue Feb 24, 2018 · 3 comments
Closed

Split out interfaces for Retrier vs HTTP Client #19

rShetty opened this issue Feb 24, 2018 · 3 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@rShetty
Copy link
Contributor

rShetty commented Feb 24, 2018

We have a common big interface for retrier related methods vs those for making http requests.
We could have Client interface for HTTP related methods and Retriable interface for retrier related methods, segregating the interfaces

@rShetty rShetty added help wanted Extra attention is needed good first issue Good for newcomers labels Feb 24, 2018
@darshanime
Copy link
Contributor

(hello, I am learning Go and would like to contribute (please pardon my asking dumb questions!))

Currently we have separate interfaces: Client and Retriable:

// client.go
type Client interface {
	Get(url string, headers http.Header) (*http.Response, error)
	Post(url string, body io.Reader, headers http.Header) (*http.Response, error)
	Put(url string, body io.Reader, headers http.Header) (*http.Response, error)
	Patch(url string, body io.Reader, headers http.Header) (*http.Response, error)
	Delete(url string, headers http.Header) (*http.Response, error)
	Do(req *http.Request) (*http.Response, error)

	SetRetryCount(count int)
	SetRetrier(retrier Retriable)
}

// retry.go
type Retriable interface {
	NextInterval(retry int) time.Duration
}

What does this issue aim to achieve?

On a side note, we could look at merging the Backoff and Retriable interface. They are both related and this could simplify the heimdall API.

type Backoff interface {
	Next(retry int) time.Duration
}

@rShetty
Copy link
Contributor Author

rShetty commented Mar 23, 2018

@darshanime It is always good to have one method interfaces. These help provide composeability when using these/implementing them.

The point of this issue was that we have Client interface which has grown quite a lot and needs splitting. The way we do it still being discussed and debated.

@rShetty
Copy link
Contributor Author

rShetty commented Jun 22, 2018

With #33 this might not be required.

@rShetty rShetty closed this as completed Jul 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants