Differential D5977 Diff 21537 swh/lister/pypi/lister.py

Changeset View

Standalone View

swh/lister/pypi/lister.py

# 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

anlambertUnsubmitted

Done

since the pypi instance was visited

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 = False,

): ):

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

olasdUnsubmitted

Done

maybe self.last_processed_serial?

olasd: maybe `self.last_processed_serial`?

ardumontAuthorUnsubmitted

Done

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

anlambertUnsubmitted

Done

You 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.

ardumontAuthorUnsubmitted

Done

totally, thx ;)

ardumont: totally, thx ;)

) -> List[Tuple[str, str, int, str]]:

anlambertUnsubmitted

Done

Execute 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.

ardumontAuthorUnsubmitted

Done

@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…

olasdUnsubmitted

Done

My 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…

ardumontAuthorUnsubmitted

Done

I actually don't know how to do what you suggest, i'll check.
Thanks.

ardumont: I actually don't know how to do what you suggest, i'll check. Thanks.

ardumontAuthorUnsubmitted

Done

reading 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:

anlambertUnsubmitted

Done

The 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)

anlambertUnsubmitted

Done

Is is not client.changelog instead here ?

anlambert: Is is not `client.changelog` instead here ?

ardumontAuthorUnsubmitted

Done

totally, here falls apart the mocking part ;)
nice catch!

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 [

olasdUnsubmitted

Done

client = ServerProxy(self.url)

- last_serial = self.state.last_serial

- max_serial = last_serial if last_serial is not None else -1

- highest_serial = self._changelog_last_serial(client)

+ last_processed_serial = -1

+ if self.state.last_serial is not None:

+ last_processed_serial = self.state.last_serial

+ upstream_last_serial = self._changelog_last_serial(client)

# Paginate through result of pypi, until we read everything

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…

ardumontAuthorUnsubmitted

Done

thx, +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."""

olasdUnsubmitted

Done

highest_serial = self._changelog_last_serial(client)

# Paginate through result of pypi, until we read everything

- while max_serial < highest_serial:

+ while last_processed_serial < last_upstream_serial:

updated_packages = defaultdict(list)

for package, _, release_date, _, serial in self._changelog_since_serial(

client, max_serial

updated_packages[package].append(release_date)

# Compute the max serial so we can stop when done

- max_serial = max(max_serial, serial)

+ last_processed_serial = max(last_processed_serial, serial)

# Returns pages of result to flush regularly

yield [

(

pypi_url(package),

datetime.fromtimestamp(max(release_dates)).replace(

tzinfo=timezone.utc

)

for package, release_dates in updated_packages.items()

]

- self.highest_serial = highest_serial

+ self.last_processed_serial = last_processed_serial

def get_origins_from_page(

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"],

) )

olasdUnsubmitted

Done

actually happened.

"""

self.updated = (

self.state

and self.state.last_serial

- and self.state.last_serial < self.highest_serial

- ) or (not self.state.last_serial and self.highest_serial)

+ and self.state.last_serial < self.last_processed_serial

+ ) or (not self.state.last_serial and self.last_processed_serial)

if self.updated:

- self.state.last_serial = self.highest_serial

+ self.state.last_serial = self.last_processed_serial

olasd: