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.

Sign up for GitHub

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

Jump to bottom

Allow get_export_names to skip configuration check #1471

Merged
merged 2 commits into from
Apr 26, 2021
Merged

Allow get_export_names to skip configuration check #1471

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
46df060
Allow get_export_names to skip configuration check
rmoe Nov 20, 2020
aacba56
Update nbconvert/exporters/tests/test_exporter.py
rmoe Apr 23, 2021
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
4 changes: 4 additions & 0 deletions docs/autogen_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@
CLI Flags and Aliases
---------------------

The dynamic loading of exporters can be disabled by setting the environment
variable ``NBCONVERT_DISABLE_CONFIG_EXPORTERS``. This causes all exporters
to be loaded regardless of the value of their ``enabled`` attribute.

When using Nbconvert from the command line, a number of aliases and flags are
defined as shortcuts to configuration options for convience.

Expand Down
5 changes: 5 additions & 0 deletions nbconvert/exporters/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# Copyright (c) Jupyter Development Team.
# Distributed under the terms of the Modified BSD License.

import os
import warnings

import entrypoints
Expand Down Expand Up @@ -135,6 +136,10 @@ def get_export_names(config=get_config()):
them as an nbconvert.exporter entrypoint.
"""
exporters = sorted(entrypoints.get_group_named('nbconvert.exporters'))
if os.environ.get("NBCONVERT_DISABLE_CONFIG_EXPORTERS"):
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: might be good to have a comment here so people know what this is for, i.e. bypass the (potentially slow) enable/disable check per extension if you know that you're supporting all exporters.

I also wonder if there is somewhere in the docs or config options that we could mention this environment variable otherwise people would only ever know about it if they were familiar with this code. Like maybe somewhere in here:

https://github.com/jupyter/nbconvert/blob/master/docs/source/api/exporters.rst

Though that's just auto-generated module doc.

Looking at https://nbconvert.readthedocs.io/en/latest/config_options.html#exporter-options maybe Exporter.enabled should mention this env var? Or https://nbconvert.readthedocs.io/en/latest/external_exporters.html?

A CODEOWNER might have better ideas, but I'd think mentioning it with the Exporter.enabled config option would be the most discoverable place for mentioning this variable.

Copy link
Contributor

Choose a reason for hiding this comment

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

To @mriedem 's point, https://nbconvert.readthedocs.io/en/latest/config_options.html#exporter-options would be the best place to document the environment variable action. You can add this note by editing the CLI Flags and Aliases section in autogen_config.py.

Additionally we should probably log out a quick message when we're disabling config exporters so it's visible that the flag was set when helping future nbconvert issues. Use from traitlets.log import get_logger and get_logger().info("Config exporter loading disabled, no additional exporters will be automatically included.")

get_logger().info("Config exporter loading disabled, no additional exporters will be automatically included.")
return exporters

enabled_exporters = []
for exporter_name in exporters:
try:
Expand Down
31 changes: 30 additions & 1 deletion nbconvert/exporters/tests/test_exporter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@
#-----------------------------------------------------------------------------
# Imports
#-----------------------------------------------------------------------------

import os
from traitlets.config import Config

from unittest.mock import patch

from .base import ExportersTestsBase
from ...preprocessors.base import Preprocessor
from ..exporter import Exporter
Expand Down Expand Up @@ -73,3 +75,30 @@ def test_get_export_names_disable(self):
})
export_names = get_export_names(config=config)
self.assertEqual(export_names, ['notebook'])

def test_get_exporter_disable_config_exporters(self):
"""
Does get_export_names behave correctly with respect to
NBCONVERT_DISABLE_CONFIG_EXPORTERS being set in the
environment?
"""
config = Config({
'Exporter': {'enabled': False},
'NotebookExporter': {'enabled': True}
})
os.environ["NBCONVERT_DISABLE_CONFIG_EXPORTERS"] = "1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Fun fact, you can use patch.dict to temporarily set values in os.environ, there is an example in the docs:

https://docs.python.org/3.7/library/unittest.mock.html#unittest.mock.patch.dict

with patch("nbconvert.exporters.base.get_exporter") as exp:
export_names = get_export_names(config=config)
# get_export_names should not call get_exporter for
# any of the entry points because we return before then.
exp.assert_not_called()

# We should have all exporters, not just the ones
# enabled in the config
self.assertNotEqual(export_names, ['notebook'])

# In the absence of this variable we should revert to
# the normal behavior.
del os.environ["NBCONVERT_DISABLE_CONFIG_EXPORTERS"]
export_names = get_export_names(config=config)
self.assertEqual(export_names, ['notebook'])