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

Pull: include error message in warnings/errors #11555

Merged
merged 2 commits into from Mar 25, 2024

Conversation

felixfontein
Copy link
Contributor

What I did
When warnings or errors happen during pulling when running docker compose up, simply "<service_name> Warning" resp. "<service_name> Error" is printed without any information on what went wrong.

This PR changes this by adding the error message.

Before:

[+] Running 3/3
 ! dummy3 Warning                                                                                                                                                                        2.4s 
 ! dummy2 Warning                                                                                                                                                                        2.4s 
 ✘ dummy Error                                                                                                                                                                           2.4s 

After:

[+] Running 3/3
 ! dummy2 Warning Error response from daemon: pull access denied for dummy2, repository does not exist or may require 'docker login': denied: requested access to the resource is denied2.2s 
 ✘ dummy Error    Error response from daemon: pull access denied for dummy2, repository does not exist or may require 'docker login': denied: requested access to the resource is denied2.2s 
 ! dummy3 Warning Error response from daemon: pull access denied for dummy, repository does not exist or may require 'docker login': denied: requested access to the resource is denied2.2s 

Related issue
Ref: ansible-collections/community.docker#807

(not mandatory) A picture of a cute animal, if possible in relation to what you did
A cute cat with rather sharp claws, watch out when petting her

@ndeloof
Copy link
Contributor

ndeloof commented Mar 1, 2024

I wonder we could use Unwrap to get down to the interesting diagnostic here, and avoid generic daemon message, as such a long status text will probably get truncated

Copy link

codecov bot commented Mar 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.34%. Comparing base (3371227) to head (3a786a3).

❗ Current head 3a786a3 differs from pull request most recent head 6274fe9. Consider uploading reports for the commit 6274fe9 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11555      +/-   ##
==========================================
+ Coverage   54.76%   58.34%   +3.58%     
==========================================
  Files         145      135      -10     
  Lines       12585    11557    -1028     
==========================================
- Hits         6892     6743     -149     
+ Misses       5000     4149     -851     
+ Partials      693      665      -28     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@felixfontein
Copy link
Contributor Author

@ndeloof I added some code for the unwrapping. This specific message needs to be unpacked at least twice, only unpacking once yields exactly the same output.

@felixfontein
Copy link
Contributor Author

@ndeloof is there a chance to get this actually merged?

@ndeloof ndeloof enabled auto-merge (rebase) March 25, 2024 06:29
Signed-off-by: Felix Fontein <felix@fontein.de>
Signed-off-by: Felix Fontein <felix@fontein.de>
@ndeloof ndeloof merged commit 466374b into docker:main Mar 25, 2024
27 checks passed
@felixfontein
Copy link
Contributor Author

@ndeloof thanks a lot for reviewing and merging this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants