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

Add context to 2.0 storage interfaces #2823

Open
AlekSi opened this Issue Jun 8, 2017 · 9 comments

Comments

Projects
None yet
6 participants
@AlekSi
Copy link
Contributor

AlekSi commented Jun 8, 2017

Having context.Context in Storage, Querier, Appender and probably few other types would be helpful for remote storages. Even if Prometheus will use context.TODO() in the first 2.0 release, having at least somewhat stable interface is a good thing.

I can implement it if you agree.

@gouthamve

This comment has been minimized.

Copy link
Member

gouthamve commented Jun 8, 2017

@AlekSi Thanks for the interest. Yes, the plan is to add context.Context to those types. The bigger issue is how you will plumb into the storage and its presented here: prometheus/tsdb#84

I think it makes sense to tackle the issue in tsdb first before we implement it in Prometheus as the wrapper is pretty thin.

@AlekSi

This comment has been minimized.

Copy link
Contributor Author

AlekSi commented Jun 8, 2017

Yepp, I saw that tsdb issue. This issue is about adding context to generic storage interfaces which, eventually, will be used for cancellation signals. How they will be handled depends on specific implementations, including tsdb. In the simplest case, they can just ignore them.
I'm prototyping remote storage implementation now, that's why I'm interested in adding contexts now rather then when/if tsdb support them.

@fabxc

This comment has been minimized.

Copy link
Member

fabxc commented Jun 8, 2017

@tomwilkie

This comment has been minimized.

Copy link
Member

tomwilkie commented Jun 15, 2017

+1 I just came across the need for this working on the remote APIs for 2.0.

@AlekSi

This comment has been minimized.

Copy link
Contributor Author

AlekSi commented Jul 7, 2017

I can implement it if you agree.

We postponed that work. Feel free to work on it if you need it right now. I expect to return to it later.

@AlekSi

This comment has been minimized.

Copy link
Contributor Author

AlekSi commented Aug 3, 2017

Actively working on it again.
Update: Sorry, have to postpone it again.

@krasi-georgiev

This comment has been minimized.

Copy link
Member

krasi-georgiev commented Mar 6, 2018

@AlekSi can we close this one since there is an open issue for the same in the tsdb repo
prometheus/tsdb#84

@AlekSi

This comment has been minimized.

Copy link
Contributor Author

AlekSi commented Mar 6, 2018

Please see discussion above why those are separate issues.

@krasi-georgiev

This comment has been minimized.

Copy link
Member

krasi-georgiev commented Mar 6, 2018

yep , sorry. I missed some important bits on the first read.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.