Open
Conversation
If this function gets fewer responses than expected, that means some of the authorizations were deleted before their containing order expired (or were never written due to a bug). Error out. Fix a test that fails with this extra check in place.
beautifulentropy
requested changes
Apr 23, 2026
| } | ||
|
|
||
| if len(validities) != len(ids) { | ||
| return nil, fmt.Errorf("getAuthorizationStatuses got %d results, expected %d", len(validities), len(ids)) |
Member
There was a problem hiding this comment.
We could provide a bit more context here.
Suggested change
| return nil, fmt.Errorf("getAuthorizationStatuses got %d results, expected %d", len(validities), len(ids)) | |
| return nil, fmt.Errorf("getAuthorizationStatuses got %d results, expected %d for ids %v", len(validities), len(ids), ids) |
| return nil, err | ||
| } | ||
|
|
||
| if len(validities) != len(ids) { |
Member
There was a problem hiding this comment.
With this check in place, this method no longer tolerates duplicate ids. We don't necessarily expect duplicate ids, but previously they would not have errored. We should stipulate this requirement in the doc comment.
| }, | ||
| NewAuthzs: []*sapb.NewAuthzRequest{ | ||
| { | ||
| Identifier: &corepb.Identifier{Type: "dns", Value: "example.com"}, |
Member
There was a problem hiding this comment.
I think you could use the same identifier.NewDNS("example.com").ToProto() as used above, here.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
If this function gets fewer responses than expected, that means some of the authorizations were deleted before their containing order expired (or were never written due to a bug). Error out.
Fix a test that fails with this extra check in place.