Skip to content

Moving the ApplicationReadyEvent to before the execution of *Runners breaks Spring Cloud Task #360

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
mminella opened this issue Jan 2, 2018 · 2 comments
Assignees
Milestone

Comments

@mminella
Copy link
Contributor

mminella commented Jan 2, 2018

Prior to issue spring-projects/spring-boot#7656, Spring Cloud Task was able to use the ApplicationReadyEvent to be notified that the ApplicationRunners and CommandLineRunners have been executed. The resolution of that issue breaks Spring Cloud Task in that it moves that event to before the runners have been executed without providing a replacement notification.

@sabbyanandan sabbyanandan added this to the 2.0.0.RC1 milestone Jan 2, 2018
@snicoll snicoll changed the title Boot moving the ApplicationReadyEvent to before the execution of *Runners breaks Spring Cloud Task Moving the ApplicationReadyEvent to before the execution of *Runners breaks Spring Cloud Task Jan 3, 2018
@snicoll
Copy link

snicoll commented Jan 3, 2018

Given that an ApplicationRunner can be infinite, using it to determine what a task is seems an abuse to me. Of course, an infinite runner for a Spring Cloud Task app doesn't make any sense at all but ApplicationRunner is a Spring Boot concept and its semantic is wider to what you're using it for. That alone is a reason not to expose it directly.

IMO, it is much better that Spring Cloud Task defines a contract of its own for several reasons:

  1. You get control over the API and you can improve it without relying on the “core” contract in Spring Boot (this issue shows that the semantics do not match)
  2. You get control over the lifecycle and what happens in case of error, etc.

You could wrap an invoker that is an ApplicationRunner behind the scenes and have full control over which task is currently running (in case you have more than one). It's also easier to report what is going on in the logs.

@mminella
Copy link
Contributor Author

mminella commented Jan 3, 2018

This issue will be addressed by the adding of new events in Spring Boot per: spring-projects/spring-boot#11484

@cppwfs cppwfs added in progress and removed ready labels Jan 18, 2018
@cppwfs cppwfs self-assigned this Jan 18, 2018
cppwfs added a commit to cppwfs/spring-cloud-task that referenced this issue Jan 18, 2018
Did build and smoke tests and it resolves the issues identified in this issue.
resolves spring-cloud#360
@cppwfs cppwfs added in pr and removed in progress labels Jan 18, 2018
@mminella mminella removed the in pr label Jan 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants