CASSGO-81 Add missing Context variant methods #1898
Conversation
|
|
session.go
Outdated
| // the applied boolean as the first argument to *Iter.Scan | ||
| func (b *Batch) ExecCAS(dest ...interface{}) (applied bool, iter *Iter, err error) { | ||
| iter = b.session.executeBatch(b, nil) | ||
| return b.ExecCASContext(nil, dest...) |
There was a problem hiding this comment.
The context docs say:
Do not pass a nil Context, even if a function permits it. Pass context.TODO if you are unsure about which Context to use.
We should either pass a TODO or pass context.Background()
There was a problem hiding this comment.
yeah I thought about that but then the internal code that defaults to the old context field that is deprecated will not work...
There was a problem hiding this comment.
maybe I can use context.TODO and treat context.TODO as if the user didn't provide it
There was a problem hiding this comment.
Oh I didn't make the connection with newQueryOptions and it checking for nil specifically. Could we just do q.Context() in the query ones and b.Context() in the batch ones here instead off nil?
There was a problem hiding this comment.
oh I didn't think about that, yeah I'll do that
In CASSGO-22 we deprecated WithContext but we didn't deprecate Context(). IterContext and ExecContext were added as replacements but there are other execution methods that lack a Context variant. This patch addresses both of these issues. Patch by João Reis; reviewed by James Hartig for CASSGO-81
joao-r-reis
force-pushed
the
cassgo81-context
branch
from
a8073d7 to
e3e5be3
Compare
2 participants
In #1868 we deprecated Query.WithContext and Batch.WithContext. We added ExecContext and IterContext as alternatives but we didn't add these alternatives for other ways to execute queries and batches like Query.Scan or Batch.ExecCAS.
We also didn't deprecate Query.Context() and we should have