Skip to content

Commit 60df235

Browse files
Enhance GitHub results cache (#87)
* Includes enhancement to how false/true values are cached for membership checks. This should resolve pain around needing to wait for 2 days for a membership check to clear. * Adds additional logging fields for cache hits to make triaging issues easier. Future enhancements may include allowing the caching values to be set via App Configuration.
1 parent 0013196 commit 60df235

File tree

1 file changed

+24
-6
lines changed

1 file changed

+24
-6
lines changed

src/services/gitHubCache.ts

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ export class GitHubClientCache implements InstalledClient {
5656
properties: {
5757
"Data": id,
5858
"Operation": "IsUserMember",
59-
"Group": "GitHub"
59+
"Group": "GitHub",
60+
"Value": result
6061
}
6162
})
6263

@@ -68,10 +69,26 @@ export class GitHubClientCache implements InstalledClient {
6869

6970
const actualResult = await this.client.IsUserMember(id);
7071

71-
if (actualResult.successful) {
72-
await this.cacheClient.set(cacheKey, actualResult.data.toString(), {
73-
EX: 172800 // Expire every 2 days
74-
});
72+
if (actualResult.successful) {
73+
// TODO: switch all cache expirations to application configuration values.
74+
75+
const userIsMember = actualResult.data;
76+
77+
if(userIsMember) {
78+
await this.cacheClient.set(cacheKey, userIsMember.toString(), {
79+
EX: 172800 // Expire every 2 days
80+
});
81+
}
82+
else {
83+
// If membership check comes back as "not a member," we still want to cache
84+
// the value so that we aren't hitting GitHub's APIs "too much."
85+
// With that being said, we don't want to cache it for "too long" as then
86+
// it will start to cause user abrasion.
87+
await this.cacheClient.set(cacheKey, userIsMember.toString(), {
88+
EX: 1800 // Expire every 30 minutes
89+
// It is unlikely that 30 minutes will cause much pain
90+
});
91+
}
7592
}
7693

7794
return actualResult;
@@ -100,7 +117,8 @@ export class GitHubClientCache implements InstalledClient {
100117
properties: {
101118
"Data": gitHubId,
102119
"Operation": "DoesUserExist",
103-
"Group": "GitHub"
120+
"Group": "GitHub",
121+
"Value": result
104122
}
105123
})
106124

0 commit comments

Comments
 (0)