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

Jump to bottom

Use os.makedirs with exist_ok=True #292

Merged
merged 1 commit into from
Aug 11, 2022
Merged

Use os.makedirs with exist_ok=True #292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 2 additions & 6 deletions m3u8/model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,12 +368,8 @@ def dump(self, filename):

def _create_sub_directories(self, filename):
basename = os.path.dirname(filename)
try:
if basename:
os.makedirs(basename)
except OSError as error:
if error.errno != errno.EEXIST:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This block in the CPython source explains why this check isn't enough:

Cannot rely on checking for EEXIST, since the operating system could give priority to other errors like EACCES or EROFS

In #291, EACCES is the error code rather than EEXIST.

Copy link
Member

Choose a reason for hiding this comment

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

Good to know! Thank you!

raise
if basename:
os.makedirs(basename, exist_ok=True)


class Segment(BasePathMixin):
Expand Down
27 changes: 12 additions & 15 deletions tests/test_model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -612,21 +612,18 @@ def test_dump_should_create_sub_directories(tmpdir):

assert_file_content(filename, expected)

def test_dump_should_raise_if_create_sub_directories_fails(tmpdir, monkeypatch):
def raiseOSError(*args):
raise OSError

monkeypatch.setattr(os, "makedirs", raiseOSError)

raised = False
try:
obj = m3u8.M3U8(playlists.SIMPLE_PLAYLIST)
obj.dump(str(tmpdir.join('subdir1', 'playlist.m3u8')))
except OSError as e:
raised = True
finally:
assert raised

def test_dump_should_raise_if_create_sub_directories_fails(tmpdir):
Copy link
Member

Choose a reason for hiding this comment

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

Nice tests

# The first subdirectory is read-only
subdir_1 = os.path.join(tmpdir, 'subdir1')
os.mkdir(subdir_1, mode=0o400)

# The file is to be stored in a second subdirectory that's underneath the first
subdir_2 = os.path.join(subdir_1, 'subdir2')
file_name = os.path.join(subdir_2, 'playlist.m3u8')

# When we try to write it, we'll be prevented from creating the second subdirectory
with pytest.raises(OSError):
m3u8.M3U8(playlists.SIMPLE_PLAYLIST).dump(file_name)

def test_dump_should_work_for_variant_streams():
obj = m3u8.M3U8(playlists.VARIANT_PLAYLIST)
Expand Down