Changeset View
Standalone View
swh/lister/pypi/lister.py
# Copyright (C) 2018-2021 The Software Heritage developers | # Copyright (C) 2018-2021 The Software Heritage developers | ||||||||||||||||||||||||||||||||||
# See the AUTHORS file at the top-level directory of this distribution | # See the AUTHORS file at the top-level directory of this distribution | ||||||||||||||||||||||||||||||||||
# License: GNU General Public License version 3, or any later version | # License: GNU General Public License version 3, or any later version | ||||||||||||||||||||||||||||||||||
# See top-level LICENSE file for more information | # See top-level LICENSE file for more information | ||||||||||||||||||||||||||||||||||
from collections import defaultdict | |||||||||||||||||||||||||||||||||||
from dataclasses import asdict, dataclass | |||||||||||||||||||||||||||||||||||
from datetime import datetime, timezone | |||||||||||||||||||||||||||||||||||
import logging | import logging | ||||||||||||||||||||||||||||||||||
from typing import Iterator, List, Optional | from typing import Any, Dict, Iterator, List, Optional, Tuple | ||||||||||||||||||||||||||||||||||
from xmlrpc.client import ServerProxy | |||||||||||||||||||||||||||||||||||
from bs4 import BeautifulSoup | from bs4 import BeautifulSoup | ||||||||||||||||||||||||||||||||||
import iso8601 | |||||||||||||||||||||||||||||||||||
import requests | import requests | ||||||||||||||||||||||||||||||||||
from swh.scheduler.interface import SchedulerInterface | from swh.scheduler.interface import SchedulerInterface | ||||||||||||||||||||||||||||||||||
from swh.scheduler.model import ListedOrigin | from swh.scheduler.model import ListedOrigin | ||||||||||||||||||||||||||||||||||
from .. import USER_AGENT | from .. import USER_AGENT | ||||||||||||||||||||||||||||||||||
from ..pattern import CredentialsType, StatelessLister | from ..pattern import CredentialsType, Lister | ||||||||||||||||||||||||||||||||||
logger = logging.getLogger(__name__) | logger = logging.getLogger(__name__) | ||||||||||||||||||||||||||||||||||
PackageListPage = List[str] | PackageListPage = List[Dict] | ||||||||||||||||||||||||||||||||||
class PyPILister(StatelessLister[PackageListPage]): | @dataclass | ||||||||||||||||||||||||||||||||||
class PyPIListerState: | |||||||||||||||||||||||||||||||||||
"""State of PyPI lister""" | |||||||||||||||||||||||||||||||||||
last_visit: Optional[datetime] = None | |||||||||||||||||||||||||||||||||||
anlambert: since the pypi instance was visited | |||||||||||||||||||||||||||||||||||
"""Last visit date and time since we visited the pypi instance (incremental pass)""" | |||||||||||||||||||||||||||||||||||
class PyPILister(Lister[PyPIListerState, PackageListPage]): | |||||||||||||||||||||||||||||||||||
"""List origins from PyPI. | """List origins from PyPI. | ||||||||||||||||||||||||||||||||||
""" | """ | ||||||||||||||||||||||||||||||||||
LISTER_NAME = "pypi" | LISTER_NAME = "pypi" | ||||||||||||||||||||||||||||||||||
INSTANCE = "pypi" # As of today only the main pypi.org is used | INSTANCE = "pypi" # As of today only the main pypi.org is used | ||||||||||||||||||||||||||||||||||
PACKAGE_LIST_URL = "https://pypi.org/simple/" | PACKAGE_LIST_URL = "https://pypi.org/simple/" | ||||||||||||||||||||||||||||||||||
PACKAGE_URL = "https://pypi.org/project/{package_name}/" | PACKAGE_URL = "https://pypi.org/project/{package_name}/" | ||||||||||||||||||||||||||||||||||
RPC_URL = "https://pypi.org/pypi" | |||||||||||||||||||||||||||||||||||
def __init__( | def __init__( | ||||||||||||||||||||||||||||||||||
self, | self, | ||||||||||||||||||||||||||||||||||
scheduler: SchedulerInterface, | scheduler: SchedulerInterface, | ||||||||||||||||||||||||||||||||||
credentials: Optional[CredentialsType] = None, | credentials: Optional[CredentialsType] = None, | ||||||||||||||||||||||||||||||||||
incremental: bool = True, | |||||||||||||||||||||||||||||||||||
): | ): | ||||||||||||||||||||||||||||||||||
super().__init__( | super().__init__( | ||||||||||||||||||||||||||||||||||
scheduler=scheduler, | scheduler=scheduler, | ||||||||||||||||||||||||||||||||||
url=self.PACKAGE_LIST_URL, | url=self.PACKAGE_LIST_URL, | ||||||||||||||||||||||||||||||||||
instance=self.INSTANCE, | instance=self.INSTANCE, | ||||||||||||||||||||||||||||||||||
credentials=credentials, | credentials=credentials, | ||||||||||||||||||||||||||||||||||
) | ) | ||||||||||||||||||||||||||||||||||
self.incremental = incremental | |||||||||||||||||||||||||||||||||||
self.session = requests.Session() | self.session = requests.Session() | ||||||||||||||||||||||||||||||||||
self.session.headers.update( | self.session.headers.update( | ||||||||||||||||||||||||||||||||||
{"Accept": "application/html", "User-Agent": USER_AGENT} | {"Accept": "application/html", "User-Agent": USER_AGENT} | ||||||||||||||||||||||||||||||||||
) | ) | ||||||||||||||||||||||||||||||||||
self.client: Optional[ServerProxy] = None | |||||||||||||||||||||||||||||||||||
Done Inline Actionsmaybe self.last_processed_serial? olasd: maybe `self.last_processed_serial`? | |||||||||||||||||||||||||||||||||||
Done Inline Actions+1 ardumont: +1 | |||||||||||||||||||||||||||||||||||
self.current_visit: Optional[datetime] = None | |||||||||||||||||||||||||||||||||||
def state_from_dict(self, d: Dict[str, Any]) -> PyPIListerState: | |||||||||||||||||||||||||||||||||||
last_visit = d.get("last_visit") | |||||||||||||||||||||||||||||||||||
if last_visit is not None: | |||||||||||||||||||||||||||||||||||
d["last_visit"] = iso8601.parse_date(last_visit) | |||||||||||||||||||||||||||||||||||
return PyPIListerState(**d) | |||||||||||||||||||||||||||||||||||
def state_to_dict(self, state: PyPIListerState) -> Dict[str, Any]: | |||||||||||||||||||||||||||||||||||
d = asdict(state) | |||||||||||||||||||||||||||||||||||
last_visit = d.get("last_visit") | |||||||||||||||||||||||||||||||||||
if last_visit is not None: | |||||||||||||||||||||||||||||||||||
d["last_visit"] = last_visit.isoformat() | |||||||||||||||||||||||||||||||||||
return d | |||||||||||||||||||||||||||||||||||
def _last_updates_since( | |||||||||||||||||||||||||||||||||||
self, last_visit_timestamp: int | |||||||||||||||||||||||||||||||||||
Done Inline ActionsYou could use a dataclass PackageUpdate instead of a tuple here for better readability. anlambert: You could use a dataclass `PackageUpdate` instead of a tuple here for better readability. | |||||||||||||||||||||||||||||||||||
Done Inline Actionstotally, thx ;) ardumont: totally, thx ;) | |||||||||||||||||||||||||||||||||||
) -> List[Tuple[str, str, int, str]]: | |||||||||||||||||||||||||||||||||||
Done Inline ActionsExecute the listing of the last updates since last_visit_timestamp anlambert: Execute the listing of the last updates since last_visit_timestamp | |||||||||||||||||||||||||||||||||||
"""Execute the listing of the last update since the last_visit_timestamp. | |||||||||||||||||||||||||||||||||||
Done Inline Actions@olasd to explicit ^ when mocking the ServerProxy, here is the issue with whatever methods i'd like to mock ("the *annoying* implementation detail) AttributeError: <class 'xmlrpc.client.ServerProxy'> does not have the attribute 'last_serial' AttributeError: <class 'xmlrpc.client.ServerProxy'> does not have the attribute 'changelog_since_serial' ardumont: @olasd to explicit ^ when mocking the ServerProxy, here is the issue with whatever methods i'd… | |||||||||||||||||||||||||||||||||||
Done Inline ActionsMy suggestion would be to substitute xmlrpc.client.ServerProxy() with a (stubbed) class implementing just these two methods with hardcoded return values. I don't know how Mock() or MagicMock() behaves on classes which have dynamically generated attributes/methods, like the ones xmlrpc.client generates. olasd: My suggestion would be to substitute `xmlrpc.client.ServerProxy()` with a (stubbed) class… | |||||||||||||||||||||||||||||||||||
Done Inline ActionsI actually don't know how to do what you suggest, i'll check. ardumont: I actually don't know how to do what you suggest, i'll check.
Thanks. | |||||||||||||||||||||||||||||||||||
Done Inline Actionsreading it more slowly, i think i got it. ardumont: reading it more slowly, i think i got it. | |||||||||||||||||||||||||||||||||||
The indirection method exists so the testing is actually doable. Technically, | |||||||||||||||||||||||||||||||||||
the ServerProxy class does not expose the changelog method due to internal | |||||||||||||||||||||||||||||||||||
implementation detail which makes the testing hard for no good reason. | |||||||||||||||||||||||||||||||||||
Args: | |||||||||||||||||||||||||||||||||||
Done Inline ActionsThe last visit timestamp anlambert: The last visit timestamp | |||||||||||||||||||||||||||||||||||
last_visit_timestamp: The last timestamp since we visited | |||||||||||||||||||||||||||||||||||
Returns: | |||||||||||||||||||||||||||||||||||
The list of tuple information (package-name, version, last-update, | |||||||||||||||||||||||||||||||||||
description) | |||||||||||||||||||||||||||||||||||
""" | |||||||||||||||||||||||||||||||||||
if not self.client: | |||||||||||||||||||||||||||||||||||
self.client = ServerProxy(self.RPC_URL) | |||||||||||||||||||||||||||||||||||
Done Inline ActionsIs is not client.changelog instead here ? anlambert: Is is not `client.changelog` instead here ? | |||||||||||||||||||||||||||||||||||
Done Inline Actionstotally, here falls apart the mocking part ;) ardumont: totally, here falls apart the mocking part ;)
nice catch! | |||||||||||||||||||||||||||||||||||
return self.client_changelog(last_visit_timestamp) # type: ignore | |||||||||||||||||||||||||||||||||||
def finalize(self): | |||||||||||||||||||||||||||||||||||
"""Finalize incremental visit state with the current visit we did | |||||||||||||||||||||||||||||||||||
""" | |||||||||||||||||||||||||||||||||||
if self.incremental and self.current_visit: | |||||||||||||||||||||||||||||||||||
self.updated = True | |||||||||||||||||||||||||||||||||||
self.state.last_visit = self.current_visit | |||||||||||||||||||||||||||||||||||
def get_pages(self) -> Iterator[PackageListPage]: | def get_pages(self) -> Iterator[PackageListPage]: | ||||||||||||||||||||||||||||||||||
response = self.session.get(self.PACKAGE_LIST_URL) | if ( | ||||||||||||||||||||||||||||||||||
self.incremental | |||||||||||||||||||||||||||||||||||
and self.state is not None | |||||||||||||||||||||||||||||||||||
and self.state.last_visit is not None | |||||||||||||||||||||||||||||||||||
): # incremental behavior will do its best to fetch latest change with | |||||||||||||||||||||||||||||||||||
# last_update information | |||||||||||||||||||||||||||||||||||
last_visit_timestamp: int = int(self.state.last_visit.timestamp()) | |||||||||||||||||||||||||||||||||||
updated_packages = defaultdict(list) | |||||||||||||||||||||||||||||||||||
self.current_visit = datetime.now(tz=timezone.utc) | |||||||||||||||||||||||||||||||||||
for package, _, last_update, _ in self._last_updates_since( | |||||||||||||||||||||||||||||||||||
last_visit_timestamp | |||||||||||||||||||||||||||||||||||
): | |||||||||||||||||||||||||||||||||||
updated_packages[package].append(last_update) | |||||||||||||||||||||||||||||||||||
yield [ | |||||||||||||||||||||||||||||||||||
Done Inline Actions
I'm a bit confused by these three variables. It seems that last_serial is never reused, so here's my proposal! olasd: I'm a bit confused by these three variables. It seems that `last_serial` is never reused, so… | |||||||||||||||||||||||||||||||||||
Done Inline Actionsthx, +1 again ardumont: thx, +1 again | |||||||||||||||||||||||||||||||||||
{ | |||||||||||||||||||||||||||||||||||
"name": package, | |||||||||||||||||||||||||||||||||||
"last_update": datetime.fromtimestamp(max(releases)).replace( | |||||||||||||||||||||||||||||||||||
tzinfo=timezone.utc | |||||||||||||||||||||||||||||||||||
), | |||||||||||||||||||||||||||||||||||
} | |||||||||||||||||||||||||||||||||||
for package, releases in updated_packages.items() | |||||||||||||||||||||||||||||||||||
] | |||||||||||||||||||||||||||||||||||
else: # Full lister behavior | |||||||||||||||||||||||||||||||||||
response = self.session.get(self.PACKAGE_LIST_URL) | |||||||||||||||||||||||||||||||||||
response.raise_for_status() | response.raise_for_status() | ||||||||||||||||||||||||||||||||||
page = BeautifulSoup(response.content, features="html.parser") | page = BeautifulSoup(response.content, features="html.parser") | ||||||||||||||||||||||||||||||||||
page_results = [p.text for p in page.find_all("a")] | yield [ | ||||||||||||||||||||||||||||||||||
{"name": package.text, "last_update": None} | |||||||||||||||||||||||||||||||||||
yield page_results | for package in page.find_all("a") | ||||||||||||||||||||||||||||||||||
] | |||||||||||||||||||||||||||||||||||
def get_origins_from_page( | def get_origins_from_page( | ||||||||||||||||||||||||||||||||||
self, packages_name: PackageListPage | self, packages: PackageListPage | ||||||||||||||||||||||||||||||||||
) -> Iterator[ListedOrigin]: | ) -> Iterator[ListedOrigin]: | ||||||||||||||||||||||||||||||||||
"""Convert a page of PyPI repositories into a list of ListedOrigins.""" | """Convert a page of PyPI repositories into a list of ListedOrigins.""" | ||||||||||||||||||||||||||||||||||
Done Inline Actions
olasd: | |||||||||||||||||||||||||||||||||||
assert self.lister_obj.id is not None | assert self.lister_obj.id is not None | ||||||||||||||||||||||||||||||||||
for package_name in packages_name: | for package in packages: | ||||||||||||||||||||||||||||||||||
package_url = self.PACKAGE_URL.format(package_name=package_name) | package_url = self.PACKAGE_URL.format(package_name=package["name"]) | ||||||||||||||||||||||||||||||||||
yield ListedOrigin( | yield ListedOrigin( | ||||||||||||||||||||||||||||||||||
lister_id=self.lister_obj.id, | lister_id=self.lister_obj.id, | ||||||||||||||||||||||||||||||||||
url=package_url, | url=package_url, | ||||||||||||||||||||||||||||||||||
visit_type="pypi", | visit_type="pypi", | ||||||||||||||||||||||||||||||||||
last_update=None, # available on PyPI JSON API | last_update=package["last_update"], | ||||||||||||||||||||||||||||||||||
) | ) | ||||||||||||||||||||||||||||||||||
Done Inline Actions
olasd: |
since the pypi instance was visited