Display Docker build errors when builds fail (Fixes #35)#347
Display Docker build errors when builds fail (Fixes #35)#347T-Rajeev30 wants to merge 4 commits intoosrf:mainfrom
Conversation
|
Hi @tfoote gentle follow-up on this PR. A review at your convenience would help me proceed, as I’m planning to work on additional issues and would like to align with you before moving forward . Thanks for your time. |
|
@tfoote can you please review it and can you please assign me some issue so that i can solve them |
tfoote
left a comment
tfoote
left a comment
There was a problem hiding this comment.
I see the error coming out and that the error messages are shown.
^^^^^^
test/test_core.py Writing dockerfile to /tmp/tmp0r2m3yyt/Dockerfile
vvvvvv
FROM ubuntu:bionic
USER root
^^^^^^
Building docker file with arguments: {'path': '/tmp/tmp0r2m3yyt', 'rm': True, 'nocache': False, 'pull': False}
build > Step 1/2 : FROM ubuntu:bionic
build > ---> f9a80a55f492
build > Step 2/2 : USER root
build > ---> Using cache
build > ---> c3f42b90a98d
build > Successfully built c3f42b90a98d
Writing dockerfile to /tmp/tmpzvky9i3i/Dockerfile
vvvvvv
# Preamble from extension [broken_snippet]
FROM ubuntu:bionic
USER root
# Snippet from extension [broken_snippet]
BAD KEYWORD for a dockerfile
# User Snippet from extension [broken_snippet]
^^^^^^
Building docker file with arguments: {'path': '/tmp/tmpzvky9i3i', 'rm': True, 'nocache': False, 'pull': False}
Docker build failed
400 Client Error for http+docker://localhost/v1.52/build?q=False&nocache=False&rm=True&forcerm=False&pull=False: Bad Request ("dockerfile parse error on line 7: unknown instruction: BAD (did you mean ADD?)")
.
But I'm not seeing the failed build outputs along with the error. As far as I can tell nothing is going through the stream output as you can see from the lack of the different console output prefixes in my overridden print.
| # dig.output_callback = lambda output: print("OVERRIDE build > %s" % output) | ||
| self.assertIn('BAD KEYWORD', dig.dockerfile) | ||
| self.assertEqual(dig.build(), 1) | ||
| # TODO(tfoote) capture the console output by passing in a custom output_callback and assert we see the error there |
There was a problem hiding this comment.
Please make sure that the error message can be confirmed in the console output by overriding the output_callback.
| # Add broken build which breaks | ||
| mock_cli_args = {'broken_snippet': True} | ||
| dig = DockerImageGenerator(active_extensions, mock_cli_args, 'ubuntu:bionic') | ||
| # dig.output_callback = lambda output: print("OVERRIDE build > %s" % output) |
There was a problem hiding this comment.
I quickly mocked overriding the output callback but it doesn't appear to be working.
|
@tfoote can i do all these changes by next 24 hrs ?? |
|
For future reference the past iteration of this was #345 In fhe future please just iterate and evolve the PR instead of creating a new one so that the history is clear to future reviewers. |
|
This doesn't seem to resolve the issue and is out of date. |
2 participants
Summary
Fixes #35 by displaying actual Docker build errors instead of the generic failure message when builds fail.
Notes
I previously opened a PR for this issue, but it grew beyond scope and included multiple changes, so I closed it and prepared this smaller PR focused only on surfacing Docker build errors.
@tfoote Could you please review this updated, minimal version?