Changeset View
Standalone View
swh/core/api/model.py
- This file was added.
# Copyright (C) 2015-2020 The Software Heritage developers | |||||
# See the AUTHORS file at the top-level directory of this distribution | |||||
# License: GNU General Public License version 3, or any later version | |||||
# See top-level LICENSE file for more information | |||||
import attr | |||||
from typing import ( | |||||
Generic, | |||||
List, | |||||
Optional, | |||||
TypeVar, | |||||
) | |||||
from attrs_strict import type_validator | |||||
T = TypeVar("T") | |||||
olasd: Please give this type variable a meaningful name (e.g. `ResultType`). | |||||
vlorentzUnsubmitted Not Done Inline ActionsThis goes against conventions in multiple languages (at least Python, C++, Rust) and PEP8:
https://www.python.org/dev/peps/pep-0008/#type-variable-names vlorentz: This goes against conventions in multiple languages (at least Python, C++, Rust) and PEP8:
>… | |||||
olasdUnsubmitted Not Done Inline ActionsResultType is a short, CapWords name. olasd: `ResultType` is a short, CapWords name. | |||||
vlorentzUnsubmitted Not Done Inline ActionsI disagree that it's short. The examples in PEP8 all shorten nouns (ie. they are not AnyString nor Number) But as a compromise we could use TRes, or even TResult. vlorentz: I disagree that it's short. The examples in PEP8 all shorten nouns (ie. they are not… | |||||
@attr.s(frozen=True) | |||||
class PagedResult(Generic[T]): | |||||
"""Represents a page of results; with a token to get the next page""" | |||||
results = attr.ib(type=List[T], default=[]) | |||||
olasdUnsubmitted Not Done Inline ActionsIf the class is frozen then this shouldn't be a mutable object. But I don't think making it frozen has much value in the first place. I'm not enthused by the lack of runtime type checking. Deserializing a PagedResult containing serialized results of an unexpected type would not report any error. olasd: If the class is frozen then this shouldn't be a mutable object. But I don't think making it… | |||||
next_page_token = attr.ib( | |||||
type=Optional[str], default=None, validator=type_validator() | |||||
olasdUnsubmitted Not Done Inline ActionsI think this should be a generic type too. We may need to use composite keys (e.g. what swh.scheduler needs for get_listed_origins). olasd: I think this should be a generic type too. We may need to use composite keys (e.g. what `swh. | |||||
vlorentzUnsubmitted Not Done Inline ActionsThe whole point of next_page_token is that it's an opaque token that can change at the server's will. If you need composite values, the server can use msgpack + base64 (that's what swh-search does) vlorentz: The whole point of next_page_token is that it's an opaque token that can change at the server's… | |||||
olasdUnsubmitted Not Done Inline ActionsThis is a return type for our backend API/RPC calls. It doesn't have to be opaque, let's not make it gratuitously so. olasd: This is a return type for our backend API/RPC calls. It doesn't have to be opaque, let's not… | |||||
vlorentzUnsubmitted Not Done Inline ActionsIt's not gratuitous, it's hiding implementation details. It also keeps PagedResult simpler and with a type validator vlorentz: It's not gratuitous, it's hiding implementation details. It also keeps PagedResult simpler and… | |||||
ardumontAuthorUnsubmitted Done Inline ActionsYeah, i was gonna ask to "type validate" it. ardumont: Yeah, i was gonna ask to "type validate" it. | |||||
ardumontAuthorUnsubmitted Done Inline Actionshow* to "type validate" it... ardumont: how* to "type validate" it... | |||||
) |
Please give this type variable a meaningful name (e.g. ResultType).