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

Support contexts #34

Merged
merged 2 commits into from Mar 9, 2017
Merged

Support contexts #34

merged 2 commits into from Mar 9, 2017

Conversation

arnaud-lb
Copy link
Contributor

Implements #32

At the API level, this adds:

  • BackOffContext interface: A BackOff that also supports contexts
  • WithContext(BackOff, context.Context): Returns a BackOffContext from a BackOff

Retry() and Ticker still accept BackOff implementations, but also support BackOffContext implementations.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 82.051% when pulling eff3c63 on arnaud-lb:context into b02f2bb on cenkalti:master.

@arnaud-lb
Copy link
Contributor Author

arnaud-lb commented Feb 1, 2017

This also breaks the compatibility with older Go versions which don't have the context package.

If you want to maintain the compatibility, we should be able to support older Go versions by declaring a backoff.Context interface, and accepting this interface instead of context.Context:

type Context interface {
 	Deadline() (deadline time.Time, ok bool)
 	Done() <-chan struct{}
 	Err() error
 	Value(key interface{}) interface{}
 }

@arnaud-lb arnaud-lb mentioned this pull request Feb 1, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 82.051% when pulling 376009d on arnaud-lb:context into b02f2bb on cenkalti:master.

@coveralls
Copy link

coveralls commented Feb 2, 2017

Coverage Status

Coverage increased (+0.02%) to 81.529% when pulling 4af6cef on arnaud-lb:context into b02f2bb on cenkalti:master.

@arnaud-lb
Copy link
Contributor Author

Added support for older Go versions:.

This depends on "golang.org/x/net/context" instead of "context", as the former is always available.

This makes the package compatible with all Go versions supported by "golang.org/x/net/context".

It's still compatible with both "context" and "golang.org/x/net/context" packages.

@coveralls
Copy link

coveralls commented Feb 2, 2017

Coverage Status

Coverage increased (+0.02%) to 81.529% when pulling 997c21a on arnaud-lb:context into b02f2bb on cenkalti:master.

@coveralls
Copy link

coveralls commented Feb 2, 2017

Coverage Status

Coverage increased (+0.02%) to 81.529% when pulling 997c21a on arnaud-lb:context into b02f2bb on cenkalti:master.

@cenkalti
Copy link
Owner

cenkalti commented Feb 9, 2017

@arnaud-lb Sorry for the delay, I was quite busy this week. I still couldn't review this. I will do it as soon as possible.

@cenkalti cenkalti merged commit 8bd6a0c into cenkalti:master Mar 9, 2017
@metalmatze
Copy link

Is there any particular reason not to use the, in Go 1.7 introduced, context from the stdlib?
Do you want to be backwards compatible to 1.3, as it's defined in .travis.yml?

@cenkalti
Copy link
Owner

@metalmatze see #40

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.

None yet

4 participants