Skip to content
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

Terminate native image build container when Quarkus build terminates #19060

Merged
merged 1 commit into from
Jul 28, 2021

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Jul 28, 2021

Fixes: #7331

Copy link
Contributor

@gastaldi gastaldi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@geoand geoand added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jul 28, 2021
.redirectError(ProcessBuilder.Redirect.DISCARD)
.start();
removeProcess.waitFor(2_000, TimeUnit.SECONDS);
} catch (IOException | InterruptedException e) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right some DB are very slow to stop, but 2000 seconds is a lot! I expect people to think it's hung?

What happens if we don't wait at all, or just wait for some ~10 seconds ?

Also might be nice to include containerName in the logged message, and upgrade it from debug to INFO or WARN ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh shoot... I mean to have 2 seconds :).

Basically this is just meant to do docker rm -f build-native so it's really esoteric, not much the user can do anyway and it doesn't have much of an effect even if it fails.

@geoand geoand merged commit ab9defa into quarkusio:main Jul 28, 2021
@quarkus-bot quarkus-bot bot added this to the 2.2 - main milestone Jul 28, 2021
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jul 28, 2021
@geoand geoand deleted the #7331 branch July 29, 2021 08:29
@gsmet gsmet modified the milestones: 2.2 - main, 2.1.1.Final Aug 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cancellation of Maven build should stop spawned GraalVM build, too
4 participants