-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
correct startup error message #9369
correct startup error message #9369
Conversation
Note that the error message is still quite broad as it assumes that all errors would be caused by a remote-connection error but it's improving over the previous state. |
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.
LGTM
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: saschagrunert, vrothberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
I was wondering where that error message was comping from. |
CI failures should be fixed with #9370 |
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.
I think it would be better to move the error wrap to pkg/bindings here
podman/pkg/bindings/connection.go
Line 126 in 30607d7
if err := pingNewConnection(ctx); err != nil { |
Currently you can get something like this
$ bin/podman-remote --url abc ps
Error: Cannot connect to the Podman socket, make sure there is a Podman REST API service running.: unable to create connection. "" is not a supported schema
If the url is invalid it is a client error and should not show something about the server.
@Luap99 sounds good! I'll update once CI is back green. |
Very good catch! LGTM |
fb4529c
to
1f027e3
Compare
The error message when failing to create an image engine unconditionally pointed to the Podman socket which is quite confusing when running locally. Move the error message to the point where the first ping to the service fails. [NO TESTS NEEDED] Signed-off-by: Valentin Rothberg <[email protected]>
1f027e3
to
39c1fdb
Compare
/lgtm |
/hold cancel |
The error message when failing to create an image engine unconditionally
pointed to the Podman socket which is quite confusing when running
locally.
Move the error message to the point where the first ping to the service
fails.
[NO TESTS NEEDED]
Signed-off-by: Valentin Rothberg [email protected]
@containers/podman-maintainers