Validate plugins removed from hub before marking them stale (#1301)

* Updating processor to sanity check deletion

* Adding Classifier Adapters

* Adding back Zulip credentials

* Fixing linting

* Fixing linting for napari-hub-commons

* Installing libffi7

* Updating ubuntu distros

* Updating tests

* Revert "Installing libffi7"

This reverts commit d9e4039.

* Revert "Updating ubuntu distros"

This reverts commit b9b3320.

* Revert "Adding back Zulip credentials"

This reverts commit 6597f45.

* Fix linting

data-workflows/plugin/classifier_adapter.py

@@ -0,0 +1,28 @@
+import logging
+from functools import cache
+
+from nhcommons.utils.adapter_helpers import GithubClientHelper
+
+
+REPOSITORY = "https://github.com/napari/npe2api"
+FILE_NAME = "public/classifiers.json"
+LOGGER = logging.getLogger(__name__)
+
+
+@cache
+def _get_recent_query_data() -> dict:
+    github_client_helper = GithubClientHelper(REPOSITORY)
+    data = github_client_helper.get_file(FILE_NAME, "json")
+    if not data:
+        raise RuntimeError(f"Unable to fetch {FILE_NAME} from {REPOSITORY}")
+    return data
+
+
+def is_plugin_active(name: str, version: str) -> bool:
+    try:
+        recent_query_update = _get_recent_query_data()
+        active_versions = set(recent_query_update.get("active", {}).get(name, []))
+        return version in active_versions
+    except RuntimeError:
+        LOGGER.warning(f"Returning {name} {version} is active due to RuntimeError")
+        return True

data-workflows/plugin/processor.py

@@ -2,6 +2,7 @@
 from concurrent import futures
 from typing import Optional
 
+from plugin.classifier_adapter import is_plugin_active
 from plugin.lambda_adapter import LambdaAdapter
 from nhcommons.models.plugin_utils import PluginMetadataType
 from nhcommons.utils import pypi_adapter
@@ -39,15 +40,23 @@ def _is_new_plugin(plugin_version_pair):
     # update for removed plugins and existing older version of plugins
     for name, version in dynamo_latest_plugins.items():
         pypi_plugin_version = pypi_latest_plugins.get(name)
-        if pypi_plugin_version != version:
-            logger.info(f"Updating old plugin={name} version={version}")
-            put_plugin_metadata(
-                plugin=name,
-                version=version,
-                plugin_metadata_type=PluginMetadataType.PYPI,
+        if pypi_plugin_version == version:
+            continue
+        if pypi_plugin_version is None and is_plugin_active(name, version):
+            logger.info(
+                f"Skipping marking plugin={name} version={version} stale as the "
+                f"plugin is still active in npe2api"
             )
-            if pypi_plugin_version is None:
-                zulip.plugin_no_longer_on_hub(name)
+            continue
+
+        logger.info(f"Updating old plugin={name} version={version}")
+        put_plugin_metadata(
+            plugin=name,
+            version=version,
+            plugin_metadata_type=PluginMetadataType.PYPI,
+        )
+        if pypi_plugin_version is None:
+            zulip.plugin_no_longer_on_hub(name)
 
 
 def _update_for_new_plugin(name: str, version: str, old_version: Optional[str]) -> None:

data-workflows/plugin/tests/test_classifier_adapter.py

@@ -0,0 +1,61 @@
+from unittest.mock import Mock, call
+
+import pytest
+
+import nhcommons.utils.adapter_helpers
+import plugin.classifier_adapter
+
+REPOSITORY = "https://github.com/napari/npe2api"
+FILE_NAME = "public/classifiers.json"
+
+
+class TestClassifierAdapter:
+    @pytest.fixture(autouse=True)
+    def clear_cache(self):
+        plugin.classifier_adapter._get_recent_query_data.cache_clear()
+
+    @pytest.fixture(autouse=True)
+    def github_client_helper(self, monkeypatch):
+        self._github_client_helper = Mock()
+        self._github_client_helper.get_file.side_effect = (
+            lambda _, __: self._classifier_json
+        )
+        self._github_client_helper_call = Mock(
+            spec=nhcommons.utils.adapter_helpers.GithubClientHelper,
+            return_value=self._github_client_helper,
+        )
+        monkeypatch.setattr(
+            plugin.classifier_adapter,
+            "GithubClientHelper",
+            self._github_client_helper_call,
+        )
+
+    def test_handle_valid_query_data(self):
+        self._classifier_json = {"active": {"foo": ["1.0.0", "1.0.1", "1.0.2"]}}
+        assert True == plugin.classifier_adapter.is_plugin_active("foo", "1.0.2")
+        assert False == plugin.classifier_adapter.is_plugin_active("foo", "0.0.9")
+        assert False == plugin.classifier_adapter.is_plugin_active("bar", "1.0.4")
+        self._github_client_helper_call.assert_called_once_with(REPOSITORY)
+        self._github_client_helper.get_file.assert_called_once_with(FILE_NAME, "json")
+
+    def test_handle_invalid_query_data(self):
+        self._classifier_json = {"inactive": []}
+        assert False == plugin.classifier_adapter.is_plugin_active("foo", "1.0.2")
+        assert False == plugin.classifier_adapter.is_plugin_active("bar", "1.0.2")
+        self._github_client_helper_call.assert_called_once_with(REPOSITORY)
+        self._github_client_helper.get_file.assert_called_once_with(FILE_NAME, "json")
+
+    @pytest.mark.parametrize("init_classifier_json", [None, {}])
+    def test_is_plugin_live_does_not_cache_error(self, init_classifier_json):
+        # When the classifier json is not a dict with values, _get_recent_query_data()
+        # should throw a RuntimeError
+        self._classifier_json = init_classifier_json
+        assert True == plugin.classifier_adapter.is_plugin_active("foo", "1.0.2")
+        self._classifier_json = {"active": {"foo": ["1.0.0", "1.0.1", "1.0.2"]}}
+        assert True == plugin.classifier_adapter.is_plugin_active("foo", "1.0.2")
+        self._github_client_helper_call.assert_has_calls(
+            [call(REPOSITORY), call(REPOSITORY)]
+        )
+        self._github_client_helper.get_file.assert_has_calls(
+            [call(FILE_NAME, "json"), call(FILE_NAME, "json")]
+        )

data-workflows/plugin/tests/test_processor.py

@@ -3,6 +3,7 @@
 import pytest
 
 import plugin.lambda_adapter
+import plugin.classifier_adapter
 import plugin.metadata
 from nhcommons.models.plugin_utils import PluginMetadataType as PMType
 
@@ -68,6 +69,15 @@ def mock_lambda_adapter(self, monkeypatch) -> Mock:
         monkeypatch.setattr(processor, "LambdaAdapter", mock)
         return mock
 
+    @pytest.fixture
+    def mock_classifier_adapter(self, monkeypatch) -> Mock:
+        mock = Mock(
+            side_effect=lambda _, __: self._is_plugin_live,
+            spec=plugin.classifier_adapter,
+        )
+        monkeypatch.setattr(processor, "is_plugin_active", mock)
+        return mock
+
     @pytest.fixture
     def mock_zulip(self, monkeypatch) -> Mock:
         mock = Mock(spec=zulip)
@@ -83,6 +93,7 @@ def setup(
         mock_get_existing_types,
         mock_get_formatted_metadata,
         mock_lambda_adapter,
+        mock_classifier_adapter,
         mock_zulip,
     ) -> None:
         self._get_latest_plugins = mock_get_latest_plugins
@@ -91,6 +102,7 @@ def setup(
         self._get_existing_types = mock_get_existing_types
         self._get_formatted_metadata = mock_get_formatted_metadata
         self._lambda_adapter = mock_lambda_adapter
+        self._classifier_adapter = mock_classifier_adapter
         self._zulip = mock_zulip
 
     @pytest.fixture
@@ -103,6 +115,7 @@ def _verify_calls(
             get_formatted_metadata_called: bool = False,
             lambda_invoked: bool = False,
             put_pm_calls: list = None,
+            classifier_adapter_not_called: bool = True,
         ) -> None:
             verify_call(True, self._get_latest_plugins, empty_call_list)
             verify_call(True, self._get_all_plugins, empty_call_list)
@@ -125,6 +138,8 @@ def _verify_calls(
                     default_call_list
                     == self._lambda_adapter.return_value.invoke.call_args_list
                 )
+            if classifier_adapter_not_called:
+                self._classifier_adapter.assert_not_called()
 
         return _verify_calls
 
@@ -138,16 +153,27 @@ def test_all_latest_plugin_in_dynamo(self, verify_calls):
         verify_calls()
         self._zulip.assert_not_called()
 
-    def test_stale_plugin_in_dynamo(self, verify_calls):
+    @pytest.mark.parametrize("is_plugin_live", [False, True])
+    def test_stale_plugin_in_dynamo(
+        self,
+        is_plugin_live,
+        verify_calls,
+    ):
         self._dynamo_latest_plugins = {PLUGIN: VERSION, "bar": "2.4.6"}
         self._pypi_latest_plugins = {"bar": "2.4.6"}
-        put_pm_calls = [_create_put_pm_call(PMType.PYPI)]
+        self._is_plugin_live = is_plugin_live
 
         processor.update_plugin()
 
-        verify_calls(put_pm_calls=put_pm_calls)
-        assert len(self._zulip.method_calls) == 1
-        self._zulip.plugin_no_longer_on_hub.assert_called_once_with(PLUGIN)
+        if is_plugin_live:
+            assert len(self._zulip.method_calls) == 0
+            put_pm_calls = []
+        else:
+            assert len(self._zulip.method_calls) == 1
+            self._zulip.plugin_no_longer_on_hub.assert_called_once_with(PLUGIN)
+            put_pm_calls = [_create_put_pm_call(PMType.PYPI)]
+        verify_calls(put_pm_calls=put_pm_calls, classifier_adapter_not_called=False)
+        self._classifier_adapter.assert_called_once_with(PLUGIN, VERSION)
 
     @pytest.mark.parametrize(
         "existing_types, put_pm_data, formatted_metadata",

napari-hub-commons/src/nhcommons/tests/models/test_plugin.py

@@ -135,7 +135,7 @@ def seed_data(self, put_item):
             "2.2",
             release_date="2023-03-01",
             data=plugin_data("plugin-1", "2.2"),
-            is_latest=True
+            is_latest=True,
         )
         put_item(
             "Plugin-2",

napari-hub-commons/src/nhcommons/utils/adapter_helpers.py

@@ -1,5 +1,5 @@
 import logging
-from typing import Dict, List, Optional
+from typing import Dict, List, Optional, Union
 
 import yaml
 from cffconvert import Citation
@@ -47,7 +47,9 @@ def get_first_valid_file(
                 return file
         return None
 
-    def get_file(self, file: str = "", file_format: str = "") -> Optional[str]:
+    def get_file(
+        self, file: str = "", file_format: str = ""
+    ) -> Optional[Union[str, dict]]:
         """
         Get file from github.
         :param file: filename to get if specified
@@ -61,7 +63,7 @@ def get_file(self, file: str = "", file_format: str = "") -> Optional[str]:
             response = get_request(api_url, auth=self._auth)
             return response.json() if file_format == "json" else response.text
         except (HTTPError, JSONDecodeError):
-            logger.error(f"Encountered error fetching {api_url}")
+            logger.error(f"Encountered error fetching {api_url}", exc_info=True)
             return None
 
     @classmethod