Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

avoid creating asyncio.Lock at import time #935

Merged
merged 1 commit into from
Jul 29, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 23 additions & 18 deletions jupyter_server/services/nbconvert/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,16 @@
AUTH_RESOURCE = "nbconvert"


LOCK = asyncio.Lock()


class NbconvertRootHandler(APIHandler):
auth_resource = AUTH_RESOURCE
_exporter_lock: asyncio.Lock

def initialize(self, **kwargs):
super().initialize(**kwargs)
# share lock across instances of this handler class
if not hasattr(self.__class__, "_exporter_lock"):
self.__class__._exporter_lock = asyncio.Lock()
self._exporter_lock = self.__class__._exporter_lock

@web.authenticated
@authorized
Expand All @@ -28,22 +33,22 @@ async def get(self):
# Some exporters use the filesystem when instantiating, delegate that
# to a thread so we don't block the event loop for it.
exporters = await run_sync(base.get_export_names)
for exporter_name in exporters:
try:
async with LOCK:
async with self._exporter_lock:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved acquisition of the lock outside the loop so we aren't acquiring and re-acquiring the same lock for each exporter.

for exporter_name in exporters:
try:
exporter_class = await run_sync(base.get_exporter, exporter_name)
except ValueError:
# I think the only way this will happen is if the entrypoint
# is uninstalled while this method is running
continue
# XXX: According to the docs, it looks like this should be set to None
# if the exporter shouldn't be exposed to the front-end and a friendly
# name if it should. However, none of the built-in exports have it defined.
# if not exporter_class.export_from_notebook:
# continue
res[exporter_name] = {
"output_mimetype": exporter_class.output_mimetype,
}
except ValueError:
# I think the only way this will happen is if the entrypoint
# is uninstalled while this method is running
continue
# XXX: According to the docs, it looks like this should be set to None
# if the exporter shouldn't be exposed to the front-end and a friendly
# name if it should. However, none of the built-in exports have it defined.
# if not exporter_class.export_from_notebook:
# continue
res[exporter_name] = {
"output_mimetype": exporter_class.output_mimetype,
}

self.finish(json.dumps(res))

Expand Down