-
Notifications
You must be signed in to change notification settings - Fork 850
VS Code extension: Auto-restore on workspace open and config change #15546
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
base: main
Are you sure you want to change the base?
Changes from all commits
b026fd5
a58fc94
c91c4f7
500bf15
a3b7631
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -96,6 +96,11 @@ export const openCliInstallInstructions = vscode.l10n.t('See CLI installation in | |||||
| export const cliNotAvailable = vscode.l10n.t('Aspire CLI is not available on PATH. Please install it and restart VS Code.'); | ||||||
| export const cliFoundAtDefaultPath = (path: string) => vscode.l10n.t('Aspire CLI found at {0}. The extension will use this path.', path); | ||||||
| export const selectDirectoryTitle = vscode.l10n.t('Select directory'); | ||||||
| export const runningAspireRestore = (configPath: string) => vscode.l10n.t('Running aspire restore on {0}...', configPath); | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| export const runningAspireRestoreProgress = (completed: number, total: number) => vscode.l10n.t('Running aspire restore ({0}/{1} projects)...', completed, total); | ||||||
radical marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| export const aspireRestoreCompleted = (configPath: string) => vscode.l10n.t('Aspire restore completed for {0}.', configPath); | ||||||
| export const aspireRestoreAllCompleted = vscode.l10n.t('Aspire restore completed'); | ||||||
| export const aspireRestoreFailed = (configPath: string, error: string) => vscode.l10n.t('aspire restore failed for {0}: {1}', configPath, error); | ||||||
| export const selectFileTitle = vscode.l10n.t('Select file'); | ||||||
| export const enterPipelineStep = vscode.l10n.t('Enter the pipeline step to execute'); | ||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,204 @@ | ||
| import * as vscode from 'vscode'; | ||
| import path from 'path'; | ||
| import { aspireConfigFileName } from './cliTypes'; | ||
| import { findAspireSettingsFiles } from './workspace'; | ||
| import { ChildProcessWithoutNullStreams } from 'child_process'; | ||
| import { spawnCliProcess } from '../debugger/languages/cli'; | ||
| import { AspireTerminalProvider } from './AspireTerminalProvider'; | ||
| import { extensionLogOutputChannel } from './logging'; | ||
| import { getEnableAutoRestore } from './settings'; | ||
| import { runningAspireRestore, runningAspireRestoreProgress, aspireRestoreCompleted, aspireRestoreAllCompleted, aspireRestoreFailed } from '../loc/strings'; | ||
|
|
||
| /** | ||
| * Runs `aspire restore` on workspace open and whenever aspire.config.json content changes | ||
| * (e.g. after a git branch switch). | ||
| */ | ||
| export class AspirePackageRestoreProvider implements vscode.Disposable { | ||
| private static readonly _maxConcurrency = 4; | ||
|
|
||
| private readonly _disposables: vscode.Disposable[] = []; | ||
| private readonly _terminalProvider: AspireTerminalProvider; | ||
| private readonly _statusBarItem: vscode.StatusBarItem; | ||
| private readonly _lastContent = new Map<string, string>(); // fsPath → content | ||
| private readonly _active = new Map<string, string>(); // configDir → relativePath | ||
| private readonly _childProcesses = new Set<ChildProcessWithoutNullStreams>(); | ||
| private readonly _timeouts = new Set<ReturnType<typeof setTimeout>>(); | ||
| private readonly _pendingRestore = new Set<string>(); // configDirs needing re-restore | ||
| private _total = 0; | ||
| private _completed = 0; | ||
|
|
||
| constructor(terminalProvider: AspireTerminalProvider) { | ||
| this._terminalProvider = terminalProvider; | ||
| this._statusBarItem = vscode.window.createStatusBarItem(vscode.StatusBarAlignment.Left, 0); | ||
| this._disposables.push(this._statusBarItem); | ||
| } | ||
|
|
||
| async activate(): Promise<void> { | ||
| this._disposables.push( | ||
| vscode.workspace.onDidChangeConfiguration(e => { | ||
| if (e.affectsConfiguration('aspire.enableAutoRestore') && getEnableAutoRestore()) { | ||
| this._restoreAll(); | ||
| } | ||
| }) | ||
| ); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This should also call
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The call site in void this._restoreAll().catch(err => {
extensionLogOutputChannel.warn(`Auto-restore failed: ${String(err)}`);
}); |
||
|
|
||
| if (!getEnableAutoRestore()) { | ||
| extensionLogOutputChannel.info('Auto-restore is disabled'); | ||
| return; | ||
| } | ||
|
|
||
| await this._restoreAll(); | ||
| this._watchConfigFiles(); | ||
| } | ||
|
|
||
| private async _restoreAll(): Promise<void> { | ||
| const allConfigs = await findAspireSettingsFiles(); | ||
| const configs = allConfigs.filter(uri => uri.fsPath.endsWith(aspireConfigFileName)); | ||
| if (configs.length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| this._total = configs.length; | ||
| this._completed = 0; | ||
|
|
||
| const pending = new Set<Promise<void>>(); | ||
| for (const uri of configs) { | ||
| const p = this._restoreIfChanged(uri, true).finally(() => pending.delete(p)); | ||
| pending.add(p); | ||
| if (pending.size >= AspirePackageRestoreProvider._maxConcurrency) { | ||
| await Promise.race(pending); | ||
| } | ||
| } | ||
| await Promise.all(pending); | ||
| } | ||
adamint marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| private _watchConfigFiles(): void { | ||
| for (const folder of vscode.workspace.workspaceFolders ?? []) { | ||
| const watcher = vscode.workspace.createFileSystemWatcher( | ||
| new vscode.RelativePattern(folder, `**/${aspireConfigFileName}`) | ||
| ); | ||
| watcher.onDidChange(uri => this._onChanged(uri)); | ||
| watcher.onDidCreate(uri => this._onChanged(uri)); | ||
| this._disposables.push(watcher); | ||
| } | ||
| } | ||
|
|
||
| private async _onChanged(uri: vscode.Uri): Promise<void> { | ||
| if (!getEnableAutoRestore()) { | ||
| return; | ||
| } | ||
| if (this._active.size === 0) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to worry about
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be safer to cancel an existing restore task for a given uri, if it exists. I’ll make that change |
||
| this._total = 1; | ||
| this._completed = 0; | ||
| } | ||
| await this._restoreIfChanged(uri, false); | ||
| } | ||
|
|
||
| private async _restoreIfChanged(uri: vscode.Uri, isInitial: boolean): Promise<void> { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When Minor UX issue — consider incrementing |
||
| let content: string; | ||
| try { | ||
| content = (await vscode.workspace.fs.readFile(uri)).toString(); | ||
| } catch (error) { | ||
| extensionLogOutputChannel.warn(`Failed to read ${uri.fsPath}: ${error}`); | ||
| return; | ||
| } | ||
|
|
||
| const prev = this._lastContent.get(uri.fsPath); | ||
| if (!isInitial && prev === content) { | ||
| return; | ||
| } | ||
|
|
||
| const configDir = path.dirname(uri.fsPath); | ||
| const relativePath = vscode.workspace.asRelativePath(uri); | ||
| extensionLogOutputChannel.info(`${isInitial ? 'Initial' : 'Changed'} restore for ${relativePath}`); | ||
|
|
||
| // Don't update baseline until restore succeeds; queue re-restore if one is already active | ||
| if (this._active.has(configDir)) { | ||
| this._pendingRestore.add(configDir); | ||
| return; | ||
| } | ||
|
|
||
| this._lastContent.set(uri.fsPath, content); | ||
| try { | ||
| await this._runRestore(configDir, relativePath); | ||
| } catch (error) { | ||
| extensionLogOutputChannel.warn(`Restore failed for ${relativePath}: ${error}`); | ||
| } | ||
|
|
||
| // If a change arrived while we were restoring, re-read and restore again | ||
| if (this._pendingRestore.delete(configDir)) { | ||
| await this._restoreIfChanged(uri, false); | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comment on line 124 says "Don't update baseline until restore succeeds" but The content should only be saved in |
||
| } | ||
|
|
||
| private async _runRestore(configDir: string, relativePath: string): Promise<void> { | ||
| this._active.set(configDir, relativePath); | ||
| this._showProgress(); | ||
|
|
||
| const cliPath = await this._terminalProvider.getAspireCliExecutablePath(); | ||
| await new Promise<void>((resolve, reject) => { | ||
| const proc = spawnCliProcess(this._terminalProvider, cliPath, ['restore'], { | ||
| workingDirectory: configDir, | ||
| noExtensionVariables: true, | ||
| exitCallback: code => { | ||
| if (code === 0) { | ||
| extensionLogOutputChannel.info(aspireRestoreCompleted(relativePath)); | ||
| resolve(); | ||
| } else { | ||
| extensionLogOutputChannel.warn(aspireRestoreFailed(relativePath, `exit code ${code}`)); | ||
| reject(new Error(`exit code ${code}`)); | ||
| } | ||
| }, | ||
| errorCallback: error => { | ||
| extensionLogOutputChannel.warn(aspireRestoreFailed(relativePath, error.message)); | ||
| reject(error); | ||
| }, | ||
| }); | ||
| this._childProcesses.add(proc); | ||
| const timeout = setTimeout(() => { proc.kill(); reject(new Error('restore timed out')); }, 120_000); | ||
| this._timeouts.add(timeout); | ||
| proc.on('close', () => { | ||
| clearTimeout(timeout); | ||
| this._timeouts.delete(timeout); | ||
| this._childProcesses.delete(proc); | ||
| }); | ||
| }).finally(() => { | ||
| this._active.delete(configDir); | ||
| this._completed++; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When the timeout fires, it calls Consider adding a |
||
| this._showProgress(); | ||
| if (this._active.size === 0) { | ||
| const hideTimeout = setTimeout(() => { | ||
| this._timeouts.delete(hideTimeout); | ||
| if (this._active.size === 0) { this._statusBarItem.hide(); } | ||
| }, 5000); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, it would be a good idea to-I’ll add |
||
| this._timeouts.add(hideTimeout); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| private _showProgress(): void { | ||
| if (this._active.size === 0) { | ||
| this._statusBarItem.text = `$(check) ${aspireRestoreAllCompleted}`; | ||
| } else if (this._total <= 1) { | ||
| this._statusBarItem.text = `$(sync~spin) ${runningAspireRestore([...this._active.values()][0])}`; | ||
| } else { | ||
| this._statusBarItem.text = `$(sync~spin) ${runningAspireRestoreProgress(this._completed, this._total)}`; | ||
| } | ||
| this._statusBarItem.show(); | ||
| } | ||
|
|
||
| dispose(): void { | ||
| for (const proc of this._childProcesses) { | ||
| proc.kill(); | ||
| } | ||
| this._childProcesses.clear(); | ||
| for (const timeout of this._timeouts) { | ||
| clearTimeout(timeout); | ||
| } | ||
| this._timeouts.clear(); | ||
| for (const d of this._disposables) { | ||
| d.dispose(); | ||
| } | ||
| this._disposables.length = 0; | ||
|
Comment on lines
+191
to
+202
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm a TS noob. Do we need to handle exceptions here like if killing a process fails? |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,13 @@ | ||
| { | ||
| "sdk": { | ||
| "version": "13.3.0-preview.1.26163.4" | ||
| "appHost": { | ||
| "path": "AppHost/apphost.ts", | ||
| "language": "typescript/nodejs" | ||
| }, | ||
| "channel": "daily", | ||
| "packages": { | ||
| "Aspire.Hosting.Azure.Storage": "13.3.0-preview.1.26167.8", | ||
| "Aspire.Hosting.JavaScript": "13.3.0-preview.1.26167.8", | ||
| "Aspire.Hosting.Azure": "13.3.0-preview.1.26167.8", | ||
| "Aspire.Hosting.Azure.EventHubs": "13.3.0-preview.1.26167.8" | ||
| "Aspire.Hosting.Azure.Storage": "13.2.0", | ||
| "Aspire.Hosting.JavaScript": "13.2.0", | ||
| "Aspire.Hosting.Azure": "13.2.0", | ||
| "Aspire.Hosting.Azure.EventHubs": "13.2.0" | ||
| } | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,13 +2,9 @@ | |
| "appHost": { | ||
| "path": "apphost.ts" | ||
| }, | ||
| "sdk": { | ||
| "version": "13.3.0-preview.1.26163.4" | ||
| }, | ||
| "channel": "daily", | ||
| "packages": { | ||
| "Aspire.Hosting.Python": "13.3.0-preview.1.26167.8", | ||
| "Aspire.Hosting.PostgreSQL": "13.3.0-preview.1.26167.8", | ||
| "Aspire.Hosting.JavaScript": "13.3.0-preview.1.26167.8" | ||
| "Aspire.Hosting.Python": "13.2.0", | ||
| "Aspire.Hosting.PostgreSQL": "13.2.0", | ||
| "Aspire.Hosting.JavaScript": "13.2.0" | ||
|
Comment on lines
-10
to
+8
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't be using the latest
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Practically, no difference. What would be really nice is if there were a way to avoid putting a version altogether and just taking the latest (or just allowing a “latest” tag). |
||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.