- Notifications
You must be signed in to change notification settings - Fork913
chore(vpn): upsert agents with their network status#15659
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
ethanndickson commentedNov 26, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
This stack of pull requests is managed byGraphite. Learn more aboutstacking. |
cf43bdd
to594fbc6
CompareUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
594fbc6
to1347916
CompareUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
vpn/tunnel.go Outdated
resp.Msg = &TunnelMessage_PeerUpdate{ | ||
PeerUpdate:convertWorkspaceUpdate(state), | ||
PeerUpdate:update, |
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.
Is it OK to send a nilPeerUpdate
in the response?
ethanndicksonDec 2, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
Good catch, I don't think we should.
For pushed updates, if there's no active connection (for whatever reason, should be impossible once the tunnel has started), we'll just send a nil last handshake time.
For requested updates, I think we have to send a nilPeerUpdate
if we can't callCurentWorkspaceState
.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
d150a25
toa456281
Compare@@ -175,8 +175,6 @@ linters-settings: | |||
- name: modifies-value-receiver | |||
- name: package-comments | |||
- name: range | |||
- name: range-val-address | |||
- name: range-val-in-closure |
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.
@@ -238,7 +241,6 @@ linters: | |||
- errname | |||
- errorlint | |||
- exhaustruct | |||
- exportloopref |
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.
default: | ||
t.logger.Warn(t.ctx, "unhandled manager request", slog.F("request", msg)) | ||
return resp | ||
} | ||
if err := req.sendReply(resp); err != nil { |
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.
The original code also sentresp := &TunnelMessage{}
as a reply for thedefault:
branch, so I assume it's still fine.
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.
LGTM
469ff7a
to07dc614
Comparee3f8508
to561f65f
Compare07dc614
to359aaeb
Compare561f65f
to6cd9c76
Compare359aaeb
toba48069
Compare6cd9c76
tob2fd151
Compareb2fd151
toacdd09d
Comparea0a5683
intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Closes#14734.