Notebooks: Only have One Connection in Attach To Dropdown (#8582)

* Have only one connection in attach to dropdown

* LGTM fixes

* Test fixes not unnecessarily changing context 2x

* PR Feedback
This commit is contained in:
Chris LaFreniere
2019-12-08 11:03:51 -08:00
committed by GitHub
parent de177c0335
commit 62565e0577
11 changed files with 94 additions and 321 deletions

View File

@@ -316,7 +316,7 @@ export class CellModel implements ICellModel {
this.sendNotification(notificationService, Severity.Info, localize('runCellCancelled', "Cell execution cancelled"));
} else {
// TODO update source based on editor component contents
if (kernel.requiresConnection && !this.notebookModel.activeConnection) {
if (kernel.requiresConnection && !this.notebookModel.context) {
let connected = await this.notebookModel.requestConnection();
if (!connected) {
return false;
@@ -509,11 +509,11 @@ export class CellModel implements ICellModel {
let result = output as nb.IDisplayResult;
if (result && result.data && result.data['text/html']) {
let model = this._options.notebook as NotebookModel;
if (model.activeConnection) {
let gatewayEndpointInfo = this.getGatewayEndpoint(model.activeConnection);
if (model.context) {
let gatewayEndpointInfo = this.getGatewayEndpoint(model.context);
if (gatewayEndpointInfo) {
let hostAndIp = notebookUtils.getHostAndPortFromEndpoint(gatewayEndpointInfo.endpoint);
let host = hostAndIp.host ? hostAndIp.host : model.activeConnection.serverName;
let host = hostAndIp.host ? hostAndIp.host : model.context.serverName;
let port = hostAndIp.port ? ':' + hostAndIp.port : defaultPort;
let html = result.data['text/html'];
// CTP 3.1 and earlier Spark link

View File

@@ -214,11 +214,6 @@ export interface IClientSession extends IDisposable {
onKernelChanging(changeHandler: ((kernel: nb.IKernelChangedArgs) => Promise<void>)): void;
}
export interface IDefaultConnection {
defaultConnection: ConnectionProfile;
otherConnections: ConnectionProfile[];
}
/**
* A kernel preference.
*/
@@ -323,10 +318,10 @@ export interface INotebookModel {
readonly specs: nb.IAllKernels | undefined;
/**
* The specs for available contexts, or undefined if these have
* The specs for available context, or undefined if this has
* not been loaded yet
*/
readonly contexts: IDefaultConnection | undefined;
readonly context: ConnectionProfile | undefined;
/**
* Event fired on first initialization of the cells and

View File

@@ -3,113 +3,44 @@
* Licensed under the Source EULA. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import { nb } from 'azdata';
import { localize } from 'vs/nls';
import { IDefaultConnection } from 'sql/workbench/contrib/notebook/browser/models/modelInterfaces';
import { IConnectionManagementService } from 'sql/platform/connection/common/connectionManagement';
import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile';
import { IConnectionProfile } from 'sql/platform/connection/common/interfaces';
import { mssqlProviderName } from 'sql/platform/connection/common/constants';
import { find } from 'vs/base/common/arrays';
export class NotebookContexts {
public static get DefaultContext(): IDefaultConnection {
let defaultConnection: ConnectionProfile = <any>{
public static get DefaultContext(): ConnectionProfile {
return <any>{
providerName: mssqlProviderName,
id: '-1',
serverName: localize('selectConnection', "Select Connection")
};
return {
// default context if no other contexts are applicable
defaultConnection: defaultConnection,
otherConnections: [defaultConnection]
};
}
public static get LocalContext(): IDefaultConnection {
let localConnection: ConnectionProfile = <any>{
public static get LocalContext(): ConnectionProfile {
return <any>{
providerName: mssqlProviderName,
id: '-1',
serverName: localize('localhost', "localhost")
};
return {
// default context if no other contexts are applicable
defaultConnection: localConnection,
otherConnections: [localConnection]
};
}
/**
* Get all of the applicable contexts for a given kernel
* @param connectionService connection management service
* Get the applicable context for a given kernel
* @param context current connection profile
* @param connProviderIds array of connection provider ids applicable for a kernel
* @param kernelChangedArgs kernel changed args (both old and new kernel info)
* @param profile current connection profile
*/
public static getContextsForKernel(connectionService: IConnectionManagementService, connProviderIds: string[], kernelChangedArgs?: nb.IKernelChangedArgs, profile?: IConnectionProfile): IDefaultConnection {
let connections: IDefaultConnection = this.DefaultContext;
if (!profile) {
if (!kernelChangedArgs || !kernelChangedArgs.newValue ||
(kernelChangedArgs.oldValue && kernelChangedArgs.newValue.id === kernelChangedArgs.oldValue.id)) {
// nothing to do, kernels are the same or new kernel is undefined
return connections;
}
}
if (kernelChangedArgs && kernelChangedArgs.newValue && kernelChangedArgs.newValue.name && connProviderIds.length < 1) {
return connections;
} else {
connections = this.getActiveContexts(connectionService, connProviderIds, profile);
}
return connections;
}
/**
* Get all active contexts and sort them
* @param connectionService connection service
* @param connProviderIds array of applicable connection providers to filter connections
* @param profile connection profile passed when launching notebook
*/
public static getActiveContexts(connectionService: IConnectionManagementService, connProviderIds: string[], profile?: IConnectionProfile): IDefaultConnection {
let defaultConnection: ConnectionProfile = NotebookContexts.DefaultContext.defaultConnection;
let activeConnections: ConnectionProfile[] = connectionService.getActiveConnections();
if (activeConnections && activeConnections.length > 0) {
activeConnections = activeConnections.filter(conn => conn.id !== '-1');
}
public static getContextForKernel(context: ConnectionProfile, connProviderIds: string[]): ConnectionProfile {
// If no connection provider ids exist for a given kernel, the attach to should show localhost
if (connProviderIds.length === 0) {
return NotebookContexts.LocalContext;
}
// If no active connections exist, show "Select connection" as the default value
if (activeConnections.length === 0) {
return NotebookContexts.DefaultContext;
if (context && context.providerName && connProviderIds.filter(p => p === context.providerName).length > 0) {
return context;
}
// Filter active connections by their provider ids to match kernel's supported connection providers
else if (activeConnections.length > 0) {
let connections = activeConnections.filter(connection => {
return connProviderIds.some(x => x === connection.providerName);
});
if (connections && connections.length > 0) {
defaultConnection = connections[0];
if (profile && profile.options) {
let matchingConn = find(connections, connection => connection.serverName === profile.serverName);
if (matchingConn) {
defaultConnection = matchingConn;
}
}
} else if (connections.length === 0) {
return NotebookContexts.DefaultContext;
}
activeConnections = [];
connections.forEach(connection => activeConnections.push(connection));
}
return {
otherConnections: activeConnections,
defaultConnection: defaultConnection
};
return NotebookContexts.DefaultContext;
}
}

View File

@@ -9,7 +9,7 @@ import { localize } from 'vs/nls';
import { Event, Emitter } from 'vs/base/common/event';
import { Disposable } from 'vs/base/common/lifecycle';
import { IClientSession, INotebookModel, IDefaultConnection, INotebookModelOptions, ICellModel, NotebookContentChange, notebookConstants } from 'sql/workbench/contrib/notebook/browser/models/modelInterfaces';
import { IClientSession, INotebookModel, INotebookModelOptions, ICellModel, NotebookContentChange, notebookConstants } from 'sql/workbench/contrib/notebook/browser/models/modelInterfaces';
import { NotebookChangeType, CellType, CellTypes } from 'sql/workbench/contrib/notebook/common/models/contracts';
import { nbversion } from 'sql/workbench/contrib/notebook/common/models/notebookConstants';
import * as notebookUtils from 'sql/workbench/contrib/notebook/browser/models/notebookUtils';
@@ -56,7 +56,6 @@ export class NotebookModel extends Disposable implements INotebookModel {
private _sessionLoadFinished: Promise<void>;
private _onClientSessionReady = new Emitter<IClientSession>();
private _onProviderIdChanged = new Emitter<string>();
private _activeContexts: IDefaultConnection;
private _trustedMode: boolean;
private _onActiveCellChanged = new Emitter<ICellModel>();
@@ -69,7 +68,6 @@ export class NotebookModel extends Disposable implements INotebookModel {
private readonly _nbformat: number = nbversion.MAJOR_VERSION;
private readonly _nbformatMinor: number = nbversion.MINOR_VERSION;
private _activeConnection: ConnectionProfile;
private _otherConnections: ConnectionProfile[] = [];
private _activeCell: ICellModel;
private _providerId: string;
private _defaultKernel: nb.IKernelSpec;
@@ -190,8 +188,8 @@ export class NotebookModel extends Disposable implements INotebookModel {
return this._cells;
}
public get contexts(): IDefaultConnection {
return this._activeContexts;
public get context(): ConnectionProfile {
return this._activeConnection;
}
public get specs(): nb.IAllKernels | undefined {
@@ -244,10 +242,6 @@ export class NotebookModel extends Disposable implements INotebookModel {
});
}
public get activeConnection(): IConnectionProfile {
return this._activeConnection;
}
/**
* Indicates the server has finished loading. It may have failed to load in
* which case the view will be in an error state.
@@ -457,14 +451,8 @@ export class NotebookModel extends Disposable implements INotebookModel {
}
let profile = new ConnectionProfile(this._notebookOptions.capabilitiesService, this.connectionProfile);
// TODO: this code needs to be fixed since it is called before the this._savedKernelInfo is set.
// This means it always fails, and we end up using the default connection instead. If you right-click
// and run "New Notebook" on a disconnected server this means you get the wrong connection (global active)
// instead of the one you chose, or it'll fail to connect in general
if (this.isValidConnection(profile)) {
this._activeConnection = profile;
} else {
this._activeConnection = undefined;
}
clientSession.onKernelChanging(async (e) => {
@@ -687,19 +675,13 @@ export class NotebookModel extends Disposable implements INotebookModel {
public async changeContext(title: string, newConnection?: ConnectionProfile, hideErrorMessage?: boolean): Promise<void> {
try {
if (!newConnection) {
newConnection = find(this._activeContexts.otherConnections, (connection) => connection.title === title);
}
if ((!newConnection) && (this._activeContexts.defaultConnection.title === title)) {
newConnection = this._activeContexts.defaultConnection;
if ((!newConnection) && this._activeConnection && (this._activeConnection.title === title)) {
newConnection = this._activeConnection;
}
if (newConnection) {
if (this._activeConnection && this._activeConnection.id !== newConnection.id) {
this._otherConnections.push(this._activeConnection);
}
this._activeConnection = newConnection;
this.refreshConnections(newConnection);
this.setActiveConnectionIfDifferent(newConnection);
this._activeClientSession.updateConnection(newConnection.toIConnectionProfile()).then(
result => {
//Remove 'Select connection' from 'Attach to' drop-down since its a valid connection
@@ -723,17 +705,12 @@ export class NotebookModel extends Disposable implements INotebookModel {
}
}
private refreshConnections(newConnection: ConnectionProfile) {
private setActiveConnectionIfDifferent(newConnection: ConnectionProfile) {
if (this.isValidConnection(newConnection) &&
this._activeConnection.id !== '-1' &&
this._activeConnection.id !== this._activeContexts.defaultConnection.id) {
// Put the defaultConnection to the head of otherConnections
if (this.isValidConnection(this._activeContexts.defaultConnection)) {
this._activeContexts.otherConnections = this._activeContexts.otherConnections.filter(conn => conn.id !== this._activeContexts.defaultConnection.id);
this._activeContexts.otherConnections.unshift(this._activeContexts.defaultConnection);
}
// Change the defaultConnection to newConnection
this._activeContexts.defaultConnection = newConnection;
this._activeConnection.id !== newConnection.id) {
// Change the active connection to newConnection
this._activeConnection = newConnection;
}
}
@@ -803,17 +780,9 @@ export class NotebookModel extends Disposable implements INotebookModel {
public async handleClosed(): Promise<void> {
try {
if (this.notebookOptions && this.notebookOptions.connectionService) {
if (this._otherConnections) {
notebookUtils.asyncForEach(this._otherConnections, async (conn) => {
await this.disconnectNotebookConnection(conn);
});
this._otherConnections = [];
}
if (this._activeConnection) {
await this.disconnectNotebookConnection(this._activeConnection);
this._activeConnection = undefined;
}
if (this.notebookOptions && this.notebookOptions.connectionService && this._activeConnection) {
await this.disconnectNotebookConnection(this._activeConnection);
this._activeConnection = undefined;
}
await this.shutdownActiveSession();
} catch (err) {
@@ -837,11 +806,11 @@ export class NotebookModel extends Disposable implements INotebookModel {
private async loadActiveContexts(kernelChangedArgs: nb.IKernelChangedArgs): Promise<void> {
if (kernelChangedArgs && kernelChangedArgs.newValue && kernelChangedArgs.newValue.name) {
let kernelDisplayName = this.getDisplayNameFromSpecName(kernelChangedArgs.newValue);
this._activeContexts = NotebookContexts.getContextsForKernel(this._notebookOptions.connectionService, this.getApplicableConnectionProviderIds(kernelDisplayName), kernelChangedArgs, this.connectionProfile);
this._contextsChangedEmitter.fire();
if (this.contexts.defaultConnection !== undefined && this.contexts.defaultConnection.serverName !== undefined && this.contexts.defaultConnection.title !== undefined) {
await this.changeContext(this.contexts.defaultConnection.title, this.contexts.defaultConnection);
let context = NotebookContexts.getContextForKernel(this._activeConnection, this.getApplicableConnectionProviderIds(kernelDisplayName));
if (context !== undefined && context.serverName !== undefined && context.title !== undefined) {
await this.changeContext(context.title, context);
}
this._contextsChangedEmitter.fire();
}
}
@@ -938,7 +907,7 @@ export class NotebookModel extends Disposable implements INotebookModel {
}
}
// Disconnect any connections that were added through the "Add new connection" functionality in the Attach To dropdown
// Disconnect any connections that were added through the "Change connection" functionality in the Attach To dropdown
private async disconnectAttachToConnections(): Promise<void> {
notebookUtils.asyncForEach(this._connectionUrisToDispose, async conn => {
await this.notebookOptions.connectionService.disconnect(conn).catch(e => this.logService.error(e));