Fix #53, Fix #56: Don't corrupt notebook.json when no oauth_client_id #57
Conversation
|
Also see related change to make this kind of thing less severe on the notebook side: jupyter/notebook#3305 |
Adds Python 3.6, drops Python 3.3 Fixes version conflict around flake8 Adds tornado (which wasn't getting installed automatically into pypy) Use different Travis Python environments
mdboom
force-pushed
the
fix-oath-client-id
branch
from
6907c2f
to
0d28456
Compare
Codecov Report
@@ Coverage Diff @@
## master #57 +/- ##
==========================================
- Coverage 63.33% 61.41% -1.93%
==========================================
Files 3 3
Lines 180 184 +4
Branches 25 26 +1
==========================================
- Hits 114 113 -1
- Misses 64 68 +4
- Partials 2 3 +1
Continue to review full report at Codecov.
|
mdboom
force-pushed
the
fix-oath-client-id
branch
from
e4f9345
to
0d28456
Compare
…auth_client_id When there is no `c.NotebookGist.oauth_client_id` in jupyter_notebook_config.py, `self.config.NotebookGist.oauth_client_id` is a lazy value, which can't be serialized to JSON. This crashes the writing of `notebook.json` by Jupyter, and corrupts the `notebook.json` file. This fix makes sure that value is a string, otherwise it just sets it to `None`. When `None`, the frontend will helpfully warn (as it always has) that the user hasn't set up their oauth ids.
mdboom
force-pushed
the
fix-oath-client-id
branch
from
0d28456
to
c175659
Compare
src/jupyter_notebook_gist/config.py
Outdated
| @@ -19,6 +19,9 @@ def __init__(self, *args, **kwargs): | |||
| super(NotebookGist, self).__init__(*args, **kwargs) | |||
| # update the frontend settings with the currently passed | |||
| # OAUTH client id | |||
| client_id = self.config.NotebookGist.oauth_client_id | |||
| if not isinstance(client_id, (str, bytes)): | |||
There was a problem hiding this comment.
This won't work in case the client id is a unicode value on Python 2.x (since str == bytes there) and fail with a hard to catch thing. The typical solution is to use basestring on 2.x but that's gone on 3.x. We could use six for this, but I guess that seems like a lot of code to rely on just for this silly thing, so lets do something like this here instead. You could also add it to a _compat.py or something.
# ...
try:
basestring
except NameError:
basestring = str
# ...
if not isinstance(client_id, basestring):
# ...Of course the real fix is to stop supporting Python 2.x. 🤡
There was a problem hiding this comment.
Ah, yes. Of course, six is a dependency of Jupyter notebook, and this package of course makes no sense without the notebook, so I think it's safe to rely on it.
There was a problem hiding this comment.
Good point! That means we'll have to list it explicitly in the setup.py install_requires list though, just in case notebook ever stops using six for compatibility.
mdboom
force-pushed
the
fix-oath-client-id
branch
from
091ef48
to
f4abd1b
Compare
mdboom
force-pushed
the
fix-oath-client-id
branch
from
f4abd1b
to
0d2816e
Compare
When there is no
c.NotebookGist.oauth_client_idin jupyter_notebook_config.py,self.config.NotebookGist.oauth_client_idis a lazy value, which can't beserialized to JSON. This crashes the writing of
notebook.jsonby Jupyter,and corrupts the
notebook.jsonfile. This fix makes sure that value is astring, otherwise it just sets it to
None. WhenNone, the frontend willhelpfully warn (as it always has) that the user hasn't set up their oauth ids.
r? @jezdez