Change securesystemslib.dsse.Envelope.signatures to dict upstream#743
Conversation
Signed-off-by: E3E <[email protected]>
| @@ -26,7 +26,7 @@ def __init__( | |||
| ): | |||
There was a problem hiding this comment.
I wasn't sure if I should change the last param from signatures: List[Signature] to signatures: Dict[str, Signature] or if it is fine to leave it
There was a problem hiding this comment.
Good question! Please do change the constructor argument to Dict[str, Signature] and implement the translation in from_dict and to_dict. This is also what we do with python-tuf Metadata.
There was a problem hiding this comment.
I also just saw that we raise, if there are duplicate (by keyid) signatures there. I suggest to do the same in Envelope.from_dict.
|
Hey @lukpueh , I was wondering if you could take a look at this pr when you get a chance? thanks |
secure-systems-lab/securesystemslib#743 Signed-off-by: Lukas Puehringer <[email protected]>
secure-systems-lab/securesystemslib#743 Signed-off-by: Lukas Puehringer <[email protected]>
lukpueh
left a comment
There was a problem hiding this comment.
Thanks for the clean patch! I replied to your question inline. Please address and we can merge this. FYI I already tried out what changes this will need in tuf and in-toto, and it looks reasonable (see referenced commits above).
| @@ -26,7 +26,7 @@ def __init__( | |||
| ): | |||
There was a problem hiding this comment.
Good question! Please do change the constructor argument to Dict[str, Signature] and implement the translation in from_dict and to_dict. This is also what we do with python-tuf Metadata.
| @@ -26,7 +26,7 @@ def __init__( | |||
| ): | |||
There was a problem hiding this comment.
I also just saw that we raise, if there are duplicate (by keyid) signatures there. I suggest to do the same in Envelope.from_dict.
Signed-off-by: E3E <[email protected]>
Signed-off-by: E3E <[email protected]>
| if signature.keyid in signatures: | ||
| raise ValueError( | ||
| f"Multiple signatures found for keyid {signature.keyid}" | ||
| ) |
There was a problem hiding this comment.
Thanks for the check. Would you mind adding a small test?
|
|
||
| signatures = [] | ||
| for signature in self.signatures: | ||
| for signature in list(self.signatures.values()): |
There was a problem hiding this comment.
Is there a reason to convert to list? You could iterate directly over dict_values. Same comment applies below.
There was a problem hiding this comment.
ah no - I just followed the format here comment . Will change to just iterating over dict_values
There was a problem hiding this comment.
Yeah, that PR has a few such conversions for type checking reasons.
Signed-off-by: E3E <[email protected]>
Adopt `securesystemslib.dsse.Envelope.signatures` type change from list to dict (secure-systems-lab/securesystemslib/pull/743) in `tuf.api.dsse.SimpleEnvelope` subclass. fixes theupdateframework#2564 Signed-off-by: Lukas Puehringer <[email protected]>
Adopt `securesystemslib.dsse.Envelope.signatures` type change from list to dict (secure-systems-lab/securesystemslib/pull/743) in `in_toto.models.metadata.Envelope` subclass. Signed-off-by: Lukas Puehringer <[email protected]>
Adopt `securesystemslib.dsse.Envelope.signatures` type change from list to dict (secure-systems-lab/securesystemslib/pull/743) in `in_toto.models.metadata.Envelope` subclass. Signed-off-by: Lukas Puehringer <[email protected]>
Adopt `securesystemslib.dsse.Envelope.signatures` type change from list to dict (secure-systems-lab/securesystemslib/pull/743) in `in_toto.models.metadata.Envelope` subclass. Signed-off-by: Lukas Puehringer <[email protected]>
Adopt `securesystemslib.dsse.Envelope.signatures` type change from list to dict (secure-systems-lab/securesystemslib/pull/743) in `in_toto.models.metadata.Envelope` subclass. Signed-off-by: Lukas Puehringer <[email protected]>
Adopt `securesystemslib.dsse.Envelope.signatures` type change from list to dict (secure-systems-lab/securesystemslib/pull/743) in `in_toto.models.metadata.Envelope` subclass. Signed-off-by: Lukas Puehringer <[email protected]>
Adopt `securesystemslib.dsse.Envelope.signatures` type change from list to dict (secure-systems-lab/securesystemslib/pull/743) in `in_toto.models.metadata.Envelope` subclass. Signed-off-by: Lukas Puehringer <[email protected]>
Fixes: theupdateframework/python-tuf#2564
Description of the changes being introduced by the pull request:
Please verify and check that the pull request fulfils the following requirements: