Page MenuHomeSoftware Heritage
Differential D3316 Diff 11753 swh/web/common/service.py

Changeset View

Standalone View

View Options

swh/web/common/service.py

# Copyright (C) 2015-2020 The Software Heritage developers # Copyright (C) 2015-2020 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 Affero General Public License version 3, or any later version # License: GNU Affero 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
import itertools import itertools
import os import os
import re import re
from collections import defaultdict from collections import defaultdict
from typing import Any, Dict, List, Set, Iterable, Iterator, Optional, Tuple from typing import Any, Dict, List, Set, Iterable, Iterator, Optional, Tuple
from swh.model import hashutil from swh.model import hashutil
from swh.model.identifiers import CONTENT, DIRECTORY, RELEASE, REVISION, SNAPSHOT from swh.model.identifiers import CONTENT, DIRECTORY, RELEASE, REVISION, SNAPSHOT
from swh.storage.algos import diff, revisions_walker from swh.storage.algos import diff, revisions_walker
from swh.storage.algos.origin import origin_get_latest_visit_status
from swh.storage.algos.snapshot import snapshot_get_latest from swh.storage.algos.snapshot import snapshot_get_latest
from swh.vault.exc import NotFoundExc as VaultNotFoundExc from swh.vault.exc import NotFoundExc as VaultNotFoundExc
from swh.web import config from swh.web import config
from swh.web.common import converters from swh.web.common import converters
from swh.web.common import query from swh.web.common import query
from swh.web.common.exc import BadInputExc, NotFoundExc from swh.web.common.exc import BadInputExc, NotFoundExc
from swh.web.common.origin_visits import get_origin_visit from swh.web.common.origin_visits import get_origin_visit
from swh.web.common.typing import OriginInfo, OriginVisitInfo from swh.web.common.typing import OriginInfo, OriginVisitInfo
▲ Show 20 Lines • Show All 874 Lines • ▼ Show 20 Lines) -> Iterator[OriginVisitInfo]:
""" """
visits = _lookup_origin_visits(origin, last_visit=last_visit, limit=per_page) visits = _lookup_origin_visits(origin, last_visit=last_visit, limit=per_page)
for visit in visits: for visit in visits:
yield converters.from_origin_visit(visit) yield converters.from_origin_visit(visit)
def lookup_origin_visit_latest( def lookup_origin_visit_latest(
origin_url: str, require_snapshot: bool origin_url: str, require_snapshot: bool
) -> OriginVisitInfo: ) -> Optional[OriginVisitInfo]:
ardumontAuthorUnsubmitted
Done
Inline Actions

I got mislead by the type here.

It should be Optional so a NotFoundExc is raised and properly dealt with in the
upper layer (to return a 404 and not a 500).

If we return the empty dict, the test about the not found use case raises a
500... [1]

[1] Those are not easy to see when hypothesis tests fail... we got a huge

stacktrace about flaky dataset. I got finally out of this one using pdb ¯\_(ツ)_/¯
ardumont: I got mislead by the type here. It should be Optional so a NotFoundExc is raised and properly…
anlambertUnsubmitted
Not Done
Inline Actions

yes, hypothesis makes it hard to debug tests.

Passing --hypothesis-verbosity=verbose to pytest usually helps.

anlambert: yes, hypothesis makes it hard to debug tests. Passing `--hypothesis-verbosity=verbose` to…
"""Return the origin's latest visit """Return the origin's latest visit
Args: Args:
origin_url (str): origin to list visits for origin_url (str): origin to list visits for
require_snapshot (bool): filter out origins without a snapshot require_snapshot (bool): filter out origins without a snapshot
Returns: Returns:
dict: The origin_visit concerned The origin_visit as dict if found
""" """
visit = storage.origin_visit_get_latest(
origin_url, require_snapshot=require_snapshot visit_and_status = origin_get_latest_visit_status(
storage, origin_url, require_snapshot=require_snapshot
)
return (
converters.from_origin_visit(
{**visit_and_status[0].to_dict(), **visit_and_status[1].to_dict()}
)
if visit_and_status
else None
) )
return converters.from_origin_visit(visit)
anlambertUnsubmitted
Not Done
Inline Actions

How about that instead ?

visit, visit_status = visit_and_status if visit_status else ({}, {})
visit_info_dict = {**visit.to_dict(), **visit_status.to_dict()}
anlambert: How about that instead ? ```lang=python visit, visit_status = visit_and_status if visit_status…
anlambertUnsubmitted
Not Done
Inline Actions

Fix typo in my previous comment

visit, visit_status = visit_and_status if visit_and_status else ({}, {})
visit_info_dict = {**visit.to_dict(), **visit_status.to_dict()}
anlambert: Fix typo in my previous comment

```lang=python
visit, visit_status = visit_and_status if…
ardumontAuthorUnsubmitted
Done
Inline Actions
visit, visit_status = visit_and_status if visit_and_status else ({}, {})



That i did not thought of



visit_info_dict = {visit.to_dict(), visit_status.to_dict()}



yes, i used that initially but we are only interested by the type visit.



The other fields are either redundant (visit, origin) or soon to be removed (status, snapshot, metadata).



:)



Although, if the issue is the multiple return statements, yes, i gather there is no choice but this then.
ardumont: > visit, visit_status = visit_and_status if visit_and_status else ({}, {})

That i did not…
ardumontAuthorUnsubmitted
Done
Inline Actions
ah also, that won't work.

When not visit_and_status, we'd have {}.to_dict()which won't work.
ardumont: ah also, that won't work.
When not visit_and_status, we'd have `{}.to_dict()`which won't work.
anlambertUnsubmitted
Not Done
Inline Actions
Use this then:



visit, visit_status = (
    {visit_and_status[0].to_dict(), visit_and_status[1].to_dict()}
    if visit_and_status
    else ({}, {})
)
anlambert: Use this then:

```lang=python
visit, visit_status = (
    {visit_and_status[0].to_dict()…
anlambertUnsubmitted
Not Done
Inline Actions
Typo day ;-)



visit, visit_status = (
    (visit_and_status[0].to_dict(), visit_and_status[1].to_dict())
    if visit_and_status
    else ({}, {})
)
anlambert: Typo day ;-)

```lang=python
visit, visit_status = (
    (visit_and_status[0].to_dict()…
ardumontAuthorUnsubmitted
Done
Inline Actions
typo day



lol ;)
ardumont: > typo day

lol ;)



def lookup_origin_visit(origin_url: str, visit_id: int) -> OriginVisitInfo:
def lookup_origin_visit(origin_url: str, visit_id: int) -> OriginVisitInfo:

    """Return information about visit visit_id with origin origin.
    """Return information about visit visit_id with origin origin.




    Args:
    Args:

        origin (str): origin concerned by the visit
        origin (str): origin concerned by the visit

        visit_id: the visit identifier to lookup
        visit_id: the visit identifier to lookup




▲ Show 20 Lines • Show All 359 Lines • Show Last 20 Lines
About · Developer information · Legal