Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions nslsii/utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import os
import re
import subprocess


def run(cmd, path="", ignoreErrors=True, returnError=False, debug=False):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to name the kwargs in the snake_case style, i.e.:

  • ignoreErrors -> ignore_errors
  • returnError -> return_error

"""cmd should be a list, e.g. ["ls", "-lh"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring should have a break between the first line and the following lines. Maybe an introductory sentence about this function can be useful.

Also, run seems to be too vague. Maybe something more specific such as execute_command can be more appropriate.

path is for the cmd, not the same as cwd
"""
cmd[0] = path + cmd[0]
p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
out, err = p.communicate()
if debug:
print(out.decode(), err.decode())
if len(err) > 0 and not ignoreErrors:
print(err.decode())
raise Exception(err.decode())
if returnError:
return out.decode(), err.decode()
else:
return out.decode()
Comment on lines +13 to +21
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out.decode() and err.decode() are used a few times. Maybe worth calling them once, and reuse via variables?



def check_access(fn):
if not os.path.exists(fn):
raise Exception(f"{fn} does not exist ...")
if os.access(fn, os.W_OK):
print(f"write access to {fn} verified ...")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this message, I think it will make sense to add a few words about this being verified via Unix permissions.

return

# this below may not be necessary
out = run(["getfacl", "-cn", fn])
wgrps = [int(t[:-4].lstrip("group:")) for t in re.findall("groups:[0-9]*:rw.", out)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is -4 here for? Can it ever be more or less than 4 symbols?
Maybe it's worth adding an example (anonymized) output of that search to have a better understanding of what kind of data we are dealing with here.

ugrps = os.getgroups()
if len(set(wgrps) & set(ugrps)) == 0:
print("groups with write permission: ", wgrps)
print("user group membership: ", ugrps)
raise Exception(f"the current user does not have write access to {fn}")
else:
print(f"write access to {fn} verified ...")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this message, I think it will make sense to add a few words about this being verified via ACL (getfacl).