Differential D2820 Diff 10040 swh/storage/interface.py

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

from typing import Any, Dict, Iterable, List, Optional, Union		from typing import Any, Dict, Iterable, List, Optional, Union

from swh.core.api import remote_api_endpoint		from swh.core.api import remote_api_endpoint
from swh.model.model import (		from swh.model.model import (
Content, Directory, Origin, OriginVisit, Revision, Release,		Content, Directory, Origin, OriginVisit, Revision, Release,
Snapshot, SkippedContent		Snapshot, SkippedContent
)		)

▲ Show 20 Lines • Show All 749 Lines • ▼ Show 20 Lines	def snapshot_get_random(self):

Returns:		Returns:
a sha1_git		a sha1_git
"""		"""
...		...

@remote_api_endpoint('origin/visit/add')		@remote_api_endpoint('origin/visit/add')
def origin_visit_add(		def origin_visit_add(
self, origin, date, type) -> Optional[Dict[str, Union[str, int]]]:		self, origin: Origin,
		vlorentzUnsubmitted Not Done Inline Actions If you're willing to break compatibility, you might as well remove support for `str`. vlorentz: If you're willing to break compatibility, you might as well remove support for `str`.
		ardumontAuthorUnsubmitted Done Inline Actions I'm doing this iteratively but yes later. ardumont: I'm doing this iteratively but yes later.
		date: Union[str, datetime.datetime],
		type: str) -> OriginVisit:
		vlorentzUnsubmitted Not Done Inline Actions I don't think `origin` should be an Origin object. Here it's used as ID, not as complete node. vlorentz: I don't think `origin` should be an Origin object. Here it's used as ID, not as complete node.
		ardumontAuthorUnsubmitted Done Inline Actions Except that when we pass other objects, they are complete. I don't see why we don't do the same thing here. ardumont: Except that when we pass other objects, they are complete. I don't see why we don't do the same…
		vlorentzUnsubmitted Not Done Inline Actions But here, we're not passing an object but the id of an object. It would be like passing an entire snapshot to `origin_visit_update` instead of the snapshot's id. vlorentz: But here, we're not passing an object but the id of an object. It would be like passing an…
		ardumontAuthorUnsubmitted Done Inline Actions It would be like passing an entire snapshot to origin_visit_update instead of the snapshot's id. yes and that'd be more consistent with i do here indeed. Overall, i think i sense what you mean. But somehow i found it best we pass a compliant object Origin (when here, client created the origin properly, as returned from "origin_add" mostly likely). Then also, passing the object makes the implementation detail of the id hidden from the user. That qualifies as opaque to me. When we pass the string, it's from my point of view less so. Also i understand you might be bothered by this because i recall you did the opposite job a while ago (when origin were dict, you changed it with its string url here...). Hopefully, we'll converge on something. From a test here point of view, it's annoying to create origin to call this method (some tests do...). But i think most of our tests here are not that well written (because we need to make a refactoring pass at some point) so somehow that must be kept out of the reasoning loop here. From a "real" client point of view (loader), i found it simpler to pass along the origin object just created. Then let the storage deal with the pk itself. This, instead of unwraping the origin when calling origin_visit.... ardumont:* > It would be like passing an entire snapshot to origin_visit_update instead of the snapshot's…
		ardumontAuthorUnsubmitted Done Inline Actions Apparently i'm the only one annoyed by this. So let's keep it then. Maybe I'll propose an `id` alias on the origin at some point. I don't find "url" clear that it's the id of the origin. ardumont: Apparently i'm the only one annoyed by this. So let's keep it then. Maybe I'll propose an `id`…
"""Add an origin_visit for the origin at ts with status 'ongoing'.		"""Add an origin_visit for the origin at ts with status 'ongoing'.

Args:		Args:
origin (str): visited origin's identifier or URL		origin: visited origin's identifier or URL
date (Union[str,datetime]): timestamp of such visit		date: timestamp of such visit
type (str): the type of loader used for the visit (hg, git, ...)		type: the type of loader used for the visit (hg, git, ...)

		Raises:
		StorageArgumentException if the date is mistyped, or the origin
		is unknown.
		vlorentzUnsubmitted Not Done Inline Actions needs an update vlorentz: needs an update

Returns:		Returns:
dict: dictionary with keys origin and visit where:		dict: dictionary with keys origin and visit where:
		- origin: origin object
- origin: origin identifier		- visit: the visit object for the new visit occurrence
- visit: the visit identifier for the new visit occurrence

"""		"""
...		...

@remote_api_endpoint('origin/visit/update')		@remote_api_endpoint('origin/visit/update')
def origin_visit_update(		def origin_visit_update(
self, origin: str, visit_id: int, status: Optional[str] = None,		self, origin: str, visit_id: int, status: Optional[str] = None,
metadata: Optional[Dict] = None, snapshot: Optional[bytes] = None):		metadata: Optional[Dict] = None, snapshot: Optional[bytes] = None):
		vlorentzUnsubmitted Not Done Inline Actions Same comment as above, it doesn't make sense to pass a complete `Origin` object, let alone a complete `OriginVisit` object. vlorentz: Same comment as above, it doesn't make sense to pass a complete `Origin` object, let alone a…
		ardumontAuthorUnsubmitted Done Inline Actions I agree for origin, thus why it goes away in a diff i thought i submitted ;) ardumont: I agree for origin, thus why it goes away in a diff i thought i submitted ;)
		ardumontAuthorUnsubmitted Done Inline Actions i submitted it but i did not find it immediately for some reason D2822 ardumont: i submitted it but i did not find it immediately for some reason D2822
"""Update an origin_visit's status.		"""Update an origin_visit's status.

Args:		Args:
origin (str): visited origin's URL		origin (str): visited origin's URL
visit_id: Visit's id		visit_id: Visit's id
		vlorentzUnsubmitted Not Done Inline Actions needs an update. vlorentz: needs an update.
status: Visit's new status		status: Visit's new status
metadata: Data associated to the visit		metadata: Data associated to the visit
snapshot (sha1_git): identifier of the snapshot to add to		snapshot (sha1_git): identifier of the snapshot to add to
the visit		the visit

Returns:		Returns:
None		None

▲ Show 20 Lines • Show All 470 Lines • Show Last 20 Lines