Skip to content
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.

Sign up for GitHub

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

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

Closed
wants to merge 9 commits into from
Closed

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

Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
cf4c772
Exception scope 02:42
Preetam-Das26 Mar 18, 2024
90e276b
Exception scope 02:56
Preetam-Das26 Mar 18, 2024
5c6af93
Merge branch 'stable' of https://github.com/Preetam-Das26/MSS into ex…
Preetam-Das26 Mar 21, 2024
81b2bce
made changes based on reviews 20:51
Preetam-Das26 Mar 21, 2024
e520f41
22:06
Preetam-Das26 Mar 21, 2024
0e71792
rectify 13:51
Preetam-Das26 Mar 23, 2024
aed4d2f
Merge branch 'stable' into exception
ReimarBauer Mar 26, 2024
dde7e8c
Merge branch 'stable' into exception
ReimarBauer Apr 15, 2024
67ad22d
Merge branch 'stable' into exception
ReimarBauer Apr 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions mslib/msui/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if '://' in MSUI_CONFIG_PATH:
try:
_fs = fs.open_fs(MSUI_CONFIG_PATH)
_fs = fs.open_fs(fs.path.dirname(MSUI_CONFIG_PATH))
Copy link
Member

@ReimarBauer ReimarBauer Mar 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

except fs.errors.CreateFailed:
_fs.makedirs(MSUI_CONFIG_PATH)
_fs.makedirs(fs.path.basename(MSUI_CONFIG_PATH))
except fs.opener.errors.UnsupportedProtocol:
logging.error('FS url "%s" not supported', MSUI_CONFIG_PATH)
else:
Expand Down
Loading