Skip to content

Add option to disable downloads #263

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

Merged
merged 1 commit into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@
"type": "string",
"default": ""
},
"coder.enableDownloads": {
"markdownDescription": "Allow the plugin to download the CLI when missing or out of date.",
"type": "boolean",
"default": true
},
Comment on lines +66 to +70
Copy link
Member

Choose a reason for hiding this comment

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

I'm still trying to get up to speed on VS Code extension stuff. I think I get the point of the contribution point properties, but aside from the default values, how do these properties get set over time?

Looks like it's strictly the config.update method?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, you can update it with config.update() from plugin code, although we have no need to update it ourselves in this case, we just let it get set via the settings window (which is presumably using config.update()).

Funny thing about the default is that it seems to have no actual effect on the code, it only changes what is displayed in the settings window. In the code you just get undefined.

Copy link
Member Author

Choose a reason for hiding this comment

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

Technically you could also munge the settings.json file and VS Code will pick that up but that is a pretty janky way to update a value. I have seen it done though although I forget why.

"coder.headerCommand": {
"markdownDescription": "An external command that outputs additional HTTP headers added to all requests. The command must output each header as `key=value` on its own line. The following environment variables will be available to the process: `CODER_URL`. Defaults to the value of `CODER_HEADER_COMMAND` if not set.",
"type": "string",
Expand Down
22 changes: 20 additions & 2 deletions src/storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,9 @@ export class Storage {
/**
* Download and return the path to a working binary. If there is already a
* working binary and it matches the server version, return that, skipping the
* download. Throw if unable to download a working binary.
* download. If it does not match but downloads are disabled, return whatever
* we have and log a warning. Otherwise throw if unable to download a working
* binary, whether because of network issues or downloads being disabled.
*/
public async fetchBinary(): Promise<string> {
const baseURL = this.getURL()
Expand All @@ -130,13 +132,19 @@ export class Storage {
}
this.output.appendLine(`Using deployment URL: ${baseURL}`)

// Settings can be undefined when set to their defaults (true in this case),
// so explicitly check against false.
const enableDownloads = vscode.workspace.getConfiguration().get("coder.enableDownloads") !== false
this.output.appendLine(`Downloads are ${enableDownloads ? "enabled" : "disabled"}`)

// Get the build info to compare with the existing binary version, if any,
// and to log for debugging.
const buildInfo = await getBuildInfo()
this.output.appendLine(`Got server version: ${buildInfo.version}`)

// Check if there is an existing binary and whether it looks valid. If it
// is valid and matches the server, we can return early.
// is valid and matches the server, or if it does not match the server but
// downloads are disabled, we can return early.
const binPath = this.binaryPath()
this.output.appendLine(`Using binary path: ${binPath}`)
const stat = await cli.stat(binPath)
Expand All @@ -151,6 +159,11 @@ export class Storage {
if (version === buildInfo.version) {
this.output.appendLine("Using existing binary since it matches the server version")
return binPath
} else if (!enableDownloads) {
this.output.appendLine(
"Using existing binary even though it does not match the server version because downloads are disabled",
)
return binPath
}
this.output.appendLine("Downloading since existing binary does not match the server version")
} catch (error) {
Expand All @@ -159,6 +172,11 @@ export class Storage {
}
}

if (!enableDownloads) {
this.output.appendLine("Unable to download CLI because downloads are disabled")
throw new Error("Unable to download CLI because downloads are disabled")
}

// Remove any left-over old or temporary binaries.
const removed = await cli.rmOld(binPath)
removed.forEach(({ fileName, error }) => {
Expand Down