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

Fix win32pipe.WaitNamedPipe throw exception in windows container #2421

Merged
merged 1 commit into from
Nov 6, 2019

Conversation

SLdragon
Copy link

WaitNamedPipe API call in the npipesocket.py module doesn't work correctly from inside a Windows container.
This is because currently WaitNamedPipeis not supported from inside of Windows Containers (however CreateNamedPipeClient works ok). It fails with ERROR_FILE_NOT_FOUND

related issues:
#2018
docker/compose#5934
docker/for-win#4012
https://github.com/dotnet/corefx/issues/24594

@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "master" git@github.com:SLdragon/docker-py.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

Signed-off-by: Renlong Tu <rentu@microsoft.com>
@SLdragon
Copy link
Author

ping @shin- , please help approve this pull request, thank you very much!

@romerod
Copy link

romerod commented Oct 23, 2019

what is missing for this to be merged?

@eNettMattD
Copy link

Any update on when this can be merged?

@StefanScherer
Copy link
Member

I have tested this PR in a Windows container with a small test environment from StefanScherer/dockerfiles-windows#432 and it works.

Previously docker-compose just shows this error

ERROR: Couldn't connect to Docker daemon. You might need to start Docker for Windows. 

Thanks for the contribution. Ping @chris-crone @ndeloof WDYT? Is the recursion ok here for max 10 iterations?

Copy link
Member

@chris-crone chris-crone left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @SLdragon and @StefanScherer !

@rumpl
Copy link
Member

rumpl commented Nov 6, 2019

Indeed, 10 iterations seems reasonable seeing what the code was before.

Thanks!

@rumpl rumpl merged commit a0b9c3d into docker:master Nov 6, 2019
@chris-crone chris-crone added this to the 4.2.0 milestone Nov 6, 2019
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.

7 participants