spring-cloud / spring-cloud-commons

Common classes used in different Spring Cloud implementations
Apache License 2.0
707 stars 702 forks source link

/restart endpoint stops working after adding devtools #102

Open gmarziou opened 8 years ago

gmarziou commented 8 years ago

Hello,

I'm working on JHipster project and I am unable to make /restart work with Brixton.M5

I have been able to reproduce the problem with a minimal spring boot app, when I post to /restart the app restarts as expected. But when I add spring boot devtools to the pom, restart fails saying that there's no bean of type JHipsterProperties to inject which is enabled this way:

@EnableConfigurationProperties({ JHipsterProperties.class})

Is this a known issue? Or is there something wrong in the way we init our app?

My sample app is available in this repo: https://github.com/gmarziou/issue-restart

spencergibb commented 8 years ago

I'm unable to reproduce the problem from your project (which is missing JHipsterProperties, .mvn and bad character in the @pom.xml). How are you running the project? I ran in intellij, mvn spring-boot:run and java -jar.

gmarziou commented 8 years ago

Sorry about the issues in my repo, I fixed them. I don't have the problem with mvn spring-boot:run, I have it only when running in IntelliJ as a java app using main class as below:

image

gmarziou commented 8 years ago

IntelliJ IDEA 2016.1 Community Edition Build #IC-145.258, built on March 17, 2016 JRE: 1.8.0_40-release-b132 x86_64 JVM: OpenJDK 64-Bit Server VM by JetBrains s.r.o

gmarziou commented 8 years ago

It works also as java -jar

spencergibb commented 8 years ago

I still don't have the problem, but I'm using Intellij 15.0.4

Maybe that's the problem?

Either way, not sure it's a spring-cloud problem at this point. @philwebb any ideas?

gmarziou commented 8 years ago

I have same problem with Intellij 15.0.4 Community Edition I'm on OSX 10.11.3, but my original issue with full JHipster app was on Ubuntu (also with IntelliJ CE) Maybe something about the Run Configuration settings?

odrotbohm commented 3 years ago

I am running into the same issue. The error manifests itself by missing bean references when application components are wired. In my case, the situation looks as follows:

Starting the application with DevTools on the classpath works fine. When I trigger a restart, I can see the argument resolution trying to find a bean in the new ApplicationContext for the argument type but the type match failing due to the fact that one class was still loaded from the DevTools classloader and the reference one is loaded by the classloader set by RestartEndpoint.overrideClassLoaderForRestart(). Thus they never match and Boot claims a bean missing.

spencergibb commented 3 years ago

@philwebb what would we need to do to support devtools?

dsyer commented 3 years ago

Does it even make sense to use /restart with devtools? I mean, they are trying to do the same thing at some level. It's no surprise if they can't agree on how to do it.

odrotbohm commented 3 years ago

I am not sure what "makes sense" means in this context. I configure the actuator and would like to run the application and potentially trigger that functionality and make sure it works. The concrete use case here is running mvn spring-boot:run in preparation of integration tests being executed.

@spencergibb – I assume reusing the DevTools classloader?

dsyer commented 3 years ago

What I meant was, once the app is running and the resources are not changing (i.e. post dev) then you don't need devtools. It seems like that probably fits your use case - it's post dev because you are running automated tests, and probably the tests do not themselves change the source code. Maybe you should switch off devtools for the test app?

odrotbohm commented 3 years ago

The tests previously just used mvn spring-boot:run in a GitHub Actions workflow and then ran tests against that. I now have to package up the application (to basically get rid off DevTools during the execution) and run the jar.

Maybe you should switch off devtools for the test app?

You can turn of the restart functionality but that doesn't skip the classloader kung-fu.

dsyer commented 3 years ago

You could use a profile to exclude the devtools dependency. But, yes, I see, it would be good to have a switch to shut off the devtools class loader on the command line instead.