Code cleanup, fix two bugs #4
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 6 commits
May 24, 2017 08:40
If the afs python modue isn't installed, then when libdelete is imported, this was getting displayed instead of the intended warning about afs not being available: >`No handlers could be found for logger "libdelete"` This is because libdelete was attempting to import afs to check for its availability at module scope, before the main script importing libdelete had configured logging. Fix this by changing `have_AFS` from a module-scope variable to a memoized function so that the check occurs after logging is configured.
`delete` doesn't ever use `afs.fs` so it doesn't need to import it.
|
I am also contemplating modifying |
added 3 commits
May 24, 2017 09:18
The code in `debug_callback` checking if the specified loggers were valid was using a set equals operator when it should have been using a subset operator.
When lsdel and expunge list files they shouldn't output a blank line when there are no files to list.
This change is intended to fix the following:
```
$ touch foo
$ delete foo
$ touch foo
$ expunge
Traceback (most recent call last):
File "/usr/local/bin/expunge", line 183, in <module>
sys.exit(main())
File "/usr/local/bin/expunge", line 176, in main
rv += expunge(deleted_files, options)
File "/usr/local/bin/expunge", line 90, in expunge
os.unlink(f)
OSError: [Errno 2] No such file or directory: './.#foo'
$
```
The problem was that "./.#foo" was getting added to the deleted file
list twice, because it glob-matched twice, once in its undeleted form
and again in its deleted form.
To avoid this issue we use a uniquifying list class so that we don't
add the same path to our list of deleted files twice.
We use a uniquifying list rather than a set to preserve order.
yilundu
approved these changes
Nov 11, 2017
|
Knock knock, anybody here? |
kaduk
reviewed
Feb 11, 2020
| @@ -156,6 +139,7 @@ def main(): | |||
| rv = 1 | |||
| return rv | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
Why so many extra blank lines?
(Sorry for the super sparse review; I'm on vacation this week and just skimming.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Clean up handling of presence/absence of PyAFS. Fix a missing import and a typo in a variable reference. Make all the code flake8-clean.