From e765e66925505f5dfb6c1348079cfdbd7dea7e4f Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Fri, 19 Apr 2024 13:29:02 +0200 Subject: [PATCH 1/5] Handle PyFilesystem2 errors more gracefully --- lib/galaxy/files/sources/_pyfilesystem2.py | 34 +++++++++++++--------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/lib/galaxy/files/sources/_pyfilesystem2.py b/lib/galaxy/files/sources/_pyfilesystem2.py index 3336f72a2165..e604a6c8f75f 100644 --- a/lib/galaxy/files/sources/_pyfilesystem2.py +++ b/lib/galaxy/files/sources/_pyfilesystem2.py @@ -12,9 +12,11 @@ ) import fs +import fs.errors from fs.base import FS from typing_extensions import Unpack +from galaxy.exceptions import MessageException from . import ( BaseFilesSource, FilesSourceOptions, @@ -42,19 +44,25 @@ def _open_fs(self, user_context=None, opts: Optional[FilesSourceOptions] = None) def _list(self, path="/", recursive=False, user_context=None, opts: Optional[FilesSourceOptions] = None): """Return dictionary of 'Directory's and 'File's.""" - - with self._open_fs(user_context=user_context, opts=opts) as h: - if recursive: - res: List[Dict[str, Any]] = [] - for p, dirs, files in h.walk(path): - to_dict = functools.partial(self._resource_info_to_dict, p) - res.extend(map(to_dict, dirs)) - res.extend(map(to_dict, files)) - return res - else: - res = h.scandir(path, namespaces=["details"]) - to_dict = functools.partial(self._resource_info_to_dict, path) - return list(map(to_dict, res)) + try: + with self._open_fs(user_context=user_context, opts=opts) as h: + if recursive: + res: List[Dict[str, Any]] = [] + for p, dirs, files in h.walk(path): + to_dict = functools.partial(self._resource_info_to_dict, p) + res.extend(map(to_dict, dirs)) + res.extend(map(to_dict, files)) + return res + else: + res = h.scandir(path, namespaces=["details"]) + to_dict = functools.partial(self._resource_info_to_dict, path) + return list(map(to_dict, res)) + except fs.errors.PermissionDenied as e: + raise MessageException( + f"Permission Denied. Reason: {e}. Please check your credentials in your preferences for {self.label}." + ) + except fs.errors.FSError as e: + raise MessageException(f"Problem listing file source path {path}. Reason: {e}") from e def _realize_to(self, source_path, native_path, user_context=None, opts: Optional[FilesSourceOptions] = None): with open(native_path, "wb") as write_file: From 0bbb982cae3af878c3ff52673dfb7b1c8588d433 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Fri, 19 Apr 2024 13:31:30 +0200 Subject: [PATCH 2/5] Handle errors creating new entries more gracefully --- lib/galaxy/files/sources/_rdm.py | 4 ++-- lib/galaxy/managers/remote_files.py | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/files/sources/_rdm.py b/lib/galaxy/files/sources/_rdm.py index 14f7e9e1daa0..7d9a97d2fb5f 100644 --- a/lib/galaxy/files/sources/_rdm.py +++ b/lib/galaxy/files/sources/_rdm.py @@ -7,7 +7,7 @@ from typing_extensions import Unpack -from galaxy.exceptions import AuthenticationRequired +from galaxy.exceptions import MessageException from galaxy.files import ProvidesUserFileSourcesUserContext from galaxy.files.sources import ( BaseFilesSource, @@ -199,7 +199,7 @@ def get_authorization_token(self, user_context: OptionalUserContext) -> str: effective_props = self._serialization_props(user_context) token = effective_props.get("token") if not token: - raise AuthenticationRequired( + raise MessageException( f"Please provide a personal access token in your user's preferences for '{self.label}'" ) return token diff --git a/lib/galaxy/managers/remote_files.py b/lib/galaxy/managers/remote_files.py index 06980dac8580..985dd4980dbd 100644 --- a/lib/galaxy/managers/remote_files.py +++ b/lib/galaxy/managers/remote_files.py @@ -162,6 +162,9 @@ def create_entry(self, user_ctx: ProvidesUserContext, entry_data: CreateEntryPay file_source = file_source_path.file_source try: result = file_source.create_entry(entry_data.dict(), user_context=user_file_source_context) + except exceptions.MessageException: + log.warning(f"Problem creating entry {entry_data.name} in file source {entry_data.target}", exc_info=True) + raise except Exception: message = f"Problem creating entry {entry_data.name} in file source {entry_data.target}" log.warning(message, exc_info=True) From 91bf0c69aa8440f7df932320727581512160d2ea Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Fri, 19 Apr 2024 13:32:24 +0200 Subject: [PATCH 3/5] Handle Dropbox special case --- lib/galaxy/files/sources/dropbox.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/files/sources/dropbox.py b/lib/galaxy/files/sources/dropbox.py index 880834411d5a..865338da2bbd 100644 --- a/lib/galaxy/files/sources/dropbox.py +++ b/lib/galaxy/files/sources/dropbox.py @@ -8,6 +8,7 @@ Union, ) +from galaxy.exceptions import MessageException from . import ( FilesSourceOptions, FilesSourceProperties, @@ -27,8 +28,17 @@ def _open_fs(self, user_context=None, opts: Optional[FilesSourceOptions] = None) if "accessToken" in props: props["access_token"] = props.pop("accessToken") - handle = DropboxFS(**{**props, **extra_props}) - return handle + try: + handle = DropboxFS(**{**props, **extra_props}) + return handle + except Exception as e: + # This plugin might raise dropbox.dropbox_client.BadInputException + # which is not a subclass of fs.errors.FSError + if "OAuth2" in str(e): + raise MessageException( + f"Permission Denied. Reason: {e}. Please check your credentials in your preferences for {self.label}." + ) + raise MessageException(f"Error connecting to Dropbox. Reason: {e}") __all__ = ("DropboxFilesSource",) From 53f5b5fd1f1e2a059413cbb16206731f189d014e Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Fri, 19 Apr 2024 13:44:08 +0200 Subject: [PATCH 4/5] Use less cryptic error when unexpected error --- lib/galaxy/managers/remote_files.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/managers/remote_files.py b/lib/galaxy/managers/remote_files.py index 985dd4980dbd..2be2274dbc70 100644 --- a/lib/galaxy/managers/remote_files.py +++ b/lib/galaxy/managers/remote_files.py @@ -9,6 +9,7 @@ from galaxy import exceptions from galaxy.files import ( ConfiguredFileSources, + FileSourcePath, ProvidesUserFileSourcesUserContext, ) from galaxy.files.sources import ( @@ -94,10 +95,10 @@ def index( opts=opts, ) except exceptions.MessageException: - log.warning(f"Problem listing file source path {file_source_path}", exc_info=True) + log.warning(self._get_error_message(file_source_path), exc_info=True) raise except Exception: - message = f"Problem listing file source path {file_source_path}" + message = self._get_error_message(file_source_path) log.warning(message, exc_info=True) raise exceptions.InternalServerError(message) if format == RemoteFilesFormat.flat: @@ -131,6 +132,9 @@ def index( return index + def _get_error_message(self, file_source_path: FileSourcePath) -> str: + return f"Problem listing file source path {file_source_path.file_source.get_uri_root()}{file_source_path.path}" + def get_files_source_plugins( self, user_context: ProvidesUserContext, From 0d1c41f0cc27ff4338e71283563d35e4cff16c54 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Fri, 19 Apr 2024 16:36:51 +0200 Subject: [PATCH 5/5] Prefer AuthenticationRequired for credential issues --- lib/galaxy/files/sources/_pyfilesystem2.py | 7 +++++-- lib/galaxy/files/sources/_rdm.py | 4 ++-- lib/galaxy/files/sources/dropbox.py | 7 +++++-- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/galaxy/files/sources/_pyfilesystem2.py b/lib/galaxy/files/sources/_pyfilesystem2.py index e604a6c8f75f..efdb5ac79ca8 100644 --- a/lib/galaxy/files/sources/_pyfilesystem2.py +++ b/lib/galaxy/files/sources/_pyfilesystem2.py @@ -16,7 +16,10 @@ from fs.base import FS from typing_extensions import Unpack -from galaxy.exceptions import MessageException +from galaxy.exceptions import ( + AuthenticationRequired, + MessageException, +) from . import ( BaseFilesSource, FilesSourceOptions, @@ -58,7 +61,7 @@ def _list(self, path="/", recursive=False, user_context=None, opts: Optional[Fil to_dict = functools.partial(self._resource_info_to_dict, path) return list(map(to_dict, res)) except fs.errors.PermissionDenied as e: - raise MessageException( + raise AuthenticationRequired( f"Permission Denied. Reason: {e}. Please check your credentials in your preferences for {self.label}." ) except fs.errors.FSError as e: diff --git a/lib/galaxy/files/sources/_rdm.py b/lib/galaxy/files/sources/_rdm.py index 7d9a97d2fb5f..14f7e9e1daa0 100644 --- a/lib/galaxy/files/sources/_rdm.py +++ b/lib/galaxy/files/sources/_rdm.py @@ -7,7 +7,7 @@ from typing_extensions import Unpack -from galaxy.exceptions import MessageException +from galaxy.exceptions import AuthenticationRequired from galaxy.files import ProvidesUserFileSourcesUserContext from galaxy.files.sources import ( BaseFilesSource, @@ -199,7 +199,7 @@ def get_authorization_token(self, user_context: OptionalUserContext) -> str: effective_props = self._serialization_props(user_context) token = effective_props.get("token") if not token: - raise MessageException( + raise AuthenticationRequired( f"Please provide a personal access token in your user's preferences for '{self.label}'" ) return token diff --git a/lib/galaxy/files/sources/dropbox.py b/lib/galaxy/files/sources/dropbox.py index 865338da2bbd..70c994e3e2e4 100644 --- a/lib/galaxy/files/sources/dropbox.py +++ b/lib/galaxy/files/sources/dropbox.py @@ -8,7 +8,10 @@ Union, ) -from galaxy.exceptions import MessageException +from galaxy.exceptions import ( + AuthenticationRequired, + MessageException, +) from . import ( FilesSourceOptions, FilesSourceProperties, @@ -35,7 +38,7 @@ def _open_fs(self, user_context=None, opts: Optional[FilesSourceOptions] = None) # This plugin might raise dropbox.dropbox_client.BadInputException # which is not a subclass of fs.errors.FSError if "OAuth2" in str(e): - raise MessageException( + raise AuthenticationRequired( f"Permission Denied. Reason: {e}. Please check your credentials in your preferences for {self.label}." ) raise MessageException(f"Error connecting to Dropbox. Reason: {e}")