Remove duplicate groupshared option by adding to CourseDirectory

- Thanks to suggestion from @jhamrick

nbgrader/converters/assign.py

@@ -41,7 +41,7 @@ class Assign(BaseConverter):
 
     @default("permissions")
     def _permissions_default(self):
-        return 664 if self.groupshared else 644
+        return 664 if self.coursedir.groupshared else 644
 
     @property
     def _input_directory(self):

nbgrader/converters/base.py

@@ -43,20 +43,9 @@ class BaseConverter(LoggingConfigurable):
         )
     ).tag(config=True)
 
-    groupshared = Bool(
-        False,
-        help=dedent(
-            """
-            Be less strict about user permissions (instructor files are by
-            default group writeable.  Requires that admins ensure that primary
-            groups are correct!
-            """
-        )
-    ).tag(config=True)
-
     @default("permissions")
     def _permissions_default(self):
-        return 664 if self.groupshared else 644
+        return 664 if self.coursedir.groupshared else 644
 
 
     coursedir = Instance(CourseDirectory, allow_none=True)
@@ -261,7 +250,7 @@ def set_permissions(self, assignment_id, student_id):
             for filename in filenames:
                 os.chmod(os.path.join(dirname, filename), permissions)
             # If groupshared, set dir permissions - see comment below.
-            if self.groupshared and os.stat(dirname).st_uid == os.getuid():
+            if self.coursedir.groupshared and os.stat(dirname).st_uid == os.getuid():
                 #for subdirname in subdirnames:
                 #    subdirname = os.path.join(dirname, subdirname)
                 try:   os.chmod(dirname, (os.stat(dirname).st_mode|0o2770) & 0o2777)
@@ -271,7 +260,7 @@ def set_permissions(self, assignment_id, student_id):
         # nbconvert.writer, (unless there are supplementary files) with a
         # default mode of 755 and there is no way to pass the mode arguments
         # all the way to there!  So we have to walk and fix.
-        if self.groupshared:
+        if self.coursedir.groupshared:
             # Root may be created in this step, and is not included above.
             rootdir = self.coursedir.format_path(self._output_directory, '.', '.')
             # Add 2770 to existing dir permissions (don't unconditionally override)

nbgrader/coursedir.py

@@ -4,7 +4,7 @@
 from textwrap import dedent
 
 from traitlets.config import LoggingConfigurable
-from traitlets import Unicode, List, default, validate, TraitError
+from traitlets import Unicode, List, Bool, default, validate, TraitError
 
 from .utils import full_split, parse_utc
 
@@ -218,6 +218,17 @@ def _root_default(self):
         )
     ).tag(config=True)
 
+    groupshared = Bool(
+        False,
+        help=dedent(
+            """
+            Be less strict about user permissions (instructor files are by
+            default group writeable).  Requires that admins ensure that primary
+            groups are correct!
+            """
+        )
+    ).tag(config=True)
+
     def format_path(self, nbgrader_step, student_id, assignment_id, escape=False):
         kwargs = dict(
             nbgrader_step=nbgrader_step,

nbgrader/exchange/exchange.py

@@ -76,18 +76,6 @@ def _cache_default(self):
 
     coursedir = Instance(CourseDirectory, allow_none=True)
 
-    groupshared = Bool(
-        False,
-        help=dedent(
-            """
-            Be less strict about user permissions (instructor files are by
-            default group writeable.  Requires that admins ensure that primary
-            groups are correct!
-            """
-        )
-    ).tag(config=True)
-
-
     def __init__(self, coursedir=None, **kwargs):
         self.coursedir = coursedir
         super(Exchange, self).__init__(**kwargs)
@@ -135,7 +123,7 @@ def do_copy(self, src, dest):
         shutil.copytree(src, dest, ignore=shutil.ignore_patterns(*self.coursedir.ignore))
         # copytree copies access mode too - so we must add go+rw back to it if
         # we are in groupshared.
-        if self.groupshared:
+        if self.coursedir.groupshared:
             for dirname, _, filenames in os.walk(dest):
                 # dirs become ug+rwx
                 try:   os.chmod(dirname, (os.stat(dirname).st_mode|0o2770) & 0o2777)
@@ -150,7 +138,7 @@ def start(self):
         if sys.platform == 'win32':
             self.fail("Sorry, the exchange is not available on Windows.")
 
-        if not self.groupshared:
+        if not self.coursedir.groupshared:
             # This just makes sure that directory is o+rwx.  In group shared
             # case, it is up to admins to ensure that instructors can write
             # there.

nbgrader/exchange/release.py

@@ -18,7 +18,7 @@ class ExchangeRelease(Exchange):
     force = Bool(False, help="Force overwrite existing files in the exchange.").tag(config=True)
 
     def ensure_root(self):
-        perms = S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IWGRP|S_IXGRP|S_IROTH|S_IWOTH|S_IXOTH|((S_IWGRP|S_ISGID) if self.groupshared else 0)
+        perms = S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IWGRP|S_IXGRP|S_IROTH|S_IWOTH|S_IXOTH|((S_IWGRP|S_ISGID) if self.coursedir.groupshared else 0)
 
         # if root doesn't exist, create it and set permissions
         if not os.path.exists(self.root):
@@ -66,19 +66,19 @@ def init_dest(self):
         # groupshared: +2040
         self.ensure_directory(
             self.course_path,
-            S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH|((S_ISGID|S_IWGRP) if self.groupshared else 0)
+            S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH|((S_ISGID|S_IWGRP) if self.coursedir.groupshared else 0)
         )
         # 0755
         # groupshared: +2040
         self.ensure_directory(
             self.outbound_path,
-            S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH|((S_ISGID|S_IWGRP) if self.groupshared else 0)
+            S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH|((S_ISGID|S_IWGRP) if self.coursedir.groupshared else 0)
         )
         # 0733 with set GID so student submission will have the instructors group
         # groupshared: +0040
         self.ensure_directory(
             self.inbound_path,
-            S_ISGID|S_IRUSR|S_IWUSR|S_IXUSR|S_IWGRP|S_IXGRP|S_IWOTH|S_IXOTH|(S_IRGRP if self.groupshared else 0)
+            S_ISGID|S_IRUSR|S_IWUSR|S_IXUSR|S_IWGRP|S_IXGRP|S_IWOTH|S_IXOTH|(S_IRGRP if self.coursedir.groupshared else 0)
         )
 
     def ensure_directory(self, path, mode):
@@ -89,7 +89,7 @@ def ensure_directory(self, path, mode):
             # so we have to create and then chmod.
             os.chmod(path, mode)
         else:
-            if not self.groupshared and not self_owned(path):
+            if not self.coursedir.groupshared and not self_owned(path):
                 self.fail("You don't own the directory: {}".format(path))
 
     def copy_files(self):
@@ -108,6 +108,6 @@ def copy_files(self):
         self.do_copy(self.src_path, self.dest_path)
         self.set_perms(
             self.dest_path,
-            fileperms=(S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH|(S_IWGRP if self.groupshared else 0)),
-            dirperms=(S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH|((S_ISGID|S_IWGRP) if self.groupshared else 0)))
+            fileperms=(S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH|(S_IWGRP if self.coursedir.groupshared else 0)),
+            dirperms=(S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH|((S_ISGID|S_IWGRP) if self.coursedir.groupshared else 0)))
         self.log.info("Released as: {} {}".format(self.course_id, self.coursedir.assignment_id))