Skip to content

Move publication of ApplicationReadyEvent back to after runners have been called and provide new event that's published before #11484

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
wilkinsona opened this issue Jan 3, 2018 · 4 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@wilkinsona
Copy link
Member

#7656 moved the logic that calls any CommandLineRunners and ApplicationRunners to a point after the ApplicationReadyEvent has been fired. This means there's no longer an event that can be used to learn when the runners have been called.

The proposal is to introduce a new event (RunnersCompletedEvent?) that is fired when the runners have been called. We may also want to introduce a second event (RunnersFailedEvent?) that can be fired when one of the runners fails (throws an exception). As things stand an exception from a runner will result in ApplicationFailedEvent being published despite ApplicationReadyEvent already having been published.

@wilkinsona
Copy link
Member Author

wilkinsona commented Jan 10, 2018

I'm currently of the opinion that implementing #7656 was a mistake. That change has created the problem described above, while also meaning that ContextRefreshedEvent and ApplicationReadyEvent are published at, essentially, the same point in the application's lifecycle. The only thing of any real substance that happens between the two is the call to afterRefresh on SpringApplication and that's a no-op by default.

I think we should consider reverting the fix for #7656 and declining this issue.

@wilkinsona
Copy link
Member Author

Actually, rather than completely reverting the fix for #7656, we could continue logging the application started message before calling the runners.

@snicoll
Copy link
Member

snicoll commented Jan 10, 2018

That's what I was about to write. This looks good to me.

@wilkinsona
Copy link
Member Author

The advantage of a SpringApplicationEvent is that it provides access to SpringApplication. ContextRefreshedEvent does not do so. We're considering introducing an ApplicationStartedEvent that would be published once the context is refreshed but before the runners have been called. ApplicationReadyEvent (possibly renamed to ApplicationRunningEvent) would then be published once the runners have been called and immediately prior to returning from run().

@wilkinsona wilkinsona removed the for: team-attention An issue we'd like other members of the team to review label Jan 10, 2018
@wilkinsona wilkinsona self-assigned this Jan 10, 2018
@wilkinsona wilkinsona changed the title Provide events for completion or failure of CommandLineRunners and ApplicationRunners Move publication of ApplicationReadyEvent back to after runners have been called and provide new event that's published before Jan 11, 2018
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

2 participants