Skip to content

gateway: benchmark: fix 'all websocket requests fail' case #1131

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

Merged
merged 2 commits into from
Dec 6, 2024
Merged

Conversation

emidoots
Copy link
Member

@emidoots emidoots commented Dec 6, 2024

The first commit here replaces the authentication with a simpler / more correct implementation (no transport) and improves usage doc commands slightly.

The second commit fixes a major issue: we would connect the websocket at the very start, which meant that if the tests which run before we use the websocket take >60s.. by the time we use the websocket, the server would already consider the connection dead due to inactivity. This fixes the issue by only connecting the first time we want to use the connection, and also adding reconnect logic.

Test plan

Manually tested

Stephen Gutekanst added 2 commits December 6, 2024 07:09
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
…ct logic

Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
@emidoots emidoots requested a review from a team as a code owner December 6, 2024 14:13
Copy link
Contributor

@vdavid vdavid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both changes make sense. I'm AFK but went through all the code on mobile, and they look good. Approving to unblock.

@emidoots emidoots merged commit 28df838 into main Dec 6, 2024
7 of 8 checks passed
@emidoots emidoots deleted the sg/fixes branch December 6, 2024 14:28
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

Successfully merging this pull request may close these issues.

2 participants