8000 Fix error handling for bind mount spec parser. by cpuguy83 · Pull Request #39251 · moby/moby · GitHub
[go: up one dir, main page]
Skip to content

Fix error handling for bind mount spec parser. #39251

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 23, 2019
Merged

Fix error handling for bind mount spec parser. #39251

merged 1 commit into from
May 23, 2019

Conversation

cpuguy83
Copy link
Member
< 8000 /div>

Errors were being ignored and always telling the user that the path
doesn't exist even if it was some other problem, such as a permission
error.

Closes docker/for-linux#674

@cpuguy83 cpuguy83 requested review from thaJeztah and tiborvass May 21, 2019 22:37
@cpuguy83 cpuguy83 mentioned this pull request May 21, 2019
Closed
3 tasks
@cpuguy83
Copy link
Member Author

Adding cherry-pick because this is a quick fix for a really confusing situation.

@cpuguy83 cpuguy83 force-pushed the these_pretzels_are_making_me_thirsty branch from 69c6733 to b698469 Compare May 21, 2019 22:59
thaJeztah
thaJeztah previously approved these changes May 22, 2019
View reviewed changes
Copy link
Member
@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah
Copy link
Member

arf; looks like it's failing on Windows;

23:09:37 --- FAIL: TestParseMountSpecBindWithFileinfoError (0.00s)
23:09:37     parser_test.go:529: assertion failed: invalid mount path: '/apples' (e.err *errors.errorString) != some error (testErr *errors.errorString)
23:09:37 FAIL

@thaJeztah thaJeztah dismissed their stale review May 22, 2019 12:21

see comment

@cpuguy83 cpuguy83 force-pushed the these_pretzels_are_making_me_thirsty branch from b698469 to 50d8a41 Compare May 22, 2019 17:30
@cpuguy83
Copy link
Member Author

:( Github notifications are having issues, CI is not being triggered.

@codecov
Copy link
codecov bot commented May 22, 2019 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@0805242). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master   #39251   +/-   ##
=========================================
  Coverage          ?   37.04%           
=========================================
  Files             ?      612           
  Lines             ?    45489           
  Branches          ?        0           
=========================================
  Hits              ?    16850           
  Misses            ?    26349           
  Partials          ?     2290

@cpuguy83
Fix error handling for bind mount spec parser.
ebcef28 
Errors were being ignored and always telling the user that the path
doesn't exist even if it was some other problem, such as a permission
error.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
@cpuguy83 cpuguy83 force-pushed the these_pretzels_are_making_me_thirsty branch from 50d8a41 to ebcef28 Compare May 22, 2019 18:54
@cpuguy83
Copy link
Member Author

Well, PR is fixed for Windows... Github is still being a bit funny.

8000
thaJeztah
thaJeztah approved these changes May 22, 2019
View reviewed changes
Copy link
Member
@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

janky failure is docker-py, and doesn't look related

This was referenced May 22, 2019
Merged
Merged
kolyshkin
kolyshkin approved these changes May 23, 2019
View reviewed changes
Copy link
Contributor
@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah merged commit 8a208a1 into moby:master May 23, 2019
@cpuguy83 cpuguy83 deleted the these_pretzels_are_making_me_thirsty branch May 23, 2019 18:47
@thaJeztah thaJeztah added this to the 20.03.0 milestone Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thaJeztah thaJeztah thaJeztah approved these changes

@kolyshkin kolyshkin kolyshkin approved these changes

@tiborvass tiborvass Awaiting requested review from tiborvass

Assignees
No one assigned
Labels
process/cherry-picked status/2-code-review
Projects
None yet
Milestone
20.10.0
Development

Successfully merging this pull request may close these issues.

"Path does not exist" when path definitely exists
4 participants
@cpuguy83 @thaJeztah @kolyshkin @GordonTheTurtle
0