Repository: gabrieldemarmiesse/python-on-whales (language: python)

Issue:
it forces me to download docker client binary file on windows
I ran this code.
```
from python_on_whales import DockerClient
docker = DockerClient(host="ssh://root@xx.xxx.xx.xx")
docker.ps()
```
and it suddenly started downloading docker and to make it worse, it crashed. i have downloaded this 3 times. Also, i ran the equivalent for this with docker-py and it worked perfectly. i am using windows.

```
C:/Users/USER/AppData/Local/Programs/Python/Python37/python.exe c:/Users/USER/Desktop/cvbn/eyede
/pywale.py
C:\Users\USER\AppData\Local\Programs\Python\Python37\lib\site-packages\python_on_whales\client_config.py:65: UserWarning: The docker client binary file was not found on your system.
Docker on whales will try to download it for you.
Don't worry, it won't be in the PATH and won't have anything to do with the package manager of your system.
Note: We are not installing the docker daemon, which is a lot heavier and harder to install. We're just downloading a single standalone binary file.
If you want to trigger the download of the client binary file manually (for example if you want to do it in a Dockerfile), you can run the following command:
 $ python-on-whales download-cli

  "The docker client binary file was not found on your system. \n"
100%|████████████████████████████████████████████████████████████████████████████████████████████████████| 65.4M/65.4M [00:25<00:00, 2.53MiB/s]
Traceback (most recent call last):
  File "C:\Users\USER\AppData\Local\Programs\Python\Python37\lib\shutil.py", line 932, in _unpack_tarfile
    tarobj = tarfile.open(filename)
  File "C:\Users\USER\AppData\Local\Programs\Python\Python37\lib\tarfile.py", line 1578, in open
    raise ReadError("file could not be opened successfully")
tarfile.ReadError: file could not be opened successfully

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "c:/Users/USER/Desktop/cvbn/eyede/pywale.py", line 4, in <module>
    docker.ps()
  File "C:\Users\USER\AppData\Local\Programs\Python\Python37\lib\site-packages\python_on_whales\components\container\cli_wrapper.py", line 947,
in list
    full_cmd = self.docker_cmd
  File "C:\Users\USER\AppData\Local\Programs\Python\Python37\lib\site-packages\python_on_whales\client_config.py", line 134, in docker_cmd
    return self.client_config.docker_cmd
  File "C:\Users\USER\AppData\Local\Programs\Python\Python37\lib\site-packages\python_on_whales\client_config.py", line 85, in docker_cmd
    result = Command([self.get_docker_path()])
  File "C:\Users\USER\AppData\Local\Programs\Python\Python37\lib\site-packages\python_on_whales\client_config.py", line 78, in get_docker_path
    download_docker_cli()
  File "C:\Users\USER\AppData\Local\Programs\Python\Python37\lib\site-packages\python_on_whales\download_binaries.py", line 39, in download_docker_cli
    shutil.unpack_archive(str(tar_file), str(extract_dir))
  File "C:\Users\USER\AppData\Local\Programs\Python\Python37\lib\shutil.py", line 1002, in unpack_archive
    func(filename, extract_dir, **kwargs)
  File "C:\Users\USER\AppData\Local\Programs\Python\Python37\lib\shutil.py", line 935, in _unpack_tarfile
    "%s is not a compressed or uncompressed tar file" % filename)
shutil.ReadError: C:\Users\USER\AppData\Local\Temp\tmpv86v5tuj\docker.tgz is not a compressed or uncompressed tar file
```


Discussion / hints:
Thank you for the great bug report! I believe I know what is going on. Fetching the docker binary is quite different in windows and it doesn't come in a tar. I believe we need a switch. I'll take a look at this bug, I'll try the fix on my windows PC

Fix patch:
diff --git a/python_on_whales/client_config.py b/python_on_whales/client_config.py
index 454940a9..93e00b6e 100644
--- a/python_on_whales/client_config.py
+++ b/python_on_whales/client_config.py
@@ -8,7 +8,10 @@
 
 import pydantic
 
-from python_on_whales.download_binaries import DOCKER_BINARY_PATH, download_docker_cli
+from python_on_whales.download_binaries import (
+    download_docker_cli,
+    get_docker_binary_path,
+)
 from python_on_whales.utils import to_list
 
 from .utils import ValidPath, run
@@ -58,8 +61,8 @@ def get_docker_path(self) -> ValidPath:
             docker_sys = shutil.which("docker")
             if docker_sys is not None:
                 self.client_binary_path = docker_sys
-            elif DOCKER_BINARY_PATH.exists():
-                self.client_binary_path = DOCKER_BINARY_PATH
+            elif get_docker_binary_path().exists():
+                self.client_binary_path = get_docker_binary_path()
             else:
                 warnings.warn(
                     "The docker client binary file was not found on your system. \n"
@@ -76,7 +79,7 @@ def get_docker_path(self) -> ValidPath:
                     "$ python-on-whales download-cli \n"
                 )
                 download_docker_cli()
-                self.client_binary_path = DOCKER_BINARY_PATH
+                self.client_binary_path = get_docker_binary_path()
         return self.client_binary_path
 
     @property
diff --git a/python_on_whales/download_binaries.py b/python_on_whales/download_binaries.py
index 92382275..8f121144 100644
--- a/python_on_whales/download_binaries.py
+++ b/python_on_whales/download_binaries.py
@@ -18,7 +18,8 @@
 WINDOWS_CLI_URL = "https://github.com/StefanScherer/docker-cli-builder/releases/download/{version}/docker.exe"
 
 
-DOCKER_BINARY_PATH = CACHE_DIR / "docker-cli" / DOCKER_VERSION / "docker"
+def get_docker_binary_path():
+    return CACHE_DIR / "docker-cli" / DOCKER_VERSION / "docker"
 
 
 def get_docker_cli_url():
@@ -31,19 +32,26 @@ def get_docker_cli_url():
 
 def download_docker_cli():
     file_to_download = get_docker_cli_url()
+
+    extension = file_to_download.split(".")[-1]
     with tempfile.TemporaryDirectory() as tmp_dir:
         tmp_dir = Path(tmp_dir)
-        tar_file = tmp_dir / "docker.tgz"
-        download_from_url(file_to_download, tar_file)
-        extract_dir = tmp_dir / "extracted"
-        shutil.unpack_archive(str(tar_file), str(extract_dir))
+        downloaded_file_path = tmp_dir / f"docker.{extension}"
+        download_from_url(file_to_download, downloaded_file_path)
+
+        docker_binary_path = get_docker_binary_path()
+        docker_binary_path.parent.mkdir(exist_ok=True, parents=True)
 
-        DOCKER_BINARY_PATH.parent.mkdir(exist_ok=True, parents=True)
-        shutil.move(extract_dir / "docker" / "docker", DOCKER_BINARY_PATH)
+        if extension == "tgz":
+            extract_dir = tmp_dir / "extracted"
+            shutil.unpack_archive(str(downloaded_file_path), str(extract_dir))
+            shutil.move(extract_dir / "docker" / "docker", docker_binary_path)
+        elif extension == "exe":
+            shutil.move(downloaded_file_path, docker_binary_path)
 
     warnings.warn(
         f"The docker client binary file {DOCKER_VERSION} was downloaded and put "
-        f"in `{DOCKER_BINARY_PATH.absolute()}`. \n"
+        f"in `{docker_binary_path.absolute()}`. \n"
         f"You can feel free to remove it if you wish, Python on whales will download "
         f"it again if needed."
     )


Test patch:
diff --git a/tests/python_on_whales/test_download_binaries.py b/tests/python_on_whales/test_download_binaries.py
index 8623c146..ea1fd081 100644
--- a/tests/python_on_whales/test_download_binaries.py
+++ b/tests/python_on_whales/test_download_binaries.py
@@ -1,3 +1,5 @@
+import platform
+
 from python_on_whales import download_binaries
 from python_on_whales.utils import run
 
@@ -8,14 +10,12 @@ def test_download_from_url(tmp_path):
     assert (tmp_path / "dodo.png").exists()
 
 
-def test_download_cli():
-    try:
-        download_binaries.DOCKER_BINARY_PATH.unlink()
-    except FileNotFoundError:
-        pass
+def test_download_cli(mocker, tmp_path):
+    mocker.patch.object(download_binaries, "CACHE_DIR", tmp_path)
+
     download_binaries.download_docker_cli()
     simple_command = [
-        download_binaries.DOCKER_BINARY_PATH,
+        download_binaries.get_docker_binary_path(),
         "run",
         "--rm",
         "hello-world",
@@ -26,15 +26,27 @@ def test_download_cli():
 
 def test_download_cli_from_cli():
     try:
-        download_binaries.DOCKER_BINARY_PATH.unlink()
+        download_binaries.get_docker_binary_path().unlink()
     except FileNotFoundError:
         pass
     run(["python-on-whales", "download-cli"])
     simple_command = [
-        download_binaries.DOCKER_BINARY_PATH,
+        download_binaries.get_docker_binary_path(),
         "run",
         "--rm",
         "hello-world",
     ]
     output = run(simple_command, capture_stdout=True, capture_stderr=True)
     assert "Hello from Docker!" in output
+
+
+def test_download_windows_binaries(mocker, tmp_path):
+    mocker.patch.object(platform, "system", lambda: "Windows")
+    mocker.patch.object(download_binaries, "CACHE_DIR", tmp_path)
+
+    assert not download_binaries.get_docker_binary_path().exists()
+    assert download_binaries.get_user_os() == "windows"
+
+    download_binaries.download_docker_cli()
+
+    assert download_binaries.get_docker_binary_path().exists()