flake8
-
bot/reviewbot/utils/filesystem.py (Diff revision 1) Show all issues -
-
Review Request #10247 — Created Oct. 19, 2018 and submitted
When a RepositoryTool is applying the review's patches to the cloned
repository, it simply writes usingopen(p, 'wb')
. This does not create
directories that the patch implies.This leads to the write failing with:
IOError: [Errno 2] No such file or directory: u'dir/file'
This patch creates directories for each file if the path does not exist
already.
Replciated the error by posting a review for a file in a new directory,
applied patch, and RepositoryTool successfully runs on patched files.
Description | From | Last Updated |
---|---|---|
F401 'os.path' imported but unused |
![]() |
|
F811 redefinition of unused 'path' from line 8 |
![]() |
|
E722 do not use bare except' |
![]() |
|
Blank line between these. |
|
|
This doesn't always create directories -- it ensures they exist, so let's call this ensure_directories. |
|
|
No space after " |
|
|
The path for which directories should be created if they don't exist. |
|
|
Blank line between here. |
|
|
E722 do not use bare except' |
![]() |
|
We never want to use bare except. This can catch things that should never be caught, like KeyboardInterrupt or RuntimeError. … |
|
|
It seems like we'd probably want to surface the error so that the caller can bail out. |
|
|
Function name would be better as ensure_directories_exist |
|
bot/reviewbot/utils/filesystem.py (Diff revision 1) |
---|
Removed unneccessary import
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 2 (+17 -1) |
bot/reviewbot/utils/filesystem.py (Diff revision 2) |
---|
This doesn't always create directories -- it ensures they exist, so let's call this
ensure_directories
.
bot/reviewbot/utils/filesystem.py (Diff revision 2) |
---|
The path for which directories should be created if they don't exist.
bot/reviewbot/utils/filesystem.py (Diff revision 2) |
---|
We never want to use bare
except
. This can catch things that should never be caught, likeKeyboardInterrupt
orRuntimeError
. Instead, this needs to specify the exact exception we expect, which should beOSError
.
Renamed create_dirs, formatting, and catch OSError instead of bare except.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 3 (+20 -1) |
bot/reviewbot/utils/filesystem.py (Diff revision 3) |
---|
It seems like we'd probably want to surface the error so that the caller can bail out.
Converted to enforcing absolute paths.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 4 (+28 -1) |
bot/reviewbot/utils/filesystem.py (Diff revision 4) |
---|
Function name would be better as
ensure_directories_exist
Renamed ensure_directories
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 5 (+28 -1) |