bitsofinfo / hazelcast-docker-swarm-discovery-spi

Docker Swarm based discovery strategy SPI for Hazelcast enabled applications
Apache License 2.0
39 stars 33 forks source link

Added new constructor to SwarmAddressPicker #6

Closed tunix closed 7 years ago

tunix commented 7 years ago

There are 3 important changes in this PR:

  1. Removed shading for docker-client since it causes AbstractMethodError when used in another project.
  2. Updated dependencies to their latest version
  3. Added a new constructor to SwarmAddressPicker so that it's possible to pass swarm properties without the need for JVM properties. Otherwise one has to define both hazelcast.xml properties (as DefaultDiscoveryService forces this) and JVM properties.
bitsofinfo commented 7 years ago

Thanks, if you are going to remove the shaded jar, please ensure the project has a separate build task for your uses case, because this works for some folks and I don't want to break the existing functionality. It is there because without the shaded jar, things simply will not work for certain projects.

tunix commented 7 years ago

@bitsofinfo Do you mean I need to keep it as shaded? It causes me the following error:

Caused by: java.lang.AbstractMethodError
       at org.glassfish.jersey.model.internal.CommonConfig.configureAutoDiscoverableProviders(CommonConfig.java:624)
       at org.glassfish.jersey.client.ClientConfig$State.configureAutoDiscoverableProviders(ClientConfig.java:364)
       at org.glassfish.jersey.client.ClientConfig$State.initRuntime(ClientConfig.java:399)
       at org.glassfish.jersey.client.ClientConfig$State.access$000(ClientConfig.java:90)
       at org.glassfish.jersey.client.ClientConfig$State$3.get(ClientConfig.java:122)
       at org.glassfish.jersey.client.ClientConfig$State$3.get(ClientConfig.java:119)
       at org.glassfish.jersey.internal.util.collection.Values$LazyValueImpl.get(Values.java:340)
       at org.glassfish.jersey.client.ClientConfig.getRuntime(ClientConfig.java:733)
       at org.glassfish.jersey.client.ClientRequest.getConfiguration(ClientRequest.java:286)
       at org.glassfish.jersey.client.JerseyInvocation.validateHttpMethodAndEntity(JerseyInvocation.java:135)
       at org.glassfish.jersey.client.JerseyInvocation.<init>(JerseyInvocation.java:105)
       at org.glassfish.jersey.client.JerseyInvocation.<init>(JerseyInvocation.java:101)
       at org.glassfish.jersey.client.JerseyInvocation.<init>(JerseyInvocation.java:92)
       at org.glassfish.jersey.client.JerseyInvocation$AsyncInvoker.method(JerseyInvocation.java:629)
       at com.spotify.docker.client.DefaultDockerClient.request(DefaultDockerClient.java:2576)
       at com.spotify.docker.client.DefaultDockerClient.listNetworks(DefaultDockerClient.java:2322)
       at org.bitsofinfo.hazelcast.discovery.docker.swarm.SwarmDiscoveryUtil.discoverContainers(SwarmDiscoveryUtil.java:268)
       at org.bitsofinfo.hazelcast.discovery.docker.swarm.SwarmDiscoveryUtil.discoverSelf(SwarmDiscoveryUtil.java:124)
       at org.bitsofinfo.hazelcast.discovery.docker.swarm.SwarmDiscoveryUtil.<init>(SwarmDiscoveryUtil.java:118)
       at org.bitsofinfo.hazelcast.discovery.docker.swarm.SwarmAddressPicker.initialize(SwarmAddressPicker.java:77)
       at org.bitsofinfo.hazelcast.discovery.docker.swarm.SwarmAddressPicker.<init>(SwarmAddressPicker.java:57)
       at com.foo.bar.config.HazelcastConfig.getNodeContext(HazelcastConfig.java:135)
       at com.foo.bar.config.HazelcastConfig.awsConfig(HazelcastConfig.java:92)
       at com.foo.bar.config.HazelcastConfig$$EnhancerBySpringCGLIB$$f803a015.CGLIB$awsConfig$1(<generated>)
       at com.foo.bar.config.HazelcastConfig$$EnhancerBySpringCGLIB$$f803a015$$FastClassBySpringCGLIB$$e1da177f.invoke(<generated>)
       at org.springframework.cglib.proxy.MethodProxy.invokeSuper(MethodProxy.java:228)
       at org.springframework.context.annotation.ConfigurationClassEnhancer$BeanMethodInterceptor.intercept(ConfigurationClassEnhancer.java:358)
       at com.foo.bar.config.HazelcastConfig$$EnhancerBySpringCGLIB$$f803a015.awsConfig(<generated>)
       at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
       at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
       at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
       at java.lang.reflect.Method.invoke(Method.java:498)
       at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:162)
       ... 119 more

Do you think that this plugin will work (as taken from bintray repository) for the majority without this issue? If not, we could try to add a task to include docker-client as shaded although I currently don't know how to achieve that.

tunix commented 7 years ago

It worked when I added an exclusion to my project for jersey-common:

            <dependency> <!-- We can remove this when our changes are merged & released -->
                <groupId>org.bitsofinfo</groupId>
                <artifactId>hazelcast-docker-swarm-discovery-spi</artifactId>
                <version>1.0-RC2</version>
                <exclusions>
                    <exclusion>
                        <groupId>org.glassfish.jersey.core</groupId>
                        <artifactId>jersey-common</artifactId>
                    </exclusion>
                </exclusions>
            </dependency>

CommonConfig.java comes from both this plugin and docker-client which caused that stacktrace. Do you think we can sort it out in this repo?

tunix commented 7 years ago

Maybe we can change dependency declaration like the following?

    compile('com.spotify:docker-client:8.9.0:shaded') {
        exclude module: 'jersey-common'
    }

Would this work for you?

bitsofinfo commented 7 years ago

Please just do this, in the README, add a new section "troubleshooting" and add your note about "if you get this kind of exception...." with instructions on that exclusion workaround and links to this PR etc.

tunix commented 7 years ago

@bitsofinfo Here you go! 👍

bitsofinfo commented 7 years ago

thx!