Gorp new sql context support

[soniabhishek]

Hey when are we expecting to support context for gorp and if no one else is working can I give it a try, It seems easy to workaround.

[srenatus]

@soniabhishek Heya -- any updates on this? I think it would be nice to have that.

[nelsam]

d'oh, this is probably something I should have commented on. There are no current plans, but feel free! If you don't get to it, I'll keep it in mind for the rearch I've been working through.

[bryanpaluch]

I'm also interested in this as I've just started using tracing libraries. If I had some guidance on the desired API, I may be able to take a swing at it.

[MatthewDolan]

Looks like it might be pretty simple to pipe through.

What are you thinking in terms of the change to the API?

(1) Add new FooContext methods that copy the existing methods.

type SqlExecutor interface {
	Get(i interface{}, keys ...interface{}) (interface{}, error)
	GetContext(ctx context.Context, i interface{}, keys ...interface{}) (interface{}, error)
	
	Insert(list ...interface{}) error
	InsertContext(ctx context.Context, list ...interface{}) error
	
	Update(list ...interface{}) (int64, error)
	UpdateContext(ctx context.Context, list ...interface{}) (int64, error)
	
	Delete(list ...interface{}) (int64, error)
	DeleteContext(ctx context.Context, list ...interface{}) (int64, error)
	
	Exec(query string, args ...interface{}) (sql.Result, error)
	ExecContext(ctx context.Context, query string, args ...interface{}) (sql.Result, error)
	
	...
}

(2) Add a WithContext(ctx) which attaches the context to the dbMap.

type SqlExecutor interface {
	WithContext(ctx context.Context) SqlExecutor

	Get(i interface{}, keys ...interface{}) (interface{}, error)
	Insert(list ...interface{}) error
	Update(list ...interface{}) (int64, error)
	Delete(list ...interface{}) (int64, error)
	Exec(query string, args ...interface{}) (sql.Result, error)
	...
}

(3) I guess it's possible to do something backward incompatible that forces context, but I assume that's not desirable.

(1) is probably most similar to how go changed the underlying sql.Db, but it add the most new methods.

[nelsam]

I would currently choose #2. I have ideas to mess with the syntax to make gorp more flexible, but that'll be for later. For now, returning a new SqlExecutor that uses the context is probably the best choice. In the distant future, I have a really vague thought of something like `dbmap.Query(someSQL, gorp.Args(arg1, arg2, arg3), gorp.WithContext(ctx), gorp.SomeOtherCoolOption()).Unmarshal(&foo, gorp.SomeCoolUnmarshalOption())`. But for now, #2. On Aug 30, 2017 18:46, "Matthew Dolan" <[email protected]> wrote: Looks like it might be pretty simple to pipe through. What are you thinking in terms of the change to the API? (1) Add new FooContext methods that copy the existing methods. type SqlExecutor interface { Get(i interface{}, keys ...interface{}) (interface{}, error) GetContext(ctx context.Context, i interface{}, keys ...interface{}) (interface{}, error) Insert(list ...interface{}) error InsertContext(ctx context.Context, list ...interface{}) error Update(list ...interface{}) (int64, error) UpdateContext(ctx context.Context, list ...interface{}) (int64, error) Delete(list ...interface{}) (int64, error) DeleteContext(ctx context.Context, list ...interface{}) (int64, error) Exec(query string, args ...interface{}) (sql.Result, error) ExecContext(ctx context.Context, query string, args ...interface{}) (sql.Result, error) ... } (2) Add a WithContext(ctx) which attaches the context to the dbMap. type SqlExecutor interface { WithContext(ctx context.Context) SqlExecutor Get(i interface{}, keys ...interface{}) (interface{}, error) Insert(list ...interface{}) error Update(list ...interface{}) (int64, error) Delete(list ...interface{}) (int64, error) Exec(query string, args ...interface{}) (sql.Result, error) ... } (3) I guess it's possible to do something backward incompatible that forces context, but I assume that's not desirable. (1) is probably most similar to how go changed the underlying sql.Db, but it add the most new methods. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#344 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA-QBDn3zi_so5fO7qxyjrXGvyTCrGZCks5sdgJ8gaJpZM4MLQAc> .

[soniabhishek]

@srenatus @nelsam I would like to work over this, just wanted to confirm that above-mentioned approach (i.e. 2) is what needs to be implemented

[nelsam]

@soniabhishek yeah, the WithContext method on SqlExecutor is the desired approach. @MatthewDolan is working on a solution in #352.

[MatthewDolan]

This is merged.

@nelsam would it be possible to get a new release (2.1.0) associated with this?

[nelsam]

Sure thing, I'll do that a little bit later today (ideally)

[sinni800]

So, WithContext does return a new Gorp with a context, but the SQLs won't be executed with that ctx apparently?