register contents_manager.files_handler_class directly #2917
Conversation
notebook/files/handlers.py
Outdated
| Normally used when ContentsManager is not a FileContentsManager. | ||
|
|
||
| FileContentsManager subclasses use AuthenticatedFilesManager by default, | ||
| a subclass of StaticFileManager. |
There was a problem hiding this comment.
"AuthenticatedFilesManager" and "StaticFileManager" should be "...Handler", I think?
There was a problem hiding this comment.
I got Managers on the mind. Fixed.
| @@ -299,7 +299,8 @@ def init_handlers(self, settings): | |||
| handlers.extend(load_handlers('services.kernelspecs.handlers')) | |||
| handlers.extend(load_handlers('services.security.handlers')) | |||
| handlers.extend(load_handlers('services.shutdown')) | |||
|
|
|||
| handlers.extend(settings['contents_manager'].get_extra_handlers()) | |||
There was a problem hiding this comment.
This is being added after the default files handler (the one which uses the contents manager). Doesn't that mean that this one (using static files) will never actually be used?
There was a problem hiding this comment.
I removed the global registration of the default files handler here, so this is where it gets registered, whichever class it is. On the base ContentsManager, it will be the generic FilesHandler, but on FileContentsManager subclasses it will be AuthenticatedFileHandler.
There was a problem hiding this comment.
The relevant change below is that the base ContentsManager has files_handler_class = FilesHandler by default, so get_extra_handlers always returns something unless a ContentsManager explicitly overrides files_handler_class = None, in which case the /files/ endpoint is disabled.
|
Thank you @minrk, just for the sake of curiosity, do you know why this could cause more issue on Safari? Thanks |
rather than trying to call one handler from another, which is unreliable and can cause misbehavior.
minrk
force-pushed
the
fix-auth-file-handler
branch
from
618b99e
to
a69ddb6
Compare
|
@ebraminio I have no idea why I can only reproduce this in Safari. My guess is that it's a coincidence of how it implements fetching and/or caching of requests that makes it more likely to trigger. |
| @@ -299,7 +299,8 @@ def init_handlers(self, settings): | |||
| handlers.extend(load_handlers('services.kernelspecs.handlers')) | |||
| handlers.extend(load_handlers('services.security.handlers')) | |||
| handlers.extend(load_handlers('services.shutdown')) | |||
|
|
|||
| handlers.extend(settings['contents_manager'].get_extra_handlers()) | |||
|
Confirmed the fix, thanks! |
rather than trying to call one handler from another, which is unreliable and can cause misbehavior.
closes jupyter/jupyter#292