Tool cache install from a manifest file #382
Conversation
|
Adding @ericsciple to have a glance. This is early exploration on an idea in the referenced ADR. Basically if a repo has a json file in the root with all the tool binary distributions as releases in that repo, then a setup-*** need only pull that file, at a couple lines and it just works. It also protects against external service distruptions. See the ADR referenced. |
bryanmacfarlane
changed the title
bryanmacfarlane
changed the title
| export type IToolRelease = mm.IToolRelease | ||
| export type IToolReleaseFile = mm.IToolReleaseFile | ||
|
|
||
| export async function getManifestFromUrl(url: string): Promise<IToolRelease[]> { |
There was a problem hiding this comment.
How confident do you feel about the behavior?
To buy leeway, I'm wondering if whether it would be good to add a function description, indicating experimental and behavior may change.
There was a problem hiding this comment.
Good suggestion. I may go one step further and release a actions/tool-cache@preview out of this branch. Same on the setup-node side. Start a v2-beta
|
|
||
| let chk = item.arch === archFilter && item.platform === platFilter | ||
| if (chk && item.platform_version) { | ||
| const osVersion = module.exports._getOsVersion() |
There was a problem hiding this comment.
Since _getOsVersion() is heavy (launches a process (mac) or reads a file from disk (linux)), consider caching the result.
For example:
if (osVersion === undefined) {
osVersion = _getOsVersion()
}
| } | ||
|
|
||
| if (match && file) { | ||
| // clone since we're mutating the file list to be only the file that matches |
There was a problem hiding this comment.
+1 for cloning the object :)
| @@ -18,7 +18,7 @@ | |||
| "@typescript-eslint/await-thenable": "error", | |||
| "@typescript-eslint/ban-ts-ignore": "error", | |||
| "camelcase": "off", | |||
| "@typescript-eslint/camelcase": "error", | |||
| "@typescript-eslint/camelcase": "off", | |||
There was a problem hiding this comment.
I got tired of adding all the line by line exceptions. Fact is as we deserialize these requests into structures dictated externally, many have non camel cased properties.
| @@ -32,7 +34,8 @@ const userAgent = 'actions/tool-cache' | |||
| */ | |||
| export async function downloadTool( | |||
There was a problem hiding this comment.
Can we update the function's description to include this new input?
If a setup-* tool is cached to insulate against target disriptions and the repo contains a versions-manifest.json advertising versions available by platform, version and arch, this api will match a semver spec (ranges, etc.) against the latest and discover where to pull it from. The abstraction allows for putting on a CDN and changing locations later which are opaque and non breaking to the consumers (the setup-* tasks).
This also allows the hosted images to cache n versions directly on the image for instant resolution and as older versions roll off the image, it still works but may incur a performance of typically 10s of seconds to pull from the CDN cache. On miss, the setup task can ultimately still pull from the origin (the source tools distribution) JIT.