Skip to content

Commit 0755ca4

Browse files
authored
Fix twifkak@ last comments from #349. (#369)
1 parent e416691 commit 0755ca4

File tree

1 file changed

+2
-9
lines changed

1 file changed

+2
-9
lines changed

packager/certcache/certcache.go

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ const ocspCheckInterval = 1 * time.Hour
5858
const certCheckInterval = 24 * time.Hour
5959

6060
// Max number of OCSP request tries.
61-
// This will timeout after 2^10 minutes or ~16 hours.
61+
// This will timeout after 1 + 2 + 4 + 8 + 10 * 6 = 75 minutes.
6262
const maxOCSPTries = 10
6363

6464
// Recommended renewal duration for certs. This is duration before next cert expiry.
@@ -108,7 +108,7 @@ type CertCache struct {
108108
// Callers can use the uninitialized CertCache for testing certificates (without doing OCSP or
109109
// cert refreshes).
110110
//
111-
// TODO(banaag): per greigable@ comments:
111+
// TODO(banaag): per gregable@ comments:
112112
// The long argument list makes the callsites tricky to read and easy to get wrong, especially if several of the arguments have the same type.
113113
//
114114
// An alternative pattern would be to create an IsInitialized() bool or similarly named function that verifies all of the required fields have
@@ -358,13 +358,6 @@ func (this *CertCache) readOCSPHelper(numTries int, exhaustedRetries bool) ([]by
358358
}
359359

360360
// Returns the OCSP response and expiry, refreshing if necessary.
361-
// TODO(banaag): Per twifkak's suggestion, consider:
362-
// It may also be interesting to try to separate the retry logic from the fetch logic. One approach comes to mind:
363-
//
364-
// retryWithBackoff(func() {
365-
// ocsp, err := ...
366-
// return true if successful
367-
// }, initialWaitTime, maxTries)
368361
func (this *CertCache) readOCSP(allowRetries bool) ([]byte, time.Time, error) {
369362
var ocspUpdateAfter time.Time
370363
var err error

0 commit comments

Comments
 (0)