Skip to content

Allow SpringApplicationRunListeners step action to be optional #22845

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
wants to merge 1 commit into from

Conversation

mdeinum
Copy link
Contributor

@mdeinum mdeinum commented Aug 10, 2020

Prior to this commit the StartupStep.end method was being
called from the default step action. However when overriding
the default step action this might lead to the StartupStep.end
method not being called. As in the case of a failure, as that
enriches the information being written.

This commit also introduces a test for the failure case showing that
there is a missed call to end with the initial solution.

See: gh-22776

@mdeinum
Copy link
Contributor Author

mdeinum commented Aug 10, 2020

@bclozel There appears to be still a missing call to a StartupStep.end() in case of failure during startup, which that is I couldn't determine (might be one in Spring itself.

Another note I couldn't run the tests locally it was hanging on the BuildImageTests (or at least it appeared that way as it didn't progress).

@mdeinum
Copy link
Contributor Author

mdeinum commented Aug 10, 2020

@bclozel spring-projects/spring-framework#25572 (in Spring Framework Repository) fixes the missing call to end. It was the spring.beans.instantiate that wasn't ended when an error occurred.

@philwebb
Copy link
Member

This one was my fault, I messed up in a polish commit.

@philwebb philwebb self-assigned this Aug 10, 2020
@philwebb philwebb added type: task A general task and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 10, 2020
@philwebb philwebb added this to the 2.4.x milestone Aug 10, 2020
@mdeinum mdeinum force-pushed the gh-22776 branch 2 times, most recently from 6fe8b32 to d9e0c14 Compare August 11, 2020 05:56
Prior to this commit the StartupStep.end method was being
called from the default step action. However when overriding
the default step action this might lead to the StartupStep.end
method not being called. As in the case of a failure, as that
enriches the information being written.

This commit also introduces a test for the failure case showing that
there is a missed call to end with the initial solution.

See: spring-projectsgh-22776
@mdeinum
Copy link
Contributor Author

mdeinum commented Aug 11, 2020

Just noticed 4 test failures on Concourse-ci, however, it is 4 times the same test, appears that tests are either counted or run multiple times?

The test is failing due to the linked Spring Framework issue.

@philwebb philwebb changed the title Use no-op lambda as default action for step action Allow SpringApplicationRunListeners step action to be optional Aug 11, 2020
@philwebb philwebb modified the milestones: 2.4.x, 2.4.0-M2 Aug 11, 2020
philwebb pushed a commit that referenced this pull request Aug 11, 2020
Update `SpringApplicationRunListeners` so that the step action
is optional and does not need to call `end()`.

This commit also introduces a test for the failure case showing
that there is a missed call to end with the initial solution.

See gh-22845
philwebb added a commit that referenced this pull request Aug 11, 2020
@philwebb philwebb closed this in 07e59df Aug 11, 2020
@philwebb
Copy link
Member

Thanks @mdeinum! I've tweaked the test for now until the Framework issue is fixed.

@mdeinum mdeinum deleted the gh-22776 branch April 29, 2021 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: task A general task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants