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

Add support for spring-boot-starter-webflux #152

Closed
dorzey opened this issue May 9, 2018 · 6 comments
Closed

Add support for spring-boot-starter-webflux #152

dorzey opened this issue May 9, 2018 · 6 comments

Comments

@dorzey
Copy link

dorzey commented May 9, 2018

I have a Spring Boot app wrapping a Spark model that uses org.springframework.boot:spring-boot-starter-webflux version 2.0.0.RELEASE.

The existing seldon-core-wrapper uses an older version of Spring Boot and assumes I am using Tomcat as a web container. Thus, when the app starts I get a java.lang.ClassNotFoundException: org.springframework.boot.context.embedded.EmbeddedServletContainerCustomizer.

From performance tests I know that Netty (the default container in webflux) is more performant for my application than Tomcat.

If I wished to use seldon-core what are the possible ways forward?

Thanks!

@ukclivecox
Copy link
Contributor

We would need to modify https://github.com/SeldonIO/seldon-core/tree/master/wrappers/s2i/java/wrapper project to make the setup configurable.

Would the specific change be just to swap out Tomcat for Netty?
Could the pom.xml stay the same or just be added to?
could the setup a user presently needs to do be change to point to different config classes:

@EnableAsync
@SpringBootApplication(scanBasePackages = {"io.seldon.wrapper","io.seldon.example"})
@Import({ io.seldon.wrapper.config.AppConfig.class })
public class App {
    public static void main(String[] args) throws Exception {
            SpringApplication.run(App.class, args);
	    }
}

@dorzey
Copy link
Author

dorzey commented May 14, 2018

I've taken a better look at what the issue might be.

It seems that org.springframework.boot.context.embedded.EmbeddedServletContainerCustomizer doesn't exist in Spring Boot 2.

This StackOverflow answer seems to suggest a way forward.

Does this seem a reasonable way forward? I assume I could create my own equivalent to io.seldon.wrapper.config.AppConfig to return a org.springframework.boot.web.embedded.netty.NettyReactiveWebServerFactory bean.

@ukclivecox
Copy link
Contributor

Be great if this could be an extension of our wrapper code to allow either to be used. But yes that sounds like the right idea.

Its the config in io.seldon.wrapper.config that needs to be selected by the user. The one currently in that package is for Tomcat.

Any suggestions on how we could have one for Tomcat and one for Netty and allow the user to decide? We could delete it and its up to the user to create appropriate bindings? But then that makes it harder for people to get running quickly.

@dorzey
Copy link
Author

dorzey commented Jun 2, 2018

I've created an example spring-boot-starter-webflux that compiles against seldon-core - https://github.com/dorzey/demo-seldon-app

I had to add an exclusion filter for io.seldon.wrapper.config.AppConfig. If supporting multiple configs is something that needs to be supported then it might be best to move them out of io.seldon.wrapper

I've not had the chance to deploy this to K8 with seldon yet.

@gsunner
Copy link
Member

gsunner commented Jun 5, 2018

@dorzey I had a look at your example "https://github.com/dorzey/demo-seldon-app"

I didn't see this work as you expected, it still starts the Tomcat embedded webserver.

Another approach can be to exclude the tomcat dependency thats in seldon-core-wrapper, and use the dependency you want ie. "spring-boot-starter-reactor-netty"

so the change to your pom.xml can be as follows:

	<dependency>
		<groupId>io.seldon.wrapper</groupId>
		<artifactId>seldon-core-wrapper</artifactId>
		<version>0.1.0</version>
		<exclusions>
			<exclusion>
				<groupId>log4j</groupId>
				<artifactId>log4j</artifactId>
			</exclusion>
			<exclusion>
				<artifactId>spring-boot-starter-tomcat</artifactId>
				<groupId>org.springframework.boot</groupId>
			</exclusion>
		</exclusions>
	</dependency>

	<dependency>
		<groupId>org.springframework.boot</groupId>
		<artifactId>spring-boot-starter-reactor-netty</artifactId>
	</dependency>

This should start your app with netty.

@ukclivecox
Copy link
Contributor

Please reopen if still an issue.

agrski pushed a commit that referenced this issue Dec 2, 2022
* add trigger types to pipeline steps

* finish triggers to allow for tensor spec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants