Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TRAP Caching: Re-introduce workaround for download timeout #1373

Merged
merged 1 commit into from
Nov 16, 2022

Conversation

edoardopirovano
Copy link
Contributor

Looking at our telemetry, I found a run where the download timeout provided by the Actions library didn't do what we expect. In particular, looking at the timestamps shows that while the downloading happened quickly, the check to see if the key was present in the cache took over 15 minutes (much longer than our 2 minute timeout). Clearly, this is problematic. While it isn't a widespread problem as far as I can see in our telemetry, an outage in the Actions cache could cause this to break all CodeQL runs which is not good.

Let's use our workaround that wraps the entire call in a timeout here too like we already do for uploading.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.
  • Confirm the changelog has been updated if necessary.

@edoardopirovano edoardopirovano requested a review from a team as a code owner November 16, 2022 11:24
Copy link
Contributor

@henrymercer henrymercer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we download trap caches in the init Action, do we need to add await checkForTimeout(); to its entrypoint to prevent things hanging?

@edoardopirovano
Copy link
Contributor Author

Since we download trap caches in the init Action, do we need to add await checkForTimeout(); to its entrypoint to prevent things hanging?

Indeed we do, good point. I've amended the PR to do this.

@edoardopirovano edoardopirovano merged commit 53060aa into main Nov 16, 2022
@edoardopirovano edoardopirovano deleted the edoardo/workaround-download branch November 16, 2022 17:28
sfoslund added a commit to microsoft/accessibility-insights-web that referenced this pull request Nov 29, 2022
#### Details

The codeql action has been timing out in our CI consistently since this
weekend. It appears that it's the same issue described in the
description of github/codeql-action#1373, so
since the work around implemented in that PR has not yet been released,
I'm working around it here by just temporarily disabling trap caching.

##### Motivation

Work around codeql timeouts

#### Pull request checklist
<!-- If a checklist item is not applicable to this change, write "n/a"
in the checkbox -->
- [n/a] Addresses an existing issue: #0000
- [n/a] Ran `yarn null:autoadd`
- [n/a] Ran `yarn fastpass`
- [n/a] Added/updated relevant unit test(s) (and ran `yarn test`)
- [n/a] Verified code coverage for the changes made. Check coverage
report at: `<rootDir>/test-results/unit/coverage`
- [x] PR title *AND* final merge commit title both start with a semantic
tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See
`CONTRIBUTING.md`.
- [n/a] (UI changes only) Added screenshots/GIFs to description above
- [n/a] (UI changes only) Verified usability with NVDA/JAWS
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants