Skip to content
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

resolver: move dns and passthrough to internal #3116

Merged
merged 2 commits into from Oct 22, 2019
Merged

Conversation

dfawley
Copy link
Member

@dfawley dfawley commented Oct 21, 2019

Nobody should directly need to reference these packages.

This is technically a breaking change. However:

  • Package dns was exporting a NewBuilder method. This should never have been necessary to use, but if so, it can be replaced by importing the "grpc" package and then using resolver.Get("dns").
  • Package passthrough was not exporting any symbols and there was never a need to even blank-import it.

@dfawley dfawley added the Type: API Change Breaking API changes (experimental APIs only!) label Oct 21, 2019
@dfawley dfawley added this to the 1.25 Release milestone Oct 21, 2019
@dfawley dfawley requested a review from menghanl October 21, 2019 22:48
@easwars
Copy link
Contributor

easwars commented Oct 22, 2019

Would it be better to move the files to internal/resolver/dns and internal/resolver/passthrough directories?

@dfawley
Copy link
Member Author

dfawley commented Oct 22, 2019

Would it be better to move the files to internal/resolver/dns and internal/resolver/passthrough directories?

I'm not too bothered either way. We can rearrange it now or later if we want since it's all internal. What are the pros and cons?

@easwars
Copy link
Contributor

easwars commented Oct 22, 2019

Would it be better to move the files to internal/resolver/dns and internal/resolver/passthrough directories?

I'm not too bothered either way. We can rearrange it now or later if we want since it's all internal. What are the pros and cons?

I guess it's just a matter of preference :)
Although, both of them being resolvers, might be logical to put them under a parent resolverdirectory.

@dfawley
Copy link
Member Author

dfawley commented Oct 22, 2019

Both seem equally fine to me. passthrough is a bit of a weird name by itself, and @menghanl also preferred resolver/ so I moved them.

@dfawley dfawley merged commit c0909e9 into grpc:master Oct 22, 2019
@dfawley dfawley deleted the dnssrv branch October 22, 2019 20:02
@owend
Copy link

owend commented Nov 7, 2019

Just curious, but does this change make it impossible to build older versions of any packages which might still depend on older versions of grpc which then recursively require the resolver/dns and resolver/passthrough packages being at the old location?

Oh, while I was typing this up I noticed there's a new bug open #3157

This change was released in 1.25.0 two days ago and now I am seeing build failures:

Solving failure: No versions of go.etcd.io/etcd met constraints:
v3.4.3: Could not introduce go.etcd.io/etcd@v3.4.3, as it requires problematic packages from google.golang.org/grpc (current version v1.25.0):
google.golang.org/grpc/resolver/dns is missing.
google.golang.org/grpc/resolver/passthrough is missing.

@lock lock bot locked as resolved and limited conversation to collaborators May 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: API Change Breaking API changes (experimental APIs only!)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants