abcxyz / team-link

Apache License 2.0
1 stars 0 forks source link

Consider caching tokens #45

Open lock14 opened 2 months ago

lock14 commented 2 months ago

TL;DR

Currently we are not caching tokens we mint from the GitHub app. We should consider adding this as it reduce the amount of unneccessary calls to mint a token.

Detailed design

No response

Alternatives considered

No response

Additional information

No response

sethvargo commented 2 months ago

I debating caching in the pkg, but then I decided it wasn't worth it. How often are we minting tokens?

lock14 commented 2 months ago

We mint a token whenever we need to sync a team to GitHub as we need to the token for write permissions. The token is scoped to the GitHub Org. So caching would only help if we got many requests for the same Org in short succession.

Its reasonable to assume team structures won't change frequently enough to necessitate caching, however we can't be sure until we have some actual usage patterns. This issue was left as an item to circle back to once we had actual traffic flowing through the system.

TLDR: We don't know how often we will mint tokens, it could be many times in a short span (on the order of seconds), or infrequenty (on the order of hours/days). My intuition says its likely to be the latter.

lock14 commented 2 months ago

That being said, I think supporting token caching in pkg would be useful as caching tokens has come up before in other abcxyz projects.

sethvargo commented 2 months ago

We can add it to pkg, but I don't know if we want to expire on a fixed duration or inspect the token's ttl. There's also the risk that a token is prematurely invalidated, so we need a way to force getting a new token (even if one is cached).