Skip to content

Commit

Permalink
add stricter formatting because it was easy
Browse files Browse the repository at this point in the history
  • Loading branch information
weaverba137 committed Jun 3, 2024
1 parent 8716c45 commit 7b55451
Show file tree
Hide file tree
Showing 10 changed files with 64 additions and 70 deletions.
11 changes: 5 additions & 6 deletions py/desitransfer/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def empty_rsync(out):
``True`` if there are no files to transfer.
"""
rr = re.compile(r'(receiving|sent [0-9]+ bytes|total size)')
return all([rr.match(l) is not None for l in out.split('\n') if l])
return all([rr.match(out_line) is not None for out_line in out.split('\n') if out_line])


def new_exposures(out):
Expand All @@ -53,8 +53,8 @@ def new_exposures(out):
"""
e = set()
e_re = re.compile(r'([0-9]{8})/?')
for l in out.split('\n'):
m = e_re.match(l)
for out_line in out.split('\n'):
m = e_re.match(out_line)
if m is not None:
e.add(m.groups()[0])
return e
Expand Down Expand Up @@ -126,7 +126,7 @@ def ensure_scratch(directories):
"""
for d in directories:
try:
l = os.listdir(d)
dir_list = os.listdir(d)
except FileNotFoundError:
continue
return d
Expand All @@ -144,7 +144,7 @@ def today():
This formulation, with the offset ``7/24+0.5``, is inherited from previous
nightwatch transfer scripts.
"""
return (dt.datetime.utcnow() - dt.timedelta(7/24+0.5)).strftime('%Y%m%d')
return (dt.datetime.utcnow() - dt.timedelta(7 / 24 + 0.5)).strftime('%Y%m%d')


def idle_time(start=8, end=12, tz=None):
Expand Down Expand Up @@ -191,4 +191,3 @@ def exclude_years(start_year):
A list suitable for appending to a command.
"""
return (' '.join([f'--exclude {y:d}*' for y in range(start_year, time.localtime().tm_year)])).split()

27 changes: 13 additions & 14 deletions py/desitransfer/daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,10 @@ def __init__(self, options):
self._ini = options.configuration
self.test = options.test
self.tape = options.backup
getlist = lambda x: x.split(',')
getdict = lambda x: dict([tuple(i.split(':')) for i in x.split(',')])
self.conf = ConfigParser(defaults=os.environ, strict=True,
interpolation=ExtendedInterpolation(),
converters={'list': getlist, 'dict': getdict})
converters={'list': lambda x: x.split(','),
'dict': lambda x: dict([tuple(i.split(':')) for i in x.split(',')])})
files = self.conf.read(self._ini)
# assert files[0] == self._ini
self.sections = [s for s in self.conf.sections()
Expand Down Expand Up @@ -183,11 +182,11 @@ def directory(self, d):
_, out, err = _popen(cmd)
links = sorted([x for x in out.split('\n') if x])
if links:
for l in links:
if self._link_re.search(l) is None:
log.warning("Malformed symlink detected: %s. Skipping.", l)
for link in links:
if self._link_re.search(link) is None:
log.warning("Malformed symlink detected: %s. Skipping.", link)
else:
self.exposure(d, l, status)
self.exposure(d, link, status)
else:
log.warning('No links found, check connection.')
#
Expand Down Expand Up @@ -440,9 +439,9 @@ def backup(self, d, night, status):
if self.tape:
log.debug(' '.join(cmd))
_, out, err = _popen(cmd)
with open(ls_file) as l:
data = l.read()
backup_files = [l.split()[-1] for l in data.split('\n') if l]
with open(ls_file) as ls_fileobj:
data = ls_fileobj.read()
backup_files = [ls_out.split()[-1] for ls_out in data.split('\n') if ls_out]
else:
backup_files = []
backup_file = hpss_file + '_' + night + '.tar'
Expand Down Expand Up @@ -544,9 +543,9 @@ def verify_checksum(checksum_file):
checksum_file, n_lines)
errors += "{0:d} file(s) listed but not downloaded.\n".format(n_lines)
if n_lines < 0:
log.error("%d files are not listed in %s!", -1*n_lines, checksum_file)
errors += "{0:d} file(s) downloaded but not listed.\n".format(-1*n_lines)
digest = dict([(l.split()[1], l.split()[0]) for l in lines if l])
log.error("%d files are not listed in %s!", -1 * n_lines, checksum_file)
errors += "{0:d} file(s) downloaded but not listed.\n".format(-1 * n_lines)
digest = dict([(cl.split()[1], cl.split()[0]) for cl in lines if cl])
for f in files:
ff = os.path.join(d, f)
if ff != checksum_file:
Expand Down Expand Up @@ -668,5 +667,5 @@ def main():
options.kill)
return 0
transfer.transfer()
time.sleep(sleep*60)
time.sleep(sleep * 60)
return 0
28 changes: 14 additions & 14 deletions py/desitransfer/daily.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,14 @@ def transfer(self, permission=True):
if self.extra:
for i, e in enumerate(self.extra):
cmd.insert(cmd.index('--omit-dir-times') + 1 + i, e)
with open(self.log, 'ab') as l:
l.write(("DEBUG: desi_daily_transfer %s\n" % dtVersion).encode('utf-8'))
l.write(("DEBUG: %s\n" % ' '.join(cmd)).encode('utf-8'))
l.write(("DEBUG: Transfer start: %s\n" % stamp()).encode('utf-8'))
l.flush()
p = sub.Popen(cmd, stdout=l, stderr=sub.STDOUT)
with open(self.log, 'ab') as logfile:
logfile.write(("DEBUG: desi_daily_transfer %s\n" % dtVersion).encode('utf-8'))
logfile.write(("DEBUG: %s\n" % ' '.join(cmd)).encode('utf-8'))
logfile.write(("DEBUG: Transfer start: %s\n" % stamp()).encode('utf-8'))
logfile.flush()
p = sub.Popen(cmd, stdout=logfile, stderr=sub.STDOUT)
status = p.wait()
l.write(("DEBUG: Transfer complete: %s\n" % stamp()).encode('utf-8'))
logfile.write(("DEBUG: Transfer complete: %s\n" % stamp()).encode('utf-8'))
if status == 0:
self.lock()
if permission:
Expand All @@ -80,8 +80,8 @@ def lock(self):
fpath = os.path.join(dirpath, f)
if stat.S_IMODE(os.stat(fpath).st_mode) != file_perm:
os.chmod(fpath, file_perm)
with open(self.log, 'ab') as l:
l.write(("DEBUG: Lock complete: %s\n" % stamp()).encode('utf-8'))
with open(self.log, 'ab') as logfile:
logfile.write(("DEBUG: Lock complete: %s\n" % stamp()).encode('utf-8'))

def permission(self):
"""Set permissions for DESI collaboration access.
Expand All @@ -95,12 +95,12 @@ def permission(self):
The status returned by :command:`fix_permissions.sh`.
"""
cmd = ['fix_permissions.sh', self.destination]
with open(self.log, 'ab') as l:
l.write(("DEBUG: %s\n" % ' '.join(cmd)).encode('utf-8'))
l.flush()
p = sub.Popen(cmd, stdout=l, stderr=sub.STDOUT)
with open(self.log, 'ab') as logfile:
logfile.write(("DEBUG: %s\n" % ' '.join(cmd)).encode('utf-8'))
logfile.flush()
p = sub.Popen(cmd, stdout=logfile, stderr=sub.STDOUT)
status = p.wait()
l.write(("DEBUG: Permission reset complete: %s\n" % stamp()).encode('utf-8'))
logfile.write(("DEBUG: Permission reset complete: %s\n" % stamp()).encode('utf-8'))
return status


Expand Down
2 changes: 1 addition & 1 deletion py/desitransfer/nightwatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ def main():
options = _options()
_configure_log(options.debug)
errcount = 0
wait = options.sleep*60
wait = options.sleep * 60
source = '/exposures/nightwatch'
basedir = os.path.join(os.environ['DESI_ROOT'], 'spectro', 'nightwatch')
kpnodir = os.path.join(basedir, 'kpno')
Expand Down
2 changes: 1 addition & 1 deletion py/desitransfer/test/test_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ def test_today(self, mock_dt):
"""Test today's date.
"""
mock_dt.datetime.utcnow.return_value = datetime(2019, 7, 3, 5, 0, 0)
mock_dt.timedelta.return_value = timedelta(7/24+0.5)
mock_dt.timedelta.return_value = timedelta(7 / 24 + 0.5)
y = today()
self.assertEqual(y, '20190702')

Expand Down
31 changes: 15 additions & 16 deletions py/desitransfer/test/test_daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -1045,49 +1045,48 @@ def test_verify_checksum(self):
d = os.path.dirname(c)
with patch('os.listdir') as mock_listdir:
mock_listdir.return_value = ['t.sha256sum', 'test_file_1.txt', 'test_file_2.txt']
with patch('desitransfer.daemon.log') as l:
with patch('desitransfer.daemon.log') as mock_log:
o = verify_checksum(c)
self.assertEqual(o, "")
l.debug.assert_has_calls([call("%s is valid.", os.path.join(d, 'test_file_1.txt')),
call("%s is valid.", os.path.join(d, 'test_file_2.txt'))])
mock_log.debug.assert_has_calls([call("%s is valid.", os.path.join(d, 'test_file_1.txt')),
call("%s is valid.", os.path.join(d, 'test_file_2.txt'))])
#
# Wrong number of files.
#
with patch('os.listdir') as mock_listdir:
mock_listdir.return_value = ['t.sha256sum', 'test_file_1.txt']
with patch('desitransfer.daemon.log') as l:
with patch('desitransfer.daemon.log') as mock_log:
o = verify_checksum(c)
self.assertEqual(o, "1 file(s) listed but not downloaded.\n")
l.error.assert_has_calls([call("%s lists %d file(s) that are not present!", c, 1)])
mock_log.error.assert_has_calls([call("%s lists %d file(s) that are not present!", c, 1)])
with patch('os.listdir') as mock_listdir:
mock_listdir.return_value = ['t.sha256sum', 'test_file_1.txt', 'test_file_2.txt', 'test_file_3.txt']
with patch('desitransfer.daemon.log') as l:
with patch('desitransfer.daemon.log') as mock_log:
o = verify_checksum(c)
self.assertEqual(o, "1 file(s) downloaded but not listed.\ntest_file_3.txt not listed in checksum file.\n")
l.error.assert_has_calls([call("%d files are not listed in %s!", 1, c)])
mock_log.error.assert_has_calls([call("%d files are not listed in %s!", 1, c)])
#
# Bad list of files.
#
with patch('os.listdir') as mock_listdir:
mock_listdir.return_value = ['t.sha256sum', 'test_file_1.txt', 'test_file_3.txt']
with patch('desitransfer.daemon.log') as l:
with patch('desitransfer.daemon.log') as mock_log:
o = verify_checksum(c)
self.assertEqual(o, "test_file_3.txt not listed in checksum file.\n")
l.debug.assert_has_calls([call("%s is valid.", os.path.join(d, 'test_file_1.txt'))])
l.error.assert_has_calls([call("%s does not appear in %s!", os.path.join(d, 'test_file_3.txt'), c)])
mock_log.debug.assert_has_calls([call("%s is valid.", os.path.join(d, 'test_file_1.txt'))])
mock_log.error.assert_has_calls([call("%s does not appear in %s!", os.path.join(d, 'test_file_3.txt'), c)])
#
# Hack hashlib to produce incorrect checksums.
#
with patch('os.listdir') as mock_listdir:
mock_listdir.return_value = ['t.sha256sum', 'test_file_1.txt', 'test_file_2.txt']
with patch('desitransfer.daemon.log') as l:
with patch('hashlib.sha256') as h:
# h.sha256 = MagicMock()
h.hexdigest.return_value = 'abcdef'
with patch('desitransfer.daemon.log') as mock_log:
with patch('hashlib.sha256') as mock_hash:
mock_hash.hexdigest.return_value = 'abcdef'
o = verify_checksum(c)
self.assertEqual(o, "test_file_1.txt had a checksum mismatch.\ntest_file_2.txt had a checksum mismatch.\n")
l.error.assert_has_calls([call("Checksum mismatch for %s in %s!", os.path.join(d, 'test_file_1.txt'), c),
call("Checksum mismatch for %s in %s!", os.path.join(d, 'test_file_2.txt'), c)])
mock_log.error.assert_has_calls([call("Checksum mismatch for %s in %s!", os.path.join(d, 'test_file_1.txt'), c),
call("Checksum mismatch for %s in %s!", os.path.join(d, 'test_file_2.txt'), c)])

@patch('os.walk')
@patch('os.chmod')
Expand Down
20 changes: 10 additions & 10 deletions py/desitransfer/test/test_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,17 @@ def test_TransferStatus_init(self):
# New directory.
#
d = '/desi/spectro/status'
with patch('desitransfer.status.log') as l:
with patch('os.makedirs') as m:
with patch('shutil.copy') as cp:
with patch('shutil.copyfile') as cf:
with patch('desitransfer.status.log') as mock_log:
with patch('os.makedirs') as mock_makedirs:
with patch('shutil.copy') as mock_copy:
with patch('shutil.copyfile') as mock_copyfile:
s = TransferStatus(d)
l.debug.assert_has_calls([call("os.makedirs('%s', exist_ok=True)", d),
call("shutil.copyfile('%s', '%s')", h, os.path.join(d, 'index.html')),
call("shutil.copy('%s', '%s')", j, d)])
m.assert_called_once_with(d, exist_ok=True)
cp.assert_called_once_with(j, d)
cf.assert_called_once_with(h, os.path.join(d, 'index.html'))
mock_log.debug.assert_has_calls([call("os.makedirs('%s', exist_ok=True)", d),
call("shutil.copyfile('%s', '%s')", h, os.path.join(d, 'index.html')),
call("shutil.copy('%s', '%s')", j, d)])
mock_makedirs.assert_called_once_with(d, exist_ok=True)
mock_copy.assert_called_once_with(j, d)
mock_copyfile.assert_called_once_with(h, os.path.join(d, 'index.html'))

@patch('desitransfer.status.log')
def test_TransferStatus_handle_malformed_with_log(self, mock_log):
Expand Down
3 changes: 1 addition & 2 deletions py/desitransfer/test/test_top_level.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ class TestTopLevel(unittest.TestCase):

@classmethod
def setUpClass(cls):
cls.versionre = re.compile(
r'([0-9]+!)?([0-9]+)(\.[0-9]+)*((a|b|rc|\.post|\.dev)[0-9]+)?')
cls.versionre = re.compile(r'([0-9]+!)?([0-9]+)(\.[0-9]+)*((a|b|rc|\.post|\.dev)[0-9]+)?')

@classmethod
def tearDownClass(cls):
Expand Down
2 changes: 1 addition & 1 deletion py/desitransfer/tucson.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ def main():
for s in suffix:
if options.sleep.endswith(s):
try:
sleepy_time = int(options.sleep[0:-1])*suffix[s]
sleepy_time = int(options.sleep[0:-1]) * suffix[s]
except ValueError:
log.error("Invalid value for sleep interval: '%s'!", options.sleep)
return 1
Expand Down
8 changes: 3 additions & 5 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,7 @@ exclude_lines =
# These are normally ignored by default:
# ignore = E121, E123, E126, E133, E226, E241, E242, E704, W503, W504
#
# In addition to the default set we add:
# These are the explicitly ignored styles:
# - E501: line too long (82 > 79 characters)
# - E731: do not assign a lambda expression, use a def
# - E741: do not use variables named 'l', 'O', or 'I' -- because, for example,
# 'l' might refer to Galactic longitude.
ignore = E121, E123, E126, E133, E226, E241, E242, E501, E704, E731, E741, W503, W504
# - W504: line break after binary operator
ignore = E501, W504

0 comments on commit 7b55451

Please sign in to comment.