fix: labels from first invocation were being applied to future calls #251
Conversation
googlebot
added
the
cla: yes
| if (!labelsCache) { | ||
| await refreshLabels(github, repoPath); | ||
| } | ||
| return labelsCache; | ||
| const labels = { | ||
| labels: labelsCache.labels.slice(0), |
There was a problem hiding this comment.
What's the purpose of a labels.slice(0)?
There was a problem hiding this comment.
this creates a copy of the labels so that we don't end up populating additional labels in the cache each time we access labels.
creating a cache means that the labels are cached between invocations of cloud functions.
| const labels = { | ||
| labels: labelsCache.labels.slice(0), | ||
| } as Labels; | ||
| const apiLabelsRes = await handler.getApiLabels(repoPath); |
There was a problem hiding this comment.
Little worried here. Isn't this essentially completely bypassing the cache? Will this end up calling out to GCS constantly?
There was a problem hiding this comment.
It's only called once per function invocation, which only occurs rarely, e.g., when a new repo is created, or a label is deleted.
I think we're going to need to get rid of the push functionality any ways, because it takes like an hour to backfill all labels, and the maximum execution time of a cloud function is 9 minutes.
…251) 84ec6e7 commit 84ec6e7 Author: Benjamin E. Coe <[email protected]> Date: Tue Jan 28 17:54:16 2020 -0800 fix: labels from first invocation were being applied to future calls (#251)
caching was interfering with our ability to return labels based on the context of the repo firing a webhook, this lead to the label for the first repo invoking the cloud function to be applied to subsequent calls.