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 #16

Open
derkan opened this issue Jun 4, 2018 · 2 comments
Open

Context support #16

derkan opened this issue Jun 4, 2018 · 2 comments

Comments

@derkan
Copy link
Contributor

derkan commented Jun 4, 2018

Hi Sam,

What do you think about adding Context support? My suggestions are:
1- Setting it connection-wide like db.WithContext(context.Context) and each runner interface check if ctx is nil or not and call suitable sql func.
2- For each runner interface add context param with naming suffix ...Ctx like db.SelectFrom("books").DoCtx(&books, ctx). This one will need every runner and Transaction methods to be duplicated for Context parameter with Ctx suffix but more Go-like.
3- Without changing any interface, altering current runners with contex'ed ones internally, and get/set context with a function. For ex: db.SelectFrom("books").Do() internally will run ExecTx with ctx.Background.

If you can check and give directions I can add a PR for this according to your suggestions or better you can add this to godb.

Erkan.

@samonzeweb
Copy link
Owner

Hi Derkan,

It's on my todo-list.

The way is to add DoCtx functions, but the context has to be the first argument, it's coherent with the stdlib. It's also explicit, I like the python mantra "explicit is better than implicit", and it matches well with Go.

The WithContext is interesting but less explicit, less go-ish. The stdlib has already a WithContext function in http package, but the usage is different : https://golang.org/pkg/net/http/#Request.WithContext

It's not a big work, and I'll be happy to do it. I'll take some time

Sam.

@derkan
Copy link
Contributor Author

derkan commented Jun 6, 2018

Thanks, looking forward for your implementation. Especially I needed this to use CockroachDB(uses pq's client)'s transactions which can retry. To use godb's transaction directly it needs to implement ExecContext as in here.

So, active transaction from db.CurrentTx() can be used in ExcuteInTx for retrying transactions for CockroachDB.

Please also add ExecContext method to DB for this to work. Otherwise I had to add a wrapper transactor on db.

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

No branches or pull requests

2 participants