Skip checking for updates for completion commands

[thommahoney]

I have my shell (ZSH) configured to support completion of fastly CLI commands. To do this, I have placed the following into my ~/.zshrc:

eval "$(fastly --completion-script-zsh)"

When on high-latency internet connections, the asynchronous update check adds 3 seconds to the loading of my shell. Furthermore, because the check fails with such spotty internet, I see the following message before my shell prompt following the 3 second timeout:

ERROR: there was a problem updating the versioning information for the Fastly CLI (we won't check again until 5m have passed):

Get "https://developer.fastly.com/api/internal/cli-config": context deadline exceeded (Client.Timeout exceeded while awaiting headers).

If you believe this error is the result of a bug, please file an issue: https://github.com/fastly/cli/issues/new?labels=bug&template=bug_report.md

There is a fallback version of the configuration provided with the CLI install (run `fastly config` to view the config) which enables the CLI to continue to be usable even though the config couldn't be updated.

Instead of filing a bug, I thought I'd try to fix it myself. I'm not sure if this actually works because I can't build the source code given my spotty airplane wi-fi.

[Integralist]

This looks fine to me but I don't think it will fix your issue with the error:

'there was a problem updating the versioning information...'

Because that error looks to come from a separate call to load the config here:

cli/cmd/fastly/main.go

Lines 133 to 165 in 2306872

	// Validate if configuration is older than its TTL
	if check.Stale(file.CLI.LastChecked, file.CLI.TTL) {
	if verboseOutput {
	text.Info(out, `
	Compatibility and versioning information for the Fastly CLI is being updated in the background. The updated data will be used next time you execute a fastly command.
	`)
	}
	wait = true
	go func() {
	// NOTE: we no longer use the hardcoded config.RemoteEndpoint constant.
	// Instead we rely on the values inside of the application
	// configuration file to determine where to load the config from.
	err := file.Load(file.CLI.RemoteConfig, config.FilePath, httpClient)
	if err != nil {
	// If there is an error loading the configuration, then there is no
	// point trying to retry the operation on the next CLI invocation as we
	// already have a static backup that can be used. Defer another attempt
	// until after the TTL has expired.
	file.CLI.LastChecked = time.Now().Format(time.RFC3339)
	file.Write(config.FilePath)
	errNotice := "there was a problem updating the versioning information for the Fastly CLI"
	checkAgain := fmt.Sprintf("we won't check again until %s have passed", file.CLI.TTL)
	errLoadConfig = fsterr.RemediationError{
	Inner: fmt.Errorf("%s (%s):\n\n%w", errNotice, checkAgain, err),
	Remediation: fsterr.BugRemediation + "\n\n" + fsterr.ConfigRemediation,
	}
	}
	waitForWrite <- true
	}()
	}

[thommahoney]

Weird I didn't see the contents of checkAgain in my output but perhaps I was running an outdated version of the CLI. Is the CLI config useful in the context of shell completion? If not, perhaps we should wrap that in a similar conditional.

[Integralist]

@thommahoney

Is the CLI config useful in the context of shell completion?

Nope. I don't think having the config would be useful so we should try to avoid it when handling the shell autocomplete command.

That said, the logic for checking whether the config is stale is happening in cmd/fastly/main.go and the logic for identifying the current command is happening in pkg/app/run.go.

So you could go big and refactor the main func so that the stale check is moved into the run func, or maybe because we're only checking for a single flag, --completion-script-<T>, maybe just tack on an extra conditional check to keep things simple.

[thommahoney]

I was able to reproduce the latency and error output by installing the Network Link Conditioner tooling from Apple. I made an "Extremely Bad Network" configuration to test out this functionality:

In order to properly test this, one must override the cli.last_checked value in the config.toml to something greater than 5 minutes in the past.

Using this technique, I arrived at the latest commit: f7be6c3

[Integralist]

Ah nice, I forgot I had moved that stuff into a separate package 🙂