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

Context Support? #84

Open
dd-caleb opened this issue Jul 26, 2018 · 1 comment
Open

Context Support? #84

dd-caleb opened this issue Jul 26, 2018 · 1 comment

Comments

@dd-caleb
Copy link

dd-caleb commented Jul 26, 2018

Was wondering if there was any desire to support context.Context?

I'm working on adding tracing support to some commonly used libraries and Context support is usually a pre-requisite. I thought of a couple ways:

  1. The appengine API has:

    func Add(c context.Context, item *Item) error
    func AddMulti(c context.Context, item []*Item) error
    func CompareAndSwap(c context.Context, item *Item) error
    ...

    So maybe there could be a new type of Client with these method signatures instead of the current ones? Something like NewContextualClient(server ... string) *ContextualClient?

  2. The github.com/go-redis/redis library has:

    func (c *Client) WithContext(ctx context.Context) *Client

    So you could chain calls like: client.WithContext(ctx).Add(item)

I briefly looked into the second option and I think it could work, but there are a few places where it's not super-clean in order to preserve the existing API (the field names on the Client should probably be in some sort of Pool, and keeping zero-value-struct support is tricky)

If context.Context isn't a great fit for this library, it's probably not essential for tracing because memcache is a terminal span in a trace anyway. I could add the WithContext method to my wrapper and any deadline on the context would just be ignored.

@JensRantil
Copy link

I'd vote for not adding tracing support in this package and instead use a wrapper like you propose. However, there might be a benefit in having context support for cancellation of operations (which is something I'm interested in).

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

3 participants