-
Notifications
You must be signed in to change notification settings - Fork 0
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
Fix user and group determination #19
Conversation
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.
Looks good, I think!
Could not find the Popen line to suggest the modification from #18 (comment) though. Maybe that should be added, as you suggested!
I explicitly set |
Yeah, I do not know. If this was a full Linux system, there will be issues from not assigning supplemental groups. However, within docker, I do not expect issues to appear... |
Anyway... there seem to be issues with dropping the extra groups in sagemaker afterall... in the commandline you can run:
that list (not this literal one, of course!) should be the list of supplemental groups for the user. It is only available when the user actually exists. |
...
Forgot that it also nicely lists the "groups=..." bit. However, could still be that sagemaker somehow needs access to group |
tests/test_models.py
Outdated
[23746, *USER_GROUPS], | ||
), | ||
# User does not exist, but is an int | ||
("23746", 23746, None, None, []), | ||
(f"23746:{grp.getgrgid(0).gr_name}", 23746, 0, None, []), | ||
(f"23746:{os.getgid()}", 23746, os.getgid(), None, []), | ||
(f"23746:{grp.getgrgid(0).gr_name}", 23746, 0, None, [0]), | ||
(f"23746:{os.getgid()}", 23746, os.getgid(), None, [os.getgid()]), | ||
# User and group do not exist, but are ints | ||
("23746:23746", 23746, 23746, None, []), | ||
("23746:23746", 23746, 23746, None, [23746]), |
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.
Neat!
Closes #18