Differential D2949 Diff 11301 swh/loader/package/nixguix/loader.py

Changeset View

Standalone View

swh/loader/package/nixguix/loader.py

Show First 20 Lines • Show All 91 Lines • ▼ Show 20 Lines	def known_artifacts(self, snapshot: Optional[Snapshot]) -> Dict[Sha1Git, BaseModel]:
if not revision: # revision_get can return None		if not revision: # revision_get can return None
continue		continue
ret[revision["id"]] = revision["metadata"]		ret[revision["id"]] = revision["metadata"]
return ret		return ret

def resolve_revision_from(		def resolve_revision_from(
self, known_artifacts: Dict, artifact_metadata: Dict		self, known_artifacts: Dict, artifact_metadata: Dict
) -> Optional[bytes]:		) -> Optional[bytes]:

for rev_id, known_artifact in known_artifacts.items():		for rev_id, known_artifact in known_artifacts.items():
		try:
		lewoUnsubmitted Not Done Inline Actions We should catch a `keyError` exception instead since it could come from several keys. lewo: We should catch a `keyError` exception instead since it could come from several keys.
		ardumontAuthorUnsubmitted Done Inline Actions yes ardumont: yes
		douarddaUnsubmitted Not Done Inline Actions I agree a try/except KeyError is better here. Also why exit the for loop? Isn't there any chance the searched item is found later in the for loop? douardda: I agree a try/except KeyError is better here. Also why exit the for loop? Isn't there any…
		ardumontAuthorUnsubmitted Done Inline Actions Yes, i changed it locally already. Thanks. I can only assume i was thinking (partially wrongly), the other revisions must be in the same bad format. It was true in staging when i first hit that issue, that's possibly not the case in production. ardumont: Yes, i changed it locally already. Thanks. I can only assume i was thinking (partially…
known_integrity = known_artifact["extrinsic"]["raw"]["integrity"]		known_integrity = known_artifact["extrinsic"]["raw"]["integrity"]
		lewoUnsubmitted Done Inline Actions When the sources.json file is parsed, we check the integrity attribute is present. So, I think it should never be missing. In this case, I think it would improve code clearness to move this expression out of the try block. lewo: When the sources.json file is parsed, we check the integrity attribute is present. So, I think…
		ardumontAuthorUnsubmitted Done Inline Actions ack, adapted accordingly, thx. ardumont: ack, adapted accordingly, thx.
		except KeyError as e:
		logger.warning(
		"Divergent metadata revision structure detected: %(context)s",
		vlorentzUnsubmitted Not Done Inline Actions I'd rather call it "unexpected" than "divergent" (divergent evokes an infinite data structure for me) vlorentz: I'd rather call it "unexpected" than "divergent" (divergent evokes an infinite data structure…
		{
		"context": {
		"revision": hashutil.hash_to_hex(rev_id),
		"reason": str(e),
		"known_artifact": known_artifact,
		}
		},
		)
		vlorentzUnsubmitted Not Done Inline Actions why not `log.exception`? it looks important enough to deserve it vlorentz: why not `log.exception`? it looks important enough to deserve it
		# metadata field for the revision is not as expected by the loader
		# nixguix. We consider this not the right revision and continue checking
		# the other revisions
		continue
		else:
if artifact_metadata["integrity"] == known_integrity:		if artifact_metadata["integrity"] == known_integrity:
return rev_id		return rev_id
return None		return None

def extra_branches(self) -> Dict[bytes, Mapping[str, Any]]:		def extra_branches(self) -> Dict[bytes, Mapping[str, Any]]:
"""We add a branch to the snapshot called 'evaluation' pointing to the		"""We add a branch to the snapshot called 'evaluation' pointing to the
revision used to generate the sources.json file. This revision		revision used to generate the sources.json file. This revision
is specified in the sources.json file itself. For the nixpkgs		is specified in the sources.json file itself. For the nixpkgs
origin, this revision is coming from the		origin, this revision is coming from the
github.com/nixos/nixpkgs repository.		github.com/nixos/nixpkgs repository.
▲ Show 20 Lines • Show All 104 Lines • Show Last 20 Lines