Skip to content

Call runners after ApplicationReadyEvent has been published #7656

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

Closed
rupert-madden-abbott opened this issue Dec 15, 2016 · 6 comments
Closed
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@rupert-madden-abbott
Copy link
Contributor

rupert-madden-abbott commented Dec 15, 2016

Spring Boot provides the very useful ApplicationRunner and CommandLineRunner for executing code immediately before the SpringApplication.run method finishes.

There are many articles and StackOverflow questions (example 1, example 2) that advocate using these for the bulk of the logic within a Command Line application.

However, if you use one of these, then the "Started ApplicationName in X seconds" log message is output after the runners have finished. Additionally, the event ApplicationReadyEvent is called afterwards as well.

This implies that runners are only intended for initialisation work. You should not, in fact, be using them for work that should be considered part of running the application and doing so is a misuse of their intended purpose.

Can we please have some clarity on the proper usage added to the documentation if this is indeed a misuse?

If it is not a misuse then can the runner execution get moved to after the ApplicationReadyEvent, and after the "Started" log message, to more clearly indicate that the application is already, in fact, ready, and now the runner execution is part of the application itself running?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 15, 2016
@philwebb philwebb added type: enhancement A general enhancement for: team-attention An issue we'd like other members of the team to review and removed status: waiting-for-triage An issue we've not yet triaged labels Dec 15, 2016
@philwebb
Copy link
Member

Thanks for raising this.

I think that they should probably happen after the started event, although that's not something we'll be able to do until Boot 2.0. I'll tag this for team discussion to see what others think.

@philwebb philwebb added this to the 2.0.0.M1 milestone Jan 4, 2017
@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Jan 4, 2017
@philwebb philwebb modified the milestones: 2.0.0.M1, 2.0.0, 2.0.0.RC1 Jan 12, 2017
@philwebb philwebb modified the milestones: 2.0.0.RC1, 2.0.0.M6 Sep 20, 2017
@wilkinsona wilkinsona changed the title Proper use of Runners is ambiguous Call runners after ApplicationReadyEvent has been published Oct 30, 2017
@wilkinsona wilkinsona self-assigned this Oct 30, 2017
@snicoll
Copy link
Member

snicoll commented Nov 4, 2017

@wilkinsona this one has a side effect on a web application. If you use to run something in an ApplicationRunner to initialize something, then you may end up in a situation where the HTTP connector is started before the initializer completes.

This is easy to run in problem with devtools and LiveReload as the browser might refresh "too fast" and your app is in an inconsistent state then.

I think we need to make sure to update the release notes to:

  1. Clearly describe that side effect
  2. Provide alternatives (i.e. if you want to run something when everything has completed, but before the connection actually starts

@snicoll snicoll added the for: team-attention An issue we'd like other members of the team to review label Nov 4, 2017
@wilkinsona
Copy link
Member

IMO, an application runner shouldn't be used for initialisation. If you need to initialise something it should be done as part of application context refresh. If you need that initialisation to happen after something, then the standard dependency mechanisms offered by the Framework should be used. I'm struggling to think of a use case where a component's dependency would effectively be everything.

@frederikvanhoyweghen
Copy link

frederikvanhoyweghen commented Jan 2, 2019

+1 on documenting that CommandLineRunner and ApplicationRunner should only be used for initialization work and nothing more.
Found this out while testing spring-boot-admin. Clients just weren't registering with the admin server and it took quite a while to figure out that ApplicationReadyEvent just wasn't ever fired because of what we were doing inside a CommandLineRunner bean..

@snicoll
Copy link
Member

snicoll commented Jan 2, 2019

@frederikvanhoyweghen this issue is closed so please do not comment here to suggest a change. Your use case is also quite specific to Spring Boot Admin so I'd rather ask there but if you have some ideas to generically improve the documentation, please create a separate issue.

@frederikvanhoyweghen
Copy link

Sorry, I didn't notice the issue was closed. I took it up with the spring boot admin maintainers, I'll create a separate issue if need be. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

6 participants