-
Notifications
You must be signed in to change notification settings - Fork 457
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
use network callback to update DNS config when network changes #147
Conversation
-Use requestNetwork, which gets the best network matching the passed in network request, to listen for changes to network and cache DNS config -Call netmon.InjectEvent on network change to indicate a change Follow-up will fix issue in netmon where IsMajorChangeFrom doesn't identify major changes when a network is added Fixes #10107
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.
Looks good in general, a couple of suggestions for code organization and stuff.
Oh, and you need to make the commit say |
-Use requestNetwork, which gets the best network matching the passed in network request, to listen for changes to network and cache DNS config -Call netmon.InjectEvent on network change to indicate a change Follow-up will fix issue in netmon where IsMajorChangeFrom doesn't identify major changes when a network is added Updates tailscale/tailscale/#10107 hi
-Use requestNetwork, which gets the best network matching the passed in network request, to listen for changes to network and cache DNS config -Call netmon.InjectEvent on network change to indicate a change Follow-up will fix issue in netmon where IsMajorChangeFrom doesn't identify major changes when a network is added Updates tailscale/tailscale/#10107
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.
Looks right to me! One small change suggestion but no need for another round of review.
} | ||
|
||
s = getDnsServersFromSystemProperties(); | ||
String s = getDnsConfigs(); | ||
if (!s.trim().isEmpty()) { |
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.
unnecessary? We're now returning empty string regardless
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.
We return the string if it's nonempty
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.
Sure, but if the string is empty, we return ""
below. So, you could skip the conditional and always return getDnsConfigs().trim()
for the same effect
…cale#147) * use network callback to update DNS config when network changes -Use requestNetwork, which gets the best network matching the passed in network request, to listen for changes to network and cache DNS config -Call netmon.InjectEvent on network change to indicate a change Follow-up will fix issue in netmon where IsMajorChangeFrom doesn't identify major changes when a network is added Fixes #10107 * use network callback to update DNS config when network changes -Use requestNetwork, which gets the best network matching the passed in network request, to listen for changes to network and cache DNS config -Call netmon.InjectEvent on network change to indicate a change Follow-up will fix issue in netmon where IsMajorChangeFrom doesn't identify major changes when a network is added Updates tailscale/tailscale/#10107 hi * hi * . * use network callback to update DNS config when network changes -Use requestNetwork, which gets the best network matching the passed in network request, to listen for changes to network and cache DNS config -Call netmon.InjectEvent on network change to indicate a change Follow-up will fix issue in netmon where IsMajorChangeFrom doesn't identify major changes when a network is added Updates tailscale/tailscale/#10107 * fixed missing connectivity manager
-Use requestNetwork, which gets the best network matching the passed in network request, to listen for changes to network and cache DNS config
-Call netmon.InjectEvent on network change to indicate a change
Follow-up will fix issue in netmon where IsMajorChangeFrom doesn't identify major changes when a network is added
Updates tailscale/tailscale#10107