Skip to content

Commit

Permalink
Merge branch 'main' into milively/context-key-debug
Browse files Browse the repository at this point in the history
  • Loading branch information
Yoyokrazy authored Aug 28, 2024
2 parents 75d6010 + 2a11eb0 commit 31bb99d
Show file tree
Hide file tree
Showing 26 changed files with 295 additions and 1,167 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/build-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -394,13 +394,13 @@ jobs:
python-version: ${{matrix.pythonVersion}}

- name: Install Conda environment from environment.yml
uses: mamba-org/provision-with-micromamba@3c96c0c27676490c63c18bc81f5c51895ac3e0e6
uses: mamba-org/setup-micromamba@v1
if: matrix.python == 'conda'
with:
cache-downloads: true
environment-name: functional_test_env
environment-file: ./build/conda-test-requirements.yml
extra-specs: |
create-args: |
python=${{matrix.pythonVersion}}
- name: Set CI Path
Expand Down
928 changes: 8 additions & 920 deletions package-lock.json

Large diffs are not rendered by default.

7 changes: 3 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"theme": "light"
},
"engines": {
"vscode": "^1.92.0"
"vscode": "^1.93.0"
},
"l10n": "./l10n",
"extensionKind": [
Expand Down Expand Up @@ -137,7 +137,7 @@
{
"command": "jupyter.runcurrentcell",
"key": "ctrl+enter",
"when": "editorTextFocus && !editorHasSelection && jupyter.hascodecells && !notebookEditorFocused && !jupyter.havenativecells"
"when": "editorTextFocus && !editorHasSelection && jupyter.hascodecells && !notebookEditorFocused"
},
{
"command": "jupyter.runcurrentcellandaddbelow",
Expand Down Expand Up @@ -2290,7 +2290,6 @@
"gulp": "^5.0.0",
"gulp-filter": "^7.0.0",
"gulp-rename": "^2.0.0",
"gulp-typescript": "^6.0.0-alpha.1",
"husky": "^8.0.3",
"jsdom": "^22.1.0",
"json2csv": "^5.0.7",
Expand Down Expand Up @@ -2330,7 +2329,7 @@
"tsconfig-paths-webpack-plugin": "^3.2.0",
"tsx": "^3.13.0",
"typemoq": "^2.1.0",
"typescript": "^5.4.5",
"typescript": "^5.5.4",
"unicode-properties": "^1.3.1",
"utf-8-validate": "^5.0.8",
"util": "^0.12.4",
Expand Down
2 changes: 2 additions & 0 deletions src/extension.common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import { StopWatch } from './platform/common/utils/stopWatch';
import { sendTelemetryEvent } from './telemetry';
import { IExtensionActivationManager } from './platform/activation/types';
import { getVSCodeChannel } from './platform/common/application/applicationEnvironment';
import { getExtensionTempDir } from './platform/common/temp';

export function initializeLoggers(
context: IExtensionContext,
Expand Down Expand Up @@ -82,6 +83,7 @@ export function initializeLoggers(
if (options?.platform) {
standardOutputChannel.appendLine(`Platform: ${options.platform} (${options.arch}).`);
}
standardOutputChannel.appendLine(`Temp Storage folder ${getDisplayPath(getExtensionTempDir(context))}`);
if (!workspace.workspaceFolders || workspace.workspaceFolders.length === 0) {
standardOutputChannel.appendLine(`No workspace folder opened.`);
} else if (workspace.workspaceFolders.length === 1) {
Expand Down
2 changes: 2 additions & 0 deletions src/extension.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ import {
postActivateLegacy
} from './extension.common';
import { activateNotebookTelemetry } from './kernels/telemetry/notebookTelemetry';
import { deleteOldTempDirs } from './platform/common/temp.node';

durations.codeLoadingTime = stopWatch.elapsedTime;

Expand Down Expand Up @@ -103,6 +104,7 @@ export async function activate(context: IExtensionContext): Promise<IExtensionAp
// Otherwise Telemetry is send via the error handler.
durations.endActivateTime = stopWatch.elapsedTime;
sendStartupTelemetry(durations, stopWatch);
deleteOldTempDirs(context).catch(noop);
return api;
} catch (ex) {
// We want to completely handle the error
Expand Down
2 changes: 2 additions & 0 deletions src/gdpr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,8 @@
// Failed to show the data viewer via the variable view.
/* __GDPR__
"DATASCIENCE.FAILED_SHOW_DATA_EXPLORER" : {
"reason": {"classification":"SystemMetaData","purpose":"FeatureInsight","comment":"","owner":"amunger"},
"fromVariableView": {"classification":"SystemMetaData","purpose":"FeatureInsight","comment":"","owner":"amunger"},
"${include}": [
"${F1}"
Expand Down
12 changes: 0 additions & 12 deletions src/interactive-window/editor-integration/cellFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,18 +75,6 @@ export function generateCells(
}
}

export function hasCells(document: TextDocument, settings?: IJupyterSettings): boolean {
const matcher = new CellMatcher(settings);
for (let index = 0; index < document.lineCount; index += 1) {
const line = document.lineAt(index);
if (matcher.isCell(line.text)) {
return true;
}
}

return false;
}

export function generateCellRangesFromDocument(document: TextDocument, settings?: IJupyterSettings): ICellRange[] {
// Implmentation of getCells here based on Don's Jupyter extension work
const matcher = new CellMatcher(settings);
Expand Down
24 changes: 22 additions & 2 deletions src/interactive-window/editor-integration/codelensprovider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
EditorContexts,
InteractiveInputScheme,
NotebookCellScheme,
PYTHON_LANGUAGE,
Telemetry
} from '../../platform/common/constants';
import { IDataScienceCodeLensProvider, ICodeWatcher } from './types';
Expand Down Expand Up @@ -56,6 +57,19 @@ export class DataScienceCodeLensProvider implements IDataScienceCodeLensProvider
if (this.debugLocationTracker) {
disposableRegistry.push(this.debugLocationTracker.updated(this.onDebugLocationUpdated.bind(this)));
}

disposableRegistry.push(vscode.window.onDidChangeActiveTextEditor(() => this.onChangedActiveTextEditor()));
this.onChangedActiveTextEditor();
}

private onChangedActiveTextEditor() {
const activeEditor = vscode.window.activeTextEditor;

if (!activeEditor || activeEditor.document.languageId != PYTHON_LANGUAGE) {
// set the context to false so our command doesn't run for other files
const hasCellsContext = new ContextKey(EditorContexts.HasCodeCells);
hasCellsContext.set(false).catch((ex) => logger.warn('Failed to set jupyter.HasCodeCells context', ex));
}
}

public dispose() {
Expand All @@ -65,6 +79,10 @@ export class DataScienceCodeLensProvider implements IDataScienceCodeLensProvider
duration: this.totalExecutionTimeInMs / this.totalGetCodeLensCalls
});
}

const editorContext = new ContextKey(EditorContexts.HasCodeCells);
editorContext.set(false).catch(noop);

dispose(this.activeCodeWatchers);
}

Expand Down Expand Up @@ -112,8 +130,10 @@ export class DataScienceCodeLensProvider implements IDataScienceCodeLensProvider
// Update the hasCodeCells context at the same time we are asked for codelens as VS code will
// ask whenever a change occurs. Do this regardless of if we have code lens turned on or not as
// shift+enter relies on this code context.
const editorContext = new ContextKey(EditorContexts.HasCodeCells);
editorContext.set(codeLenses && codeLenses.length > 0).catch(noop);
const hasCellsContext = new ContextKey(EditorContexts.HasCodeCells);
hasCellsContext
.set(codeLenses && codeLenses.length > 0)
.catch((ex) => logger.debug('Failed to set jupyter.HasCodeCells context', ex));

// Don't provide any code lenses if we have not enabled data science
const settings = this.configuration.getSettings(document.uri);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,9 @@ import { setPythonApi } from '../../../platform/interpreter/helpers';
when(extensionChecker.isPythonExtensionInstalled).thenReturn(true);
const memento = mock<Memento>();
const context = mock<IExtensionContext>();
when(context.extension).thenReturn({ packageJSON: { version: '1.0.0' } } as any);
when(context.extensionUri).thenReturn(Uri.file(EXTENSION_ROOT_DIR));
when(context.globalStorageUri).thenReturn(Uri.joinPath(Uri.file(EXTENSION_ROOT_DIR), 'temp'));
when(memento.get(anything(), anything())).thenCall((_, defaultValue) => {
if (Array.isArray(defaultValue)) {
return defaultValue;
Expand Down
35 changes: 31 additions & 4 deletions src/kernels/raw/finder/jupyterPaths.node.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import * as crypto from 'crypto';
import { inject, injectable, named } from 'inversify';
import * as fs from 'fs-extra';
import * as path from '../../../platform/vscode-path/path';
import * as uriPath from '../../../platform/vscode-path/resources';
import { CancellationToken, Memento, Uri } from 'vscode';
Expand All @@ -27,6 +29,7 @@ import { getDisplayPath } from '../../../platform/common/platform/fs-paths';
import { StopWatch } from '../../../platform/common/utils/stopWatch';
import { ResourceMap, ResourceSet } from '../../../platform/common/utils/map';
import { getPythonEnvDisplayName, getSysPrefix } from '../../../platform/interpreter/helpers';
import { getExtensionTempDir } from '../../../platform/common/temp';

const winJupyterPath = path.join('AppData', 'Roaming', 'jupyter', 'kernels');
const linuxJupyterPath = path.join('.local', 'share', 'jupyter', 'kernels');
Expand Down Expand Up @@ -70,9 +73,10 @@ export class JupyterPaths {
/**
* Contains the name of the directory where the Jupyter extension will temporary register Kernels when using non-raw.
* (this way we don't register kernels in global path).
* This path needs to be writable, as we store the kernelspecs here when we spawn kernels using Jupyter Server.
*/
public async getKernelSpecTempRegistrationFolder() {
const dir = uriPath.joinPath(this.context.extensionUri, 'temp', 'jupyter', 'kernels');
const dir = uriPath.joinPath(getExtensionTempDir(this.context), 'jupyter', 'kernels');
await this.fs.createDirectory(dir);
return dir;
}
Expand Down Expand Up @@ -110,8 +114,23 @@ export class JupyterPaths {
/**
* Returns the value for `JUPYTER_RUNTIME_DIR`, location where Jupyter stores runtime files.
* Such as kernel connection files.
* This path needs to be writable, as we store the connection files in here.
*/
public async getRuntimeDir(): Promise<Uri | undefined> {
public async getRuntimeDir(): Promise<Uri> {
const runtimeDir = await this.getRuntimeDirImpl();
if (runtimeDir) {
return runtimeDir;
}

// Run time directory doesn't exist or no permissions.
const extensionRuntimeDir = Uri.joinPath(getExtensionTempDir(this.context), 'jupyter', 'runtime');
await fs.ensureDir(extensionRuntimeDir.fsPath);
logger.trace(`Using extension runtime directory ${extensionRuntimeDir.fsPath}`);
return extensionRuntimeDir;
}

private runtimeDirIsWritable = false;
private async getRuntimeDirImpl(): Promise<Uri | undefined> {
let runtimeDir: Uri | undefined;
const userHomeDir = this.platformService.homeDir;
if (userHomeDir) {
Expand All @@ -133,11 +152,19 @@ export class JupyterPaths {
}

try {
// Make sure the local file exists
// Make sure the directory exists and is writable.
await this.fs.createDirectory(runtimeDir);
if (!this.runtimeDirIsWritable) {
// Ensure this folder is writable as well, we've found cases where this folder is not writable.
const tempFileName = `temp-test-write-access-${crypto.randomBytes(20).toString('hex')}.txt`;
const tempFile = uriPath.joinPath(runtimeDir, tempFileName);
// If this fails, then thats find, at least we know the folder is not writable, even if it exists.
await fs.writeFile(tempFile.fsPath, '');
await fs.unlink(tempFile.fsPath).catch(noop);
}
return runtimeDir;
} catch (ex) {
logger.error(`Failed to create runtime directory, reverting to temp directory ${runtimeDir}`, ex);
logger.error(`Failed to create/or verify write access to runtime directory ${runtimeDir}`, ex);
}
}
/**
Expand Down
22 changes: 5 additions & 17 deletions src/kernels/raw/launcher/kernelProcess.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import { ChildProcess } from 'child_process';
import { kill } from 'process';
import * as fs from 'fs-extra';
import * as crypto from 'crypto';
import * as os from 'os';
import * as path from '../../../platform/vscode-path/path';
import { CancellationError, CancellationToken, Event, EventEmitter, Uri } from 'vscode';
Expand Down Expand Up @@ -456,7 +457,10 @@ export class KernelProcess extends ObservableDisposable implements IKernelProces
);
}

this.connectionFile = await this.createConnectionFile();
const runtimeDir = await this.jupyterPaths.getRuntimeDir();
const connectionFileName = `kernel-v3${crypto.randomBytes(20).toString('hex')}.json`;
this.connectionFile = Uri.joinPath(runtimeDir, connectionFileName);

// Python kernels are special. Handle the extra arguments.
if (this.isPythonKernel) {
// Slice out -f and the connection file from the args
Expand Down Expand Up @@ -495,22 +499,6 @@ export class KernelProcess extends ObservableDisposable implements IKernelProces
}
}
}
private async createConnectionFile() {
const runtimeDir = await this.jupyterPaths.getRuntimeDir();
const tempFile = await this.fileSystem.createTemporaryLocalFile({
fileExtension: '.json',
prefix: 'kernel-v2-'
});
// Note: We have to dispose the temp file and recreate it else the file
// system will hold onto the file with an open handle. THis doesn't work so well when
// a different process tries to open it.
const connectionFile = runtimeDir
? path.join(runtimeDir.fsPath, path.basename(tempFile.filePath))
: tempFile.filePath;
// Ensure we dispose this, and don't maintain a handle on this file.
await tempFile.dispose(); // Do not remove this line.
return Uri.file(connectionFile);
}
// Add the command line arguments
private addPythonConnectionArgs(connectionFile: Uri): string[] {
const newConnectionArgs: string[] = [];
Expand Down
Loading

0 comments on commit 31bb99d

Please sign in to comment.