From d84f8f9b4b1b9bc6aa461fb118fbfe41917b32de Mon Sep 17 00:00:00 2001 From: Aditya Jha Date: Thu, 4 Dec 2025 11:52:38 +0530 Subject: [PATCH 1/5] optimize management api calls --- singlestoredb/management/files.py | 69 +++++++++++++++++++++---------- 1 file changed, 48 insertions(+), 21 deletions(-) diff --git a/singlestoredb/management/files.py b/singlestoredb/management/files.py index ac3561ad7..91a2455a8 100644 --- a/singlestoredb/management/files.py +++ b/singlestoredb/management/files.py @@ -908,9 +908,9 @@ 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 ---------- @@ -918,28 +918,35 @@ def _listdir(self, path: PathLike, *, recursive: bool = False) -> List[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 [])] def listdir( self, path: PathLike = '/', *, recursive: bool = False, - ) -> List[str]: + return_meta: bool = False, + ) -> List[Union[str, Dict[str, Any]]]: """ List the files / folders at the given path. @@ -948,21 +955,32 @@ 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): + # Single validation GET (info) rather than is_dir + info later + info = self.info(path) + if info.type != 'directory': raise NotADirectoryError(f'path is not a directory: {path}') - out = self._listdir(path, recursive=recursive) + out = self._listdir(path, recursive=recursive, return_meta=return_meta) if path != '/': path_n = len(path.split('/')) - 1 - out = ['/'.join(x.split('/')[path_n:]) for x in out] + if return_meta: + for i in range(len(out)): + if isinstance(out[i], dict): + rel = '/'.join(out[i]['path'].split('/')[path_n:]) + out[i]['path'] = rel + else: + out = ['/'.join(str(x).split('/')[path_n:]) for x in out] return out def download_file( @@ -1036,17 +1054,26 @@ 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): + # Validate directory with single info call + info = self.info(path) + if info.type != 'directory': 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): + 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 + 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 - 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) + 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) def remove(self, path: PathLike) -> None: """ From 70870ffa785b496dcff5eb58f3ee07ed23c6e675 Mon Sep 17 00:00:00 2001 From: Aditya Jha Date: Thu, 4 Dec 2025 14:51:15 +0530 Subject: [PATCH 2/5] remove duplicate calls --- singlestoredb/management/files.py | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/singlestoredb/management/files.py b/singlestoredb/management/files.py index 91a2455a8..1556fc50e 100644 --- a/singlestoredb/management/files.py +++ b/singlestoredb/management/files.py @@ -966,12 +966,12 @@ def listdir( path = re.sub(r'^(\./|/)+', r'', str(path)) path = re.sub(r'/+$', r'', path) + '/' - # Single validation GET (info) rather than is_dir + info later - info = self.info(path) - if info.type != 'directory': - raise NotADirectoryError(f'path is not a directory: {path}') - - out = self._listdir(path, recursive=recursive, return_meta=return_meta) + # 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 if path != '/': path_n = len(path.split('/')) - 1 if return_meta: @@ -990,6 +990,7 @@ def download_file( *, overwrite: bool = False, encoding: Optional[str] = None, + _skip_dir_check: bool = False, ) -> Optional[Union[bytes, str]]: """ Download the content of a file path. @@ -1013,7 +1014,7 @@ def download_file( """ 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( @@ -1054,11 +1055,7 @@ 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') - # Validate directory with single info call - info = self.info(path) - if info.type != 'directory': - raise NotADirectoryError(f'path is not a directory: {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 @@ -1073,7 +1070,7 @@ def download_folder( 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) + self.download_file(remote_path, target_file, overwrite=overwrite, _skip_dir_check=True) def remove(self, path: PathLike) -> None: """ From 618a7b0af5d3aba2ff75c7f3ce997ebc6036a164 Mon Sep 17 00:00:00 2001 From: Aditya Jha Date: Thu, 18 Dec 2025 11:45:19 +0530 Subject: [PATCH 3/5] review comment --- singlestoredb/management/files.py | 42 +++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/singlestoredb/management/files.py b/singlestoredb/management/files.py index 1556fc50e..e62851fde 100644 --- a/singlestoredb/management/files.py +++ b/singlestoredb/management/files.py @@ -990,7 +990,6 @@ def download_file( *, overwrite: bool = False, encoding: Optional[str] = None, - _skip_dir_check: bool = False, ) -> Optional[Union[bytes, str]]: """ Download the content of a file path. @@ -1011,6 +1010,45 @@ 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') @@ -1070,7 +1108,7 @@ def download_folder( 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) + self._download_file(remote_path, target_file, overwrite=overwrite, _skip_dir_check=True) def remove(self, path: PathLike) -> None: """ From 18a1f4c27cb26aac572bd7c7efd13110f28de696 Mon Sep 17 00:00:00 2001 From: Aditya Jha Date: Thu, 18 Dec 2025 17:50:52 +0530 Subject: [PATCH 4/5] fix ci --- singlestoredb/management/files.py | 80 +++++++++++++++++++++++++------ 1 file changed, 65 insertions(+), 15 deletions(-) diff --git a/singlestoredb/management/files.py b/singlestoredb/management/files.py index e62851fde..790ac4570 100644 --- a/singlestoredb/management/files.py +++ b/singlestoredb/management/files.py @@ -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]]]]: pass @abstractmethod @@ -908,7 +911,11 @@ def is_file(self, path: PathLike) -> bool: return False raise - def _listdir(self, path: PathLike, *, recursive: bool = False, return_meta: bool = False) -> List[Union[str, Dict[str, Any]]]: + def _listdir( + self, path: PathLike, *, + recursive: bool = False, + return_meta: bool = False, + ) -> List[Union[str, Dict[str, Any]]]: """ Return the names (or metadata) of files in a directory. @@ -929,24 +936,55 @@ def _listdir(self, path: PathLike, *, recursive: bool = False, return_meta: bool 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']}) + 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, return_meta=return_meta)) + out.extend( + self._listdir( + item['path'], + recursive=recursive, + return_meta=return_meta, + ), + ) return out if return_meta: - return [{'path': x['path'], 'type': x['type']} for x in (res.get('content') or [])] + 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, - ) -> List[Union[str, Dict[str, Any]]]: + ) -> Union[List[str], List[Dict[str, Any]]]: """ List the files / folders at the given path. @@ -972,16 +1010,23 @@ def listdir( 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 + if path != '/': path_n = len(path.split('/')) - 1 if return_meta: - for i in range(len(out)): - if isinstance(out[i], dict): - rel = '/'.join(out[i]['path'].split('/')[path_n:]) - out[i]['path'] = rel - else: - out = ['/'.join(str(x).split('/')[path_n:]) for x in out] - return out + 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, @@ -1106,9 +1151,14 @@ def download_folder( 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)) + 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) + self._download_file( + remote_path, target_file, + overwrite=overwrite, _skip_dir_check=True, + ) def remove(self, path: PathLike) -> None: """ From a2af567983c8830c20d4fde6393fb6bcf9f5cd6f Mon Sep 17 00:00:00 2001 From: Aditya Jha Date: Thu, 18 Dec 2025 17:55:45 +0530 Subject: [PATCH 5/5] fix ci mypy --- singlestoredb/fusion/handlers/files.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/singlestoredb/fusion/handlers/files.py b/singlestoredb/fusion/handlers/files.py index d823aeaec..b657b8a42 100644 --- a/singlestoredb/fusion/handlers/files.py +++ b/singlestoredb/fusion/handlers/files.py @@ -32,6 +32,7 @@ def run(self, params: Dict[str, Any]) -> Optional[FusionSQLResult]: for x in file_space.listdir( params['at_path'] or '/', recursive=params['recursive'], + return_meta=False, ): info = file_space.info(x) files.append( @@ -47,6 +48,7 @@ def run(self, params: Dict[str, Any]) -> Optional[FusionSQLResult]: res.set_rows([(x,) for x in file_space.listdir( params['at_path'] or '/', recursive=params['recursive'], + return_meta=False, )]) if params['like']: