-
Notifications
You must be signed in to change notification settings - Fork 691
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
Execute, Query, Request timeout tweaks #1667
Conversation
@mauri870 -- feel free to ask me why I am making these changes. |
@@ -23,7 +23,7 @@ message Statement { | |||
message Request { | |||
bool transaction = 1; | |||
repeated Statement statements = 2; | |||
int64 DBTimeout = 3; | |||
int64 dbTimeout = 3; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While Go style requires DB, protos do not. But the generated code will not be quite good Go style. That's OK.
@@ -876,6 +876,18 @@ func (db *DB) RequestStringStmts(stmts []string) ([]*command.ExecuteQueryRespons | |||
return db.Request(req, false) | |||
} | |||
|
|||
// RequestStringStmtsWithTimeout processes a request that can contain both executes and queries. | |||
func (db *DB) RequestStringStmtsWithTimeout(stmts []string, timeout time.Duration) ([]*command.ExecuteQueryResponse, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was missed in the first change.
@@ -1044,7 +1044,19 @@ func Test_ExecShouldTimeout(t *testing.T) { | |||
INSERT INTO test_table (key1, key_id, key2, key3, key4, key5, key6, data) | |||
SELECT t1.key1 || t2.key1, t1.key_id || t2.key_id, t1.key2 || t2.key2, t1.key3 || t2.key3, t1.key4 || t2.key4, t1.key5 || t2.key5, t1.key6 || t2.key6, t1.data || t2.data | |||
FROM test_table t1 LEFT OUTER JOIN test_table t2` | |||
assertExecTimeoutReached(t, db, q, 1*time.Millisecond) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this function was only used once, it's just clearer to inline it.
@@ -1082,19 +1090,26 @@ func assertExecTimeoutReached(t *testing.T, db *DB, stmt string, timeout time.Du | |||
} | |||
} | |||
|
|||
func assertQueryTimeoutReached(t *testing.T, db *DB, stmt string, timeout time.Duration) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto.
@@ -1082,19 +1090,26 @@ func assertExecTimeoutReached(t *testing.T, db *DB, stmt string, timeout time.Du | |||
} | |||
} | |||
|
|||
func assertQueryTimeoutReached(t *testing.T, db *DB, stmt string, timeout time.Duration) { | |||
r, err := db.QueryStringStmtWithTimeout(stmt, timeout) | |||
func Test_RequestShouldTimeout(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unit was missed first time round.
Some follow up to #1666