Allow get_export_names to skip configuration check #1471
Conversation
| @@ -135,6 +136,9 @@ 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"): | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.")
|
@MSeal is there anything I need to do to help move this forward? |
|
@rmoe Sorry I originally missed this PR (was on leave). Let me leave a comment in the implementation, but I think a quick log message and adding a note to the documentation is all that's needed. |
No worries, thank you for taking a look at this. |
|
This pull request has been mentioned on Jupyter Community Forum. There might be relevant details there: https://discourse.jupyter.org/t/current-state-of-nbconvert-drop-support-for-python-3-6/8882/1 |
|
@rmoe CI should be fixed on |
This introduces an environment variable named NBCONVERT_DISABLE_CONFIG_EXPORTERS that will cause get_export_names to return all entrypoints instead of checking the "enabled" status of each one. Closes: jupyter#1439
| 'Exporter': {'enabled': False}, | ||
| 'NotebookExporter': {'enabled': True} | ||
| }) | ||
| os.environ["NBCONVERT_DISABLE_CONFIG_EXPORTERS"] = "1" |
There was a problem hiding this comment.
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
Co-authored-by: Matt Riedemann <mriedem.os@gmail.com>
|
doc build issue is unrelated, I'll look into that before the next release. |
This introduces an environment variable named
NBCONVERT_DISABLE_CONFIG_EXPORTERS that will cause get_export_names to
return all entrypoints instead of checking the "enabled" status of
each one.
Closes: #1439