Add tools to manage caching discovery results#73
Open
bmillwood wants to merge 2 commits intokrdlab:masterfrom
Open
Add tools to manage caching discovery results#73bmillwood wants to merge 2 commits intokrdlab:masterfrom
bmillwood wants to merge 2 commits intokrdlab:masterfrom
Conversation
This allows long-running services to know how often they may need to rerun discovery.
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
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.
I was running into a problem that turned out to ultimately be caused by holding onto the results of
discoverfor too long. It turns out that at least in Google's case, theExpiresandcache-controlheaders do tell you how long you can hold on to the results for (actually, they tell you an hour, whereas IME they work for a week, but eh, close enough). This code adds minimal support for reading theExpiresheader, so that I can know when I need to calldiscoveragain, and also adds a new functioncachedDiscover, which makes it easy to refetch the cache when necessary. (I've implemented it in a way that refetches the cache only when it's asked for, but you could also imagine implementing it such that it proactively refetches whenever the cache expires, even if we haven't asked for it yet.)I added a test that we do successfully get a
validUntilfrom Google, and I'm usingcachedDiscoverin my own hobbyist project, so I know it at least somewhat works :) (my hobbyist project uses a slightly revised version that debug-prints whenever it refreshes the cache, so I know the cache expiry times are coming back correctly, and the cache isn't being refreshed unnecessarily... I haven't tested OIDC providers other than Google, or timezones other than UTC, though I suspect some of these things might just always be UTC)Since the
Providertype exposes its fields, and this PR adds a new one, it would necessitate a major version bump. But I think you're already locked into such a thing by 1f50c3f