Conversation
It can be useful for a file to be created atomically with the desired user and group. Let's allow callers to specify their desired uid/gid and change ownership of the intermediate temp file before it is moved into place.
3ea7407 to
52913b5
Compare
| uid = kwargs.pop('uid', -1) | ||
| gid = kwargs.pop('gid', -1) | ||
| if uid != -1 or gid != -1: | ||
| os.fchown(descriptor, uid, gid) |
There was a problem hiding this comment.
Do we need check for whether sys.platform is on Unix?
There was a problem hiding this comment.
Maybe an assert around it. Though obviously not if we're upstreaming the patch
There was a problem hiding this comment.
May need to ensure that if one is specified and the other isn't, we don't attempt to set either UID or GID to -1 (unless I misunderstand the logic here)
There was a problem hiding this comment.
@mookerji yes I was concerned about this as well. Do you know of a way to test for "unix"? The method that comes to mind is doing something like hasattr(os, 'fchown'), but that assumes that on "non-unix" platforms the function doesn't exist in the os module.
There was a problem hiding this comment.
@borgel, According to the docs -1 will leave that field unchanged, so it is always a safe default. The if clause here was only added so we don't run the system call unnecessarily.
JoshM1994
left a comment
There was a problem hiding this comment.
Can see this being really valuable for replacing some of the manual chowning that occurs inside mqtt-bridge as well :)
| uid = kwargs.pop('uid', -1) | ||
| gid = kwargs.pop('gid', -1) | ||
| if uid != -1 or gid != -1: | ||
| os.fchown(descriptor, uid, gid) |
There was a problem hiding this comment.
May need to ensure that if one is specified and the other isn't, we don't attempt to set either UID or GID to -1 (unless I misunderstand the logic here)
It can be useful for a file to be created atomically with the desired
user and group. Let's allow callers to specify their desired uid/gid and
change ownership of the intermediate temp file before it is moved into
place.