docs: clarify ExternalServer DNS resolution behavior#2563
Open
isker wants to merge 1 commit into
Open
Conversation
From reading the source code of workerd and KJ, I can see that ExternalServer does DNS resolution only once ever. This was certainly surprising to me, and I suspect it would be a surprise to many, as ExternalServer is the seemingly most natural way to get a binding to a particular networked service, but it is in fact only suitable to look up domain names that are guaranteed to not change over the lifetime of the workerd service on which it is configured, like container sidecars. So, be more explicit about the behavior in the documentation.
Contributor
Author
|
Here's a TODO explaining just this: workerd/src/workerd/server/server.c++ Lines 435 to 436 in 249464f |
Member
|
I think this is a bug. If we document it, we should say that it's a bug that may be fixed later. But maybe we should just fix it? |
Contributor
Author
|
You added the TODO two years ago in dc27ddc. So it doesn’t seem to be a bug as much as consciously acknowledged missing functionality. And I don’t think that commit regressed DNS resolution in any way, I think ExternalServer has always been this way. Either way, I have got what it takes to clarify the documentation but not, I think, to add this functionality. So if you’d prefer the latter to the exclusion of the former, go ahead and close this PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
From reading the source code of workerd and KJ, I can see that ExternalServer does DNS resolution only once ever. This was certainly surprising to me, and I suspect it would be a surprise to many, as ExternalServer is the seemingly most natural way to get a binding to a particular networked service, but it is in fact only suitable to look up domain names that are guaranteed to not change over the lifetime of the workerd service on which it is configured, like container sidecars.
So, be more explicit about the behavior in the documentation.