NotificationsYou must be signed in to change notification settings
Fork1k
Star11.1k

chore: remove pingWebSocket since yamux runs keepalives#11914

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

spikecurtis merged 1 commit intomainfromspike/10534-no-websocket-pings

Feb 1, 2024

Merged

chore: remove pingWebSocket since yamux runs keepalives#11914

spikecurtis merged 1 commit intomainfromspike/10534-no-websocket-pings

Feb 1, 2024

Conversation

Copy link

Contributor

spikecurtis commentedJan 30, 2024•
edited
Loading

Since we run yamux over the websocket, we don't need to ping at the websocket layer because yamux has a 30 second keepalive mechanism enabled in the default config.

spikecurtis mentioned this pull request

Jan 30, 2024

chore: remove agentsdk client RPC() function#11913

Merged

Copy link

ContributorAuthor

spikecurtis commentedJan 30, 2024•
edited
Loading

main
- chore: remove agentsdk client RPC() function #11913
  - chore: remove pingWebSocket since yamux runs keepalives #11914 👈
    - chore: rename client Listen to ConnectRPC #11916

This stack of pull requests is managed by Graphite.Learn more about stacking.

Join@spikecurtis and the rest of your teammates onGraphite

github-actionsbot assignedspikecurtis

Jan 30, 2024

spikecurtis requested review fromcoadler anddeansheather

January 30, 2024 10:25

spikecurtis marked this pull request as ready for review

January 30, 2024 10:25

spikecurtis force-pushed thespike/10534-no-websocket-pings branch from6565cbc to3fe1072Compare

January 30, 2024 10:44

spikecurtis mentioned this pull request

Jan 30, 2024

chore: rename client Listen to ConnectRPC#11916

Merged

coadler approved these changes

Jan 30, 2024

View reviewed changes

Copy link

ContributorAuthor

spikecurtis commentedFeb 1, 2024•
edited
Loading

Merge activity

Jan 31, 11:21 PM EST America/New_York:@spikecurtis started a stack merge that includes this pull request viaGraphite.
Jan 31, 11:23 PM EST America/New_York:Graphite rebased this pull request as part of a merge.
Jan 31, 11:31 PM EST America/New_York:Graphite couldn't merge this PR because it was not satisfying all requirements.
Feb 1, 12:48 AM EST America/New_York:@spikecurtis merged this pull request withGraphite.