Skip to content

Commit

Permalink
Check ContentsManager base class using inspect (#1254)
Browse files Browse the repository at this point in the history
* feat: Verify base contents class using inspect
* If AsyncContentsManager is found, use jupytext contents manager.
* test: Use latest jupyter-fs==1.0.0 in unit tests
* jupytext==1.0.0 ships sync metamanager along with async. We use sync version in tests
* ci: Uninstall jupyter-fs before browser test
* jupyter-fs is overriding the browser test that check its own functionality. We should, therefore, remove it and check for jupytext's functionality
* docs: Add a note about jupytext and jupyter-fs compatbility

Signed-off-by: Mahendra Paipuri <mahendra.paipuri@gmail.com>
Co-authored-by: Marc Wouts <marc.wouts@gmail.com>
  • Loading branch information
mahendrapaipuri and mwouts authored Jul 9, 2024
1 parent d3d1a3e commit 4594d86
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 24 deletions.
12 changes: 9 additions & 3 deletions .github/workflows/step_tests-conda.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,22 @@ jobs:
- name: Install a Jupyter Kernel
run: python -m ipykernel install --name jupytext-ci --user

- name: Test with pytest
run: pytest -n logical --cov --cov-report=xml

- name: Test lab extension
run: |
# Uninstall jupyter-fs as it overrides the original browser-test.js to
# check its own functionality (https://github.com/jpmorganchase/jupyter-fs/blob/main/jupyterfs/browser-test.js)
# So uninstall jupyter-fs before running browser check
#
pip uninstall jupyter-fs -y
# Check extension
jupyter labextension list
jupyter labextension list 2>&1 | grep -ie "jupyterlab-jupytext.*OK"
python -m jupyterlab.browser_check
- name: Test with pytest
run: pytest -n logical --cov --cov-report=xml

- name: Upload coverage
uses: codecov/codecov-action@v3
with:
Expand Down
14 changes: 10 additions & 4 deletions .github/workflows/step_tests-pip.yml
Original file line number Diff line number Diff line change
Expand Up @@ -82,17 +82,23 @@ jobs:
# install it
sudo apt install ./*.deb
- name: Test with pytest
continue-on-error: ${{ matrix.experimental }}
run: pytest -n logical --cov --cov-report=xml

- name: Test lab extension
run: |
# Uninstall jupyter-fs as it overrides the original browser-test.js to
# check its own functionality (https://github.com/jpmorganchase/jupyter-fs/blob/main/jupyterfs/browser-test.js)
# So uninstall jupyter-fs before running browser check
#
pip uninstall jupyter-fs -y
# Check extension
jupyter labextension list
jupyter labextension list 2>&1 | grep -ie "jupyterlab-jupytext.*OK"
python -m jupyterlab.browser_check
- name: Test with pytest
continue-on-error: ${{ matrix.experimental }}
run: pytest -n logical --cov --cov-report=xml

- name: Upload coverage
uses: codecov/codecov-action@v3
with:
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ Jupytext ChangeLog

**Fixed**
- We have fixed a typo when `build_jupytext_contents_manager_class` can't be imported ([#1162](https://github.com/mwouts/jupytext/issues/1162))
- We use `inspect` to determine whether the current contents manager derives from `AsyncContentsManager` (which is not
supported by Jupytext at the moment). This fixes a compatibility issue with `jupyter-fs==1.0.0` ([#1239](https://github.com/mwouts/jupytext/issues/1239)). Thanks to [Mahendra Paipuri](https://github.com/mahendrapaipuri) for this PR!
- Some dependencies of the JupyterLab extensions were updated ([#1243](https://github.com/mwouts/jupytext/issues/1243), [#1245](https://github.com/mwouts/jupytext/issues/1245))

**Added**
Expand Down
11 changes: 11 additions & 0 deletions docs/install.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,17 @@ If you don't have the notebook icon on text documents after a fresh restart of y
jupyter serverextension enable jupytext
```

When [`jupyter-fs>=1.0.0`](https://github.com/jpmorganchase/jupyter-fs) is being used along with `jupytext`, use `SyncMetaManager` as the contents manager for `jupyter-fs` as `jupytext` do not support async contents manager which is used in default `MetaManager` of `jupyter-fs`. The `jupyter-fs` config must be as follows:

```json
{
"ServerApp": {
"contents_manager_class": "jupyterfs.metamanager.SyncMetaManager",
}
}
```
so that `jupytext` will create its own contents manager derived from `SyncMetaManager`.

## Jupytext commands in JupyterLab

Jupytext comes with a frontend extension for JupyterLab which provides pairing commands (accessible with View / Activate Command Palette, or Ctrl+Shift+C):
Expand Down
36 changes: 23 additions & 13 deletions jupyterlab/jupyterlab_jupytext/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
"""Jupyter server and lab extension entry points"""

import inspect

import jupyter_server

from jupytext.reraise import reraise

try:
Expand All @@ -20,22 +24,28 @@ def load_jupyter_server_extension(app): # pragma: no cover
# The server extension call is too late!
# The contents manager was set at NotebookApp.init_configurables

# Let's change the contents manager class
# Get a list of all base classes and check if they contain AsyncContentsManager
base_class = app.contents_manager_class
if "async" in base_class.__name__.lower():
app.log.warning(
"[Jupytext Server Extension] Async contents managers like {} "
"are not supported at the moment "
"(https://github.com/mwouts/jupytext/issues/1020). "
"We will derive a contents manager from LargeFileManager instead.".format(
base_class.__name__
parent_classes = inspect.getmro(base_class)
for parent_class in parent_classes:
if (
parent_class
== jupyter_server.services.contents.manager.AsyncContentsManager
):
app.log.warning(
"[Jupytext Server Extension] Async contents managers like {} "
"are not supported at the moment "
"(https://github.com/mwouts/jupytext/issues/1020). "
"We will derive a contents manager from LargeFileManager instead.".format(
parent_class.__name__
)
)
from jupyter_server.services.contents.largefilemanager import ( # noqa
LargeFileManager,
)
)
from jupyter_server.services.contents.largefilemanager import ( # noqa
LargeFileManager,
)

base_class = LargeFileManager
base_class = LargeFileManager
break

app.log.info(
"[Jupytext Server Extension] Deriving a JupytextContentsManager "
Expand Down
3 changes: 1 addition & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,7 @@ test-external = [
"gitpython",
"pre-commit",
# Interaction with other contents managers
# jupyter-fs==0.4.0 is async, which is not supported by Jupytext ATM
"jupyter-fs<0.4.0"
"jupyter-fs>=1.0"
]
# Coverage requirements
test-cov = [
Expand Down
4 changes: 2 additions & 2 deletions tests/external/jupyter_fs/test_jupyter_fs.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@

def fs_meta_manager(tmpdir):
try:
from jupyterfs.metamanager import MetaManager
from jupyterfs.metamanager import SyncMetaManager
except ImportError:
pytest.skip("jupyterfs is not available")

cm_class = jupytext.build_jupytext_contents_manager_class(MetaManager)
cm_class = jupytext.build_jupytext_contents_manager_class(SyncMetaManager)
logger = logging.getLogger("jupyter-fs")
cm = cm_class(parent=None, log=logger)
cm.initResource(
Expand Down

0 comments on commit 4594d86

Please sign in to comment.