add create system user #79
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #79 +/- ##
==========================================
+ Coverage 49.60% 57.14% +7.54%
==========================================
Files 5 5
Lines 125 154 +29
==========================================
+ Hits 62 88 +26
- Misses 63 66 +3 ☔ View full report in Codecov by Sentry. |
| useradd_cmd = ['sudo', 'useradd', '-r'] | ||
|
|
||
| if group: |
There was a problem hiding this comment.
What is the plan around adding all the users to the jupyterhub group?
There was a problem hiding this comment.
I don’t have any. We are not there yet. Maybe assign group by OAuth or some type of user DB.
There was a problem hiding this comment.
Ok, so why does the code add the users to the group then?
There was a problem hiding this comment.
think it's good to do so. Otherwise we end up with users with no group.
There was a problem hiding this comment.
Ok, so this is an add a group because users need a group type thing. But that also means that users will be able to read and write each other's files by default unless they chmod 700 them. Is that the desired behavior?
There was a problem hiding this comment.
That's how NERSC does. KBase group members can read each other's files.
There was a problem hiding this comment.
Ok, if that's intentional 👍
I presume we'd have to change this if non-kbase users started using this system, but that's a ways off
| def mock_spawner(): | ||
| spawner = VirtualEnvSpawner() | ||
| return spawner |
There was a problem hiding this comment.
Seems like you could just get rid of this fixture and create it in the test methods, since there's no mocks any more
There was a problem hiding this comment.
oh yea. removed spawner fixture.
No description provided.