Fix instantiation in _to_rekor() for optional inclusion promise
#1382
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… to TransparencyLogEntry.inclusion_promise
```
File "/usr/local/google/home/rpetgrave/src/ssp/sigstore-python/sigstore/models.py", line 262, in _to_rekor
tlog_entry = rekor_v1.TransparencyLogEntry(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/google/home/rpetgrave/src/ssp/.venv/lib/python3.12/site-packages/pydantic/_internal/_dataclasses.py", line 120, in __init__
s.__pydantic_validator__.validate_python(ArgsKwargs(args, kwargs), self_instance=s)
pydantic_core._pydantic_core.ValidationError: 1 validation error for TransparencyLogEntry
inclusion_promise
Input should be a dictionary or an instance of InclusionPromise [type=dataclass_type, input_value=None, input_type=NoneType]
```
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
…Promise] to TransparencyLogEntry.inclusion_promise" This reverts commit 46b0087. Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
…nPromise] to TransparencyLogEntry.inclusion_promise" This reverts commit 43dc468. Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
30 tasks
jku
changed the title
_to_rekor() for optional inclusion proofs_to_rekor() for optional inclusion promise
jku
requested changes
May 14, 2025
There was a problem hiding this comment.
Making Bundle work without an inclusion promise makes sense, however
- if we can, let's test the whole bundle round trip (since Bundle does a lot of the real parsing)
- I don't think this is a pydantic issue, just us not being prepared for no inclusion promise
I'm not 100% sure I'm actually "requesting changes", let me know if you disagree with the ideas
CHANGELOG.md
Outdated
Comment on lines
13
to
14
| * Avoid pydantic's instantiation issues with `TransparencyLogEntry` when `InclusionPromise` is not present. | ||
|
|
There was a problem hiding this comment.
I don't think this is really related to pydantic (although I appreciate the neater handling in _to_rekor()). Isn't the issue that our code just won't work without an inclusion promise currently?
There was a problem hiding this comment.
removed mention of pydantic.
jku
reviewed
May 14, 2025
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
Signed-off-by: Ramon Petgrave <32398091+ramonpetgrave64@users.noreply.github.com>
ramonpetgrave64
changed the title
_to_rekor() for optional inclusion promise_to_rekor() for optional inclusion promise
15 tasks
|
/gcbrun |
|
Sigh, the merge rules here are really annoying...
I'll test that and update the branch anyway. |
|
/gcbrun |
|
oh, merging main does not invalidate the review -- I did not know that. So I can merge this even though I'm the last committer 🤷 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Client support for Rekor V2: sigstore-python #289
Resolves #1383
Summary
pydantic and betterproto: avoid assigning a var of type Optional[InclusionPromise] to TransparencyLogEntry.inclusion_promise
Release Note
Avoid pydantic's instantiation issues with
TransparencyLogEntrywhenInclusionPromiseis not present.Documentation
None