Changeset View
Standalone View
swh/storage/interface.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 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 | ||||
| import datetime | import datetime | ||||
| from enum import Enum | from enum import Enum | ||||
| from typing import Any, Dict, Iterable, List, Optional, Tuple, TypeVar, Union | from typing import Any, Dict, Iterable, List, Optional, Tuple, TypeVar, Union | ||||
| from typing_extensions import TypedDict | |||||
| from swh.core.api import remote_api_endpoint | from swh.core.api import remote_api_endpoint | ||||
| from swh.core.api.classes import PagedResult as CorePagedResult | from swh.core.api.classes import PagedResult as CorePagedResult | ||||
| from swh.model.identifiers import SWHID | from swh.model.identifiers import SWHID | ||||
| from swh.model.model import ( | from swh.model.model import ( | ||||
| Content, | Content, | ||||
| Directory, | Directory, | ||||
| Origin, | Origin, | ||||
| OriginVisit, | OriginVisit, | ||||
| OriginVisitStatus, | OriginVisitStatus, | ||||
| Revision, | Revision, | ||||
| Release, | Release, | ||||
| Snapshot, | Snapshot, | ||||
| SkippedContent, | SkippedContent, | ||||
| SnapshotBranch, | |||||
| MetadataAuthority, | MetadataAuthority, | ||||
| MetadataAuthorityType, | MetadataAuthorityType, | ||||
| MetadataFetcher, | MetadataFetcher, | ||||
| MetadataTargetType, | MetadataTargetType, | ||||
| RawExtrinsicMetadata, | RawExtrinsicMetadata, | ||||
| Sha1, | Sha1, | ||||
| Sha1Git, | Sha1Git, | ||||
| ) | ) | ||||
| class ListOrder(Enum): | class ListOrder(Enum): | ||||
| """Specifies the order for paginated endpoints returning sorted results.""" | """Specifies the order for paginated endpoints returning sorted results.""" | ||||
| ASC = "asc" | ASC = "asc" | ||||
| DESC = "desc" | DESC = "desc" | ||||
anlambert: I am not a big fan of that naming. For small snapshots, the returned branches set will not be… | |||||
Not Done Inline Actionsor BranchesPagedResult ? anlambert: or `BranchesPagedResult` ? | |||||
Done Inline ActionsI couldn't find a better name. ResultSet name-clashes with cassandra (plus, it doesn't make it obvious it's only a partial view), and PagedResult with the class with that name. vlorentz: I couldn't find a better name. `ResultSet` name-clashes with cassandra (plus, it doesn't make… | |||||
Not Done Inline ActionsHow about simply Branches then ? def snapshot_get_branches(..) -> Optional[Branches] Not a shocker to me. anlambert: How about simply `Branches` then ?
`def snapshot_get_branches(..) -> Optional[Branches]`
Not… | |||||
Not Done Inline ActionsI proposed in T645 that the method be typed [1] def snapshot_get_branches(..) -> Optional[PagedResult[SnapshotBranch]] Optional in case the snapshot is (Sha1Git) does not exist... (or raise to avoid the specific type...) The PagedResult implies the result can be partial... wdyt? [1] T645#47156 ardumont: I proposed in T645 that the method be typed [1]
`def snapshot_get_branches(..) -> Optional… | |||||
Done Inline Actionsit still doesn't page it clear it's a partial view. vlorentz: it still doesn't page it clear it's a partial view. | |||||
Done Inline ActionsPagedResult isn't great, because snapshot_get_branches doesn't use an opaque token for pagination. vlorentz: PagedResult isn't great, because snapshot_get_branches doesn't use an opaque token for… | |||||
Not Done Inline Actions
sorry for the typos, one must read... Optional in case the snapshot id (Sha1Git) does not exist... (or raise to avoid the specific case of the inexisting snapshot) ardumont: > Optional in case the snapshot is (Sha1Git) does not exist... (or raise to avoid the specific… | |||||
Not Done Inline Actions
What's preventing us from doing it that? ardumont: > PagedResult isn't great, because snapshot_get_branches doesn't use an opaque token for… | |||||
Not Done Inline Actionsdoing that way*... (need some sleep, sorry again...) ardumont: doing that way*... (need some sleep, sorry again...) | |||||
Done Inline Actions
Nothing, but there is no reason to switch to an opaque token.
It is; snapshot branches are sorted by name. vlorentz: > What's preventing us from doing it that?
Nothing, but there is no reason to switch to an… | |||||
Not Done Inline ActionsThe fact that the returned branches set might be partial is clearly indicated in the docstring. If you want to keep the Partial prefix, at least remove the Dict suffix as it is not really needed. anlambert: The fact that the returned branches set might be partial is clearly indicated in the docstring. | |||||
| class PartialBranches(TypedDict): | |||||
| """Type of the dictionary returned by snapshot_get_branches""" | |||||
| id: Sha1Git | |||||
| """Identifier of the snapshot""" | |||||
| branches: Dict[bytes, Optional[SnapshotBranch]] | |||||
| """A dict of branches contained in the snapshot | |||||
| whose keys are the branches' names""" | |||||
| next_branch: Optional[bytes] | |||||
| """The name of the first branch not returned or :const:`None` if | |||||
| the snapshot has less than the request number of branches.""" | |||||
| TResult = TypeVar("TResult") | TResult = TypeVar("TResult") | ||||
| PagedResult = CorePagedResult[TResult, str] | PagedResult = CorePagedResult[TResult, str] | ||||
| # TODO: Make it an enum (too much impact) | # TODO: Make it an enum (too much impact) | ||||
| VISIT_STATUSES = ["created", "ongoing", "full", "partial"] | VISIT_STATUSES = ["created", "ongoing", "full", "partial"] | ||||
| ▲ Show 20 Lines • Show All 665 Lines • ▼ Show 20 Lines | class StorageInterface: | ||||
| @remote_api_endpoint("snapshot/get_branches") | @remote_api_endpoint("snapshot/get_branches") | ||||
| def snapshot_get_branches( | def snapshot_get_branches( | ||||
| self, | self, | ||||
| snapshot_id: Sha1Git, | snapshot_id: Sha1Git, | ||||
| branches_from: bytes = b"", | branches_from: bytes = b"", | ||||
| branches_count: int = 1000, | branches_count: int = 1000, | ||||
| target_types: Optional[List[str]] = None, | target_types: Optional[List[str]] = None, | ||||
| ) -> Optional[Dict[str, Any]]: | ) -> Optional[PartialBranches]: | ||||
| """Get the content, possibly partial, of a snapshot with the given id | """Get the content, possibly partial, of a snapshot with the given id | ||||
| The branches of the snapshot are iterated in the lexicographical | The branches of the snapshot are iterated in the lexicographical | ||||
| order of their names. | order of their names. | ||||
| Args: | Args: | ||||
| snapshot_id: identifier of the snapshot | snapshot_id: identifier of the snapshot | ||||
| branches_from: optional parameter used to skip branches | branches_from: optional parameter used to skip branches | ||||
| ▲ Show 20 Lines • Show All 543 Lines • Show Last 20 Lines | |||||
I am not a big fan of that naming. For small snapshots, the returned branches set will not be partial for instance. How about BranchesResultSet instead ?