#27 cache the volta inventory to reduce times tools need to be downloaded#137
#27 cache the volta inventory to reduce times tools need to be downloaded#137cgrabmann wants to merge 1 commit intovolta-cli:masterfrom
Conversation
cgrabmann
changed the title
|
Oh no! I'm very sorry for missing this PR. Reviewing now... |
| "build": "npm-run-all build:clean build:ts build:dist build:docs", | ||
| "build:clean": "rimraf dist lib", | ||
| "build:dist": "ncc build src/index.ts", | ||
| "build:dist": "ncc build src/index.ts && ncc build src/cache.ts -o dist-post", |
There was a problem hiding this comment.
Let's split this into separate tasks, then use npm-run-all in build:dist. Something like:
| "build:dist": "ncc build src/index.ts && ncc build src/cache.ts -o dist-post", | |
| "build:dist": "npm-run-all build:dist:*", | |
| "build:dist:main": "ncc build src/index.ts", | |
| "build:dist:post": "ncc build src/cache.ts -o dist-post", |
| @@ -19,7 +19,7 @@ | |||
| "scripts": { | |||
| "build": "npm-run-all build:clean build:ts build:dist build:docs", | |||
| "build:clean": "rimraf dist lib", | |||
There was a problem hiding this comment.
Can you add dist-post here also?
| import * as core from '@actions/core'; | ||
| import * as stateKeys from './state-keys'; | ||
|
|
||
| async function run(): Promise<void> { |
There was a problem hiding this comment.
Lets call this main (it makes more sense in my head as main, since it's basically just the mechanism to execute this script itself and still be able to use async/await).
There was a problem hiding this comment.
Maybe we can call this post.ts?
There was a problem hiding this comment.
if we call the other file post.ts, we can call this one src/cache.ts, what do you think?
| ); | ||
| } | ||
|
|
||
| function createToolFilePatter(tool: Tool): string { |
There was a problem hiding this comment.
| function createToolFilePatter(tool: Tool): string { | |
| function createToolFilePattern(tool: Tool): string { |
| @@ -0,0 +1 @@ | |||
| export const VOLTA_HOME = 'VOLTA_HOME'; | |||
There was a problem hiding this comment.
I'd rather just use the string where we need it (instead of using this constant).
| } | ||
| } | ||
|
|
||
| async function cleanInventory(voltaHome: string, installedTools: Tool[]): Promise<void> { |
There was a problem hiding this comment.
I don't fully grok how this is going to work to actually prune the inventory. If we call restoreInventory in the setup flow of the main action all of the tools & inventory will be copied over from prior runs, then (eventually) call cleanInventory in the post script for the action -- how will we ever prune things out?
Can you walk me through the high level details?
2 participants
Fixes #27
This uses the
@actions/cachepackage provided to cache the inventory (cache) of Volta. So that installing the tools needed for the build can work even if the download would be down.I was triggered to do this by the instability of the node-download-servers in resent weeks.
Since I have no expertise in how exactly Volta works internally and therefore am not 100% sure if this is enough for Volta to work with, I would greatly appreciate feedback.
Ideas on how to cover this with tests, are also very welcome.
Inner workings:
I try to clean up the Volta inventory during teardown and only keep the tools that where actually installed during the run. When uploading the cache I create a cash-key based on the content of the inventory, so that a change in it's content also results in new and updated cache entry.
Limitations right now:
As far as I can tell, the cache is shared across all workflows in a repository. Therefore a project with pipelines all installing different tools might lead to continuous outdated cache entries, since the newest one will be pulled during setup, no matter the hash-suffix.