-
Notifications
You must be signed in to change notification settings - Fork 22
optimize management api calls #110
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
base: main
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -10,9 +10,12 @@ | |
| from abc import ABC | ||
| from abc import abstractmethod | ||
| from typing import Any | ||
| from typing import cast | ||
| from typing import Dict | ||
| from typing import List | ||
| from typing import Literal | ||
| from typing import Optional | ||
| from typing import overload | ||
| from typing import Union | ||
|
|
||
| from .. import config | ||
|
|
@@ -427,7 +430,7 @@ def listdir( | |
| path: PathLike = '/', | ||
| *, | ||
| recursive: bool = False, | ||
| ) -> List[str]: | ||
| ) -> Union[List[str], List[Union[str, Dict[str, Any]]]]: | ||
|
Collaborator
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. I think it would be better to return |
||
| pass | ||
|
|
||
| @abstractmethod | ||
|
|
@@ -908,38 +911,80 @@ def is_file(self, path: PathLike) -> bool: | |
| return False | ||
| raise | ||
|
|
||
| def _listdir(self, path: PathLike, *, recursive: bool = False) -> List[str]: | ||
| def _listdir( | ||
| self, path: PathLike, *, | ||
| recursive: bool = False, | ||
| return_meta: bool = False, | ||
| ) -> List[Union[str, Dict[str, Any]]]: | ||
| """ | ||
| Return the names of files in a directory. | ||
| Return the names (or metadata) of files in a directory. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| path : Path or str | ||
| Path to the folder | ||
| recursive : bool, optional | ||
| Should folders be listed recursively? | ||
|
|
||
| return_meta : bool, optional | ||
| If True, return list of dicts with 'path' and 'type'. Otherwise just paths. | ||
| """ | ||
| res = self._manager._get( | ||
| f'files/fs/{self._location}/{path}', | ||
| ).json() | ||
|
|
||
| if recursive: | ||
| out = [] | ||
| for item in res['content'] or []: | ||
| out.append(item['path']) | ||
| out: List[Union[str, Dict[str, Any]]] = [] | ||
| for item in res.get('content') or []: | ||
| if return_meta: | ||
| out.append( | ||
| {'path': item['path'], 'type': item['type']}, | ||
| ) | ||
| else: | ||
| out.append(item['path']) | ||
| if item['type'] == 'directory': | ||
| out.extend(self._listdir(item['path'], recursive=recursive)) | ||
| out.extend( | ||
| self._listdir( | ||
| item['path'], | ||
| recursive=recursive, | ||
| return_meta=return_meta, | ||
| ), | ||
| ) | ||
| return out | ||
|
|
||
| return [x['path'] for x in res['content'] or []] | ||
| if return_meta: | ||
| return [ | ||
| {'path': x['path'], 'type': x['type']} | ||
| for x in (res.get('content') or []) | ||
| ] | ||
| return [x['path'] for x in (res.get('content') or [])] | ||
|
|
||
| @overload | ||
| def listdir( | ||
| self, | ||
| path: PathLike = '/', | ||
| *, | ||
| recursive: bool = False, | ||
| return_meta: Literal[True] = ..., | ||
| ) -> List[Dict[str, Any]]: | ||
| ... | ||
|
|
||
| @overload | ||
| def listdir( | ||
| self, | ||
| path: PathLike = '/', | ||
| *, | ||
| recursive: bool = False, | ||
| return_meta: Literal[False] = ..., | ||
| ) -> List[str]: | ||
| ... | ||
|
|
||
| def listdir( | ||
| self, | ||
| path: PathLike = '/', | ||
| *, | ||
| recursive: bool = False, | ||
| return_meta: bool = False, | ||
| ) -> Union[List[str], List[Dict[str, Any]]]: | ||
| """ | ||
| List the files / folders at the given path. | ||
|
|
||
|
|
@@ -948,22 +993,40 @@ def listdir( | |
| path : Path or str, optional | ||
| Path to the file location | ||
|
|
||
| return_meta : bool, optional | ||
| If True, return list of dicts with 'path' and 'type'. Otherwise just paths. | ||
|
|
||
| Returns | ||
| ------- | ||
| List[str] | ||
| List[str] or List[dict] | ||
|
|
||
| """ | ||
| path = re.sub(r'^(\./|/)+', r'', str(path)) | ||
| path = re.sub(r'/+$', r'', path) + '/' | ||
|
|
||
| if not self.is_dir(path): | ||
| raise NotADirectoryError(f'path is not a directory: {path}') | ||
| # Validate via listing GET; if response lacks 'content', it's not a directory | ||
| try: | ||
| out = self._listdir(path, recursive=recursive, return_meta=return_meta) | ||
| except Exception as exc: | ||
| # If the path doesn't exist or isn't a directory, _listdir will fail | ||
| raise NotADirectoryError(f'path is not a directory: {path}') from exc | ||
|
|
||
| out = self._listdir(path, recursive=recursive) | ||
| if path != '/': | ||
| path_n = len(path.split('/')) - 1 | ||
| out = ['/'.join(x.split('/')[path_n:]) for x in out] | ||
| return out | ||
| if return_meta: | ||
| result: List[Dict[str, Any]] = [] | ||
| for item in out: | ||
| if isinstance(item, dict): | ||
| rel = '/'.join(item['path'].split('/')[path_n:]) | ||
| item['path'] = rel | ||
| result.append(item) | ||
| return result | ||
| return ['/'.join(str(x).split('/')[path_n:]) for x in out] | ||
|
|
||
| # _listdir guarantees homogeneous type based on return_meta | ||
| if return_meta: | ||
| return cast(List[Dict[str, Any]], out) | ||
| return cast(List[str], out) | ||
|
|
||
| def download_file( | ||
| self, | ||
|
|
@@ -992,10 +1055,49 @@ def download_file( | |
| bytes or str - ``local_path`` is None | ||
| None - ``local_path`` is a Path or str | ||
|
|
||
| """ | ||
| return self._download_file( | ||
| path, | ||
| local_path=local_path, | ||
| overwrite=overwrite, | ||
| encoding=encoding, | ||
| _skip_dir_check=False, | ||
| ) | ||
|
|
||
| def _download_file( | ||
| self, | ||
| path: PathLike, | ||
| local_path: Optional[PathLike] = None, | ||
| *, | ||
| overwrite: bool = False, | ||
| encoding: Optional[str] = None, | ||
| _skip_dir_check: bool = False, | ||
| ) -> Optional[Union[bytes, str]]: | ||
| """ | ||
| Internal method to download the content of a file path. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| path : Path or str | ||
| Path to the file | ||
| local_path : Path or str | ||
| Path to local file target location | ||
| overwrite : bool, optional | ||
| Should an existing file be overwritten if it exists? | ||
| encoding : str, optional | ||
| Encoding used to convert the resulting data | ||
| _skip_dir_check : bool, optional | ||
| Skip the directory check (internal use only) | ||
|
|
||
| Returns | ||
| ------- | ||
| bytes or str - ``local_path`` is None | ||
| None - ``local_path`` is a Path or str | ||
|
|
||
| """ | ||
| if local_path is not None and not overwrite and os.path.exists(local_path): | ||
| raise OSError('target file already exists; use overwrite=True to replace') | ||
| if self.is_dir(path): | ||
| if not _skip_dir_check and self.is_dir(path): | ||
| raise IsADirectoryError(f'file path is a directory: {path}') | ||
|
|
||
| out = self._manager._get( | ||
|
|
@@ -1036,17 +1138,27 @@ def download_folder( | |
| if local_path is not None and not overwrite and os.path.exists(local_path): | ||
| raise OSError('target path already exists; use overwrite=True to replace') | ||
|
|
||
| if not self.is_dir(path): | ||
| raise NotADirectoryError(f'path is not a directory: {path}') | ||
|
|
||
| files = self.listdir(path, recursive=True) | ||
| for f in files: | ||
| remote_path = os.path.join(path, f) | ||
| if self.is_dir(remote_path): | ||
| # listdir validates directory; no extra info call needed | ||
| entries = self.listdir(path, recursive=True, return_meta=True) | ||
| for entry in entries: | ||
| # Each entry is a dict with path relative to root and type | ||
| if not isinstance(entry, dict): # defensive: skip unexpected | ||
| continue | ||
| target = os.path.normpath(os.path.join(local_path, f)) | ||
| os.makedirs(os.path.dirname(target), exist_ok=True) | ||
| self.download_file(remote_path, target, overwrite=overwrite) | ||
| rel_path = entry['path'] | ||
| if entry['type'] == 'directory': | ||
| # Ensure local directory exists; no remote call needed | ||
| target_dir = os.path.normpath(os.path.join(local_path, rel_path)) | ||
| os.makedirs(target_dir, exist_ok=True) | ||
| continue | ||
| remote_path = os.path.join(path, rel_path) | ||
| target_file = os.path.normpath( | ||
| os.path.join(local_path, rel_path), | ||
| ) | ||
| os.makedirs(os.path.dirname(target_file), exist_ok=True) | ||
| self._download_file( | ||
| remote_path, target_file, | ||
| overwrite=overwrite, _skip_dir_check=True, | ||
| ) | ||
|
|
||
| def remove(self, path: PathLike) -> None: | ||
| """ | ||
|
|
||
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.
Don't you need a
return_metaparameter here too?