Changeset View
Changeset View
Standalone View
Standalone View
swh/loader/package/loader.py
| Show All 21 Lines | from swh.model.model import ( | ||||
| Sha1Git, | Sha1Git, | ||||
| Content, | Content, | ||||
| SkippedContent, | SkippedContent, | ||||
| Directory, | Directory, | ||||
| Revision, | Revision, | ||||
| TargetType, | TargetType, | ||||
| Snapshot, | Snapshot, | ||||
| Origin, | Origin, | ||||
| OriginVisitStatus, | |||||
| ) | ) | ||||
| from swh.storage import get_storage | from swh.storage import get_storage | ||||
| from swh.storage.utils import now | |||||
| from swh.storage.algos.snapshot import snapshot_get_all_branches | from swh.storage.algos.snapshot import snapshot_get_all_branches | ||||
| from swh.loader.package.utils import download | from swh.loader.package.utils import download | ||||
| logger = logging.getLogger(__name__) | logger = logging.getLogger(__name__) | ||||
| ▲ Show 20 Lines • Show All 235 Lines • ▼ Show 20 Lines | def load(self) -> Dict: | ||||
| """Finalize the visit: | """Finalize the visit: | ||||
| - flush eventual unflushed data to storage | - flush eventual unflushed data to storage | ||||
| - update origin visit's status | - update origin visit's status | ||||
| - return the task's status | - return the task's status | ||||
| """ | """ | ||||
| self.storage.flush() | self.storage.flush() | ||||
| self.storage.origin_visit_update( | |||||
| snapshot_id: Optional[bytes] = None | |||||
douardda: not that it would make any difference I guess, but why keep the `and snapshot.id` part in here? | |||||
Not Done Inline ActionsI mean, it adds 3 lines of code that are not necessary not really ease to read or understand this piece of code. (once again, IMHO. Others may disagree). douardda: I mean, it adds 3 lines of code that are not necessary not really ease to read or understand… | |||||
Done Inline ActionsI'm not sure either. ardumont: I'm not sure either.
mypy annoyed me and it finally left me alone after this... as far as i… | |||||
Done Inline ActionsAlthough from the model Snapshot, the id field can be "" (as a default value) so I guess, that's needed to avoid this case. ardumont: Although from the model Snapshot, the `id` field can be "" (as a default value) so I guess… | |||||
Done Inline Actionswell i meant`b""` (so the type is correct :). ardumont: well i meant`b""` (so the type is correct :).
Still that's not a correct id we want to store.
| |||||
| if snapshot and snapshot.id: # to prevent the snapshot.id to b"" | |||||
| snapshot_id = snapshot.id | |||||
| visit_status = OriginVisitStatus( | |||||
| origin=self.url, | origin=self.url, | ||||
| visit_id=visit.visit, | visit=visit.visit, | ||||
| date=now(), | |||||
| status=status_visit, | status=status_visit, | ||||
| snapshot=snapshot and snapshot.id, | snapshot=snapshot_id, | ||||
| ) | ) | ||||
| self.storage.origin_visit_status_add([visit_status]) | |||||
| result: Dict[str, Any] = { | result: Dict[str, Any] = { | ||||
| "status": status_load, | "status": status_load, | ||||
| } | } | ||||
| if snapshot: | if snapshot_id: | ||||
| result["snapshot_id"] = hash_to_hex(snapshot.id) | result["snapshot_id"] = hash_to_hex(snapshot_id) | ||||
| return result | return result | ||||
| # Prepare origin and origin_visit | # Prepare origin and origin_visit | ||||
| origin = Origin(url=self.url) | origin = Origin(url=self.url) | ||||
| try: | try: | ||||
| self.storage.origin_add_one(origin) | self.storage.origin_add_one(origin) | ||||
| visit = self.storage.origin_visit_add( | visit = self.storage.origin_visit_add( | ||||
| self.url, date=self.visit_date, type=self.visit_type | self.url, date=self.visit_date, type=self.visit_type | ||||
| ▲ Show 20 Lines • Show All 189 Lines • Show Last 20 Lines | |||||
not that it would make any difference I guess, but why keep the and snapshot.id part in here?
(also not sure this local variable is helping really)