New file progress (#933)

CHANGELOG.D/933.feature

@@ -0,0 +1 @@
+Show progress for `neuro cp` by default.

README.md

@@ -582,7 +582,7 @@ Name | Description|
 |_\--glob / --no-glob_|Expand glob patterns in SOURCES with explicit scheme  \[default: True]|
 |_\-t, --target-directory DIRECTORY_|Copy all SOURCES into DIRECTORY|
 |_\-T, --no-target-directory_|Treat DESTINATION as a normal file|
-|_\-p, --progress_|Show progress, off by default|
+|_\-p, --progress / -P, --no-progress_|Show progress, on by default|
 |_--help_|Show this message and exit.|
 
 
@@ -1647,7 +1647,7 @@ Name | Description|
 |_\--glob / --no-glob_|Expand glob patterns in SOURCES with explicit scheme  \[default: True]|
 |_\-t, --target-directory DIRECTORY_|Copy all SOURCES into DIRECTORY|
 |_\-T, --no-target-directory_|Treat DESTINATION as a normal file|
-|_\-p, --progress_|Show progress, off by default|
+|_\-p, --progress / -P, --no-progress_|Show progress, on by default|
 |_--help_|Show this message and exit.|
 
 

neuromation/api/__init__.py

@@ -6,10 +6,12 @@
 
 from .abc import (
     AbstractDockerImageProgress,
-    AbstractStorageProgress,
+    AbstractFileProgress,
+    AbstractRecursiveFileProgress,
     StorageProgressComplete,
+    StorageProgressEnterDir,
     StorageProgressFail,
-    StorageProgressMkdir,
+    StorageProgressLeaveDir,
     StorageProgressStart,
     StorageProgressStep,
 )
@@ -70,13 +72,15 @@
     "AuthError",
     "AuthenticationError",
     "AuthorizationError",
-    "AbstractStorageProgress",
+    "AbstractFileProgress",
+    "AbstractRecursiveFileProgress",
     "AbstractDockerImageProgress",
     "StorageProgressStart",
     "StorageProgressComplete",
     "StorageProgressStep",
-    "StorageProgressMkdir",
     "StorageProgressFail",
+    "StorageProgressEnterDir",
+    "StorageProgressLeaveDir",
     "RemoteImage",
     "LocalImage",
     "Factory",

neuromation/api/abc.py

@@ -27,7 +27,13 @@ class StorageProgressStep:
 
 
 @dataclass
-class StorageProgressMkdir:
+class StorageProgressEnterDir:
+    src: URL
+    dst: URL
+
+
+@dataclass
+class StorageProgressLeaveDir:
     src: URL
     dst: URL
 
@@ -39,7 +45,7 @@ class StorageProgressFail:
     message: str
 
 
-class AbstractStorageProgress(abc.ABC):
+class AbstractFileProgress(abc.ABC):
     # design note:
     # dataclasses used instead of direct passing parameters
     # because a dataclass is forward-compatible
@@ -58,8 +64,14 @@ def complete(self, data: StorageProgressComplete) -> None:
     def step(self, data: StorageProgressStep) -> None:
         pass  # pragma: no cover
 
+
+class AbstractRecursiveFileProgress(AbstractFileProgress):
+    @abc.abstractmethod
+    def enter(self, data: StorageProgressEnterDir) -> None:
+        pass  # pragma: no cover
+
     @abc.abstractmethod
-    def mkdir(self, data: StorageProgressMkdir) -> None:
+    def leave(self, data: StorageProgressLeaveDir) -> None:
         pass  # pragma: no cover
 
     @abc.abstractmethod

neuromation/api/storage.py

@@ -12,10 +12,12 @@
 from yarl import URL
 
 from .abc import (
-    AbstractStorageProgress,
+    AbstractFileProgress,
+    AbstractRecursiveFileProgress,
     StorageProgressComplete,
+    StorageProgressEnterDir,
     StorageProgressFail,
-    StorageProgressMkdir,
+    StorageProgressLeaveDir,
     StorageProgressStart,
     StorageProgressStep,
 )
@@ -241,7 +243,7 @@ async def mv(self, src: URL, dst: URL) -> None:
     # high-level helpers
 
     async def _iterate_file(
-        self, src: Path, dst: URL, *, progress: AbstractStorageProgress
+        self, src: Path, dst: URL, *, progress: AbstractFileProgress
     ) -> AsyncIterator[bytes]:
         loop = asyncio.get_event_loop()
         src_url = URL(src.as_uri())
@@ -258,7 +260,7 @@ async def _iterate_file(
             progress.complete(StorageProgressComplete(src_url, dst, size))
 
     async def upload_file(
-        self, src: URL, dst: URL, *, progress: Optional[AbstractStorageProgress] = None
+        self, src: URL, dst: URL, *, progress: Optional[AbstractFileProgress] = None
     ) -> None:
         if progress is None:
             progress = _DummyProgress()
@@ -297,7 +299,11 @@ async def upload_file(
         await self.create(dst, self._iterate_file(path, dst, progress=progress))
 
     async def upload_dir(
-        self, src: URL, dst: URL, *, progress: Optional[AbstractStorageProgress] = None
+        self,
+        src: URL,
+        dst: URL,
+        *,
+        progress: Optional[AbstractRecursiveFileProgress] = None,
     ) -> None:
         if progress is None:
             progress = _DummyProgress()
@@ -314,8 +320,9 @@ async def upload_dir(
                 raise NotADirectoryError(errno.ENOTDIR, "Not a directory", str(dst))
         except ResourceNotFound:
             await self.mkdirs(dst)
-        progress.mkdir(StorageProgressMkdir(src, dst))
-        for child in path.iterdir():
+        progress.enter(StorageProgressEnterDir(src, dst))
+        folder = sorted(path.iterdir(), key=lambda item: (item.is_dir(), item.name))
+        for child in folder:
             if child.is_file():
                 await self.upload_file(
                     src / child.name, dst / child.name, progress=progress
@@ -335,9 +342,10 @@ async def upload_dir(
                         f"Cannot upload {child}, not regular file/directory",
                     )
                 )  # pragma: no cover
+        progress.leave(StorageProgressLeaveDir(src, dst))
 
     async def download_file(
-        self, src: URL, dst: URL, *, progress: Optional[AbstractStorageProgress] = None
+        self, src: URL, dst: URL, *, progress: Optional[AbstractFileProgress] = None
     ) -> None:
         if progress is None:
             progress = _DummyProgress()
@@ -359,16 +367,21 @@ async def download_file(
             progress.complete(StorageProgressComplete(src, dst, size))
 
     async def download_dir(
-        self, src: URL, dst: URL, *, progress: Optional[AbstractStorageProgress] = None
+        self,
+        src: URL,
+        dst: URL,
+        *,
+        progress: Optional[AbstractRecursiveFileProgress] = None,
     ) -> None:
         if progress is None:
             progress = _DummyProgress()
         src = normalize_storage_path_uri(src, self._config.auth_token.username)
         dst = normalize_local_path_uri(dst)
         path = _extract_path(dst)
         path.mkdir(parents=True, exist_ok=True)
-        progress.mkdir(StorageProgressMkdir(src, dst))
-        for child in await self.ls(src):
+        progress.enter(StorageProgressEnterDir(src, dst))
+        folder = sorted(await self.ls(src), key=lambda item: (item.is_dir(), item.name))
+        for child in folder:
             if child.is_file():
                 await self.download_file(
                     src / child.name, dst / child.name, progress=progress
@@ -385,6 +398,7 @@ async def download_dir(
                         f"Cannot download {child}, not regular file/directory",
                     )
                 )  # pragma: no cover
+        progress.leave(StorageProgressLeaveDir(src, dst))
 
 
 _magic_check = re.compile("(?:[*?[])")
@@ -412,7 +426,7 @@ def _file_status_from_api(values: Dict[str, Any]) -> FileStatus:
     )
 
 
-class _DummyProgress(AbstractStorageProgress):
+class _DummyProgress(AbstractRecursiveFileProgress):
     def start(self, data: StorageProgressStart) -> None:
         pass
 
@@ -422,7 +436,10 @@ def complete(self, data: StorageProgressComplete) -> None:
     def step(self, data: StorageProgressStep) -> None:
         pass
 
-    def mkdir(self, data: StorageProgressMkdir) -> None:
+    def enter(self, data: StorageProgressEnterDir) -> None:
+        pass
+
+    def leave(self, data: StorageProgressLeaveDir) -> None:
         pass
 
     def fail(self, data: StorageProgressFail) -> None:

neuromation/cli/formatters/images.py

@@ -60,8 +60,8 @@ def progress(self, message: str, layer_id: str) -> None:
                 lineno = self._mapping[layer_id]
                 self._printer.print(message, lineno)
             else:
-                self._printer.print(message)
                 self._mapping[layer_id] = self._printer.total_lines
+                self._printer.print(message)
         else:
             self._printer.print(message)
 

neuromation/cli/formatters/jobs.py

@@ -1,10 +1,10 @@
 import abc
 import datetime
 import itertools
+import sys
 import time
 from dataclasses import dataclass
 from math import floor
-from sys import platform
 from typing import Iterable, Iterator, List, Mapping
 
 import humanize
@@ -327,7 +327,7 @@ def __init__(self, color: bool):
         self._time = time.time()
         self._color = color
         self._prev = ""
-        if platform == "win32":
+        if sys.platform == "win32":
             self._spinner = itertools.cycle("-\\|/")
         else:
             self._spinner = itertools.cycle("◢◣◤◥")
@@ -351,8 +351,8 @@ def __call__(self, job: JobDescription) -> None:
             if self._prev:
                 self._printer.print(self._prev, lineno=self._lineno)
             self._prev = msg
-            self._printer.print(msg)
             self._lineno = self._printer.total_lines
+            self._printer.print(msg)
         else:
             self._printer.print(
                 f"{msg} {next(self._spinner)} [{dt:.1f} sec]", lineno=self._lineno