Remove Application bootstrap from spring-cloud-stream-schema-server

@guilhermeblanco commented on Thu Jun 14 2018

Currently the package spring-cloud-stream-schema-server (path: https://github.com/spring-cloud/spring-cloud-stream/tree/master/spring-cloud-stream-schema-server) contains an Application class annotated with @SpringBootApplication, and also an application.yml, which overlaps with setups where bootstrapping (through bootstrap.yml) switches application name, server port, etc. in order to bind with customized infrastructure.

Following the convention adopted by other servers (such as config-server, reference: https://github.com/spring-cloud/spring-cloud-config/tree/master/spring-cloud-config-server) and allow consumers to bootstrap their own SpringBootApplication and also turn the configuration into a more flexible system.

@olegz commented on Thu Jun 14 2018

@guilhermeblanco I am a bit confused with the second paragraph. Are you saying you're expecting something like this?:

@Configuration
@EnableAutoConfiguration
@EnableSchemaRegistryServer
public class SchemaRegistryServerApplication {..}

@guilhermeblanco commented on Thu Jun 14 2018

@olegz exactly!! Also, the yml configuration should be renamed and also loaded the same way too. =)

@olegz commented on Fri Jun 15 2018

Thank you @guilhermeblanco Seems like an easy fix. Would you be interested in contributing? Otherwise we'll handle it. Let us know

@guilhermeblanco commented on Fri Jun 15 2018

@olegz Done. =)

@olegz commented on Fri Jun 15 2018

@guilhermeblanco Thank you! Would you mind adding your name to @author?

@guilhermeblanco commented on Fri Jun 15 2018

@olegz done.

@olegz commented on Mon Jun 18 2018

@guilhermeblanco we are reopening it and reverting the commit as it created some issues in our integration/acceptance tests. Not sayin its an invalid issue, but we need more information. So, could you please provide as with a sample applications that demonstrates what you want to do but does not work?

@guilhermeblanco commented on Mon Jun 18 2018

@olegz Ouch... I'm sure the tests should be updated (I've fixed one to get the build properly pass). Would you mind pointing the tests that needs fixing?

As for the issue this PR addresses, I can explain (build a test case would be hard)... My apps run in a Kubernetes environment. This means service discovery and configuration comes from there, so my application.yml actually only exists as a ConfigMap in Kubernetes. Because of that, I have a bootstrap.yml that defines my initial settings, such as spring.application.name. The maven dependency org.springframework.cloud > spring-cloud-starter-kubernetes-all will use the application name to load the ConfigMap that matches it and load the application.yml). Since the SpringBootApplication is loaded through the schema-registry dependency, it also overrides my bootstrap.yml configuration and forces the name to match SchemaRegistryServer declared in the dependency jar. It also forces the server port to be 8990.

The solution is to prevent the SpringBootApplication to be booted inside of the dependency jar, and also move the application.yml to a different name, which is exactly what the PR I've made does.

The integration/acceptance tests might be failing because the SpringRunner was looking for a SpringBootApplication, which does not exist anymore in the package. But if you point to the class and also specify the config name, it works as intended. Look at the work I've done on https://github.com/spring-cloud/spring-cloud-stream/commit/66b7d8dbe4046327cf8084f12b691b9c2fac8229#diff-ee64c612806083837af5147c31823963

I'm more than happy to give you a hand on addressing the issues you may have encountered.

@olegz commented on Mon Jun 18 2018

I sort of understood it in such way, but what was troubling is the fact that the those tests failed which means there were no default server running. Let me dig in.

@guilhermeblanco commented on Mon Jun 18 2018

@olegz Ok, let me give you some info on what might be the changes to the tests that are failing on your end.

If the class have a @RunWith(SpringRunner.class), then it'll require this now to the class:

@SpringBootTest(
    classes = SchemaRegistryServerApplication.class,
    properties = {"spring.config.name:schemaregistryserver"},
    webEnvironment = SpringBootTest.WebEnvironment.DEFINED_PORT
)

And if it's running through SpringApplication.run(SchemaRegistryServerApplication.class), it needs to be changed to use a Builder now:

new SpringApplicationBuilder(SchemaRegistryServerApplication.class)
    .properties("spring.config.name=schemaregistryserver")
    .run();

Hope that helps. =)

@guilhermeblanco commented on Mon Jun 18 2018

Oh... and I'm on Gitter if you want to speed up our async conversation.

spring-cloud / spring-cloud-schema-registry

Remove Application bootstrap from spring-cloud-stream-schema-server #1