Repository: jupyter/nbgrader

Issue:
Make it clearer how to set permissions for the exchange directory
If the exchange directory hasn't been created when `nbgrader release` is run for the first time, an error occurs:

```
$ nbgrader release --Exchange.root=/tmp/exchange ps1
[ReleaseApp | WARNING] No nbgrader_config.py file found (rerun with --debug to see where nbgrader is looking)
[ReleaseApp | CRITICAL] Unwritable directory, please contact your instructor: /tmp/exchange
[ReleaseApp | ERROR] nbgrader release failed
```

This is confusing and not helpful for instructors. Instead, if the exchange directory doesn't exist, `nbgrader release` should create the directory for instructors and set it to have the correct permissions.

Discussion:

Patch:
diff --git a/nbgrader/apps/baseapp.py b/nbgrader/apps/baseapp.py
index ab115149..825cc8ae 100644
--- a/nbgrader/apps/baseapp.py
+++ b/nbgrader/apps/baseapp.py
@@ -211,17 +211,6 @@ class NbGrader(JupyterApp):
             cfg.Exchange.merge(cfg.TransferApp)
             del cfg.TransferApp
 
-        if 'BaseNbConvertApp' in cfg:
-            self.log.warning(
-                "Use BaseConverter in config, not BaseNbConvertApp. Outdated config:\n%s",
-                '\n'.join(
-                    'BaseNbConvertApp.{key} = {value!r}'.format(key=key, value=value)
-                    for key, value in cfg.BaseNbConvertApp.items()
-                )
-            )
-            cfg.BaseConverter.merge(cfg.BaseNbConvertApp)
-            del cfg.BaseNbConvertApp
-
         super(NbGrader, self)._load_config(cfg, **kwargs)
         if self.coursedir:
             self.coursedir._load_config(cfg)
diff --git a/nbgrader/apps/nbgraderapp.py b/nbgrader/apps/nbgraderapp.py
index 23e84b3e..c5c22f8c 100755
--- a/nbgrader/apps/nbgraderapp.py
+++ b/nbgrader/apps/nbgraderapp.py
@@ -15,7 +15,6 @@ from .. import preprocessors
 from .. import plugins
 from ..coursedir import CourseDirectory
 from .. import exchange
-from .. import converters
 from .baseapp import nbgrader_aliases, nbgrader_flags
 from . import (
     NbGrader,
@@ -267,12 +266,6 @@ class NbGraderApp(NbGrader):
             if hasattr(ex, "class_traits") and ex.class_traits(config=True):
                 classes.append(ex)
 
-        # include all the converters
-        for ex_name in converters.__all__:
-            ex = getattr(converters, ex_name)
-            if hasattr(ex, "class_traits") and ex.class_traits(config=True):
-                classes.append(ex)
-
         return classes
 
     @catch_config_error
diff --git a/nbgrader/exchange/release.py b/nbgrader/exchange/release.py
index 6d1c89ca..3760d84a 100644
--- a/nbgrader/exchange/release.py
+++ b/nbgrader/exchange/release.py
@@ -4,7 +4,7 @@ from stat import (
     S_IRUSR, S_IWUSR, S_IXUSR,
     S_IRGRP, S_IWGRP, S_IXGRP,
     S_IROTH, S_IWOTH, S_IXOTH,
-    S_ISGID
+    S_ISGID, ST_MODE
 )
 
 from traitlets import Bool
@@ -17,6 +17,30 @@ 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
+
+        # if root doesn't exist, create it and set permissions
+        if not os.path.exists(self.root):
+            self.log.warning("Creating exchange directory: {}".format(self.root))
+            try:
+                os.makedirs(self.root)
+                os.chmod(self.root, perms)
+            except PermissionError:
+                self.fail("Could not create {}, permission denied.".format(self.root))
+
+        else:
+            old_perms = oct(os.stat(self.root)[ST_MODE] & 0o777)
+            new_perms = oct(perms & 0o777)
+            if old_perms != new_perms:
+                self.log.warning(
+                    "Permissions for exchange directory ({}) are invalid, changing them from {} to {}".format(
+                        self.root, old_perms, new_perms))
+                try:
+                    os.chmod(self.root, perms)
+                except PermissionError:
+                    self.fail("Could not change permissions of {}, permission denied.".format(self.root))
+
     def init_src(self):
         self.src_path = self.coursedir.format_path(self.coursedir.release_directory, '.', self.coursedir.assignment_id)
         if not os.path.isdir(self.src_path):


Test patch:
diff --git a/nbgrader/tests/apps/test_nbgrader_release.py b/nbgrader/tests/apps/test_nbgrader_release.py
index 93db81cb..0d8bf2dc 100644
--- a/nbgrader/tests/apps/test_nbgrader_release.py
+++ b/nbgrader/tests/apps/test_nbgrader_release.py
@@ -1,4 +1,6 @@
 import os
+import shutil
+import stat
 from os.path import join
 
 from .. import run_nbgrader
@@ -55,3 +57,16 @@ class TestNbGraderRelease(BaseTestApp):
         self._copy_file(join("files", "test.ipynb"), join(course_dir, "release", "ps1", "p1.ipynb"))
         self._release("--assignment=ps1", exchange)
         assert os.path.isfile(join(exchange, "abc101", "outbound", "ps1", "p1.ipynb"))
+
+    def test_no_exchange(self, exchange, course_dir):
+        shutil.rmtree(exchange)
+        self._copy_file(join("files", "test.ipynb"), join(course_dir, "release", "ps1", "p1.ipynb"))
+        self._release("--assignment=ps1", exchange)
+        assert os.path.isfile(join(exchange, "abc101", "outbound", "ps1", "p1.ipynb"))
+
+    def test_exchange_bad_perms(self, exchange, course_dir):
+        perms = stat.S_IRUSR|stat.S_IWUSR|stat.S_IXUSR|stat.S_IRGRP
+        os.chmod(exchange, perms)
+        self._copy_file(join("files", "test.ipynb"), join(course_dir, "release", "ps1", "p1.ipynb"))
+        self._release("--assignment=ps1", exchange)
+        assert os.path.isfile(join(exchange, "abc101", "outbound", "ps1", "p1.ipynb"))