alts: Forward-fix of ALTS queuing of handshake requests. #6906
Conversation
matthewstevenson88
added
the
Type: Feature
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #6906 +/- ##
=======================================
Coverage 83.69% 83.70%
=======================================
Files 286 287 +1
Lines 30756 30832 +76
=======================================
+ Hits 25742 25807 +65
- Misses 3956 3971 +15
+ Partials 1058 1054 -4
|
matthewstevenson88
added
the
Type: Security
|
@erm-g can you review this PR first? |
|
Note that the real changes were already reviewed in #6884, the only delta here is bumping up the test timeout in alts_test.go. |
|
@erm-g Friendly ping. Also cc @cesarghali. |
| @@ -44,7 +44,7 @@ import ( | |||
| ) | |||
|
|
|||
| const ( | |||
| defaultTestLongTimeout = 10 * time.Second | |||
| defaultTestLongTimeout = 60 * time.Second | |||
There was a problem hiding this comment.
I'm not sure I fully understand this long timeout. It's used only for establishAltsConnection and has the comment
// The server is not ready yet. Try again.
However the server should be up and running because of synchronous startServer call preceding the `establishAltsConnection'. What am I missing here?
There was a problem hiding this comment.
That comment was just a bit out-of-date. I've updated it to better reflect the current state of affairs. Thanks for flagging this!
This PR builds on #6884, which was reverted in #6903 because of flakiness in
alts_test.go. It turned out that this flakiness was just due to the test occasionally bumping up on the timeout, so we've fixed this in this PR by increasing the timeout.RELEASE NOTES: none