Out of Scope Variable in Exception Block in constants.py #2283

Preetam-Das26 · 2024-03-18T21:30:31Z

Purpose of PR?:

Fixes #2274

Does this PR introduce a breaking change?

If the changes in this PR are manually verified, list down the scenarios covered::

Additional information for reviewer? :
Mention if this PR is part of any design or a continuation of previous PRs

Does this PR results in some Documentation changes?
If yes, include the list of Documentation changes

Checklist:

Bug fix. Fixes #
New feature (Non-API breaking changes that adds functionality)
PR Title follows the convention of <type>: <subject>
Commit has unit tests

matrss · 2024-03-19T16:03:52Z

mslib/msui/constants.py

@@ -34,11 +34,12 @@
 # ToDo refactor to generic functions, keep only constants
 HOME = os.path.expanduser(f"~{os.path.sep}")
 MSUI_CONFIG_PATH = os.getenv("MSUI_CONFIG_PATH", os.path.join(HOME, ".config", "msui"))
+_fs = None


This doesn't solve the issue. Now it is possible that .makedirs is called on None.

Please remove this _fs = None line, it should not be necessary.

ReimarBauer · 2024-03-19T19:35:37Z

mslib/msui/constants.py

 if '://' in MSUI_CONFIG_PATH:
    try:
-        _fs = fs.open_fs(MSUI_CONFIG_PATH)
+        _fs = fs.open_fs(fs.path.dirname(MSUI_CONFIG_PATH))


look at the idea in the issue. There should be only one attempt for the fs.errors.CreateFailed because it could be a remote system,

and it would be good to have a test for describing the problem and evaluating the fix by that

…ception

Preetam-Das26 · 2024-03-23T08:34:25Z

Kindly checkout the changes @ReimarBauer @matrss

matrss · 2024-03-25T13:03:42Z

mslib/msui/constants.py

-        _fs = fs.open_fs(MSUI_CONFIG_PATH)
-    except fs.errors.CreateFailed:
-        _fs.makedirs(MSUI_CONFIG_PATH)
+        _fs = fs.open_fs(fs.path.dirname(MSUI_CONFIG_PATH))


I am not convinced that fs.path.dirname is correct there. For the default value of ~/.config/msui defined above this would open ~/.config, which seems wrong to me.

matrss · 2024-03-25T13:17:39Z

The code seems to get more complicated than it needs to be (but I am not completely sure). Therefore, I would like to take a step back and ask this: what is this code intended to do? What is the intended outcome of the code that was found to be buggy in #2274? Because I am not sure on that yet, and I can't properly review a PR without having that knowledge... This is probably a question for @ReimarBauer.

ReimarBauer · 2024-03-26T17:50:25Z

I provide tom. a testcase. This looks now a bit too much for a constant

ReimarBauer · 2024-03-27T07:37:21Z

This shows an example to illustrate the problem"s". There can be a lot cases why a remote address can fail. I prefer to crash here, without catching all unknown possibilities dependent on the potential used module. We should just not try to create a remote folder.

import fs
import os
import logging

# ToDo refactor to generic functions, keep only constants
HOME = os.path.expanduser(f"~{os.path.sep}")
MSUI_CONFIG_PATH = "ftp://ftp.de.debian.org/debian/.config/msui"

if '://' in MSUI_CONFIG_PATH:
    try:
        _fs = fs.open_fs(MSUI_CONFIG_PATH)
    except fs.opener.errors.UnsupportedProtocol:
        logging.error('FS url "%s" not supported', MSUI_CONFIG_PATH)
    except Exception as ex:
        logging.error('FS url "%s" can''t be used, check your remote ressource. '
                  'Reasons can be: path does not exists, %s', MSUI_CONFIG_PATH, ex)
        raise fs.errors.ResourceError(MSUI_CONFIG_PATH, exc=None, msg="FS url can't be used")
else:
    _dir = os.path.expanduser(MSUI_CONFIG_PATH)
    if not os.path.exists(_dir):
        os.makedirs(_dir)

The error looks now like

ERROR:root:FS url "ftp://ftp.de.debian.org/debian/.config/msui" cant be used, check your remote ressource. Reasons can be: path does not exists, unable to create filesystem, resource 'debian/.config/msui' not found
Traceback (most recent call last):
  File "/home/user/Miniforge/envs/mssdev/lib/python3.11/site-packages/fs/ftpfs.py", line 77, in ftp_errors
    yield
  File "/home/user/Miniforge/envs/mssdev/lib/python3.11/site-packages/fs/ftpfs.py", line 674, in getinfo
    response = self.ftp.sendcmd(
               ^^^^^^^^^^^^^^^^^
  File "/home/user/Miniforge/envs/mssdev/lib/python3.11/ftplib.py", line 281, in sendcmd
    return self.getresp()
           ^^^^^^^^^^^^^^
  File "/home/user/Miniforge/envs/mssdev/lib/python3.11/ftplib.py", line 254, in getresp
    raise error_perm(resp)
ftplib.error_perm: 550 '/debian/.config/msui' cannot be listed

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/user/Miniforge/envs/mssdev/lib/python3.11/site-packages/fs/errors.py", line 125, in new_func
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/home/user/Miniforge/envs/mssdev/lib/python3.11/site-packages/fs/opener/ftpfs.py", line 55, in open_fs
    return ftp_fs.opendir(dir_path, factory=ClosingSubFS)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/user/Miniforge/envs/mssdev/lib/python3.11/site-packages/fs/base.py", line 1269, in opendir
    if not self.getinfo(path).is_dir:
           ^^^^^^^^^^^^^^^^^^
  File "/home/user/Miniforge/envs/mssdev/lib/python3.11/site-packages/fs/ftpfs.py", line 673, in getinfo
    with ftp_errors(self, path=path):
  File "/home/user/Miniforge/envs/mssdev/lib/python3.11/contextlib.py", line 155, in __exit__
    self.gen.throw(typ, value, traceback)
  File "/home/user/Miniforge/envs/mssdev/lib/python3.11/site-packages/fs/ftpfs.py", line 96, in ftp_errors
    raise errors.ResourceNotFound(path=cast(str, path))
fs.errors.ResourceNotFound: resource 'debian/.config/msui' not found

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/user/PycharmProjects/MSS/mslib/msui/constants.py", line 39, in <module>
    _fs = fs.open_fs(MSUI_CONFIG_PATH)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/user/Miniforge/envs/mssdev/lib/python3.11/site-packages/fs/opener/registry.py", line 220, in open_fs
    _fs, _path = self.open(
                 ^^^^^^^^^^
  File "/home/user/Miniforge/envs/mssdev/lib/python3.11/site-packages/fs/opener/registry.py", line 177, in open
    open_fs = opener.open_fs(fs_url, parse_result, writeable, create, cwd)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/user/Miniforge/envs/mssdev/lib/python3.11/site-packages/fs/errors.py", line 129, in new_func
    raise cls(exc=e)
fs.errors.CreateFailed: unable to create filesystem, resource 'debian/.config/msui' not found

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/user/PycharmProjects/MSS/mslib/msui/msui.py", line 48, in <module>
    from mslib.utils.qt import ui_mainwindow as ui
  File "/home/user/PycharmProjects/MSS/mslib/utils/qt.py", line 40, in <module>
    from mslib.utils.config import config_loader
  File "/home/user/PycharmProjects/MSS/mslib/utils/config.py", line 40, in <module>
    from mslib.msui import constants
  File "/home/user/PycharmProjects/MSS/mslib/msui/constants.py", line 46, in <module>
    raise fs.errors.ResourceError(MSUI_CONFIG_PATH, exc=None, msg="FS url can't be used")
fs.errors.ResourceError: FS url can't be used

ReimarBauer · 2024-05-08T09:39:28Z

there is a different solution #2343

Preetam-Das26 added 2 commits March 19, 2024 02:42

Exception scope 02:42

cf4c772

Exception scope 02:56

90e276b

matrss reviewed Mar 19, 2024

View reviewed changes

ReimarBauer reviewed Mar 19, 2024

View reviewed changes

Preetam-Das26 added 4 commits March 21, 2024 20:40

Merge branch 'stable' of https://github.com/Preetam-Das26/MSS into ex…

5c6af93

…ception

made changes based on reviews 20:51

81b2bce

22:06

e520f41

rectify 13:51

0e71792

matrss reviewed Mar 25, 2024

View reviewed changes

Merge branch 'stable' into exception

aed4d2f

ReimarBauer added 2 commits April 15, 2024 13:02

Merge branch 'stable' into exception

dde7e8c

Merge branch 'stable' into exception

67ad22d

ReimarBauer closed this May 8, 2024

Preetam-Das26 deleted the exception branch May 13, 2024 05:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Out of Scope Variable in Exception Block in constants.py #2283

Out of Scope Variable in Exception Block in constants.py #2283

Preetam-Das26 commented Mar 18, 2024

matrss Mar 19, 2024

Preetam-Das26 Mar 20, 2024

matrss Mar 25, 2024

ReimarBauer Mar 19, 2024 •

edited

Loading

Preetam-Das26 Mar 20, 2024

Preetam-Das26 commented Mar 23, 2024

matrss Mar 25, 2024

matrss commented Mar 25, 2024

ReimarBauer commented Mar 26, 2024

ReimarBauer commented Mar 27, 2024

ReimarBauer commented May 8, 2024

Out of Scope Variable in Exception Block in constants.py #2283

Out of Scope Variable in Exception Block in constants.py #2283

Conversation

Preetam-Das26 commented Mar 18, 2024

matrss Mar 19, 2024

Choose a reason for hiding this comment

Preetam-Das26 Mar 20, 2024

Choose a reason for hiding this comment

matrss Mar 25, 2024

Choose a reason for hiding this comment

ReimarBauer Mar 19, 2024 • edited Loading

Choose a reason for hiding this comment

Preetam-Das26 Mar 20, 2024

Choose a reason for hiding this comment

Preetam-Das26 commented Mar 23, 2024

matrss Mar 25, 2024

Choose a reason for hiding this comment

matrss commented Mar 25, 2024

ReimarBauer commented Mar 26, 2024

ReimarBauer commented Mar 27, 2024

ReimarBauer commented May 8, 2024

ReimarBauer Mar 19, 2024 •

edited

Loading