Skip to content
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 explorer refreshes on streamed events instead of polling so frequently #3680

Merged
merged 29 commits into from
Nov 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
c39f54f
Establish a tree prefix type
bwateratmsft Oct 27, 2022
b994f4e
Rewrite refresh code
bwateratmsft Oct 27, 2022
bf6890a
Remove some dead code
bwateratmsft Oct 27, 2022
c8a2f01
Some fixes from self-review
bwateratmsft Oct 28, 2022
dae9839
Actually refresh that tree
bwateratmsft Oct 28, 2022
e591226
Add some refresh management for contexts
bwateratmsft Oct 28, 2022
ae6dedb
Copy changes from upstream
bwateratmsft Nov 1, 2022
71ea6be
Mark some TODOs
bwateratmsft Nov 1, 2022
cc3383a
Another TODO
bwateratmsft Nov 1, 2022
2fa7ed6
Add `tree-kill` package
bwateratmsft Nov 2, 2022
93aab77
Fix build break from upstream
bwateratmsft Nov 2, 2022
127172e
Sync with upstream
bwateratmsft Nov 2, 2022
79055e5
Add second command runner shape
bwateratmsft Nov 2, 2022
0c6a17c
Fix breaks
bwateratmsft Nov 2, 2022
009aec4
Cleanup
bwateratmsft Nov 2, 2022
c1176b6
Scope events a little tighter
bwateratmsft Nov 3, 2022
06fbe9f
Normalize "Less than a second" too
bwateratmsft Nov 3, 2022
e4a8525
Remove refresh interval setting
bwateratmsft Nov 3, 2022
777b663
Pull upstream fix
bwateratmsft Nov 3, 2022
6e7c836
Eat error we don't need
bwateratmsft Nov 3, 2022
fb4671c
Explanatory comment
bwateratmsft Nov 3, 2022
f759e0c
Errors need to come through
bwateratmsft Nov 3, 2022
5c6a78f
Change the wrapping paper
bwateratmsft Nov 3, 2022
6af24fb
Refactor to remove `CommandResponse<T>`
bwateratmsft Nov 7, 2022
e37afca
Debounce rapid refreshes
bwateratmsft Nov 7, 2022
672e897
Pull upstream changes
bwateratmsft Nov 7, 2022
0481dde
Use upstream changes
bwateratmsft Nov 7, 2022
fe74c90
Fix event process not being killed
bwateratmsft Nov 7, 2022
a42a20e
Fix over-eager refresh manager
bwateratmsft Nov 7, 2022
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
14 changes: 14 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 1 addition & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1540,11 +1540,6 @@
"default": true,
"description": "%vscode-docker.config.docker.promptForRegistryWhenPushingImages%"
},
"docker.explorerRefreshInterval": {
"type": "number",
"default": 2000,
"description": "%vscode-docker.config.docker.explorerRefreshInterval%"
},
"docker.commands.build": {
"oneOf": [
{
Expand Down Expand Up @@ -2979,6 +2974,7 @@
"node-fetch": "^2.6.7",
"semver": "^7.3.7",
"tar": "^6.1.11",
"tree-kill": "^1.2.2",
"vscode-languageclient": "^8.0.2",
"vscode-nls": "^5.0.0",
"vscode-tas-client": "^0.1.22",
Expand Down
1 change: 0 additions & 1 deletion package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@
"vscode-docker.config.template.composeDown.label": "The label displayed to the user.",
"vscode-docker.config.template.composeDown.match": "The regular expression for choosing the right template. Checked against docker-compose YAML files, folder name, etc.",
"vscode-docker.config.template.composeDown.description": "Command templates for `docker-compose down` commands.",
"vscode-docker.config.docker.explorerRefreshInterval": "Docker view refresh interval (milliseconds)",
"vscode-docker.config.docker.containers.groupBy": "The property to use to group containers in Docker view: ContainerId, ContainerName, CreatedTime, FullTag, ImageId, Networks, Ports, Registry, Repository, RepositoryName, RepositoryNameAndTag, State, Status, Tag, or None",
"vscode-docker.config.docker.containers.description": "Any secondary properties to display for a container (an array). Possible elements include: ContainerId, ContainerName, CreatedTime, FullTag, ImageId, Networks, Ports, Registry, Repository, RepositoryName, RepositoryNameAndTag, State, Status, and Tag",
"vscode-docker.config.docker.containers.label": "The primary property to display for a container: ContainerId, ContainerName, CreatedTime, FullTag, ImageId, Networks, Ports, Registry, Repository, RepositoryName, RepositoryNameAndTag, State, Status, or Tag",
Expand Down
5 changes: 3 additions & 2 deletions src/commands/containers/attachShellContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Licensed under the MIT License. See LICENSE.md in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { ContainerOS } from '../../runtimes/docker';
import { ContainerOS, VoidCommandResponse } from '../../runtimes/docker';
import { IActionContext } from '@microsoft/vscode-azext-utils';
import { ext } from '../../extensionVariables';
import { localize } from '../../localize';
Expand Down Expand Up @@ -34,7 +34,8 @@ export async function attachShellContainer(context: IActionContext, node?: Conta
try {
// If this succeeds, bash is present (exit code 0)
await ext.runWithDefaultShell(client =>
client.execContainer({ container: node.containerId, interactive: true, command: ['sh', '-c', 'which bash'] })
// Since we're not interested in the output, just the exit code, we can pretend this is a `VoidCommandResponse`
client.execContainer({ container: node.containerId, interactive: true, command: ['sh', '-c', 'which bash'] }) as Promise<VoidCommandResponse>
);
shellCommand = 'bash';
} catch {
Expand Down
7 changes: 4 additions & 3 deletions src/commands/containers/composeGroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Licensed under the MIT License. See LICENSE.md in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { CommandResponse, CommonOrchestratorCommandOptions, IContainerOrchestratorClient, LogsCommandOptions } from '../../runtimes/docker';
import { CommonOrchestratorCommandOptions, IContainerOrchestratorClient, LogsCommandOptions, VoidCommandResponse } from '../../runtimes/docker';
import { IActionContext } from '@microsoft/vscode-azext-utils';
import * as path from 'path';
import { ext } from '../../extensionVariables';
Expand All @@ -13,7 +13,8 @@ import { ContainerGroupTreeItem } from '../../tree/containers/ContainerGroupTree
import { ContainerTreeItem } from '../../tree/containers/ContainerTreeItem';

export async function composeGroupLogs(context: IActionContext, node: ContainerGroupTreeItem): Promise<void> {
return composeGroup<LogsCommandOptions>(context, (client, options) => client.logs(options), node, { follow: true, tail: 1000 });
// Since we're not interested in the output, we can pretend this is a `VoidCommandResponse`
return composeGroup<LogsCommandOptions>(context, (client, options) => client.logs(options) as Promise<VoidCommandResponse>, node, { follow: true, tail: 1000 });
}

export async function composeGroupStart(context: IActionContext, node: ContainerGroupTreeItem): Promise<void> {
Expand All @@ -36,7 +37,7 @@ type AdditionalOptions<TOptions extends CommonOrchestratorCommandOptions> = Omit

async function composeGroup<TOptions extends CommonOrchestratorCommandOptions>(
context: IActionContext,
composeCommandCallback: (client: IContainerOrchestratorClient, options: TOptions) => Promise<CommandResponse<unknown>>,
composeCommandCallback: (client: IContainerOrchestratorClient, options: TOptions) => Promise<VoidCommandResponse>,
node: ContainerGroupTreeItem,
additionalOptions?: AdditionalOptions<TOptions>
): Promise<void> {
Expand Down
4 changes: 1 addition & 3 deletions src/commands/registries/logInToDockerCli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { ext } from '../../extensionVariables';
import { localize } from "../../localize";
import { registryExpectedContextValues } from '../../tree/registries/registryContextValues';
import { RegistryTreeItemBase } from '../../tree/registries/RegistryTreeItemBase';
import { runWithDefaultShell } from '../../runtimes/runners/runWithDefaultShell';

export async function logInToDockerCli(context: IActionContext, node?: RegistryTreeItemBase): Promise<void> {
if (!node) {
Expand Down Expand Up @@ -40,13 +39,12 @@ export async function logInToDockerCli(context: IActionContext, node?: RegistryT

await vscode.window.withProgress(progressOptions, async () => {
try {
await runWithDefaultShell(
await ext.runWithDefaultShell(
client => client.login({
username: username,
passwordStdIn: true,
registry: creds.registryPath,
}),
ext.runtimeManager,
{
stdInPipe: stream.Readable.from(password),
}
Expand Down
14 changes: 7 additions & 7 deletions src/commands/selectCommandTemplate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Licensed under the MIT License. See LICENSE.md in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { CommandResponse, PortBinding } from '../runtimes/docker';
import { PortBinding, VoidCommandResponse } from '../runtimes/docker';
import { IActionContext, IAzureQuickPickItem, IAzureQuickPickOptions, UserCancelledError } from '@microsoft/vscode-azext-utils';
import * as vscode from 'vscode';
import { ext } from '../extensionVariables';
Expand All @@ -29,7 +29,7 @@ export interface CommandTemplate {
match?: string;
}

export async function selectBuildCommand(context: IActionContext, folder: vscode.WorkspaceFolder, dockerfile: string, buildContext: string): Promise<CommandResponse<void>> {
export async function selectBuildCommand(context: IActionContext, folder: vscode.WorkspaceFolder, dockerfile: string, buildContext: string): Promise<VoidCommandResponse> {
return await selectCommandTemplate(
context,
'build',
Expand All @@ -39,7 +39,7 @@ export async function selectBuildCommand(context: IActionContext, folder: vscode
);
}

export async function selectRunCommand(context: IActionContext, fullTag: string, interactive: boolean, exposedPorts?: PortBinding[]): Promise<CommandResponse<void>> {
export async function selectRunCommand(context: IActionContext, fullTag: string, interactive: boolean, exposedPorts?: PortBinding[]): Promise<VoidCommandResponse> {
let portsString: string = '';
if (exposedPorts) {
portsString = exposedPorts.map(pb => `-p ${pb.containerPort}:${pb.containerPort}${pb.protocol ? '/' + pb.protocol : ''}`).join(' ');
Expand All @@ -54,7 +54,7 @@ export async function selectRunCommand(context: IActionContext, fullTag: string,
);
}

export async function selectAttachCommand(context: IActionContext, containerName: string, imageName: string, containerId: string, shellCommand: string): Promise<CommandResponse<void>> {
export async function selectAttachCommand(context: IActionContext, containerName: string, imageName: string, containerId: string, shellCommand: string): Promise<VoidCommandResponse> {
return await selectCommandTemplate(
context,
'attach',
Expand All @@ -64,7 +64,7 @@ export async function selectAttachCommand(context: IActionContext, containerName
);
}

export async function selectLogsCommand(context: IActionContext, containerName: string, imageName: string, containerId: string): Promise<CommandResponse<void>> {
export async function selectLogsCommand(context: IActionContext, containerName: string, imageName: string, containerId: string): Promise<VoidCommandResponse> {
return await selectCommandTemplate(
context,
'logs',
Expand All @@ -74,7 +74,7 @@ export async function selectLogsCommand(context: IActionContext, containerName:
);
}

export async function selectComposeCommand(context: IActionContext, folder: vscode.WorkspaceFolder, composeCommand: 'up' | 'down' | 'upSubset', configurationFile?: string, detached?: boolean, build?: boolean): Promise<CommandResponse<void>> {
export async function selectComposeCommand(context: IActionContext, folder: vscode.WorkspaceFolder, composeCommand: 'up' | 'down' | 'upSubset', configurationFile?: string, detached?: boolean, build?: boolean): Promise<VoidCommandResponse> {
let template: TemplateCommand;

switch (composeCommand) {
Expand Down Expand Up @@ -120,7 +120,7 @@ export async function selectCommandTemplate(
// The following three are overridable for test purposes, but have default values that cover actual usage
templatePicker: TemplatePicker = (i, o) => actionContext.ui.showQuickPick(i, o), // Default is the normal ext.ui.showQuickPick (this longer syntax is because doing `ext.ui.showQuickPick` alone doesn't result in the right `this` further down)
getCommandSettings: () => CommandSettings = () => vscode.workspace.getConfiguration('docker').inspect<string | CommandTemplate[]>(`commands.${command}`)
): Promise<CommandResponse<void>> {
): Promise<VoidCommandResponse> {
// Get the configured settings values
const commandSettings = getCommandSettings();
const userTemplates: CommandTemplate[] = toCommandTemplateArray(commandSettings.workspaceFolderValue ?? commandSettings.workspaceValue ?? commandSettings.globalValue);
Expand Down
55 changes: 19 additions & 36 deletions src/debugging/DockerServerReadyAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@
// Adapted from: https://github.com/microsoft/vscode/blob/8827cf5a607b6ab7abf45817604bc21883314db7/extensions/debug-server-ready/src/extension.ts
//

import * as readline from 'readline';
import * as stream from 'stream';
import * as util from 'util';
import * as vscode from 'vscode';
import { IActionContext, callWithTelemetryAndErrorHandling } from '@microsoft/vscode-azext-utils';
import { ext } from '../extensionVariables';
import { localize } from '../localize';
import { ResolvedDebugConfiguration } from './DebugHelper';
import { runWithDefaultShell } from '../runtimes/runners/runWithDefaultShell';

const PATTERN = 'listening on.* (https?://\\S+|[0-9]+)'; // matches "listening on port 3000" or "Now listening on: https://localhost:5001"
const URI_FORMAT = 'http://localhost:%s';
Expand Down Expand Up @@ -191,60 +191,43 @@ interface DockerServerReadyDetector {
}

class DockerLogsTracker extends vscode.Disposable {
private logStream: stream.PassThrough;
private readonly cts = new vscode.CancellationTokenSource();
private lineReader: readline.Interface | undefined;

public constructor(private readonly containerName: string, private readonly detector: DockerServerReadyDetector) {
super(
() => {
this.cts.cancel();
this.lineReader?.close();
});

if (!this.detector) {
return;
}

// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.startListening();
// Don't wait
void this.listen();
}

private async startListening(): Promise<void> {
return callWithTelemetryAndErrorHandling('dockerServerReadyAction.dockerLogsTracker.startListening', async (context: IActionContext) => {
// Don't actually telemetrize or show anything (same as prior behavior), but wrap call to get an IActionContext
context.telemetry.suppressAll = true;
context.errorHandling.suppressDisplay = true;
context.errorHandling.rethrow = false;

this.logStream = new stream.PassThrough();

this.logStream.on('data', (data) => {
this.detector.detectPattern(data.toString());
});

// Don't wait
void runWithDefaultShell(
private async listen(): Promise<void> {
try {
const generator = ext.streamWithDefaultShell(
client => client.logsForContainer({ container: this.containerName, follow: true }),
ext.runtimeManager,
{
cancellationToken: this.cts.token,
stdOutPipe: this.logStream,
}
).then(
() => {
// Do nothing on fulfilled
},
() => {
// Do nothing on reject
//
// The usual termination path is for `runWithDefaultShell` to throw a `CancellationError`,
// because when this `DockerLogsTracker` object is disposed, cancellation of the process
// is triggered.
//
// If we do not eat that `CancellationError` here, it bubbles up to the extension host process
// where it will be eaten, but ugly warnings show in a few places
}
);
});

this.lineReader = readline.createInterface({ input: stream.Readable.from(generator) });
for await (const line of this.lineReader) {
this.detector.detectPattern(line);
}
} catch {
// Do nothing
// The usual termination pathway is cancellation through the CTS above, so errors are expected
// TODO: for unknown reasons, the cancellation error does not actually get thrown to here, and ends
// up in the extension host output log. Ideally this should not happen.
}
}
}

Expand Down
28 changes: 16 additions & 12 deletions src/debugging/netcore/NetCoreDebugHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import * as fse from 'fs-extra';
import * as path from 'path';
import { composeArgs, ContainerOS, withArg, withQuotedArg } from '../../runtimes/docker';
import { composeArgs, ContainerOS, VoidCommandResponse, withArg, withQuotedArg } from '../../runtimes/docker';
import { DialogResponses, IActionContext, UserCancelledError } from '@microsoft/vscode-azext-utils';
import { DebugConfiguration, MessageItem, ProgressLocation, ShellQuotedString, window } from 'vscode';
import { ext } from '../../extensionVariables';
Expand Down Expand Up @@ -312,25 +312,29 @@ export class NetCoreDebugHelper implements DebugHelper {
containerCommand = 'cmd';
containerCommandArgs = composeArgs(
withArg('/C'),
withQuotedArg(`IF EXIST "${debuggerPath}" (echo true) else (echo false)`)
withQuotedArg(`IF EXIST "${debuggerPath}" (exit 0) else (exit 1)`)
)();
} else {
containerCommand = '/bin/sh';
containerCommandArgs = composeArgs(
withArg('-c'),
withQuotedArg(`if [ -f ${debuggerPath} ]; then echo true; fi;`)
withQuotedArg(`if [ -f ${debuggerPath} ]; then exit 0; else exit 1; fi;`)
)();
}

const stdout = await ext.runWithDefaultShell(client =>
client.execContainer({
container: containerName,
command: [containerCommand, ...containerCommandArgs],
interactive: true,
})
);

return /true/ig.test(stdout);
try {
await ext.runWithDefaultShell(client =>
// Since we're not interested in the output, just the exit code, we can pretend this is a `VoidCommandResponse`
client.execContainer({
container: containerName,
command: [containerCommand, ...containerCommandArgs],
interactive: true,
}) as Promise<VoidCommandResponse>
);
return true;
} catch {
return false;
}
}

private async getContainerNameToAttach(context: IActionContext): Promise<string> {
Expand Down
4 changes: 3 additions & 1 deletion src/extensionVariables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { NetworksTreeItem } from './tree/networks/NetworksTreeItem';
import { RegistriesTreeItem } from './tree/registries/RegistriesTreeItem';
import { VolumesTreeItem } from './tree/volumes/VolumesTreeItem';
import { OrchestratorRuntimeManager } from './runtimes/OrchestratorRuntimeManager';
import { runOrchestratorWithDefaultShellInternal, runWithDefaultShellInternal } from './runtimes/runners/runWithDefaultShell';
import { runOrchestratorWithDefaultShellInternal, runWithDefaultShellInternal, streamOrchestratorWithDefaultShellInternal, streamWithDefaultShellInternal } from './runtimes/runners/runWithDefaultShell';

/**
* Namespace for common variables used throughout the extension. They must be initialized in the activate() method of extension.ts
Expand Down Expand Up @@ -60,5 +60,7 @@ export namespace ext {
export let runtimeManager: ContainerRuntimeManager;
export let orchestratorManager: OrchestratorRuntimeManager;
export const runWithDefaultShell = runWithDefaultShellInternal;
export const streamWithDefaultShell = streamWithDefaultShellInternal;
export const runOrchestratorWithDefaultShell = runOrchestratorWithDefaultShellInternal;
export const streamOrchestratorWithDefaultShell = streamOrchestratorWithDefaultShellInternal;
}
Loading