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

Move NotebookControllerManager into the DI container for web #9690

Merged
merged 9 commits into from
Apr 15, 2022
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
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
1 change: 0 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ module.exports = {
'src/test/mocks/vsc/strings.ts',
'src/test/mocks/vsc/charCode.ts',
'src/test/mocks/vsc/htmlContent.ts',
'src/test/mocks/vsc/selection.ts',
'src/test/mocks/vsc/position.ts',
'src/test/mocks/vsc/telemetryReporter.ts',
'src/test/mocks/vsc/range.ts',
Expand Down
25 changes: 8 additions & 17 deletions build/webpack/webpack.extension.web.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,23 +32,6 @@ const config = {
},
module: {
rules: [
{
// JupyterServices imports node-fetch.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As soon as I started importing these into the web bundle, this code failed to work.

I believe we don't need in the webbundle anymore.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I'm not seeing the node-fetch usage anymore in jupyterlab services.

test: /@jupyterlab[\\\/]services[\\\/].*js$/,
use: [
{
loader: path.join(__dirname, 'loaders', 'fixNodeFetch.js')
}
]
},
{
test: /\.ts$/,
use: [
{
loader: path.join(__dirname, 'loaders', 'externalizeDependencies.js')
}
]
},
{
test: /\.ts$/,
exclude: /node_modules/,
Expand Down Expand Up @@ -134,6 +117,14 @@ const config = {
path: path.resolve(constants.ExtensionRootDir, 'out'),
libraryTarget: 'commonjs2',
devtoolModuleFilenameTemplate: '../[resource-path]'
},
watchOptions: {
aggregateTimeout: 200,
poll: 1000,
ignored: /node_modules/
},
stats: {
builtAt: true
}
};
// tslint:disable-next-line:no-default-export
Expand Down
20 changes: 17 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2100,7 +2100,7 @@
"compiled": "deemon npm run compile",
"kill-compiled": "deemon --kill npm run compile",
"compile-webviews-watch": "webpack --config ./build/webpack/webpack.datascience-ui.config.js --watch",
"compile-web-watch": "webpack --config ./build/webpack/webpack.extension.web.config.js --stats-error-details --watch",
"compile-web-watch": "webpack --config ./build/webpack/webpack.extension.web.config.js --stats-error-details --watch --progress",
"compile-web": "webpack --config ./build/webpack/webpack.extension.web.config.js",
"compile-web-test": "cross-env VSC_TEST_BUNDLE=true npm run compile-web",
"compile-web-test-watch": "cross-env VSC_TEST_BUNDLE=true npm run compile-web-watch",
Expand Down Expand Up @@ -2287,6 +2287,7 @@
"@types/temp": "^0.8.32",
"@types/tmp": "0.0.33",
"@types/untildify": "^3.0.0",
"@types/url-parse": "^1.4.8",
"@types/uuid": "^3.4.3",
"@types/vscode-notebook-renderer": "^1.60.0",
"@types/webpack-bundle-analyzer": "^2.13.0",
Expand Down
2 changes: 2 additions & 0 deletions src/extension.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ import { OutputChannelLogger } from './platform/logging/outputChannelLogger';
import { ConsoleLogger } from './platform/logging/consoleLogger';
import { FileLogger } from './platform/logging/fileLogger.node';
import { createWriteStream } from 'fs-extra';
import { initializeGlobals as initializeTelemetryGlobals } from './telemetry/telemetry';

durations.codeLoadingTime = stopWatch.elapsedTime;

Expand Down Expand Up @@ -160,6 +161,7 @@ async function activateUnsafe(

const [serviceManager, serviceContainer] = initializeGlobals(context);
activatedServiceContainer = serviceContainer;
initializeTelemetryGlobals(serviceContainer);
const activationPromise = activateComponents(context, serviceManager, serviceContainer);

//===============================================
Expand Down
12 changes: 12 additions & 0 deletions src/extension.web.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ import { sendErrorTelemetry, sendStartupTelemetry } from './platform/startupTele
import { noop } from './platform/common/utils/misc';
import { JUPYTER_OUTPUT_CHANNEL, PythonExtension } from './webviews/webview-side/common/constants';
import { registerTypes as registerPlatformTypes } from './platform/serviceRegistry.web';
import { registerTypes as registerTelemetryTypes } from './telemetry/serviceRegistry.web';
import { registerTypes as registerKernelTypes } from './kernels/serviceRegistry.web';
import { registerTypes as registerNotebookTypes } from './notebooks/serviceRegistry.web';
import { registerTypes as registerInteractiveTypes } from './interactive-window/serviceRegistry.web';
import { registerTypes as registerIntellisenseTypes } from './intellisense/serviceRegistry.web';
import { IExtensionActivationManager } from './platform/activation/types';
import { isCI, isTestExecution, STANDARD_OUTPUT_CHANNEL } from './platform/common/constants';
import { getJupyterOutputChannel } from './platform/devTools/jupyterOutputChannel';
Expand All @@ -72,6 +77,7 @@ import { ServiceContainer } from './platform/ioc/container';
import { ServiceManager } from './platform/ioc/serviceManager';
import { OutputChannelLogger } from './platform/logging/outputChannelLogger';
import { ConsoleLogger } from './platform/logging/consoleLogger';
import { initializeGlobals as initializeTelemetryGlobals } from './telemetry/telemetry';

durations.codeLoadingTime = stopWatch.elapsedTime;

Expand Down Expand Up @@ -146,6 +152,7 @@ async function activateUnsafe(

const [serviceManager, serviceContainer] = initializeGlobals(context);
activatedServiceContainer = serviceContainer;
initializeTelemetryGlobals(serviceContainer);
const activationPromise = activateComponents(context, serviceManager, serviceContainer);

//===============================================
Expand Down Expand Up @@ -270,6 +277,11 @@ async function activateLegacy(

// Register the rest of the types (platform is first because it's needed by others)
registerPlatformTypes(context, serviceManager, isDevMode);
registerTelemetryTypes(serviceManager);
registerNotebookTypes(serviceManager);
registerKernelTypes(serviceManager, isDevMode);
registerInteractiveTypes(serviceManager);
registerIntellisenseTypes(serviceManager, isDevMode);

// Load the two data science experiments that we need to register types
// Await here to keep the register method sync
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ import {
isPythonKernelConnection,
getKernelConnectionLanguage,
getLanguageInNotebookMetadata
} from '../kernels/helpers.node';
} from '../kernels/helpers';
import { KernelConnectionMetadata } from '../kernels/types';
import { isJupyterNotebook, getNotebookMetadata } from '../notebooks/helpers.node';
import { translateKernelLanguageToMonaco } from '../platform/common/utils.node';
import { isJupyterNotebook, getNotebookMetadata } from '../notebooks/helpers';
import { translateKernelLanguageToMonaco } from '../platform/common/utils';

export const LastSavedNotebookCellLanguage = 'DATASCIENCE.LAST_SAVED_CELL_LANGUAGE';
/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { IDisposable } from '@fluentui/react';
import { injectable, inject } from 'inversify';
import {
CancellationToken,
Expand Down Expand Up @@ -30,10 +29,10 @@ import { IExtensionSyncActivationService } from '../platform/activation/types';
import { IVSCodeNotebook, IDocumentManager } from '../platform/common/application/types';
import { PYTHON_LANGUAGE } from '../platform/common/constants';
import { disposeAllDisposables } from '../platform/common/helpers';
import { IDisposableRegistry } from '../platform/common/types';
import { IDisposable, IDisposableRegistry } from '../platform/common/types';
import { DataScience } from '../platform/common/utils/localize';
import { JupyterNotebookView } from '../notebooks/constants';
import { getAssociatedJupyterNotebook } from '../notebooks/helpers.node';
import { getAssociatedJupyterNotebook } from '../notebooks/helpers';

type CellUri = string;
type CellVersion = number;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ import { PYTHON_LANGUAGE } from '../platform/common/constants';
import { traceError } from '../platform/logging';
import { IDisposableRegistry } from '../platform/common/types';
import { noop } from '../platform/common/utils/misc';
import { chainWithPendingUpdates } from '../notebooks/execution/notebookUpdater.node';
import { isJupyterNotebook } from '../notebooks/helpers.node';
import { chainWithPendingUpdates } from '../notebooks/execution/notebookUpdater';
import { isJupyterNotebook } from '../notebooks/helpers';
import { INotebookControllerManager } from '../notebooks/types';
import { translateKernelLanguageToMonaco } from '../platform/common/utils.node';
import { translateKernelLanguageToMonaco } from '../platform/common/utils';
import { IVSCodeNotebookController } from '../notebooks/controllers/types';
/**
* If user creates a blank notebook, then they'll mostl likely end up with a blank cell with language, lets assume `Python`.
Expand Down
27 changes: 12 additions & 15 deletions src/intellisense/intellisenseProvider.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import { IExtensionSyncActivationService } from '../platform/activation/types';
import { IPythonExtensionChecker } from '../platform/api/types';
import { IVSCodeNotebook, IWorkspaceService } from '../platform/common/application/types';
import { IDisposableRegistry, IConfigurationService, IsPreRelease } from '../platform/common/types';
import { IInterpreterService } from '../platform/interpreter/contracts.node';
import { IInterpreterService } from '../platform/interpreter/contracts';
import { PythonEnvironment } from '../platform/pythonEnvironments/info';
import { getInterpreterId } from '../platform/pythonEnvironments/info/interpreter.node';
import { isJupyterNotebook, findAssociatedNotebookDocument } from '../notebooks/helpers.node';
import { getInterpreterId } from '../platform/pythonEnvironments/info/interpreter';
import { isJupyterNotebook, findAssociatedNotebookDocument } from '../notebooks/helpers';
import { INotebookLanguageClientProvider, INotebookControllerManager } from '../notebooks/types';
import { LanguageServer } from './languageServer.node';
import { IInteractiveWindowProvider } from '../interactive-window/types';
Expand Down Expand Up @@ -73,19 +73,18 @@ export class IntellisenseProvider implements INotebookLanguageClientProvider, IE
}

private handleInterpreterChange() {
const folders = [...this.activeInterpreterCache.keys()];
this.activeInterpreterCache.clear();
folders.forEach((f) => this.getActiveInterpreterSync(f));
this.getActiveInterpreterSync(undefined);
}

private getActiveInterpreterSync(fsPath: string | undefined): PythonEnvironment | undefined {
private getActiveInterpreterSync(uri: Uri | undefined): PythonEnvironment | undefined {
if (!this.extensionChecker.isPythonExtensionInstalled) {
return;
}
const folder =
this.workspaceService.getWorkspaceFolder(fsPath ? Uri.file(fsPath) : undefined)?.uri ||
(this.workspaceService.rootPath ? Uri.file(this.workspaceService.rootPath) : undefined);
const key = folder ? folder.fsPath : EmptyWorkspaceKey;
this.workspaceService.getWorkspaceFolder(uri)?.uri ||
(this.workspaceService.rootFolder ? this.workspaceService.rootFolder : undefined);
const key = folder ? getComparisonKey(folder) : EmptyWorkspaceKey;
if (!this.activeInterpreterCache.has(key)) {
this.interpreterService
.getActiveInterpreter(folder)
Expand All @@ -105,7 +104,7 @@ export class IntellisenseProvider implements INotebookLanguageClientProvider, IE
const oldController = this.knownControllers.get(e.notebook);
const oldInterpreter = oldController
? oldController.connection.interpreter
: this.getActiveInterpreterSync(e.notebook.uri.fsPath);
: this.getActiveInterpreterSync(e.notebook.uri);
const oldInterpreterId = oldInterpreter ? this.getInterpreterIdFromCache(oldInterpreter) : undefined;
const oldLanguageServer = oldInterpreterId ? await this.servers.get(oldInterpreterId) : undefined;

Expand Down Expand Up @@ -134,7 +133,7 @@ export class IntellisenseProvider implements INotebookLanguageClientProvider, IE
}

// Make sure the active interpreter cache is up to date
this.getActiveInterpreterSync(n.uri.fsPath);
this.getActiveInterpreterSync(n.uri);

// If the controller is empty, default to the active interpreter
const interpreter =
Expand Down Expand Up @@ -171,9 +170,7 @@ export class IntellisenseProvider implements INotebookLanguageClientProvider, IE
const controller = notebook
? this.notebookControllerManager.getSelectedNotebookController(notebook)
: undefined;
const notebookInterpreter = controller
? controller.connection.interpreter
: this.getActiveInterpreterSync(uri.fsPath);
const notebookInterpreter = controller ? controller.connection.interpreter : this.getActiveInterpreterSync(uri);
let notebookId = notebookInterpreter ? this.getInterpreterIdFromCache(notebookInterpreter) : undefined;

// Special case. For remote use the active interpreter as the controller's interpreter isn't
Expand All @@ -183,7 +180,7 @@ export class IntellisenseProvider implements INotebookLanguageClientProvider, IE
(controller?.connection.kind === 'startUsingRemoteKernelSpec' ||
controller?.connection.kind === 'connectToLiveRemoteKernel')
) {
const activeInterpreter = this.getActiveInterpreterSync(uri.fsPath);
const activeInterpreter = this.getActiveInterpreterSync(uri);
notebookId = activeInterpreter ? this.getInterpreterIdFromCache(activeInterpreter) : undefined;
}

Expand Down
10 changes: 6 additions & 4 deletions src/intellisense/languageServer.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@ import { createNotebookMiddleware, createPylanceMiddleware, NotebookMiddleware }
import * as uuid from 'uuid/v4';
import { NOTEBOOK_SELECTOR, PYTHON_LANGUAGE } from '../platform/common/constants';
import { traceInfo } from '../platform/logging';
import { getInterpreterId } from '../platform/pythonEnvironments/info/interpreter.node';
import { getInterpreterId } from '../platform/pythonEnvironments/info/interpreter';
import { noop } from '../platform/common/utils/misc';
import { sleep } from '../platform/common/utils/async';
import { PythonEnvironment } from '../platform/pythonEnvironments/info';
import { getFilePath } from '../platform/common/platform/fs-paths';

// eslint-disable-next-line @typescript-eslint/no-explicit-any
function ensure(target: any, key: string) {
Expand Down Expand Up @@ -134,14 +135,14 @@ export class LanguageServer implements Disposable {
() => languageClient,
() => noop, // Don't trace output. Slows things down too much
NOTEBOOK_SELECTOR,
interpreter.uri.fsPath || interpreter.uri.path,
getFilePath(interpreter.uri),
(uri) => shouldAllowIntellisense(uri, interpreterId, interpreter.uri),
getNotebookHeader
)
: createPylanceMiddleware(
() => languageClient,
NOTEBOOK_SELECTOR,
interpreter.uri.fsPath || interpreter.uri.path,
getFilePath(interpreter.uri),
(uri) => shouldAllowIntellisense(uri, interpreterId, interpreter.uri),
getNotebookHeader
);
Expand Down Expand Up @@ -227,8 +228,9 @@ export class LanguageServer implements Disposable {
if (python) {
const runJediPath = path.join(python.extensionPath, 'pythonFiles', 'run-jedi-language-server.py');
if (await fs.pathExists(runJediPath)) {
const interpreterPath = getFilePath(interpreter.uri);
const serverOptions: ServerOptions = {
command: interpreter.uri.fsPath || 'python',
command: interpreterPath.length > 0 ? interpreterPath : 'python',
args: [runJediPath]
};
return serverOptions;
Expand Down
4 changes: 2 additions & 2 deletions src/intellisense/pythonKernelCompletionProvider.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ import {
} from 'vscode';
import * as lsp from 'vscode-languageclient';
import { IVSCodeNotebook } from '../platform/common/application/types';
import { createPromiseFromCancellation } from '../platform/common/cancellation.node';
import { createPromiseFromCancellation } from '../platform/common/cancellation';
import { traceError, traceInfoIfCI, traceVerbose } from '../platform/logging';
import { getDisplayPath } from '../platform/common/platform/fs-paths';
import { IConfigurationService, IDisposableRegistry } from '../platform/common/types';
import { waitForPromise } from '../platform/common/utils/async';
import { isNotebookCell } from '../platform/common/utils/misc';
import { StopWatch } from '../platform/common/utils/stopWatch';
import { IJupyterSession, IKernelProvider } from '../kernels/types';
import { findAssociatedNotebookDocument, getAssociatedJupyterNotebook } from '../notebooks/helpers.node';
import { findAssociatedNotebookDocument, getAssociatedJupyterNotebook } from '../notebooks/helpers';
import { INotebookLanguageClientProvider } from '../notebooks/types';
import { mapJupyterKind } from './conversion.node';
import { IInteractiveWindowProvider } from '../interactive-window/types';
Expand Down
Loading