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

EurekaClientAutoConfiguration: unable to provide my own implementation of 'EurekaInstanceConfig' #302

Closed
brenuart opened this issue Apr 15, 2015 · 11 comments
Milestone

Comments

@brenuart
Copy link
Contributor

The EurekaClientAutoConfiguration creates a default EurekaInstanceConfig instance only if none is found the application context. The snippet below shows the code responsible for that behaviour:

@Bean
@ConditionalOnMissingBean(EurekaInstanceConfig.class)
 public EurekaInstanceConfigBean eurekaInstanceConfigBean() {
...

EurekaInstanceConfig is an interface (from the Netflix package) whereas EurekaInstanceConfigBean is an implementation of that interface (from spring cloud package). Because of that, one may think it can supply its own custom implementation of the interface. Unfortunately, this won't work because the EurekaDiscoveryClient expects the implementation class and not the interface as shown below:

public class EurekaDiscoveryClient implements DiscoveryClient {
    @Autowired
    private EurekaInstanceConfigBean config;
...

Consequence is user cannot provide his own implementation and is forced to extend EurekaInstanceConfigBean.
Two comments on this:
(1) Is it expected? Is there any reason why user cannot provide their own implementation?
(2) If it is the case, then shouldn't the @ConditionalOnMissingBean annotation refers to EurekaInstanceConfigBean instead?

@spencergibb
Copy link
Member

We have a little functionality in EurekaInstanceConfigBean that will get lost if we move to the interface and you supply your own implementation: registering a random spring boot server.port and being able to customize the initial InstanceStatus that is sent to eureka. If you can live without those, moving to the interface would work.

@brenuart
Copy link
Contributor Author

If you can live without those, moving to the interface would work

We tried, but unfortunately autowiring the EurekaDiscoveryClient will fail because it expects an `EurekaInstanceConfigBean', i.e. the actual implementation provided by SpringCloud and not the interface (see the @Autowired annotation in the code samples above).

registering a random spring boot server.port

What do you mean by random? A random port selected by the embedded servlet container at boot time (i.e. configured with server.port=0) ?

If so, could you please have a look at my comments on post http://stackoverflow.com/questions/29637324/spring-cloud-random-port-not-registered-to-eureka ?

Thanks for your help.

@brenuart
Copy link
Contributor Author

Anyway, if SpringCloud Netflix components expect an instance of EurekaClientInstanceConfigBean, then I think the auto configuration should be written with a condition against that class and not the interface:

@Bean
@ConditionalOnMissingBean(EurekaInstanceConfig.class)
public EurekaInstanceConfigBean eurekaInstanceConfigBean() {

Otherwise, providing my own implementation of the interface will correctly disable the creation of 'your' EurekaInstanceConfigBean - but components like the EurekaDiscoveryClient will still complain...

See what I mean?

@spencergibb
Copy link
Member

I'm talking about loosing those features if we moved to just the interface and you replaced the implementation. You're right about random server port.

Another possibility is subclassing EurekaInstanceConfigBean.

@brenuart
Copy link
Contributor Author

brenuart commented Sep 9, 2015

Issue is still there even after refactoring work done recently (Brixton).
Issue #523 may provide a solution.

@dsyer
Copy link
Contributor

dsyer commented Sep 29, 2015

I think it's gone now (EurekaDiscoveryClient only depends on the interface since eb8a902).

@spencergibb
Copy link
Member

EurekaInstanceConfigBean is still referenced in EurekaDiscoveryClientConfiguration

spencergibb added a commit that referenced this issue Sep 29, 2015
Specifically CloudEurekaInstanceConfig where needed.

see gh-302
@spencergibb
Copy link
Member

EurekaInstanceConfigBean is no longer referenced in EurekaDiscoveryClientConfiguration. Instead it references CloudEurekaInstanceConfig.

@dsyer dsyer reopened this Sep 29, 2015
@spencergibb
Copy link
Member

@dsyer any reason to reopen this?

@dsyer
Copy link
Contributor

dsyer commented Sep 29, 2015

I thought you meant we needed to because the Netflix interface isn't used. I guess you meant that as long as the user adds a bean of a different type (which is also an interface) everything is good? If so just close this again.

@spencergibb
Copy link
Member

Yeah, I created CloudEurekaInstanceConfig which is only used in one place. As long as a user's EurekaInstanceConfig implements CloudEurekaInstanceConfig, we're good.

jkschneider pushed a commit to jkschneider/spring-cloud-netflix that referenced this issue Oct 8, 2015
Specifically CloudEurekaInstanceConfig where needed.

see spring-cloudgh-302
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants