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: | |||||
lewo: We should catch a `keyError` exception instead since it could come from several keys. | |||||
Done Inline Actionsyes ardumont: yes | |||||
Not Done Inline ActionsI 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… | |||||
Done Inline ActionsYes, i changed it locally already. Thanks. I can only assume i was thinking (partially wrongly), the other revisions must 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"] | ||||
Done Inline ActionsWhen the sources.json file is parsed, we check the integrity attribute is present. So, I think it should never be missing. lewo: When the sources.json file is parsed, we check the integrity attribute is present. So, I think… | |||||
Done Inline Actionsack, adapted accordingly, thx. ardumont: ack, adapted accordingly, thx. | |||||
except KeyError as e: | |||||
logger.exception( | |||||
Not Done Inline ActionsI'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… | |||||
"Unexpected metadata revision structure detected: %(context)s", | |||||
{ | |||||
"context": { | |||||
"revision": hashutil.hash_to_hex(rev_id), | |||||
"reason": str(e), | |||||
"known_artifact": known_artifact, | |||||
} | |||||
}, | |||||
Not Done Inline Actionswhy 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 |
We should catch a keyError exception instead since it could come from several keys.