fix: Review comments

Author: arajkumar

src/v2/models.py

@@ -221,7 +221,6 @@ class StackRecommendationResult(BaseModel):  # noqa: D101
     registration_status: str
     recommendation_status: 'RecommendationStatus' = 'success'
     companion: List['RecommendedPackageData']
-    usage_outliers: List[Dict[str, Any]]
 
 
 class RecommenderRequest(StackAggregatorRequest):  # noqa: D101

src/v2/openapi.yaml

@@ -485,7 +485,6 @@ components:
         - recommendation_status
         - companion
         - manifest_file_path
-        - usage_outliers
       properties:
         _audit:
           $ref: '#/components/schemas/Audit'
@@ -511,11 +510,6 @@ components:
         manifest_file_path:
           type: string
           example: pom.xml
-        usage_outliers:
-          type: array
-          items:
-            type: object
-            properties: {}
       description: Application stack recommendations
 
     Audit:

src/v2/recommender.py

@@ -6,6 +6,7 @@
 
 import datetime
 import logging
+import time
 from typing import List, Dict
 
 from pydantic import BaseModel
@@ -18,10 +19,8 @@
 from src.v2.models import (
     RecommenderRequest,
     StackRecommendationResult,
-    PackageDetails,
     Ecosystem,
     RecommendedPackageData,
-    Package,
 )
 
 logger = logging.getLogger(__name__)
@@ -52,7 +51,7 @@ def _prepare_insights_url(host: str) -> str:
     return endpoint
 
 
-ECOSYSTEM_TO_SERVICE_HOST = {
+ECOSYSTEM_TO_INSIGHTS_URL = {
     "pypi": _prepare_insights_url(Settings().pypi_service_host),
     "npm": _prepare_insights_url(Settings().chester_service_host),
     "maven": _prepare_insights_url(Settings().maven_service_host),
@@ -69,15 +68,22 @@ def _call(payload: InsightsRequest):
         Calls the PGM model with the normalized manifest information to get
         the relevant packages.
         """
-        insights_url = ECOSYSTEM_TO_SERVICE_HOST.get(payload.ecosystem, None)
+        insights_url = ECOSYSTEM_TO_INSIGHTS_URL.get(payload.ecosystem, None)
         assert insights_url
         try:
+            started_at = time.time()
             response = get_session_retry().post(insights_url, json=[payload.dict()])
             response.raise_for_status()
         except Exception as e:
             raise InsightsCallException() from e
         else:
             json_response = response.json()
+            logger.info(
+                "Recommendation [%s] req.pkgs [%d] elapsed time [%0.2f] sec",
+                payload.ecosystem.value,
+                len(payload.package_list),
+                time.time() - started_at,
+            )
             return json_response
 
     def _get_insights_response(self, normalized_packages: NormalizedPackages):
@@ -97,67 +103,64 @@ def _get_insights_response(self, normalized_packages: NormalizedPackages):
         # Call PGM and get the response
         return self._call(insights_payload)
 
-    def _get_package_details(self, insights_response) -> Dict[str, PackageDetails]:
-        packages = list(
+    def _get_recommended_package_details(
+        self, insights_response
+    ) -> List[RecommendedPackageData]:
+        companion_packages = insights_response.get("companion_packages", [])
+        package_to_stats_map = dict(
             map(
-                lambda x: x.get("package_name"),
-                insights_response.get("companion_packages", []),
+                lambda x: (x.get("package_name"), x),
+                companion_packages,
             )
         )
+        packages = list(package_to_stats_map.keys())
         ecosystem = insights_response["ecosystem"]
         query = (
             """g.V().has('ecosystem', ecosystem).has('name', within(name)).valueMap()"""
         )
+        started_at = time.time()
         result = post_gremlin(
             query=query, bindings={"ecosystem": ecosystem, "name": packages}
         )
+        logger.info(
+            "graph req.pkgs [%d] elapsed time [%0.2f] sec",
+            len(packages),
+            time.time() - started_at,
+        )
 
-        def map_package(data):
-            name = data.get("name", [""])[0]
+        def extract_version(data):
             # all versions are not vulnerable if latest_non_cve_version doesn't exist.
             # all versions are vulnerable if latest_non_cve_version is empty.
             recommended_version = data.get(
                 "latest_non_cve_version", data.get("latest_version", [""])
             )
             version = recommended_version[0] if len(recommended_version) else ""
-            pkg = Package(name=name, version=version)
-            return name, PackageDetails(
-                **pkg.dict(),
-                github=get_github_details(data),
-                licenses=data.get("declared_licenses", []),
-                ecosystem=ecosystem,
-                url=get_snyk_package_link(ecosystem, name),
-                latest_version=data.get("latest_version", [""])[0]
-            )
+            return version
 
-        return dict(map(map_package, result["result"]["data"]))
+        def has_valid_version(data):
+            return extract_version(data) != ""
 
-    def _get_recommended_package_details(
-        self, insights_response
-    ) -> List[RecommendedPackageData]:
-        package_details = self._get_package_details(insights_response)
+        def get_recommendation_statistics(package_name: str) -> Dict[str, str]:
+            # below dict has cooccurrence_probability, cooccurrence_count, topic_list
+            return package_to_stats_map[package_name]
 
-        def map_response(r):
-            package_detail = package_details[r["package_name"]]
+        def map_to_recommendation_package_data(data):
+            name = data.get("name", [""])[0]
+            version = extract_version(data)
             return RecommendedPackageData(
-                **package_detail.dict(),
-                cooccurrence_probability=r.get("cooccurrence_probability", 0),
-                cooccurrence_count=r.get("cooccurrence_count", 0),
-                topic_list=r.get("topic_list", [])
+                name=name,
+                version=version,
+                github=get_github_details(data),
+                licenses=data.get("declared_licenses", []),
+                ecosystem=ecosystem,
+                url=get_snyk_package_link(ecosystem, name),
+                latest_version=data.get("latest_version", [""])[0],
+                # join stats from insight
+                **get_recommendation_statistics(name),
             )
 
-        # remove recommended packages which are not in database.
-        recommended_packages = filter(
-            lambda r: package_details.get(r["package_name"]),
-            insights_response["companion_packages"],
-        )
-        # map to RecommendedPackageData
-        recommended_packages = map(map_response, recommended_packages)
-        # remove packages which has empty version string.
-        recommended_packages = filter(
-            lambda pkg: pkg.version != "", recommended_packages
-        )
-        return list(recommended_packages)
+        valid_packages = filter(has_valid_version, result["result"]["data"])
+        return list(map(map_to_recommendation_package_data, valid_packages))
 
     def execute(self, arguments=None, persist=True, check_license=False):  # noqa: F841
         """Execute task."""
@@ -167,15 +170,12 @@ def execute(self, arguments=None, persist=True, check_license=False):  # noqa: F
             logging.warning("Recommendation is yet to be implemented for golang")
             return {}
 
-        external_request_id = request.external_request_id
-
         normalized_packages = NormalizedPackages(request.packages, request.ecosystem)
         insights_response = self._get_insights_response(normalized_packages)
 
         result = StackRecommendationResult(
             **arguments,
             companion=self._get_recommended_package_details(insights_response[0]),
-            usage_outliers=[]
         )
 
         recommendation = result.dict()
@@ -186,9 +186,10 @@ def execute(self, arguments=None, persist=True, check_license=False):  # noqa: F
             "version": "v2",
         }
 
+        external_request_id = request.external_request_id
         if persist:
             persist_data_in_db(
-                external_request_id=request.external_request_id,
+                external_request_id=external_request_id,
                 task_result=recommendation,
                 worker="recommendation_v2",
                 started_at=started_at,

tests/v2/test_recommender_api.py

@@ -151,7 +151,6 @@ def test_recommendation_response_with_empty_insight_response(
             "manifest_name": None,
             "recommendation_status": "success",
             "registration_status": "FREETIER",
-            "usage_outliers": [],
             "uuid": None,
         },
     }
@@ -191,7 +190,6 @@ def test_recommendation_response_with_empty_graph(
         "manifest_name": None,
         "recommendation_status": "success",
         "registration_status": "FREETIER",
-        "usage_outliers": [],
         "uuid": None,
     }
 
@@ -322,6 +320,5 @@ def test_recommendation_response_with_2_packages_from_graph(
         "manifest_name": None,
         "recommendation_status": "success",
         "registration_status": "FREETIER",
-        "usage_outliers": [],
         "uuid": None,
     }