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.

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

don't use FileExistsError in mkdir function (doesn't exist in Python 2.7) #4328

Merged
merged 4 commits into from
Aug 16, 2023

Conversation

Flamefire
Copy link
Contributor

Follow-up to #4300 which introduced a Python3 class

Fix by introducing a Python2 implementation and using that.

Same as the Python3 implementation which has a comment:
> Cannot rely on checking for EEXIST, since the operating system could give priority to other errors like EACCES or EROFS
Use the new py2vpy3.makedirs compat layer.
boegel
boegel previously approved these changes Aug 15, 2023
Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

lgtm

@boegel boegel added the bug fix label Aug 15, 2023
@boegel boegel added this to the next release (4.8.1?) milestone Aug 15, 2023
@boegel
Copy link
Member

boegel commented Aug 15, 2023

@Flamefire Hmm, new test fails:

Traceback (most recent call last):
  File "/tmp/runner/e377429f58b1fec22562d603da05e3480ca0cfff/lib/python3.6/site-packages/test/framework/filetools.py", line 3399, in test_compat_makedirs
    self.assertErrorRegex(Exception, os.path.basename(name), py2vs3.makedirs, name)
  File "/tmp/runner/e377429f58b1fec22562d603da05e3480ca0cfff/lib/python3.6/site-packages/easybuild/base/testing.py", line 178, in assertErrorRegex
    self.assertTrue(regex.search(msg), "Pattern '%s' is found in '%s'" % (regex.pattern, msg))
AssertionError: None is not true : Pattern 'folder' is found in '17'

edit: fixed in 40f9fe7

@boegel boegel changed the title Remove use of Python3 FileExistsError Remove use of Python3 FileExistsError in mkdir function Aug 15, 2023
…tion if directory already exists, so the check passes with both Python 2 and 3
@boegel
Copy link
Member

boegel commented Aug 15, 2023

We overlooked this because we stopped running the tests on top of Python 2 in #4267, ugh...

ERROR: test_mkdir (__main__.FileToolsTest)
Test mkdir function.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/arcanine/scratch/gent/vo/000/gvo00002/vsc40023/easybuild/easybuild-framework/test/framework/filetools.py", line 631, in test_mkdir
    check_mkdir(barfoodir, error="Failed.*No such file or directory")
  File "/arcanine/scratch/gent/vo/000/gvo00002/vsc40023/easybuild/easybuild-framework/test/framework/filetools.py", line 623, in check_mkdir
    self.assertErrorRegex(EasyBuildError, error, ft.mkdir, path, **kwargs)
  File "easybuild/base/testing.py", line 170, in assertErrorRegex
    call(*args, **kwargs)
  File "easybuild/tools/filetools.py", line 1924, in mkdir
    except FileExistsError as err:
NameError: global name 'FileExistsError' is not defined

@Flamefire
Copy link
Contributor Author

@Flamefire Hmm, new test fails:

Traceback (most recent call last):
  File "/tmp/runner/e377429f58b1fec22562d603da05e3480ca0cfff/lib/python3.6/site-packages/test/framework/filetools.py", line 3399, in test_compat_makedirs
    self.assertErrorRegex(Exception, os.path.basename(name), py2vs3.makedirs, name)
  File "/tmp/runner/e377429f58b1fec22562d603da05e3480ca0cfff/lib/python3.6/site-packages/easybuild/base/testing.py", line 178, in assertErrorRegex
    self.assertTrue(regex.search(msg), "Pattern '%s' is found in '%s'" % (regex.pattern, msg))
AssertionError: None is not true : Pattern 'folder' is found in '17'

edit: fixed in 40f9fe7

Your fix works, thanks! For reference: "17" is EEXISTS, strange that the name isn't always included.

@boegel boegel changed the title Remove use of Python3 FileExistsError in mkdir function don't use FileExistsError in mkdir function (doesn't exist in Python 2.7) Aug 16, 2023
@boegel
Copy link
Member

boegel commented Aug 16, 2023

I've opened #4333 to bring back running the test suite with Python 2.7...

@boegel boegel merged commit 46654a6 into easybuilders:develop Aug 16, 2023
@Flamefire Flamefire deleted the makedirs branch August 22, 2023 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants