-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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(path): change windows style path to unix #633
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
""" | ||
Helper methods that aid with changing the mount path to unix style. | ||
""" | ||
|
||
import os | ||
import posixpath | ||
import re | ||
try: | ||
import pathlib | ||
except ImportError: | ||
import pathlib2 as pathlib | ||
|
||
|
||
def to_posix_path(code_path): | ||
""" | ||
Change the code_path to be of unix-style if running on windows when supplied with an absolute windows path. | ||
|
||
Parameters | ||
---------- | ||
code_path : str | ||
Directory in the host operating system that should be mounted within the container. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Returns? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added Returns and Examples. |
||
Returns | ||
------- | ||
str | ||
Posix equivalent of absolute windows style path. | ||
Examples | ||
-------- | ||
>>> to_posix_path('/Users/UserName/sam-app') | ||
/Users/UserName/sam-app | ||
>>> to_posix_path('C:\\\\Users\\\\UserName\\\\AppData\\\\Local\\\\Temp\\\\mydir') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Had to do multiple escapes, because of quotes within docstrings, thats still a valid path and resolves. In [332]: to_posix_path('C:\\Users\\UserName\\AppData\\Local\\Temp\\mydir')
Out[332]: '/c/Users/UserName/AppData/Local/Temp/mydir'
In [333]: to_posix_path('C:\\\\Users\\\\UserName\\\\AppData\\\\Local\\\\Temp\\\\mydir')
Out[333]: '/c/Users/UserName/AppData/Local/Temp/mydir' |
||
/c/Users/UserName/AppData/Local/Temp/mydir | ||
""" | ||
|
||
return re.sub("^([A-Za-z])+:", | ||
lambda match: posixpath.sep + match.group().replace(":", "").lower(), | ||
pathlib.PureWindowsPath(code_path).as_posix()) if os.name == "nt" else code_path |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -102,7 +102,7 @@ def test_must_create_container_with_required_values(self): | |
|
||
def test_must_create_container_including_all_optional_values(self): | ||
""" | ||
Create a container with only required values. Optional values are not provided | ||
Create a container with required and optional values. | ||
:return: | ||
""" | ||
|
||
|
@@ -149,6 +149,71 @@ def test_must_create_container_including_all_optional_values(self): | |
) | ||
self.mock_docker_client.networks.get.assert_not_called() | ||
|
||
@patch("samcli.local.docker.utils.os") | ||
def test_must_create_container_translate_volume_path(self, os_mock): | ||
""" | ||
Create a container with required and optional values, with windows style volume mount. | ||
:return: | ||
""" | ||
|
||
os_mock.name = "nt" | ||
host_dir = "C:\\Users\\Username\\AppData\\Local\\Temp\\tmp1337" | ||
additional_volumes = { | ||
"C:\\Users\\Username\\AppData\\Local\\Temp\\tmp1338": { | ||
"blah": "blah value" | ||
} | ||
} | ||
|
||
translated_volumes = { | ||
"/c/Users/Username/AppData/Local/Temp/tmp1337": { | ||
"bind": self.working_dir, | ||
"mode": "ro" | ||
} | ||
} | ||
|
||
translated_additional_volumes = { | ||
"/c/Users/Username/AppData/Local/Temp/tmp1338": { | ||
"blah": "blah value" | ||
} | ||
} | ||
|
||
translated_volumes.update(translated_additional_volumes) | ||
expected_memory = "{}m".format(self.memory_mb) | ||
|
||
generated_id = "fooobar" | ||
self.mock_docker_client.containers.create.return_value = Mock() | ||
self.mock_docker_client.containers.create.return_value.id = generated_id | ||
|
||
container = Container(self.image, | ||
self.cmd, | ||
self.working_dir, | ||
host_dir, | ||
memory_limit_mb=self.memory_mb, | ||
exposed_ports=self.exposed_ports, | ||
entrypoint=self.entrypoint, | ||
env_vars=self.env_vars, | ||
docker_client=self.mock_docker_client, | ||
container_opts=self.container_opts, | ||
additional_volumes=additional_volumes | ||
) | ||
|
||
container_id = container.create() | ||
self.assertEquals(container_id, generated_id) | ||
self.assertEquals(container.id, generated_id) | ||
|
||
self.mock_docker_client.containers.create.assert_called_with(self.image, | ||
command=self.cmd, | ||
working_dir=self.working_dir, | ||
volumes=translated_volumes, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Additional volumes with windows style paths, also resolve to unix style paths. |
||
tty=False, | ||
environment=self.env_vars, | ||
ports=self.exposed_ports, | ||
entrypoint=self.entrypoint, | ||
mem_limit=expected_memory, | ||
container='opts' | ||
) | ||
self.mock_docker_client.networks.get.assert_not_called() | ||
|
||
def test_must_connect_to_network_on_create(self): | ||
""" | ||
Create a container with only required values. Optional values are not provided | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
""" | ||
Unit test for Utils | ||
""" | ||
|
||
import os | ||
from unittest import TestCase | ||
|
||
from mock import patch | ||
|
||
from samcli.local.docker.utils import to_posix_path | ||
|
||
|
||
class TestUtils(TestCase): | ||
|
||
def setUp(self): | ||
self.ntpath = "C:\\Users\\UserName\\AppData\\Local\\Temp\\temp1337" | ||
self.posixpath = "/c/Users/UserName/AppData/Local/Temp/temp1337" | ||
self.current_working_dir = os.getcwd() | ||
|
||
@patch("samcli.local.docker.utils.os") | ||
def test_convert_posix_path_if_windows_style_path(self, mock_os): | ||
mock_os.name = "nt" | ||
self.assertEquals(self.posixpath, to_posix_path(self.ntpath)) | ||
|
||
@patch("samcli.local.docker.utils.os") | ||
def test_do_not_convert_posix_path(self, mock_os): | ||
mock_os.name = "posix" | ||
self.assertEquals(self.current_working_dir, to_posix_path(self.current_working_dir)) |
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.
nice! Thakns