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
do not check mounts entering windows container studio #4668
Conversation
Signed-off-by: mwrock <matt@mattwrock.com>
Thanks for the pull request! Here is what will happen next:
Thank you for contributing! |
@@ -197,9 +201,7 @@ where | |||
cmd_args.push(vol.as_ref().into()); | |||
} | |||
|
|||
if !is_serving_windows_containers(docker_cmd) { |
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.
Was the removal of this check intentional? Is this related to mounts too?
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.
since this PR would not enter this function when using windows containers, this check would always be false
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.
@thesentinels approve |
🤘 I am testing your branch against master before merging it. We do this to ensure that the master branch is never failing tests. |
Travis CI has started testing this PR. |
💖 Travis CI reports this PR passed. It always makes me feel nice when humans approve of one anothers work. I'm merging this PR now. I just want you and the contributor to answer me one question: |
Windows containers do not use shared drives for mounting local volumes so the
check_mounts
function provides no value when run in Windows container mode. Also, this incurs adocker run
which takes much longer on Windows 10 hosts for windows containers (like ~ 20 seconds) so removing this check significantly speeds up the studio startup time.Signed-off-by: mwrock matt@mattwrock.com