Skip to content

Commit

Permalink
Port april point fixes (#5461)
Browse files Browse the repository at this point in the history
  • Loading branch information
IanMatthewHuff authored Apr 13, 2021
1 parent 11a9e59 commit 7977fcd
Show file tree
Hide file tree
Showing 18 changed files with 129 additions and 46 deletions.
2 changes: 0 additions & 2 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -847,7 +847,6 @@ module.exports = {
'src/client/datascience/liveshare/liveshareProxy.ts',
'src/client/datascience/liveshare/postOffice.ts',
'src/client/datascience/messages.ts',
'src/client/datascience/raw-kernel/rawNotebookProvider.ts',
'src/client/datascience/raw-kernel/liveshare/guestRawNotebookProvider.ts',
'src/client/datascience/raw-kernel/rawKernel.ts',
'src/client/datascience/raw-kernel/rawSocket.ts',
Expand Down Expand Up @@ -885,7 +884,6 @@ module.exports = {
'src/client/datascience/editor-integration/codelensprovider.ts',
'src/client/datascience/editor-integration/cellhashprovider.ts',
'src/client/datascience/commands/commandLineSelector.ts',
'src/client/datascience/commands/exportCommands.ts',
'src/client/datascience/cellFactory.ts',
'src/client/datascience/notebook/notebookEditorCompatibilitySupport.ts',
'src/client/datascience/notebook/constants.ts',
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/build-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -788,7 +788,7 @@ jobs:
VSC_JUPYTER_NON_RAW_NATIVE_TEST: ${{ matrix.jupyter == 'local' }}
VSC_JUPYTER_CI_RUN_JAVA_NB_TEST: ${{ matrix.python == 'conda' }}
VSC_JUPYTER_CI_IS_CONDA: ${{ matrix.python == 'conda' }}
VSC_JUPYTER_CI_TEST_VSC_CHANNEL: 'insiders'
VSC_JUPYTER_CI_TEST_VSC_CHANNEL: 'stable'
id: test_notebook_vscode
if: matrix.test-suite == 'notebook' && matrix.python != 'noPython'

Expand All @@ -808,7 +808,7 @@ jobs:
VSC_FORCE_REAL_JUPYTER: 1
VSC_JUPYTER_FORCE_LOGGING: 1
VSC_JUPYTER_CI_RUN_NON_PYTHON_NB_TEST: 1
VSC_JUPYTER_CI_TEST_VSC_CHANNEL: 'insiders'
VSC_JUPYTER_CI_TEST_VSC_CHANNEL: 'stable'
id: test_notebookWithoutPythonExt_vscode
if: matrix.python == 'noPython' && matrix.os != 'windows-latest'

Expand All @@ -834,7 +834,7 @@ jobs:
VSC_JUPYTER_REMOTE_NATIVE_TEST: 'false'
VSC_JUPYTER_NON_RAW_NATIVE_TEST: 'false'
VSC_JUPYTER_CI_RUN_JAVA_NB_TEST: 'false'
VSC_JUPYTER_CI_TEST_VSC_CHANNEL: 'insiders'
VSC_JUPYTER_CI_TEST_VSC_CHANNEL: 'stable'
id: test_notebook_webview_vscode
if: matrix.test-suite == 'notebookAndWebview' && matrix.python != 'noPython'

Expand Down
16 changes: 15 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,26 @@
# Changelog

## 2021.5.1 (07 April 2021)
## 2021.5.1 (12 April 2021)

### Code Health

1. Check the responses of prompts for installation of missing packages such as `IPyKernel`.
([#5432](https://github.com/Microsoft/vscode-jupyter/issues/5432))

### Fixes

1. Fix for 'Export as Python Script' option not appearing.
([#5403](https://github.com/Microsoft/vscode-jupyter/issues/5403))
1. Delete extension context secrets if we get an error when getting them.
Small fixes on error handling.
([#5419](https://github.com/Microsoft/vscode-jupyter/issues/5419))
1. Enable correct plot background for Native Notebooks.
([#5353](https://github.com/Microsoft/vscode-jupyter/issues/5353))
1. Stop asking users to install ipykernel on autostart, only do it when a cell is ran.
([#5368](https://github.com/microsoft/vscode-jupyter/issues/5368))
1. Invalidate cached interpreters when Python extension active interpreter changes.
([#5470](https://github.com/microsoft/vscode-jupyter/issues/5470))

## 2021.5.0 (31 March 2021)

### Enhancements
Expand Down
14 changes: 7 additions & 7 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1855,13 +1855,13 @@
"test:cover:report": "nyc report --reporter=text --reporter=html --reporter=text-summary --reporter=cobertura",
"preTestJediLSP": "node ./out/test/languageServers/jedi/lspSetup.js",
"testJediLSP": "node ./out/test/languageServers/jedi/lspSetup.js && cross-env CODE_TESTS_WORKSPACE=src/test VSC_JUPYTER_CI_TEST_GREP='Language Server:' node ./out/test/testBootstrap.js ./out/test/standardTest.js && node ./out/test/languageServers/jedi/lspTeardown.js",
"pretestVSCode": "cross-env VSC_JUPYTER_CI_TEST_VSC_CHANNEL=insiders node ./out/test/datascience/dsTestSetup.js",
"testVSCode": "cross-env CODE_TESTS_WORKSPACE=src/test/datascience VSC_JUPYTER_CI_TEST_VSC_CHANNEL=insiders TEST_FILES_SUFFIX=vscode.test VSC_JUPYTER_FORCE_LOGGING=1 VSC_JUPYTER_LOAD_EXPERIMENTS_FROM_FILE=true node ./out/test/testBootstrap.js ./out/test/standardTest.js",
"pretestNativeNotebooksInVSCode": "cross-env VSC_JUPYTER_CI_TEST_VSC_CHANNEL=insiders VSC_JUPYTER_RUN_NB_TEST=true node ./out/test/datascience/dsTestSetup.js",
"testNativeNotebooksInVSCode": "cross-env CODE_TESTS_WORKSPACE=src/test/datascience VSC_JUPYTER_CI_TEST_VSC_CHANNEL=insiders VSC_JUPYTER_RUN_NB_TEST=true TEST_FILES_SUFFIX=vscode.test VSC_JUPYTER_FORCE_LOGGING=1 VSC_JUPYTER_LOAD_EXPERIMENTS_FROM_FILE=true node ./out/test/testBootstrap.js ./out/test/standardTest.js",
"pretestNativeNotebooksWithoutPythonInVSCode": "cross-env VSC_JUPYTER_CI_TEST_VSC_CHANNEL=insiders VSC_JUPYTER_RUN_NB_TEST=true node ./out/test/datascience/dsTestSetup.js",
"testNativeNotebooksWithoutPythonInVSCode": "cross-env CODE_TESTS_WORKSPACE=src/test/datascience VSC_JUPYTER_CI_TEST_VSC_CHANNEL=insiders VSC_JUPYTER_RUN_NB_TEST=true TEST_FILES_SUFFIX=vscode.test VSC_JUPYTER_FORCE_LOGGING=1 VSC_JUPYTER_LOAD_EXPERIMENTS_FROM_FILE=true VSC_JUPYTER_CI_TEST_GREP=non-python-kernel VSC_JUPYTER_CI_TEST_DO_NOT_INSTALL_PYTHON_EXT=true node ./out/test/testBootstrap.js ./out/test/standardTest.js",
"testNativeNotebooksAndWebviews": "cross-env CODE_TESTS_WORKSPACE=src/test/datascience VSC_JUPYTER_CI_TEST_VSC_CHANNEL=insiders VSC_JUPYTER_RUN_NB_TEST=true TEST_FILES_SUFFIX=vscode.test VSC_JUPYTER_FORCE_LOGGING=1 VSC_JUPYTER_CI_TEST_GREP=webview-test VSC_JUPYTER_LOAD_EXPERIMENTS_FROM_FILE=true node ./out/test/testBootstrap.js ./out/test/standardTest.js",
"pretestVSCode": "cross-env VSC_JUPYTER_CI_TEST_VSC_CHANNEL=stable node ./out/test/datascience/dsTestSetup.js",
"testVSCode": "cross-env CODE_TESTS_WORKSPACE=src/test/datascience VSC_JUPYTER_CI_TEST_VSC_CHANNEL=stable TEST_FILES_SUFFIX=vscode.test VSC_JUPYTER_FORCE_LOGGING=1 VSC_JUPYTER_LOAD_EXPERIMENTS_FROM_FILE=true node ./out/test/testBootstrap.js ./out/test/standardTest.js",
"pretestNativeNotebooksInVSCode": "cross-env VSC_JUPYTER_CI_TEST_VSC_CHANNEL=stable VSC_JUPYTER_RUN_NB_TEST=true node ./out/test/datascience/dsTestSetup.js",
"testNativeNotebooksInVSCode": "cross-env CODE_TESTS_WORKSPACE=src/test/datascience VSC_JUPYTER_CI_TEST_VSC_CHANNEL=stable VSC_JUPYTER_RUN_NB_TEST=true TEST_FILES_SUFFIX=vscode.test VSC_JUPYTER_FORCE_LOGGING=1 VSC_JUPYTER_LOAD_EXPERIMENTS_FROM_FILE=true node ./out/test/testBootstrap.js ./out/test/standardTest.js",
"pretestNativeNotebooksWithoutPythonInVSCode": "cross-env VSC_JUPYTER_CI_TEST_VSC_CHANNEL=stable VSC_JUPYTER_RUN_NB_TEST=true node ./out/test/datascience/dsTestSetup.js",
"testNativeNotebooksWithoutPythonInVSCode": "cross-env CODE_TESTS_WORKSPACE=src/test/datascience VSC_JUPYTER_CI_TEST_VSC_CHANNEL=stable VSC_JUPYTER_RUN_NB_TEST=true TEST_FILES_SUFFIX=vscode.test VSC_JUPYTER_FORCE_LOGGING=1 VSC_JUPYTER_LOAD_EXPERIMENTS_FROM_FILE=true VSC_JUPYTER_CI_TEST_GREP=non-python-kernel VSC_JUPYTER_CI_TEST_DO_NOT_INSTALL_PYTHON_EXT=true node ./out/test/testBootstrap.js ./out/test/standardTest.js",
"testNativeNotebooksAndWebviews": "cross-env CODE_TESTS_WORKSPACE=src/test/datascience VSC_JUPYTER_CI_TEST_VSC_CHANNEL=stable VSC_JUPYTER_RUN_NB_TEST=true TEST_FILES_SUFFIX=vscode.test VSC_JUPYTER_FORCE_LOGGING=1 VSC_JUPYTER_CI_TEST_GREP=webview-test VSC_JUPYTER_LOAD_EXPERIMENTS_FROM_FILE=true node ./out/test/testBootstrap.js ./out/test/standardTest.js",
"testPerformance": "node ./out/test/testBootstrap.js ./out/test/performanceTest.js",
"testSmoke": "node ./out/test/testBootstrap.js ./out/test/smokeTest.js",
"testSmokeLogged": "cross-env VSC_JUPYTER_FORCE_LOGGING=true VSC_JUPYTER_LOG_FILE=smoke-test.log node --no-force-async-hooks-checks ./out/test/testBootstrap.js ./out/test/smokeTest.js",
Expand Down
9 changes: 8 additions & 1 deletion src/client/api/pythonApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,14 @@ export class InterpreterService implements IInterpreterService {
.then((api) => {
if (!this.eventHandlerAdded) {
this.eventHandlerAdded = true;
api.onDidChangeInterpreter(() => this.didChangeInterpreter.fire(), this, this.disposables);
api.onDidChangeInterpreter(
() => {
this.didChangeInterpreter.fire();
this.workspaceCachedActiveInterpreter.clear();
},
this,
this.disposables
);
}
})
.catch(noop);
Expand Down
12 changes: 9 additions & 3 deletions src/client/common/application/encryptedStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,14 @@ export class EncryptedStorage implements IEncryptedStorage {
if (IS_REMOTE_NATIVE_TEST && this.extensionContext.extensionMode !== ExtensionMode.Production) {
return this.testingState.get(`${service}#${key}`);
}
// eslint-disable-next-line
const val = await this.extensionContext.secrets.get(`${service}.${key}`);
return val;
try {
// eslint-disable-next-line
const val = await this.extensionContext.secrets.get(`${service}.${key}`);
return val;
} catch (e) {
// If we get an error trying to get a secret, it might be corrupted. So we delete it.
await this.extensionContext.secrets.delete(`${service}.${key}`);
return;
}
}
}
4 changes: 3 additions & 1 deletion src/client/common/errors/errorUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ export function getLastFrameFromPythonTraceback(
}
// File "/Users/donjayamanne/miniconda3/envs/env3/lib/python3.7/site-packages/appnope/_nope.py", line 38, in C

const lastFrame = traceback
// This parameter might be either a string or a string array
const fixedTraceback: string = Array.isArray(traceback) ? traceback[0] : traceback;
const lastFrame = fixedTraceback
.split('\n')
.map((item) => item.trim().toLowerCase())
.filter((item) => item.length)
Expand Down
8 changes: 6 additions & 2 deletions src/client/common/errors/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,12 @@ const taggers = [
];
export function getErrorTags(stdErrOrStackTrace: string) {
const tags: string[] = [];
stdErrOrStackTrace = stdErrOrStackTrace.toLowerCase();
taggers.forEach((tagger) => tagger(stdErrOrStackTrace, tags));

// This parameter might be either a string or a string array
let stdErrOrStackTraceLowered = Array.isArray(stdErrOrStackTrace)
? stdErrOrStackTrace[0].toLowerCase()
: stdErrOrStackTrace.toLowerCase();
taggers.forEach((tagger) => tagger(stdErrOrStackTraceLowered, tags));

return Array.from(new Set(tags)).join(',');
}
Expand Down
13 changes: 9 additions & 4 deletions src/client/datascience/commands/exportCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,15 +127,20 @@ export class ExportCommands implements IDisposable {
const items: IExportQuickPickItem[] = [];
const notebook = JSON.parse(contents) as nbformat.INotebookContent;

if (notebook.metadata && isPythonNotebook(notebook.metadata)) {
if (interpreter || (notebook.metadata && isPythonNotebook(notebook.metadata))) {
items.push({
label: DataScience.exportPythonQuickPickLabel(),
picked: true,
handler: () => {
sendTelemetryEvent(Telemetry.ClickedExportNotebookAsQuickPick, undefined, {
format: ExportFormat.python
});
this.commandManager.executeCommand(Commands.ExportAsPythonScript, contents, source, interpreter);
void this.commandManager.executeCommand(
Commands.ExportAsPythonScript,
contents,
source,
interpreter
);
}
});
}
Expand All @@ -149,7 +154,7 @@ export class ExportCommands implements IDisposable {
sendTelemetryEvent(Telemetry.ClickedExportNotebookAsQuickPick, undefined, {
format: ExportFormat.html
});
this.commandManager.executeCommand(
void this.commandManager.executeCommand(
Commands.ExportToHTML,
contents,
source,
Expand All @@ -165,7 +170,7 @@ export class ExportCommands implements IDisposable {
sendTelemetryEvent(Telemetry.ClickedExportNotebookAsQuickPick, undefined, {
format: ExportFormat.pdf
});
this.commandManager.executeCommand(
void this.commandManager.executeCommand(
Commands.ExportToPDF,
contents,
source,
Expand Down
26 changes: 16 additions & 10 deletions src/client/datascience/interactive-common/interactiveBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,9 @@ export abstract class InteractiveBase extends WebviewPanelHost<IInteractiveWindo
this.disposables.push(this.jupyterVariables.refreshRequired(this.refreshVariables.bind(this)));

// If we have already auto started our server then we can go ahead and try to create a notebook on construction
// Disable the UI to avoid errors before the user runs a cell
setTimeout(() => {
this.createNotebookIfProviderConnectionExists().ignoreErrors();
this.createNotebookIfProviderConnectionExists(true).ignoreErrors();
}, 0);
}

Expand Down Expand Up @@ -887,9 +888,9 @@ export abstract class InteractiveBase extends WebviewPanelHost<IInteractiveWindo
// ensureNotebook can be called apart from ensureNotebookAndServer and it needs
// the same protection to not be called twice
// eslint-disable-next-line @typescript-eslint/member-ordering
protected async ensureNotebook(serverConnection: INotebookProviderConnection): Promise<void> {
protected async ensureNotebook(serverConnection: INotebookProviderConnection, disableUI = false): Promise<void> {
if (!this.notebookPromise) {
this.notebookPromise = this.ensureNotebookImpl(serverConnection);
this.notebookPromise = this.ensureNotebookImpl(serverConnection, disableUI);
}
try {
await this.notebookPromise;
Expand All @@ -901,17 +902,18 @@ export abstract class InteractiveBase extends WebviewPanelHost<IInteractiveWindo
}
}

protected async createNotebookIfProviderConnectionExists(): Promise<void> {
protected async createNotebookIfProviderConnectionExists(disableUI?: boolean): Promise<void> {
// Check to see if we are already connected to our provider
const providerConnection = await this.notebookProvider.connect({
getOnly: true,
resource: this.owningResource,
metadata: this.notebookMetadata
metadata: this.notebookMetadata,
disableUI
});

if (providerConnection) {
try {
await this.ensureNotebook(providerConnection);
await this.ensureNotebook(providerConnection, disableUI);
} catch (e) {
this.errorHandler.handleError(e).ignoreErrors();
}
Expand Down Expand Up @@ -1195,15 +1197,19 @@ export abstract class InteractiveBase extends WebviewPanelHost<IInteractiveWindo
.then(noop, noop);
}

private async createNotebook(serverConnection: INotebookProviderConnection): Promise<INotebook | undefined> {
private async createNotebook(
serverConnection: INotebookProviderConnection,
disableUI: boolean
): Promise<INotebook | undefined> {
let notebook: INotebook | undefined;
while (!notebook) {
try {
notebook = await this.notebookProvider.getOrCreateNotebook({
identity: this.notebookIdentity.resource,
resource: this.owningResource,
metadata: this.notebookMetadata,
kernelConnection: this.kernelConnection
kernelConnection: this.kernelConnection,
disableUI
});
if (notebook) {
const executionActivation = { ...this.notebookIdentity, owningResource: this.owningResource };
Expand Down Expand Up @@ -1265,7 +1271,7 @@ export abstract class InteractiveBase extends WebviewPanelHost<IInteractiveWindo
notebook.registerIOPubListener(this.handleKernelMessage.bind(this));
}

private async ensureNotebookImpl(serverConnection: INotebookProviderConnection): Promise<void> {
private async ensureNotebookImpl(serverConnection: INotebookProviderConnection, disableUI: boolean): Promise<void> {
// Create a new notebook if we need to.
if (!this._notebook) {
// While waiting make the notebook look busy
Expand All @@ -1276,7 +1282,7 @@ export abstract class InteractiveBase extends WebviewPanelHost<IInteractiveWindo
language: PYTHON_LANGUAGE
}).ignoreErrors();

this._notebook = await this.createNotebook(serverConnection);
this._notebook = await this.createNotebook(serverConnection, disableUI);

// If that works notify the UI and listen to status changes.
if (this._notebook && this._notebook.identity) {
Expand Down
10 changes: 5 additions & 5 deletions src/client/datascience/interactive-common/notebookProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export class NotebookProvider implements INotebookProvider {
private readonly notebooks = new Map<string, Promise<INotebook>>();
private _notebookCreated = new EventEmitter<{ identity: Uri; notebook: INotebook }>();
private readonly _onSessionStatusChanged = new EventEmitter<{ status: ServerStatus; notebook: INotebook }>();
private _connectionMade = new EventEmitter<void>();
private _connectionMade = new EventEmitter<boolean>();
private _potentialKernelChanged = new EventEmitter<{ identity: Uri; kernelConnection: KernelConnectionMetadata }>();
private _type: 'jupyter' | 'raw' = 'jupyter';
public get activeNotebooks() {
Expand Down Expand Up @@ -83,15 +83,15 @@ export class NotebookProvider implements INotebookProvider {
if (await this.rawNotebookProvider.supported()) {
return this.rawNotebookProvider.connect({
...options,
onConnectionMade: this.fireConnectionMade.bind(this)
onConnectionMade: this.fireConnectionMade.bind(this, options.disableUI!)
});
} else if (
this.extensionChecker.isPythonExtensionInstalled ||
serverType === Settings.JupyterServerRemoteLaunch
) {
return this.jupyterNotebookProvider.connect({
...options,
onConnectionMade: this.fireConnectionMade.bind(this)
onConnectionMade: this.fireConnectionMade.bind(this, options.disableUI!)
});
} else if (!options.getOnly) {
await this.extensionChecker.showPythonExtensionInstallRequiredPrompt();
Expand Down Expand Up @@ -175,8 +175,8 @@ export class NotebookProvider implements INotebookProvider {
this._potentialKernelChanged.fire({ identity, kernelConnection: kernel });
}

private fireConnectionMade() {
this._connectionMade.fire();
private fireConnectionMade(disableUI: boolean) {
this._connectionMade.fire(disableUI);
}

// Cache the promise that will return a notebook
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 @@ -446,9 +446,9 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
return Promise.resolve();
}

protected async createNotebookIfProviderConnectionExists() {
protected async createNotebookIfProviderConnectionExists(disableUI?: boolean) {
if (this._model.isTrusted) {
await super.createNotebookIfProviderConnectionExists();
await super.createNotebookIfProviderConnectionExists(disableUI);
}
}

Expand Down
5 changes: 5 additions & 0 deletions src/client/datascience/notebook/helpers/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,11 @@ export function isPythonNotebook(metadata?: nbformat.INotebookMetadata) {
if (metadata?.language_info?.name && metadata.language_info.name !== PYTHON_LANGUAGE) {
return false;
}

if (kernelSpec?.name.includes(PYTHON_LANGUAGE)) {
return true;
}

// Valid notebooks will have a language information in the metadata.
return kernelSpec?.language === PYTHON_LANGUAGE;
}
Expand Down
Loading

0 comments on commit 7977fcd

Please sign in to comment.