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

Merge in changes to release #13976

Merged
merged 6 commits into from
Sep 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@
([#13729](https://github.com/Microsoft/vscode-python/issues/13729))
1. Fix nighly failure with beakerx.
([#13734](https://github.com/Microsoft/vscode-python/issues/13734))
## 2020.8.6 (15 September 2020)

### Fixes

1. Workaround problem caused by https://github.com/microsoft/vscode/issues/106547
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting looks funny here, did you generate this with news script?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No as I didn't want to make it look like it fixed the specific issue. Let me double check the formatting is okay when looking at it in the preview.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it looks the same (well except the url is not a bug for us)


### Thanks

Expand Down
1 change: 1 addition & 0 deletions build/ci/templates/globals.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ variables:
npm_config_cache: $(Pipeline.Workspace)/.npm
vmImageMacOS: 'macOS-10.15'
TS_NODE_FILES: true # Temporarily enabled to allow using types from vscode.proposed.d.ts from ts-node (for tests).
VSC_PYTHON_CI_TEST_VSC_CHANNEL: '1.48.0' # Enforce this until https://github.com/microsoft/vscode-test/issues/73 is fixed
80 changes: 52 additions & 28 deletions src/client/datascience/interactive-common/interactiveBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ import { captureTelemetry, sendTelemetryEvent } from '../../telemetry';
import { generateCellRangesFromDocument } from '../cellFactory';
import { CellMatcher } from '../cellMatcher';
import { addToUriList, translateKernelLanguageToMonaco } from '../common';
import { Commands, Identifiers, Telemetry } from '../constants';
import { Commands, Identifiers, Settings, Telemetry } from '../constants';
import { ColumnWarningSize, IDataViewerFactory } from '../data-viewing/types';
import {
IAddedSysInfo,
Expand Down Expand Up @@ -865,9 +865,54 @@ export abstract class InteractiveBase extends WebviewPanelHost<IInteractiveWindo
} catch (e) {
this.errorHandler.handleError(e).ignoreErrors();
}
} else {
// Just send a kernel update so it shows something
this.postMessage(InteractiveWindowMessages.UpdateKernel, {
jupyterServerStatus: ServerStatus.NotStarted,
serverName: await this.getServerDisplayName(undefined),
kernelName: '',
language: PYTHON_LANGUAGE
}).ignoreErrors();
}
}

protected async getServerDisplayName(serverConnection: INotebookProviderConnection | undefined): Promise<string> {
// If we don't have a server connection, make one if remote. We need the remote connection in order
// to compute the display name. However only do this if the user is allowing auto start.
if (
!serverConnection &&
this.configService.getSettings(this.owningResource).datascience.jupyterServerURI !==
Settings.JupyterServerLocalLaunch &&
!this.configService.getSettings(this.owningResource).datascience.disableJupyterAutoStart
) {
serverConnection = await this.notebookProvider.connect({ disableUI: true });
}

let displayName =
serverConnection?.displayName ||
(!serverConnection?.localLaunch ? serverConnection?.url : undefined) ||
(this.configService.getSettings().datascience.jupyterServerURI === Settings.JupyterServerLocalLaunch
? localize.DataScience.localJupyterServer()
: localize.DataScience.serverNotStarted());

if (serverConnection) {
// Determine the connection URI of the connected server to display
if (serverConnection.localLaunch) {
displayName = localize.DataScience.localJupyterServer();
} else {
// Log this remote URI into our MRU list
addToUriList(
this.globalStorage,
!isNil(serverConnection.url) ? serverConnection.url : serverConnection.displayName,
Date.now(),
serverConnection.displayName
);
}
}

return displayName;
}

private combineData(
oldData: nbformat.ICodeCell | nbformat.IRawCell | nbformat.IMarkdownCell | undefined,
cell: ICell
Expand Down Expand Up @@ -1154,36 +1199,15 @@ export abstract class InteractiveBase extends WebviewPanelHost<IInteractiveWindo
return notebook;
}

private getServerUri(serverConnection: INotebookProviderConnection | undefined): string {
let localizedUri = '';

if (serverConnection) {
// Determine the connection URI of the connected server to display
if (serverConnection.localLaunch) {
localizedUri = localize.DataScience.localJupyterServer();
} else {
// Log this remote URI into our MRU list
addToUriList(
this.globalStorage,
!isNil(serverConnection.url) ? serverConnection.url : serverConnection.displayName,
Date.now(),
serverConnection.displayName
);
}
}

return localizedUri;
}

private async listenToNotebookEvents(notebook: INotebook): Promise<void> {
const statusChangeHandler = async (status: ServerStatus) => {
const connectionMetadata = notebook.getKernelConnection();
const name = getDisplayNameOrNameOfKernelConnection(connectionMetadata);

await this.postMessage(InteractiveWindowMessages.UpdateKernel, {
jupyterServerStatus: status,
localizedUri: this.getServerUri(notebook.connection),
displayName: name,
serverName: await this.getServerDisplayName(notebook.connection),
kernelName: name,
language: translateKernelLanguageToMonaco(
getKernelConnectionLanguage(connectionMetadata) || PYTHON_LANGUAGE
)
Expand All @@ -1205,8 +1229,8 @@ export abstract class InteractiveBase extends WebviewPanelHost<IInteractiveWindo
// While waiting make the notebook look busy
this.postMessage(InteractiveWindowMessages.UpdateKernel, {
jupyterServerStatus: ServerStatus.Busy,
localizedUri: this.getServerUri(serverConnection),
displayName: '',
serverName: await this.getServerDisplayName(serverConnection),
kernelName: '',
language: PYTHON_LANGUAGE
}).ignoreErrors();

Expand Down Expand Up @@ -1234,8 +1258,8 @@ export abstract class InteractiveBase extends WebviewPanelHost<IInteractiveWindo
// No notebook, send update to UI anyway
this.postMessage(InteractiveWindowMessages.UpdateKernel, {
jupyterServerStatus: ServerStatus.NotStarted,
localizedUri: '',
displayName: getDisplayNameOrNameOfKernelConnection(data.kernelConnection),
serverName: await this.getServerDisplayName(undefined),
kernelName: getDisplayNameOrNameOfKernelConnection(data.kernelConnection),
language: translateKernelLanguageToMonaco(
getKernelConnectionLanguage(data.kernelConnection) || PYTHON_LANGUAGE
)
Expand Down
4 changes: 2 additions & 2 deletions src/client/datascience/interactive-ipynb/nativeEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -751,8 +751,8 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
if (!this.notebook && metadata?.kernelspec) {
this.postMessage(InteractiveWindowMessages.UpdateKernel, {
jupyterServerStatus: ServerStatus.NotStarted,
localizedUri: '',
displayName: metadata.kernelspec.display_name ?? metadata.kernelspec.name,
serverName: await this.getServerDisplayName(undefined),
kernelName: metadata.kernelspec.display_name ?? metadata.kernelspec.name,
language: translateKernelLanguageToMonaco(
(metadata.kernelspec.language as string) ?? PYTHON_LANGUAGE
)
Expand Down
7 changes: 6 additions & 1 deletion src/client/datascience/jupyter/kernels/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,12 @@ export function getDisplayNameOrNameOfKernelConnection(
kernelConnection.kind === 'connectToLiveKernel'
? kernelConnection.kernelModel.name
: kernelConnection.kernelSpec?.name;
return displayName || name || defaultValue;

const interpeterName =
kernelConnection.kind === 'startUsingPythonInterpreter' ? kernelConnection.interpreter.displayName : undefined;

const defaultKernelName = kernelConnection.kind === 'startUsingDefaultKernel' ? 'Python 3' : undefined;
return displayName || name || interpeterName || defaultKernelName || defaultValue;
}

export function getNameOfKernelConnection(
Expand Down
3 changes: 1 addition & 2 deletions src/datascience-ui/history-react/interactivePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ ${buildSettingsCss(this.props.settings)}`}</style>
}

private renderKernelSelection() {
if (this.props.kernel.localizedUri === getLocString('DataScience.localJupyterServer', 'local')) {
if (this.props.kernel.serverName === getLocString('DataScience.localJupyterServer', 'local')) {
return;
}

Expand All @@ -235,7 +235,6 @@ ${buildSettingsCss(this.props.settings)}`}</style>
selectServer={this.props.selectServer}
selectKernel={this.props.selectKernel}
shouldShowTrustMessage={false}
settings={this.props.settings}
/>
);
}
Expand Down
42 changes: 4 additions & 38 deletions src/datascience-ui/interactive-common/jupyterInfo.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
'use strict';
import { isEmpty, isNil } from 'lodash';
import * as React from 'react';
import { IDataScienceExtraSettings } from '../../client/datascience/types';
import { Image, ImageName } from '../react-common/image';
import { getLocString } from '../react-common/locReactSide';
import { IFont, IServerState, ServerStatus } from './mainState';
Expand All @@ -16,7 +14,6 @@ export interface IJupyterInfoProps {
kernel: IServerState;
isNotebookTrusted?: boolean;
shouldShowTrustMessage: boolean;
settings?: IDataScienceExtraSettings | undefined;
selectServer(): void;
launchNotebookTrustPrompt?(): void; // Native editor-specific
selectKernel(): void;
Expand All @@ -37,17 +34,10 @@ export class JupyterInfo extends React.Component<IJupyterInfoProps> {
}

public render() {
let jupyterServerDisplayName: string = this.props.kernel.localizedUri;
if (!isNil(this.props.settings) && isEmpty(jupyterServerDisplayName)) {
const jupyterServerUriSetting: string = this.props.settings.jupyterServerURI;
if (!isEmpty(jupyterServerUriSetting) && this.isUriOfComputeInstance(jupyterServerUriSetting)) {
jupyterServerDisplayName = this.getComputeInstanceNameFromId(jupyterServerUriSetting);
}
}

const jupyterServerDisplayName: string = this.props.kernel.serverName;
const serverTextSize =
getLocString('DataScience.jupyterServer', 'Jupyter Server').length + jupyterServerDisplayName.length + 4; // plus 4 for the icon
const displayNameTextSize = this.props.kernel.displayName.length + this.props.kernel.jupyterServerStatus.length;
const displayNameTextSize = this.props.kernel.kernelName.length + this.props.kernel.jupyterServerStatus.length;
const dynamicFont: React.CSSProperties = {
fontSize: 'var(--vscode-font-size)', // Use the same font and size as the menu
fontFamily: 'var(--vscode-font-family)',
Expand Down Expand Up @@ -98,11 +88,11 @@ export class JupyterInfo extends React.Component<IJupyterInfoProps> {
role="button"
aria-disabled={ariaDisabled}
>
{this.props.kernel.displayName}: {this.props.kernel.jupyterServerStatus}
{this.props.kernel.kernelName}: {this.props.kernel.jupyterServerStatus}
</div>
);
} else {
const displayName = this.props.kernel.displayName ?? getLocString('DataScience.noKernel', 'No Kernel');
const displayName = this.props.kernel.kernelName ?? getLocString('DataScience.noKernel', 'No Kernel');
return (
<div className="kernel-status-section kernel-status-status" style={displayNameTextWidth} role="button">
{displayName}: {this.props.kernel.jupyterServerStatus}
Expand Down Expand Up @@ -138,30 +128,6 @@ export class JupyterInfo extends React.Component<IJupyterInfoProps> {
: getLocString('DataScience.connected', 'Connected');
}

private isUriOfComputeInstance(uri: string): boolean {
try {
const parsedUrl: URL = new URL(uri);
return parsedUrl.searchParams.get('id') === 'azureml_compute_instances';
} catch (e) {
return false;
}
}

private getComputeInstanceNameFromId(id: string | undefined): string {
if (isNil(id)) {
return '';
}

const res: string[] | null = id.match(
/\/providers\/Microsoft.MachineLearningServices\/workspaces\/[^\/]+\/computes\/([^\/]+)(\/)?/
);
if (isNil(res) || res.length < 2) {
return '';
}

return res[1];
}

private selectServer(): void {
this.props.selectServer();
}
Expand Down
8 changes: 4 additions & 4 deletions src/datascience-ui/interactive-common/mainState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ export interface IFont {

export interface IServerState {
jupyterServerStatus: ServerStatus;
localizedUri: string;
displayName: string;
serverName: string;
kernelName: string;
language: string;
}

Expand Down Expand Up @@ -192,8 +192,8 @@ export function generateTestState(filePath: string = '', editable: boolean = fal
loaded: false,
testMode: true,
kernel: {
localizedUri: 'No Kernel',
displayName: 'Python',
serverName: '',
kernelName: 'Python',
jupyterServerStatus: ServerStatus.NotStarted,
language: PYTHON_LANGUAGE
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ export namespace Kernel {
return {
...arg.prevState,
kernel: {
localizedUri: arg.payload.data.localizedUri,
serverName: arg.payload.data.serverName,
jupyterServerStatus: arg.payload.data.jupyterServerStatus,
displayName: arg.payload.data.displayName,
kernelName: arg.payload.data.kernelName,
language: arg.payload.data.language
}
};
Expand Down
4 changes: 2 additions & 2 deletions src/datascience-ui/interactive-common/redux/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ function generateDefaultState(
monacoReady: testMode, // When testing, monaco starts out ready
loaded: false,
kernel: {
displayName: getLocString('DataScience.noKernel', 'No Kernel'),
localizedUri: getLocString('DataScience.serverNotStarted', 'Not Started'),
kernelName: getLocString('DataScience.noKernel', 'No Kernel'),
serverName: getLocString('DataScience.serverNotStarted', 'Not Started'),
jupyterServerStatus: ServerStatus.NotStarted,
language: PYTHON_LANGUAGE
},
Expand Down
1 change: 0 additions & 1 deletion src/datascience-ui/native-editor/toolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,6 @@ export class Toolbar extends React.PureComponent<INativeEditorToolbarProps> {
shouldShowTrustMessage={true}
isNotebookTrusted={this.props.isNotebookTrusted}
launchNotebookTrustPrompt={launchNotebookTrustPrompt}
settings={this.props.settings}
/>
</div>
<div className="toolbar-divider" />
Expand Down
4 changes: 2 additions & 2 deletions src/test/datascience/interactivePanel.functional.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ suite('DataScience Interactive Panel', () => {
interruptKernel: noopAny,
isAtBottom: false,
kernel: {
displayName: '',
kernelName: '',
jupyterServerStatus: ServerStatus.Busy,
localizedUri: '',
serverName: '',
language: PYTHON_LANGUAGE
},
knownDark: false,
Expand Down
9 changes: 8 additions & 1 deletion src/test/datascience/nativeEditor.functional.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ import {
typeCode,
verifyCellIndex,
verifyCellSource,
verifyHtmlOnCell
verifyHtmlOnCell,
verifyServerStatus
} from './testHelpers';
import { ITestNativeEditorProvider } from './testNativeEditorProvider';

Expand Down Expand Up @@ -713,6 +714,8 @@ df.head()`;

// Make sure it has a server
assert.ok(editor.editor.notebook, 'Notebook did not start with a server');
// Make sure it does have a name though
verifyServerStatus(editor.mount.wrapper, 'local');
} else {
context.skip();
}
Expand All @@ -721,6 +724,7 @@ df.head()`;
runMountedTest('Server load skipped', async (context) => {
if (ioc.mockJupyter) {
ioc.getSettings().datascience.disableJupyterAutoStart = true;
ioc.getSettings().datascience.jupyterServerURI = 'https://remotetest';
await ioc.activate();

// Create an editor so something is listening to messages
Expand All @@ -731,6 +735,9 @@ df.head()`;

// Make sure it does not have a server
assert.notOk(editor.editor.notebook, 'Notebook should not start with a server');

// Make sure it does have a name though
verifyServerStatus(editor.mount.wrapper, 'Not Started');
} else {
context.skip();
}
Expand Down
8 changes: 4 additions & 4 deletions src/test/datascience/nativeEditor.toolbar.functional.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ suite('DataScience Native Toolbar', () => {
font: { family: '', size: 1 },
interruptKernel: sinon.stub(),
kernel: {
displayName: '',
kernelName: '',
jupyterServerStatus: ServerStatus.Busy,
localizedUri: '',
serverName: '',
language: PYTHON_LANGUAGE
},
restartKernel: sinon.stub(),
Expand Down Expand Up @@ -245,9 +245,9 @@ suite('DataScience Native Toolbar', () => {
font: { family: '', size: 1 },
interruptKernel: sinon.stub(),
kernel: {
displayName: '',
kernelName: '',
jupyterServerStatus: ServerStatus.Busy,
localizedUri: '',
serverName: '',
language: PYTHON_LANGUAGE
},
restartKernel: sinon.stub(),
Expand Down
Loading