-
Notifications
You must be signed in to change notification settings - Fork 655
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.
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
FreeBSD support #1616
base: dev
Are you sure you want to change the base?
FreeBSD support #1616
Conversation
Thanks ascl00, hpefully it will be more stable on FreeBSD |
@@ -29,7 +29,9 @@ filename="$4" | |||
|
|||
uri="/_relay_event/?_username=$username&event=$event&motion_camera_id=$motion_camera_id" | |||
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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', shell=True)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
subprocess.CalledProcessError
it seems. So:
except subprocess.CalledProcessError as e:
logging.warning(f'Running lsb_release failed with: {e.output}')
Or something like that?
|
||
disks = [] | ||
|
||
return disks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.