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

NPE1Adapter Part 3 - caching of adapter manifests #126

Merged
merged 26 commits into from
Mar 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
e59b7c7
changes to _from_npe1 required for shim conversion
tlambert03 Mar 13, 2022
bbb530e
shim2 just the shim part
tlambert03 Mar 13, 2022
d41fea5
add tests
tlambert03 Mar 13, 2022
1a1716e
Merge branch 'shim1-conversion' into shim2-shim
tlambert03 Mar 13, 2022
3115e32
shim3 add caching
tlambert03 Mar 13, 2022
e6f507b
Merge branch 'main' into shim1-conversion
tlambert03 Mar 14, 2022
2559424
Merge branch 'shim1-conversion' into shim2-shim
tlambert03 Mar 14, 2022
3fa8b5b
Merge branch 'shim2-shim' into shim3-cache
tlambert03 Mar 14, 2022
976cbff
Merge branch 'main' into shim1-conversion
tlambert03 Mar 14, 2022
e1fd0e0
Merge branch 'main' into shim2-shim
tlambert03 Mar 14, 2022
19e1385
Merge branch 'shim2-shim' into shim3-cache
tlambert03 Mar 14, 2022
d6365c4
add test for failed import
tlambert03 Mar 14, 2022
2669e95
Merge branch 'shim2-shim' into shim3-cache
tlambert03 Mar 14, 2022
909f8c3
add appdirs
tlambert03 Mar 14, 2022
6bcf9dc
remove napari-plugin-engine
tlambert03 Mar 14, 2022
99bf394
add magic factory finding
tlambert03 Mar 16, 2022
667ee5e
add magicfactory
tlambert03 Mar 16, 2022
cea5d03
Merge branch 'shim1-conversion' into shim2-shim
tlambert03 Mar 16, 2022
e1a4df4
Merge branch 'main' into shim2-shim
tlambert03 Mar 23, 2022
1410fae
Merge branch 'main' into shim3-cache
tlambert03 Mar 23, 2022
950634f
rename to adapter
tlambert03 Mar 23, 2022
f739e16
more renaming and docs
tlambert03 Mar 23, 2022
bc9c5b9
no cover
tlambert03 Mar 23, 2022
8bbc532
Merge branch 'shim2-shim' into shim3-cache
tlambert03 Mar 23, 2022
8dd5562
Merge branch 'main' into shim3-cache
tlambert03 Mar 23, 2022
0ae93cc
more shim word changes
tlambert03 Mar 23, 2022
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: 2 additions & 2 deletions npe2/_command_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@

from psygnal import Signal

from .manifest import utils
from .manifest._validators import DOTTED_NAME_PATTERN
from .manifest.utils import import_python_name
from .types import PythonName

PDisposable = Callable[[], None]
Expand All @@ -30,7 +30,7 @@ def resolve(self) -> Callable:
raise RuntimeError("cannot resolve command without python_name")

try:
self.function = import_python_name(self.python_name)
self.function = utils.import_python_name(self.python_name)
except Exception as e:
raise RuntimeError(f"Failed to import command at {self.python_name!r}: {e}")
return self.function
Expand Down
57 changes: 56 additions & 1 deletion npe2/cli.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
import builtins
import warnings
from pathlib import Path
from textwrap import indent
from typing import Optional
from typing import List, Optional

import typer

from npe2 import PluginManifest

try:
from importlib.metadata import distribution
except ImportError:
from importlib_metadata import distribution # type: ignore

app = typer.Typer()


Expand Down Expand Up @@ -138,5 +144,54 @@ def convert(
)


@app.command()
def cache(
clear: Optional[bool] = typer.Option(
False, "--clear", "-d", help="Clear the npe1 adapter manifest cache"
),
names: List[str] = typer.Argument(
None, help="Name(s) of distributions to list/delete"
),
list_: Optional[bool] = typer.Option(
False, "--list", "-l", help="List cached manifests"
),
verbose: Optional[bool] = typer.Option(False, "--verbose", "-v", help="verbose"),
):
"""Cache utils"""
from npe2.manifest._npe1_adapter import ADAPTER_CACHE, clear_cache

if clear:
_cleared = clear_cache(names)
if _cleared:
nf = "\n".join(f" - {i.name}" for i in _cleared)
typer.secho("Cleared these files from cache:")
typer.secho(nf, fg=typer.colors.RED)
else:
msg = "Nothing to clear"
if names:
msg += f" for plugins: {','.join(names)}"
typer.secho(msg, fg=typer.colors.RED)

typer.Exit()
if list_:
files = builtins.list(ADAPTER_CACHE.glob("*.yaml"))
if names:
files = [f for f in files if any(f.name.startswith(n) for n in names)]

if not files:
if names:
typer.secho(f"Nothing cached for plugins: {','.join(names)}")
else:
typer.secho("Nothing cached")
typer.Exit()
for fname in files:
mf = PluginManifest.from_file(fname)
if verbose:
_pprint_yaml(mf.yaml()) # pragma: no cover
else:
d = distribution(mf.name)
typer.secho(f"{mf.name}: {d.version}", fg=typer.colors.GREEN)


def main():
app()
70 changes: 69 additions & 1 deletion npe2/manifest/_npe1_adapter.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
import logging
import os
import site
import warnings
from pathlib import Path
from shutil import rmtree
from typing import List, Sequence

from appdirs import user_cache_dir

from .._from_npe1 import manifest_from_npe1
from .package_metadata import PackageMetadata
Expand All @@ -12,6 +19,34 @@


logger = logging.getLogger(__name__)
ADAPTER_CACHE = Path(user_cache_dir("napari", "napari")) / "npe2" / "adapter_manifests"
NPE2_NOCACHE = "NPE2_NOCACHE"


def clear_cache(names: Sequence[str] = ()) -> List[Path]:
"""Clear cached NPE1Adapter manifests.

Parameters
----------
names : Sequence[str], optional
selection of plugin names to clear, by default, all will be cleared

Returns
-------
List[Path]
List of filepaths cleared
"""
_cleared: List[Path] = []
if ADAPTER_CACHE.exists():
if names:
for f in ADAPTER_CACHE.glob("*.yaml"):
if any(f.name.startswith(f"{n}_") for n in names):
f.unlink()
_cleared.append(f)
else:
_cleared = list(ADAPTER_CACHE.iterdir())
rmtree(ADAPTER_CACHE)
return _cleared


class NPE1Adapter(PluginManifest):
Expand Down Expand Up @@ -56,8 +91,14 @@ def __getattribute__(self, __name: str):
def _load_contributions(self) -> None:
"""import and inspect package contributions."""

self._is_loaded = True # if we fail once, we still don't try again.
if self._cache_path().exists() and not os.getenv(NPE2_NOCACHE):
mf = PluginManifest.from_file(self._cache_path())
self.contributions = mf.contributions
logger.debug("%r npe1 adapter loaded from cache", self.name)
return

with discovery_blocked():
self._is_loaded = True # if we fail once, we still don't try again.
try:
mf = manifest_from_npe1(self._dist, adapter=True)
except Exception as e:
Expand All @@ -69,3 +110,30 @@ def _load_contributions(self) -> None:

self.contributions = mf.contributions
logger.debug("%r npe1 adapter imported", self.name)

if not _is_editable_install(self._dist):
self._save_to_cache()

def _save_to_cache(self):
cache_path = self._cache_path()
cache_path.parent.mkdir(exist_ok=True, parents=True)
cache_path.write_text(self.yaml())

def _cache_path(self) -> Path:
"""Return cache path for manifest corresponding to distribution."""
return _cached_adapter_path(self.name, self.package_version or "")


def _cached_adapter_path(name: str, version: str) -> Path:
"""Return cache path for manifest corresponding to distribution."""
return ADAPTER_CACHE / f"{name}_{version}.yaml"


def _is_editable_install(dist: metadata.Distribution) -> bool:
"""Return True if dist is installed as editable.

i.e: if the package isn't in site-packages or user site-packages.
"""
root = str(dist.locate_file(""))
installed_paths = site.getsitepackages() + [site.getusersitepackages()]
return all(loc not in root for loc in installed_paths)
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ project_urls =
packages = find:
install_requires =
PyYAML
appdirs
magicgui>=0.3.3
napari-plugin-engine
psygnal>=0.3.0
pydantic
pytomlpp
Expand Down
8 changes: 8 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import pytest

from npe2 import PluginManager, PluginManifest
from npe2.manifest import _npe1_adapter

try:
from importlib import metadata
Expand Down Expand Up @@ -164,3 +165,10 @@ def _from_name(name):
new_manifest.unlink()
if (npe1_repo / "setup.py").exists():
(npe1_repo / "setup.py").unlink()


@pytest.fixture
def mock_cache(tmp_path, monkeypatch):
with monkeypatch.context() as m:
m.setattr(_npe1_adapter, "ADAPTER_CACHE", tmp_path)
yield tmp_path
1 change: 0 additions & 1 deletion tests/sample/my_plugin/napari.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ contributions:
- command: my-plugin.hello_world
mysubmenu:
- command: my-plugin.another_command
- command: my-plugin.affinder
submenus:
- id: mysubmenu
label: My SubMenu
Expand Down
41 changes: 41 additions & 0 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,44 @@ def test_cli_main(monkeypatch, sample_path):
with pytest.raises(SystemExit) as e:
main()
assert e.value.code == 0


def test_cli_cache_list_empty(mock_cache):
result = runner.invoke(app, ["cache", "--list"])
assert "Nothing cached" in result.stdout
assert result.exit_code == 0


def test_cli_cache_list_full(uses_npe1_plugin, mock_cache):
(mock_cache / "npe1-plugin.yaml").write_text("name: npe1-plugin\n")
result = runner.invoke(app, ["cache", "--list"])
assert result.stdout == "npe1-plugin: 0.1.0\n"
assert result.exit_code == 0


def test_cli_cache_list_named(uses_npe1_plugin, mock_cache):
(mock_cache / "npe1-plugin.yaml").write_text("name: npe1-plugin\n")
result = runner.invoke(app, ["cache", "--list", "not-a-plugin"])
assert result.stdout == "Nothing cached for plugins: not-a-plugin\n"
assert result.exit_code == 0


def test_cli_cache_clear_empty(mock_cache):
result = runner.invoke(app, ["cache", "--clear"])
assert "Nothing to clear" in result.stdout
assert result.exit_code == 0


def test_cli_cache_clear_full(mock_cache):
(mock_cache / "npe1-plugin.yaml").write_text("name: npe1-plugin\n")
result = runner.invoke(app, ["cache", "--clear"])
assert "Cleared these files from cache" in result.stdout
assert "- npe1-plugin.yaml" in result.stdout
assert result.exit_code == 0


def test_cli_cache_clear_named(mock_cache):
(mock_cache / "npe1-plugin.yaml").write_text("name: npe1-plugin\n")
result = runner.invoke(app, ["cache", "--clear", "not-a-plugin"])
assert result.stdout == "Nothing to clear for plugins: not-a-plugin\n"
assert result.exit_code == 0
56 changes: 51 additions & 5 deletions tests/test_npe1_adapter.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from functools import partial
from pathlib import Path
from unittest.mock import patch

import numpy as np
Expand All @@ -22,7 +23,7 @@ def test_adapter_no_npe1():
assert not pm._npe1_adapters


def test_npe1_adapter(uses_npe1_plugin):
def test_npe1_adapter(uses_npe1_plugin, mock_cache: Path):
"""Test that the plugin manager detects npe1 plugins, and can index contribs"""
pm = PluginManager()
pm.discover()
Expand All @@ -36,6 +37,8 @@ def test_npe1_adapter(uses_npe1_plugin):
assert mf.package_metadata.name == "npe1-plugin"

# it's currently unindexed and unstored
assert not mf._cache_path().exists()
assert not list(mock_cache.iterdir())

with patch.object(
_npe1_adapter,
Expand All @@ -47,11 +50,54 @@ def test_npe1_adapter(uses_npe1_plugin):
assert len(pm._npe1_adapters) == 0
# manifest_from_npe1 was called
mock.assert_called_once_with(mf._dist, adapter=True)
assert mf._cache_path().exists()
# NOTE: accessing the `.contributions` object would have also triggered
# importing, like pm.index_npe1_adapters() above, but it would not have
# injected the contributions into the pm._contrib object.
assert mf.contributions.sample_data

mock.reset_mock()
# clear and rediscover... this time we expect the cache to kick in
pm.discover(clear=True)
assert len(pm._npe1_adapters) == 1
pm.index_npe1_adapters()
assert len(pm._npe1_adapters) == 0
mock.assert_not_called()


def test_npe1_adapter_cache(uses_npe1_plugin, mock_cache: Path):
"""Test that we can clear cache, etc.."""
pm = PluginManager()
pm.discover()

with patch.object(
_npe1_adapter,
"manifest_from_npe1",
wraps=_npe1_adapter.manifest_from_npe1, # type: ignore
) as mock:

# if we clear the cache, it should import again
mf = pm.get_manifest("npe1-plugin")
assert isinstance(mf, _npe1_adapter.NPE1Adapter)
pm.index_npe1_adapters()
mock.assert_called_once_with(mf._dist, adapter=True)
assert mf._cache_path().exists()

_npe1_adapter.clear_cache()
assert not mf._cache_path().exists()

mock.reset_mock()
pm.discover(clear=True)
pm.index_npe1_adapters()
mf = pm.get_manifest("npe1-plugin")
assert isinstance(mf, _npe1_adapter.NPE1Adapter)
mock.assert_called_once_with(mf._dist, adapter=True)
assert mf._cache_path().exists()
_npe1_adapter.clear_cache(names=["not-our-plugin"])
assert mf._cache_path().exists()
_npe1_adapter.clear_cache(names=["npe1-plugin"])
assert not mf._cache_path().exists()


def _get_mf() -> _npe1_adapter.NPE1Adapter:
pm = PluginManager.instance()
Expand All @@ -62,7 +108,7 @@ def _get_mf() -> _npe1_adapter.NPE1Adapter:
return mf


def test_adapter_pyname_sample_data(uses_npe1_plugin):
def test_adapter_pyname_sample_data(uses_npe1_plugin, mock_cache):
"""Test that objects defined locally in npe1 hookspecs can be retrieved."""
mf = _get_mf()
samples = mf.contributions.sample_data
Expand All @@ -84,7 +130,7 @@ def test_adapter_pyname_sample_data(uses_npe1_plugin):
assert np.array_equal(func(), ONES)


def test_adapter_pyname_dock_widget(uses_npe1_plugin):
def test_adapter_pyname_dock_widget(uses_npe1_plugin, mock_cache):
"""Test that objects defined locally in npe1 hookspecs can be retrieved."""
mf = _get_mf()
widgets = mf.contributions.widgets
Expand Down Expand Up @@ -120,12 +166,12 @@ def read_text(self, filename):
def locate_file(self, *_):
...

shim = _npe1_adapter.NPE1Adapter(FakeDist())
adapter = _npe1_adapter.NPE1Adapter(FakeDist())

def err():
raise ImportError("No package found.")

with pytest.warns(UserWarning) as record:
with patch.object(_npe1_adapter, "manifest_from_npe1", wraps=err):
shim.contributions
adapter.contributions
assert "Error importing contributions for" in str(record[0])