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 3 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
107 changes: 14 additions & 93 deletions src/tree/LocalRootTreeItemBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
*--------------------------------------------------------------------------------------------*/

import { isCommandNotSupportedError, ListContainersItem, ListContextItem, ListImagesItem, ListNetworkItem, ListVolumeItem } from "../runtimes/docker";
import { AzExtParentTreeItem, AzExtTreeItem, AzureWizard, GenericTreeItem, IActionContext, parseError, registerEvent } from "@microsoft/vscode-azext-utils";
import { ConfigurationChangeEvent, ConfigurationTarget, ThemeColor, ThemeIcon, TreeView, TreeViewVisibilityChangeEvent, WorkspaceConfiguration, window, workspace } from "vscode";
import { AzExtParentTreeItem, AzExtTreeItem, AzureWizard, GenericTreeItem, IActionContext, parseError } from "@microsoft/vscode-azext-utils";
import { ConfigurationTarget, ThemeColor, ThemeIcon, WorkspaceConfiguration, workspace } from "vscode";
import { showDockerInstallNotification } from "../commands/dockerInstaller";
import { configPrefix } from "../constants";
import { ext } from "../extensionVariables";
Expand All @@ -20,6 +20,7 @@ import { ITreeSettingWizardInfo, ITreeSettingsWizardContext } from "./settings/I
import { TreeSettingListStep } from "./settings/TreeSettingListStep";
import { TreeSettingStep } from "./settings/TreeSettingStep";
import { DatedDockerImage } from "./images/ImagesTreeItem";
import { TreePrefix } from "./TreePrefix";

type DockerStatus = 'NotInstalled' | 'Installed' | 'Running';

Expand Down Expand Up @@ -48,7 +49,7 @@ export abstract class LocalRootTreeItemBase<TItem extends AnyContainerObject, TP
defaultProperty: 'CreatedTime',
};

public abstract treePrefix: string;
public abstract treePrefix: TreePrefix;
public abstract configureExplorerTitle: string;
public abstract childType: LocalChildType<TItem>;
public abstract childGroupType: LocalChildGroupType<TItem, TProperty>;
Expand All @@ -68,7 +69,7 @@ export abstract class LocalRootTreeItemBase<TItem extends AnyContainerObject, TP
protected failedToConnect: boolean = false;

private _currentItems: TItem[] | undefined;
private _itemsFromPolling: TItem[] | undefined;
private _cachedItems: TItem[] | undefined;
private _currentDockerStatus: DockerStatus;

public get contextValue(): string {
Expand All @@ -79,51 +80,6 @@ export abstract class LocalRootTreeItemBase<TItem extends AnyContainerObject, TP
return workspace.getConfiguration(`${configPrefix}.${this.treePrefix}`);
}

private get autoRefreshEnabled(): boolean {
return window.state.focused && LocalRootTreeItemBase.autoRefreshViews;
}

protected getRefreshInterval(): number {
const configOptions: WorkspaceConfiguration = workspace.getConfiguration('docker');
return configOptions.get<number>('explorerRefreshInterval', 2000);
}

public registerRefreshEvents(treeView: TreeView<AzExtTreeItem>): void {
let intervalId: NodeJS.Timeout;
registerEvent('treeView.onDidChangeVisibility', treeView.onDidChangeVisibility, (context: IActionContext, e: TreeViewVisibilityChangeEvent) => {
context.errorHandling.suppressDisplay = true;
context.telemetry.suppressIfSuccessful = true;
context.telemetry.properties.isActivationEvent = 'true';

if (e.visible) {
const refreshInterval: number = this.getRefreshInterval();
intervalId = setInterval(
async () => {
if (this.autoRefreshEnabled && await this.hasChanged(context)) {
// Auto refresh could be disabled while invoking the hasChanged()
// So check again before starting the refresh.
if (this.autoRefreshEnabled) {
await this.refresh(context);
}
}
},
refreshInterval);
} else {
clearInterval(intervalId);
}
});

registerEvent('treeView.onDidChangeConfiguration', workspace.onDidChangeConfiguration, async (context: IActionContext, e: ConfigurationChangeEvent) => {
context.errorHandling.suppressDisplay = true;
context.telemetry.suppressIfSuccessful = true;
context.telemetry.properties.isActivationEvent = 'true';

if (e.affectsConfiguration(`${configPrefix}.${this.treePrefix}`)) {
await this.refresh(context);
}
});
}

protected getTreeItemForEmptyList(): AzExtTreeItem[] {
return [new GenericTreeItem(this, {
label: localize('vscode-docker.tree.noItemsFound', 'No items found'),
Expand All @@ -132,17 +88,12 @@ export abstract class LocalRootTreeItemBase<TItem extends AnyContainerObject, TP
})];
}

public clearPollingCache(): void {
this._itemsFromPolling = undefined;
}

public async loadMoreChildrenImpl(clearCache: boolean, context: IActionContext): Promise<AzExtTreeItem[]> {
try {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
ext.activityMeasurementService.recordActivity('overallnoedit');

this._currentItems = this._itemsFromPolling || await this.getSortedItems(context);
this.clearPollingCache();
this._currentItems = await this.getCachedItems(context, clearCache);
this.failedToConnect = false;
this._currentDockerStatus = 'Running';
} catch (error) {
Expand Down Expand Up @@ -356,47 +307,17 @@ export abstract class LocalRootTreeItemBase<TItem extends AnyContainerObject, TP
return result;
}

private async getSortedItems(context: IActionContext): Promise<TItem[]> {
if (ext.treeInitError === undefined) {
const items: TItem[] = await this.getItems(context) || [];
return items.sort((a, b) => getTreeId(a).localeCompare(getTreeId(b)));
} else {
throw ext.treeInitError;
}
}

private async hasChanged(context: IActionContext): Promise<boolean> {
let pollingDockerStatus: DockerStatus;
let isDockerStatusChanged = false;

try {
this._itemsFromPolling = await this.getSortedItems(context);
pollingDockerStatus = 'Running';
} catch (error) {
this.clearPollingCache();
pollingDockerStatus = await runtimeInstallStatusProvider.isRuntimeInstalled() ? 'Installed' : 'NotInstalled';
isDockerStatusChanged = pollingDockerStatus !== this._currentDockerStatus;
}

const hasChanged = !this.areArraysEqual(this._currentItems, this._itemsFromPolling) || isDockerStatusChanged;
this._currentDockerStatus = pollingDockerStatus;
return hasChanged;
}

protected areArraysEqual(array1: TItem[] | undefined, array2: TItem[] | undefined): boolean {
if (array1 === array2) {
return true;
} else if (array1 && array2) {
if (array1.length !== array2.length) {
return false;
private async getCachedItems(context: IActionContext, clearCache: boolean): Promise<TItem[]> {
if (clearCache || !this._cachedItems) {
if (ext.treeInitError === undefined) {
const items: TItem[] = await this.getItems(context) || [];
this._cachedItems = items.sort((a, b) => getTreeId(a).localeCompare(getTreeId(b)));
} else {
return !array1.some((item1, index) => {
return getTreeId(item1) !== getTreeId(array2[index]);
});
throw ext.treeInitError;
}
} else {
return false;
}

return this._cachedItems;
}

private showDockerInstallNotificationIfNeeded(): void {
Expand Down
169 changes: 169 additions & 0 deletions src/tree/RefreshManager.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See LICENSE.md in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { AzExtTreeItem, callWithTelemetryAndErrorHandling, IActionContext, parseError, registerCommand } from '@microsoft/vscode-azext-utils';
import * as vscode from 'vscode';
import { ext } from '../extensionVariables';
import { localize } from '../localize';
import { isCancellationError } from '../runtimes/docker';
import { AllTreePrefixes, TreePrefix } from './TreePrefix';

const pollingIntervalMs = 60 * 1000; // One minute
const eventListenerTries = 3; // The event listener will try at most 3 times to connect for events

type RefreshTarget = AzExtTreeItem | TreePrefix;
type RefreshReason = 'interval' | 'event' | 'config' | 'manual';

export class RefreshManager extends vscode.Disposable {
private readonly refreshEmitter = new vscode.EventEmitter<[RefreshTarget, RefreshReason]>();
private readonly disposables: vscode.Disposable[] = [];
private readonly cts: vscode.CancellationTokenSource = new vscode.CancellationTokenSource();

public constructor() {
super(() => vscode.Disposable.from(...this.disposables).dispose());

this.disposables.push(
this.refreshEmitter.event(async ([target, reason]) => {
bwateratmsft marked this conversation as resolved.
Show resolved Hide resolved
await this.refresh(target, reason);
})
);

// VSCode does *not* cancel by default on disposal of a CancellationTokenSource, so we need to manually cancel
this.disposables.unshift(new vscode.Disposable(() => this.cts.cancel()));

this.setupRefreshOnInterval();
bwateratmsft marked this conversation as resolved.
Show resolved Hide resolved
this.setupRefreshOnRuntimeEvent();
this.setupRefreshOnConfigurationChange();
this.setupRefreshOnCommand();
}

private setupRefreshOnInterval(): void {
const timer = setInterval(async () => {
bwateratmsft marked this conversation as resolved.
Show resolved Hide resolved
for (const view of AllTreePrefixes) {
this.refreshEmitter.fire([view, 'interval']);
}
}, pollingIntervalMs);

// Put the interval emitter at the front so it gets disposed earlier
this.disposables.push(new vscode.Disposable(
() => clearInterval(timer)
));
}

private setupRefreshOnRuntimeEvent(): void {
void callWithTelemetryAndErrorHandling('vscode-docker.tree.eventRefresh', async (context: IActionContext) => {
context.errorHandling.suppressDisplay = true;
context.telemetry.suppressIfSuccessful = true;

// Try at most `eventListenerTries` times to (re)connect to the event stream
for (let i = 0; i < eventListenerTries; i++) {
try {
// TODO: finish this
const eventGenerator = 'foo' as unknown as AsyncGenerator<{ Type: string }>;

for await (const event of eventGenerator) {
switch (event.Type) {
case 'container':
this.refreshEmitter.fire(['containers', 'event']);
break;
case 'network':
this.refreshEmitter.fire(['networks', 'event']);
break;
case 'image':
this.refreshEmitter.fire(['images', 'event']);
break;
case 'volume':
this.refreshEmitter.fire(['volumes', 'event']);
break;
case 'context':
this.refreshEmitter.fire(['contexts', 'event']);
break;
default:
// Ignore other events
break;
}
}
} catch (err) {
const error = parseError(err);

if (isCancellationError(err) || error.isUserCancelledError) {
// Cancelled, so don't try again and don't rethrow--this is a normal termination pathway
return;
} else if (i < eventListenerTries - 1) {
// Still in the retry loop
continue;
} else {
// Emit a message and rethrow to get telemetry
ext.outputChannel.appendLine(
localize('vscode-docker.tree.refreshManager.eventSetupFailure', 'Failed to set up event listener: {0}', error.message)
);

throw error;
}
}
}
});

}

private setupRefreshOnConfigurationChange(): void {
this.disposables.push(
vscode.workspace.onDidChangeConfiguration((e: vscode.ConfigurationChangeEvent) => {
for (const view of AllTreePrefixes) {
if (e.affectsConfiguration(`docker.${view}`)) {
this.refreshEmitter.fire([view, 'config']);
}
}
})
);
}

private setupRefreshOnCommand(): void {
for (const view of AllTreePrefixes) {
// Because `registerCommand` pushes the disposables onto the `ext.context.subscriptions` array, we don't need to keep track of them
registerCommand(`vscode-docker.${view}.refresh`, () => {
this.refreshEmitter.fire([view, 'manual']);
});
}
}

private refresh(target: RefreshTarget, reason: 'interval' | 'event' | 'config' | 'manual'): Promise<void> {
return callWithTelemetryAndErrorHandling('vscode-docker.tree.refresh', async (context: IActionContext) => {
context.telemetry.properties.refreshReason = reason;
context.errorHandling.suppressDisplay = true;

if (isAzExtTreeItem(target)) {
context.telemetry.properties.refreshTarget = 'node';
return await target.refresh(context);
} else if (typeof target === 'string') {
context.telemetry.properties.refreshTarget = target;
switch (target) {
case 'containers':
return await ext.containersRoot.refresh(context);
case 'networks':
return await ext.networksRoot.refresh(context);
case 'images':
return await ext.imagesRoot.refresh(context);
case 'registries':
return await ext.registriesRoot.refresh(context);
case 'volumes':
return await ext.volumesRoot.refresh(context);
case 'contexts':
return await ext.contextsRoot.refresh(context);
default:
throw new RangeError(`Unexpected view type: ${target}`);
}
} else {
context.telemetry.properties.refreshTarget = 'none';
}
});
}
}

// TODO: temp: this function is available in newer versions of @microsoft/vscode-azext-utils, use it when available
function isAzExtTreeItem(maybeTreeItem: unknown): maybeTreeItem is AzExtTreeItem {
return typeof maybeTreeItem === 'object' &&
!!(maybeTreeItem as AzExtTreeItem).fullId;
}
15 changes: 15 additions & 0 deletions src/tree/TreePrefix.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See LICENSE.md in the project root for license information.
*--------------------------------------------------------------------------------------------*/

/**
* The prefix names for each of the tree views
* Note: the help tree is intentionally excluded because it has no refresh nor config capabilities
*/
export const AllTreePrefixes = ['containers', 'networks', 'images', 'registries', 'volumes', 'contexts'] as const;

/**
* A union type representing the tree prefix options
*/
export type TreePrefix = (typeof AllTreePrefixes)[number];
25 changes: 2 additions & 23 deletions src/tree/containers/ContainersTreeItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,14 @@ import { ITreeArraySettingInfo, ITreeSettingInfo } from "../settings/ITreeSettin
import { ContainerGroupTreeItem } from "./ContainerGroupTreeItem";
import { ContainerProperty, containerProperties, getContainerPropertyValue, NonComposeGroupName } from "./ContainerProperties";
import { ContainerTreeItem } from "./ContainerTreeItem";
import { TreePrefix } from "../TreePrefix";

export type DockerContainerInfo = ListContainersItem & {
showFiles: boolean;
};

export class ContainersTreeItem extends LocalRootTreeItemBase<DockerContainerInfo, ContainerProperty> {
public treePrefix: string = 'containers';
public treePrefix: TreePrefix = 'containers';
public label: string = localize('vscode-docker.tree.containers.label', 'Containers');
public configureExplorerTitle: string = localize('vscode-docker.tree.containers.configure', 'Configure containers explorer');

Expand Down Expand Up @@ -98,28 +99,6 @@ export class ContainersTreeItem extends LocalRootTreeItemBase<DockerContainerInf
return super.getTreeItemForEmptyList();
}

protected areArraysEqual(array1: DockerContainerInfo[] | undefined, array2: DockerContainerInfo[] | undefined): boolean {
if (!super.areArraysEqual(array1, array2)) {
// If they aren't the same according to the base class implementation, they are definitely not the same according to this either
return false;
}

// If they are both undefined, return true
// If only one is undefined, return false
// This matches the behavior of the base class implementation, and guards against null refs below
if (array1 === undefined && array2 === undefined) {
return true;
} else if (array1 === undefined || array2 === undefined) {
return false;
}

// Containers' labels/descriptions (status in particular) can change. If they do, we want to cause a refresh. But, we also don't want to change the tree ID based on status (in `getTreeId()` in LocalRootTreeItemBase.ts).
return !array1.some((item, index) => {
return this.getTreeItemLabel(item) !== this.getTreeItemLabel(array2[index]) ||
this.getTreeItemDescription(item) !== this.getTreeItemDescription(array2[index]);
});
}

private isNewContainerUser(): boolean {
return ext.context.globalState.get<boolean>('vscode-docker.container.newContainerUser', true);
}
Expand Down
Loading