fix docker volumn mount + notebook dir for admin #88
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #88 +/- ##
=======================================
Coverage 72.48% 72.48%
=======================================
Files 5 5
Lines 229 229
=======================================
Hits 166 166
Misses 63 63 ☔ View full report in Codecov by Sentry. |
| @@ -206,7 +206,8 @@ def _configure_notebook_dir(self, username: str, user_dir: Path): | |||
| """ | |||
| if self.user.admin: | |||
| self.log.info(f'Admin user detected: {username}. Setting up admin workspace.') | |||
| self.notebook_dir = '/cdm_shared_workspace' | |||
| # global users' home directory | |||
| self.notebook_dir = str(user_dir.parent) | |||
There was a problem hiding this comment.
So basically admin notebooks are all shared and live in /home and user notebooks live in /home/{user}? And users can read /home/{user} but not home?
There was a problem hiding this comment.
hmm. Maybe we should use /home/{user} for all? Now I am thinking, admin user actually don't have access to user's home dir. I was thinking only a handful person (tech team or maybe just spark_user) should be admin, and assign groups for permissions. Admin user won't create notebooks.
There was a problem hiding this comment.
I think str('/') makes more sense for admin user.
There was a problem hiding this comment.
👍 just changed it to '/' for now. I think most majority users won't be admin.
There was a problem hiding this comment.
That means any notebooks they create won't be persisted over a container recreation, I assume
There was a problem hiding this comment.
yea yea. I think they (or most likely just us) just need to remember to create notebook in the jupyterhub/users_home dir.
| @pytest.mark.parametrize("is_admin", [ | ||
| True, # Admin user case | ||
| False # Non-admin user case | ||
| ]) | ||
| def test_configure_notebook_dir(is_admin, expected_dir, spawner, caplog): | ||
| def test_configure_notebook_dir(is_admin, spawner, caplog): | ||
| spawner.user.admin = is_admin | ||
| username = 'testuser' |
There was a problem hiding this comment.
This would probably be simpler as just a loop at this point, like
for b in [True, False]:
spawner.user.admin = b
...
No description provided.