Skip to content

LCOW remotefs - return error in Read() implementation#36051

Merged
vdemeester merged 1 commit into
moby:masterfrom
microsoft:jjh/remotefs-read-return-error
Jan 19, 2018
Merged

LCOW remotefs - return error in Read() implementation#36051
vdemeester merged 1 commit into
moby:masterfrom
microsoft:jjh/remotefs-read-return-error

Conversation

@lowenna
Copy link
Copy Markdown
Member

@lowenna lowenna commented Jan 18, 2018

Signed-off-by: John Howard [email protected]

Found while debugging a slew of LCOW bugs. Turns out if getResponse fails in the remoteFS Read() implementation, the error isn't returned back. Very simple fix.

@gupta-ak @jterry75 FYI. @johnstep PTAL.

Copy link
Copy Markdown
Contributor

@gupta-ak gupta-ak left a comment

Choose a reason for hiding this comment

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

I also see the same bug in lines 108 and 171.

@lowenna lowenna force-pushed the jjh/remotefs-read-return-error branch from 3c69a38 to 6112ad6 Compare January 19, 2018 01:47
@lowenna
Copy link
Copy Markdown
Member Author

lowenna commented Jan 19, 2018

@gupta-ak Yes, you're right. Good catch, thanks. Updated.

Copy link
Copy Markdown
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

@gupta-ak
Copy link
Copy Markdown
Contributor

LGTM

Copy link
Copy Markdown
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐸

@vdemeester vdemeester merged commit 3c9d023 into moby:master Jan 19, 2018
@thaJeztah thaJeztah added the area/lcow Issues and PR's related to the experimental LCOW feature label Oct 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/lcow Issues and PR's related to the experimental LCOW feature status/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants