Conversation
|
Thanks ascl00, hpefully it will be more stable on FreeBSD |
| data="{\"filename\": \"$filename\"}" | ||
| signature=$(echo -n "POST:$uri:$data:$password" | sha1sum | cut -d ' ' -f 1) | ||
| # allow for sha1 usage as well as sha1sum | ||
| sha1=$(which sha1sum sha1) |
There was a problem hiding this comment.
Problematic if both commands exist. Also, please use command -v:
| sha1=$(which sha1sum sha1) | |
| sha1=$(command -v sha1sum sha1 | head -1) |
| # check if it exists before calling to avoid log spam | ||
| rc = subprocess.call(['which', 'lsb_release']) | ||
|
|
||
| if rc == 0: | ||
| try: | ||
| output = subprocess.check_output('lsb_release -sri', shell=True) | ||
| lines = output.strip().split() | ||
| name, version = lines | ||
| if version.lower() == 'rolling': | ||
| version = '' | ||
|
|
||
| return name, version | ||
| return name, version | ||
|
|
||
| except: | ||
| return _get_os_version_uname() | ||
| except: | ||
| return _get_os_version_uname() | ||
| else: | ||
| return _get_os_version_uname() |
There was a problem hiding this comment.
Wrong indentation? Simpler may be:
# check if it exists before calling to avoid log spam
if subprocess.call(['which', 'lsb_release']):
try:
output = subprocess.check_output(['lsb_release', '-sri'])
lines = output.strip().split()
name, version = lines
if version.lower() == 'rolling':
version = ''
return name, version
except:
pass
return _get_os_version_uname()There was a problem hiding this comment.
Can we please lose the bare except as discussed in PEP8 and
https://realpython.com/the-most-diabolical-python-antipattern
There was a problem hiding this comment.
I also see annotations on this in CodeFactor. What is the exception type in case of a failing subprocess.check_output?
There was a problem hiding this comment.
subprocess.CalledProcessError it seems. So:
except subprocess.CalledProcessError as e:
logging.warning(f'Running lsb_release failed with: {e}')Or something like that?
|
|
||
| disks = [] | ||
|
|
||
| return disks |
There was a problem hiding this comment.
I think here is something wrong since you return an empty array? Isn't it intended to return output and make it an empty array only on except?
|
@ascl00 |
| signature=$(echo -n "POST:$uri:$data:$password" | sha1sum | cut -d ' ' -f 1) | ||
| # allow for sha1 usage as well as sha1sum | ||
| sha1=$(which sha1sum sha1) | ||
| signature=$(echo -n "POST:$uri:$data:$password" | $sha1 | cut -d ' ' -f 1) |
There was a problem hiding this comment.
To assure compatibility with any kind of shell:
| signature=$(echo -n "POST:$uri:$data:$password" | $sha1 | cut -d ' ' -f 1) | |
| signature=$(printf '%s' "POST:$uri:$data:$password" | "$sha1" | cut -d ' ' -f 1) |
|
Looks like there is another command call invalid on FreeBSD: #1060 |
|
Any hope of this being merged in or should I hope for the Python3 move to capture and correct these FreeBSD issues? |
|
This PR has major conflicts. It would need to be recreated based on the new |
Some changes to make things work a little smoother on FreeBSD (well, FreeNAS technically). _list_disks_gpart() is incomplete, if I find time I'll flesh it out. The changes should make no difference on Linux.