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

Breaking change in NameResolverProvider with latest release #5556

Closed
ST-DDT opened this issue Apr 8, 2019 · 4 comments · Fixed by #5560
Closed

Breaking change in NameResolverProvider with latest release #5556

ST-DDT opened this issue Apr 8, 2019 · 4 comments · Fixed by #5560
Assignees
Milestone

Comments

@ST-DDT
Copy link
Contributor

ST-DDT commented Apr 8, 2019

What version of gRPC are you using?

1.19.0

What did you expect to see?

I use yidongnan/grpc-spring-boot-starter 2.3.0 and use custom NameResolvers to connect remote services. Due to interoperability conflicts with java's service loaders I cannot directly access/use the provided grpc NameResolvers and have to first use spring logic to determine the remote host before using the NameResolvers.
Due to the changes introduced in grpc-java 1.19 (method parameter change) I am now unable to
use the 1.18 API with NameResolverProviders.

The change in https://github.com/grpc/grpc-java/pull/5345/files#diff-0b140e8a0a77a6eda416c5a42033462b simply exchanged the parameters without providing a method for backwards compatibility.

Due to this change I'm unable to provide support for either <=1.18 or >=1.19 versions.
I know that at some point the other method will be removed, but I hope to have some time for migration.

call-hierarchy

ConfigMappedNameResolverFactory (springNameResolver)

    @Nullable
    @Override
    public NameResolver newNameResolver(final URI targetUri, final Attributes params) {
        final String clientName = targetUri.toString();
        final GrpcChannelProperties clientConfig = this.config.getChannel(clientName);
        URI remappedUri = clientConfig.getAddress();
        if (remappedUri == null) {
            remappedUri = this.defaultUriMapper.apply(clientName);
            if (remappedUri == null) {
                throw new IllegalStateException("No targetUri provided for '" + clientName + "'"
                        + " and defaultUri mapper returned null.");
            }
        }
        log.debug("Remapping target URI for {} to {} via {}", clientName, remappedUri, this.delegate);
        final Attributes extendedParas = params.toBuilder()
                .set(NameResolverConstants.PARAMS_CLIENT_NAME, clientName)
                .set(NameResolverConstants.PARAMS_CLIENT_CONFIG, clientConfig)
                .build();
        return this.delegate.newNameResolver(remappedUri, extendedParas);
    }
Caused by: java.lang.UnsupportedOperationException: This method is going to be deleted
    at io.grpc.NameResolver$Factory.newNameResolver (NameResolver.java:135)
    at net.devh.boot.grpc.client.nameresolver.ConfigMappedNameResolverFactory.newNameResolver (ConfigMappedNameResolverFactory.java:85)
    at io.grpc.NameResolver$Factory.newNameResolver (NameResolver.java:151)
    at io.grpc.internal.ManagedChannelImpl.getNameResolver (ManagedChannelImpl.java:646)
    at io.grpc.internal.ManagedChannelImpl.<init> (ManagedChannelImpl.java:561)
    at io.grpc.internal.AbstractManagedChannelImplBuilder.build (AbstractManagedChannelImplBuilder.java:449)
    at net.devh.boot.grpc.client.channelfactory.AbstractChannelFactory.newManagedChannel (AbstractChannelFactory.java:132)
    at java.util.concurrent.ConcurrentHashMap.computeIfAbsent (ConcurrentHashMap.java:1705)
    at net.devh.boot.grpc.client.channelfactory.AbstractChannelFactory.createChannel (AbstractChannelFactory.java:98)
    at net.devh.boot.grpc.client.inject.GrpcClientBeanPostProcessor.processInjectionPoint (GrpcClientBeanPostProcessor.java:110)
    at net.devh.boot.grpc.client.inject.GrpcClientBeanPostProcessor.postProcessBeforeInitialization (GrpcClientBeanPostProcessor.java:77)
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.applyBeanPostProcessorsBeforeInitialization (AbstractAutowireCapableBeanFactory.java:414)
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean (AbstractAutowireCapableBeanFactory.java:1770)
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean (AbstractAutowireCapableBeanFactory.java:593)
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean (AbstractAutowireCapableBeanFactory.java:515)
    at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0 (AbstractBeanFactory.java:320)
    at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton (DefaultSingletonBeanRegistry.java:222)
    at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean (AbstractBeanFactory.java:318)
    at org.springframework.beans.factory.support.AbstractBeanFactory.getBean (AbstractBeanFactory.java:199)
    at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons (DefaultListableBeanFactory.java:849)
    at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization (AbstractApplicationContext.java:877)
    at org.springframework.context.support.AbstractApplicationContext.refresh (AbstractApplicationContext.java:549)
    at org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext.refresh (ServletWebServerApplicationContext.java:142)
    at org.springframework.boot.SpringApplication.refresh (SpringApplication.java:775)
    at org.springframework.boot.SpringApplication.refreshContext (SpringApplication.java:397)
    at org.springframework.boot.SpringApplication.run (SpringApplication.java:316)
    at com.example.Main.main (Main.java:38)
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:62)
    at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke (Method.java:566)
    at org.springframework.boot.maven.AbstractRunMojo$LaunchRunner.run (AbstractRunMojo.java:558)
    at java.lang.Thread.run (Thread.java:834)

Did you forget to add the method override or is it intentional?

TLDR

Temporarily re-add
io.grpc.NameResolverProvider.NameResolverFactory#newNameResolver(URI targetUri, Attributes params) and delegate it to the other method.

Related

@zhangkun83
Copy link
Contributor

We took care of the migration of the implementors of NameResolver, but didn't consider callers, because we thought only gRPC would be the caller. Seems I made the wrong assumption. I can add the implementation to the old method in the upcoming 1.20.0

@ST-DDT
Copy link
Contributor Author

ST-DDT commented Apr 8, 2019

Thanks for your help.

@zhangkun83 zhangkun83 added the TODO:release blocker Issue/PR is important enough to delay the release. Removed after release issues resolved label Apr 8, 2019
@zhangkun83 zhangkun83 added this to the 1.20 milestone Apr 8, 2019
@zhangkun83
Copy link
Contributor

@ST-DDT I noticed that you are putting custom attributes. After switching away from the Attraibutes params, you will no longer be able to do it. Can you file a feature request issue to tell us about your use case, so that we can add the support for it in the new Helper-based API?

zhangkun83 added a commit to zhangkun83/grpc-java that referenced this issue Apr 8, 2019
We assumed that we were the only caller.  Turns out there are
forwarding NameResolver too.  Implementing the old override will give
them time to migrate to the new API.

Resolves grpc#5556
zhangkun83 added a commit that referenced this issue Apr 8, 2019
…5560)

We assumed that we were the only caller.  Turns out there are
forwarding NameResolver too.  Implementing the old override will give
them time to migrate to the new API.

Resolves #5556
@ST-DDT
Copy link
Contributor Author

ST-DDT commented Apr 8, 2019

@ST-DDT I noticed that you are putting custom attributes. After switching away from the Attraibutes params, you will no longer be able to do it. Can you file a feature request issue to tell us about your use case, so that we can add the support for it in the new Helper-based API?

Done in #5562

zhangkun83 added a commit to zhangkun83/grpc-java that referenced this issue Apr 8, 2019
…rpc#5560)

We assumed that we were the only caller.  Turns out there are
forwarding NameResolver too.  Implementing the old override will give
them time to migrate to the new API.

Resolves grpc#5556
zhangkun83 added a commit that referenced this issue Apr 9, 2019
…5560) (#5564)

We assumed that we were the only caller.  Turns out there are
forwarding NameResolver too.  Implementing the old override will give
them time to migrate to the new API.

Resolves #5556
zhangkun83 added a commit to zhangkun83/grpc-java that referenced this issue Apr 9, 2019
…rpc#5560)

We assumed that we were the only caller.  Turns out there are
forwarding NameResolver too.  Implementing the old override will give
them time to migrate to the new API.

Resolves grpc#5556
zhangkun83 added a commit that referenced this issue Apr 10, 2019
…(v1.19.x backport) (#5573)

We assumed that we were the only caller.  Turns out there are
forwarding NameResolver too.  Implementing the old override will give
them time to migrate to the new API.

Resolves #5556

Backport of #5560
@ejona86 ejona86 removed the TODO:release blocker Issue/PR is important enough to delay the release. Removed after release issues resolved label May 20, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Aug 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants