fix: Ignore older config lastmodified timestamps when fetching a config. #263
Conversation
…e to set an older config. Also set clientdate on events.
| if len(minimumLastModified) > 0 && storedLM.Before(minimumLastModified[0]) { | ||
| lastModified = minimumLastModified[0].Format(time.RFC1123) | ||
| } |
There was a problem hiding this comment.
Can you explain what this is trying to do? where is minimumLastModified coming from?
There was a problem hiding this comment.
Check if MinimumLastModified is passed (length check) - if it exists; then check whether or not the stored last modified timestamp is before the passed new minimum. If it is - set that as the new header to set.
There was a problem hiding this comment.
minlastmodified is used by SSE to pass a minimum request timestamp that we should expect.
configmanager.go
Outdated
| if parseError == nil { | ||
| if len(minimumLastModified) > 0 && lastModifiedHeaderTS.Before(minimumLastModified[0]) && numRetriesRemaining > 0 { | ||
| if storedLM.After(responseLastModified) { | ||
| return e.fetchConfig(numRetriesRemaining - 1) |
There was a problem hiding this comment.
should we add a warning here?
There was a problem hiding this comment.
we could - but nothing really will be done from it - it's an inactionable warning
configmanager.go
Outdated
| if storedLM.After(responseLastModified) { | ||
| return e.fetchConfig(numRetriesRemaining - 1) | ||
| } | ||
| if len(minimumLastModified) > 0 && responseLastModified.Before(minimumLastModified[0]) && numRetriesRemaining > 0 { | ||
| return e.fetchConfig(numRetriesRemaining-1, minimumLastModified...) | ||
| } |
There was a problem hiding this comment.
doesn't there need to be an else statement here that if its beyond the number of retries that it doesn't try and save the old request body?
There was a problem hiding this comment.
also need a test for this ^
There was a problem hiding this comment.
I'll add an abort out otherways - but that's not where we would want to abort.
There was a problem hiding this comment.
We do - it's the test I added
Also set clientdate on sdkconfig events, and regular tracked events.