-
Notifications
You must be signed in to change notification settings - Fork 61
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
show docker build output (#785) #882
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking great!
src/popper/runner_host.py
Outdated
@@ -212,9 +212,17 @@ def _create_container(self, cid, step): | |||
if build: | |||
log.info(f"[{step.id}] docker build {img}:{tag} {build_ctx_path}") | |||
if not self._config.dry_run: | |||
self._d.images.build( | |||
|
|||
_, build_logs = self._d.images.build( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this blocks, so the output only shows until images.build()
returns. If the output of the build is too big, this might cause memory issues. In order to avoid this, this code can use the low-level API as exemplified here. Very similar to what what's already here, but with a couple of tweaks.
thanks! I made some tweaks based on what you have in this PR and pushed them to https://github.com/getpopper/popper/tree/pr/882 . Does all that make sense to you? If it does, can you please rebase that branch from this PR? |
nice! I did the rebase and it works smoothly. |
great, thanks! can you please push the changes to this PR, so I can merge from here? otherwise your authorship is going to get lost. |
sure 😁 |
thanks! |
Shows docker build output by default and when the
--quiet flag
is given topopper run
it hides it.