Remove DebugAdapterNewPtvsd and DebugAdapterDescriptorFactory experim…

…ent. (#11769) * Remove DebugAdapterNewPtvsd and DebugAdapterDescriptorFactory experiment. * Fix tests * Remove experiments from json files
microsoft · Jun 18, 2020 · 650e4a9 · 650e4a9
1 parent 474c49d
commit 650e4a9
Show file tree

Hide file tree

Showing 15 changed files with 87 additions and 256 deletions.
diff --git a/experiments.json b/experiments.json
@@ -29,30 +29,6 @@
         "min": 0,
         "max": 20
     },
-    {
-        "name": "DebugAdapterFactory - control",
-        "salt": "DebugAdapterFactory",
-        "min": 0,
-        "max": 0
-    },
-    {
-        "name": "DebugAdapterFactory - experiment",
-        "salt": "DebugAdapterFactory",
-        "min": 0,
-        "max": 100
-    },
-    {
-        "name": "PtvsdWheels37 - control",
-        "salt": "DebugAdapterFactory",
-        "min": 0,
-        "max": 0
-    },
-    {
-        "name": "PtvsdWheels37 - experiment",
-        "salt": "DebugAdapterFactory",
-        "min": 0,
-        "max": 100
-    },
     {
         "name": "Reload - control",
         "salt": "DebugAdapterFactory",

diff --git a/package.json b/package.json
@@ -1732,8 +1732,6 @@
                             "LS - enabled",
                             "AlwaysDisplayTestExplorer - experiment",
                             "ShowExtensionSurveyPrompt - enabled",
-                            "DebugAdapterFactory - experiment",
-                            "PtvsdWheels37 - experiment",
                             "Reload - experiment",
                             "AA_testing - experiment",
                             "LocalZMQKernel - experiment",
@@ -1758,8 +1756,6 @@
                             "LS - enabled",
                             "AlwaysDisplayTestExplorer - experiment",
                             "ShowExtensionSurveyPrompt - enabled",
-                            "DebugAdapterFactory - experiment",
-                            "PtvsdWheels37 - experiment",
                             "Reload - experiment",
                             "AA_testing - experiment",
                             "LocalZMQKernel - experiment",

diff --git a/package.nls.json b/package.nls.json
@@ -271,7 +271,6 @@
     "diagnostics.consoleTypeDiagnostic": "Your launch.json file needs to be updated to change the console type string from \"none\" to \"internalConsole\", otherwise Python debugging may not work. Would you like to automatically update your launch.json file now?",
     "diagnostics.justMyCodeDiagnostic": "Configuration \"debugStdLib\" in launch.json is no longer supported. It's recommended to replace it with \"justMyCode\", which is the exact opposite of using \"debugStdLib\". Would you like to automatically update your launch.json file to do that?",
     "diagnostics.yesUpdateLaunch": "Yes, update launch.json",
-    "diagnostics.processId": "Attaching the debugger to a local process is an experimental feature. It will be available to all users soon.",
     "diagnostics.invalidTestSettings": "Your settings needs to be updated to change the setting \"python.unitTest.\" to \"python.testing.\", otherwise testing Python code using the extension may not work. Would you like to automatically update your settings now?",
     "DataScience.interruptKernel": "Interrupt IPython kernel",
     "DataScience.clearAllOutput": "Clear All Output",

diff --git a/package.nls.zh-tw.json b/package.nls.zh-tw.json
@@ -92,7 +92,6 @@
     "diagnostics.consoleTypeDiagnostic": "Your launch.json file needs to be updated to change the console type string from \"none\" to \"internalConsole\", otherwise Python debugging may not work. Would you like to automatically update your launch.json file now?",
     "diagnostics.justMyCodeDiagnostic": "Configuration \"debugStdLib\" in launch.json is no longer supported. It's recommended to replace it with \"justMyCode\", which is the exact opposite of using \"debugStdLib\". Would you like to automatically update your launch.json file to do that?",
     "diagnostics.yesUpdateLaunch": "是，更新 launch.json",
-    "diagnostics.processId": "Attaching the debugger to a local process is an experimental feature. It will be available to all users soon.",
     "diagnostics.invalidTestSettings": "Your settings needs to be updated to change the setting \"python.unitTest.\" to \"python.testing.\", otherwise testing Python code using the extension may not work. Would you like to automatically update your settings now?",
     "Common.canceled": "已取消",
     "Common.cancel": "取消",

diff --git a/src/client/common/experiments/groups.ts b/src/client/common/experiments/groups.ts
@@ -13,18 +13,6 @@ export enum ShowExtensionSurveyPrompt {
     enabled = 'ShowExtensionSurveyPrompt - enabled'
 }
 
-// Experiment to check whether the extension should use the new VS Code debug adapter API.
-export enum DebugAdapterDescriptorFactory {
-    control = 'DebugAdapterFactory - control',
-    experiment = 'DebugAdapterFactory - experiment'
-}
-
-// Experiment to check whether the ptvsd launcher should use pre-installed ptvsd wheels for debugging.
-export enum DebugAdapterNewPtvsd {
-    control = 'PtvsdWheels37 - control',
-    experiment = 'PtvsdWheels37 - experiment'
-}
-
 // Experiment to check whether to enable re-load for web apps while debugging.
 export enum WebAppReload {
     control = 'Reload - control',

diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts
@@ -68,10 +68,6 @@ export namespace Diagnostics {
         'Your settings needs to be updated to change the setting "python.unitTest." to "python.testing.", otherwise testing Python code using the extension may not work. Would you like to automatically update your settings now?'
     );
     export const updateSettings = localize('diagnostics.updateSettings', 'Yes, update settings');
-    export const processId = localize(
-        'diagnostics.processId',
-        'Attaching the debugger to a local process is an experimental feature. It will be available to all users soon.'
-    );
 }
 
 export namespace Common {

diff --git a/src/client/datascience/jupyter/jupyterDebugger.ts b/src/client/datascience/jupyter/jupyterDebugger.ts
@@ -10,10 +10,9 @@ import * as vsls from 'vsls/vscode';
 import { concatMultilineStringOutput } from '../../../datascience-ui/common';
 import { ServerStatus } from '../../../datascience-ui/interactive-common/mainState';
 import { IApplicationShell } from '../../common/application/types';
-import { DebugAdapterNewPtvsd } from '../../common/experiments/groups';
 import { traceError, traceInfo, traceWarning } from '../../common/logger';
 import { IPlatformService } from '../../common/platform/types';
-import { IConfigurationService, IExperimentsManager, Version } from '../../common/types';
+import { IConfigurationService, Version } from '../../common/types';
 import * as localize from '../../common/utils/localize';
 import { EXTENSION_ROOT_DIR } from '../../constants';
 import { captureTelemetry, sendTelemetryEvent } from '../../telemetry';
@@ -52,22 +51,13 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener {
         @inject(IJupyterDebugService)
         @named(Identifiers.MULTIPLEXING_DEBUGSERVICE)
         private debugService: IJupyterDebugService,
-        @inject(IPlatformService) private platform: IPlatformService,
-        @inject(IExperimentsManager) private readonly experimentsManager: IExperimentsManager
+        @inject(IPlatformService) private platform: IPlatformService
     ) {
-        if (this.experimentsManager.inExperiment(DebugAdapterNewPtvsd.experiment)) {
-            this.debuggerPackage = 'debugpy';
-            this.enableDebuggerCode = `import debugpy;debugpy.listen(('localhost', 0))`;
-            this.waitForDebugClientCode = `import debugpy;debugpy.wait_for_client()`;
-            this.tracingEnableCode = `from debugpy import trace_this_thread;trace_this_thread(True)`;
-            this.tracingDisableCode = `from debugpy import trace_this_thread;trace_this_thread(False)`;
-        } else {
-            this.debuggerPackage = 'ptvsd';
-            this.enableDebuggerCode = `import ptvsd;ptvsd.enable_attach(('localhost', 0))`;
-            this.waitForDebugClientCode = `import ptvsd;ptvsd.wait_for_attach()`;
-            this.tracingEnableCode = `from ptvsd import tracing;tracing(True)`;
-            this.tracingDisableCode = `from ptvsd import tracing;tracing(False)`;
-        }
+        this.debuggerPackage = 'debugpy';
+        this.enableDebuggerCode = `import debugpy;debugpy.listen(('localhost', 0))`;
+        this.waitForDebugClientCode = `import debugpy;debugpy.wait_for_client()`;
+        this.tracingEnableCode = `from debugpy import trace_this_thread;trace_this_thread(True)`;
+        this.tracingDisableCode = `from debugpy import trace_this_thread;trace_this_thread(False)`;
     }
 
     public startRunByLine(notebook: INotebook, cellHashFileName: string): Promise<void> {

diff --git a/src/client/debugger/extension/adapter/outdatedDebuggerPrompt.ts b/src/client/debugger/extension/adapter/outdatedDebuggerPrompt.ts
@@ -7,11 +7,12 @@ import { inject, injectable } from 'inversify';
 import { DebugAdapterTracker, DebugAdapterTrackerFactory, DebugSession, ProviderResult } from 'vscode';
 import { DebugProtocol } from 'vscode-debugprotocol';
 import { IApplicationShell } from '../../../common/application/types';
-import { DebugAdapterNewPtvsd } from '../../../common/experiments/groups';
-import { IBrowserService, IExperimentsManager } from '../../../common/types';
+import { IBrowserService } from '../../../common/types';
 import { Common, OutdatedDebugger } from '../../../common/utils/localize';
 import { IPromptShowState } from './types';
 
+// This situation occurs when user connects to old containers or server where
+// the debugger they had installed was ptvsd. We should show a prompt to ask them to update.
 class OutdatedDebuggerPrompt implements DebugAdapterTracker {
     constructor(
         private promptCheck: IPromptShowState,
@@ -71,14 +72,10 @@ class OutdatedDebuggerPromptState implements IPromptShowState {
 export class OutdatedDebuggerPromptFactory implements DebugAdapterTrackerFactory {
     private readonly promptCheck: OutdatedDebuggerPromptState;
     constructor(
-        @inject(IExperimentsManager) private readonly experimentsManager: IExperimentsManager,
         @inject(IApplicationShell) private readonly appShell: IApplicationShell,
         @inject(IBrowserService) private browserService: IBrowserService
     ) {
         this.promptCheck = new OutdatedDebuggerPromptState();
-        if (!this.experimentsManager.inExperiment(DebugAdapterNewPtvsd.experiment)) {
-            this.promptCheck.setShowPrompt(false);
-        }
     }
     public createDebugAdapterTracker(_session: DebugSession): ProviderResult<DebugAdapterTracker> {
         return new OutdatedDebuggerPrompt(this.promptCheck, this.appShell, this.browserService);

diff --git a/src/client/debugger/extension/configuration/resolvers/attach.ts b/src/client/debugger/extension/configuration/resolvers/attach.ts
@@ -6,10 +6,8 @@
 import { inject, injectable } from 'inversify';
 import { CancellationToken, Uri, WorkspaceFolder } from 'vscode';
 import { IDocumentManager, IWorkspaceService } from '../../../../common/application/types';
-import { DebugAdapterNewPtvsd } from '../../../../common/experiments/groups';
 import { IPlatformService } from '../../../../common/platform/types';
-import { IConfigurationService, IExperimentsManager } from '../../../../common/types';
-import { Diagnostics } from '../../../../common/utils/localize';
+import { IConfigurationService } from '../../../../common/types';
 import { AttachRequestArguments, DebugOptions, PathMapping } from '../../../types';
 import { BaseConfigurationResolver } from './base';
 
@@ -19,8 +17,7 @@ export class AttachConfigurationResolver extends BaseConfigurationResolver<Attac
         @inject(IWorkspaceService) workspaceService: IWorkspaceService,
         @inject(IDocumentManager) documentManager: IDocumentManager,
         @inject(IPlatformService) platformService: IPlatformService,
-        @inject(IConfigurationService) configurationService: IConfigurationService,
-        @inject(IExperimentsManager) private readonly experiments: IExperimentsManager
+        @inject(IConfigurationService) configurationService: IConfigurationService
     ) {
         super(workspaceService, documentManager, platformService, configurationService);
     }
@@ -29,12 +26,6 @@ export class AttachConfigurationResolver extends BaseConfigurationResolver<Attac
         debugConfiguration: AttachRequestArguments,
         _token?: CancellationToken
     ): Promise<AttachRequestArguments | undefined> {
-        if (
-            !this.experiments.inExperiment(DebugAdapterNewPtvsd.experiment) &&
-            debugConfiguration.processId !== undefined
-        ) {
-            throw Error(Diagnostics.processId());
-        }
         const workspaceFolder = this.getWorkspaceFolder(folder);
 
         await this.provideAttachDefaults(workspaceFolder, debugConfiguration as AttachRequestArguments);

diff --git a/src/client/debugger/extension/configuration/resolvers/launchConfigExperiment.ts b/src/client/debugger/extension/configuration/resolvers/launchConfigExperiment.ts
@@ -4,7 +4,7 @@
 'use strict';
 
 import { inject, injectable } from 'inversify';
-import { DebugAdapterNewPtvsd, WebAppReload } from '../../../../common/experiments/groups';
+import { WebAppReload } from '../../../../common/experiments/groups';
 import { traceInfo } from '../../../../common/logger';
 import { IExperimentsManager } from '../../../../common/types';
 import { sendTelemetryEvent } from '../../../../telemetry';
@@ -17,45 +17,43 @@ export class LaunchDebugConfigurationExperiment implements ILaunchDebugConfigura
     constructor(@inject(IExperimentsManager) private readonly experimentsManager: IExperimentsManager) {}
 
     public modifyConfigurationBasedOnExperiment(debugConfiguration: LaunchRequestArguments): void {
-        if (this.experimentsManager.inExperiment(DebugAdapterNewPtvsd.experiment)) {
-            if (this.experimentsManager.inExperiment(WebAppReload.experiment)) {
-                if (this.isWebAppConfiguration(debugConfiguration)) {
-                    traceInfo(
-                        `Configuration used for Web App Reload experiment (before):\n${JSON.stringify(
-                            debugConfiguration,
-                            undefined,
-                            4
-                        )}`
-                    );
+        if (this.experimentsManager.inExperiment(WebAppReload.experiment)) {
+            if (this.isWebAppConfiguration(debugConfiguration)) {
+                traceInfo(
+                    `Configuration used for Web App Reload experiment (before):\n${JSON.stringify(
+                        debugConfiguration,
+                        undefined,
+                        4
+                    )}`
+                );
 
-                    let subProcessModified: boolean = false;
-                    if (!debugConfiguration.subProcess) {
-                        subProcessModified = true;
-                        debugConfiguration.subProcess = true;
-                    }
-
-                    let argsModified: boolean = false;
-                    const args = debugConfiguration.args.filter((arg) => arg !== '--noreload' && arg !== '--no-reload');
-                    if (args.length !== debugConfiguration.args.length) {
-                        argsModified = true;
-                        debugConfiguration.args = args;
-                    }
+                let subProcessModified: boolean = false;
+                if (!debugConfiguration.subProcess) {
+                    subProcessModified = true;
+                    debugConfiguration.subProcess = true;
+                }
 
-                    traceInfo(
-                        `Configuration used for Web App Reload experiment (after):\n${JSON.stringify(
-                            debugConfiguration,
-                            undefined,
-                            4
-                        )}`
-                    );
-                    sendTelemetryEvent(EventName.PYTHON_WEB_APP_RELOAD, undefined, {
-                        subProcessModified,
-                        argsModified
-                    });
+                let argsModified: boolean = false;
+                const args = debugConfiguration.args.filter((arg) => arg !== '--noreload' && arg !== '--no-reload');
+                if (args.length !== debugConfiguration.args.length) {
+                    argsModified = true;
+                    debugConfiguration.args = args;
                 }
-            } else {
-                this.experimentsManager.sendTelemetryIfInExperiment(WebAppReload.control);
+
+                traceInfo(
+                    `Configuration used for Web App Reload experiment (after):\n${JSON.stringify(
+                        debugConfiguration,
+                        undefined,
+                        4
+                    )}`
+                );
+                sendTelemetryEvent(EventName.PYTHON_WEB_APP_RELOAD, undefined, {
+                    subProcessModified,
+                    argsModified
+                });
             }
+        } else {
+            this.experimentsManager.sendTelemetryIfInExperiment(WebAppReload.control);
         }
     }
 

diff --git a/src/test/debugger/attach.ptvsd.test.ts b/src/test/debugger/attach.ptvsd.test.ts
@@ -13,10 +13,9 @@ import { DebugClient } from 'vscode-debugadapter-testsupport';
 
 import { IDocumentManager, IWorkspaceService } from '../../client/common/application/types';
 import { EXTENSION_ROOT_DIR } from '../../client/common/constants';
-import { DebugAdapterNewPtvsd } from '../../client/common/experiments/groups';
 import { IS_WINDOWS } from '../../client/common/platform/constants';
 import { IPlatformService } from '../../client/common/platform/types';
-import { IConfigurationService, IExperimentsManager } from '../../client/common/types';
+import { IConfigurationService } from '../../client/common/types';
 import { MultiStepInputFactory } from '../../client/common/utils/multiStepInput';
 import { DebuggerTypeName, PTVSD_PATH } from '../../client/debugger/constants';
 import { PythonDebugConfigurationService } from '../../client/debugger/extension/configuration/debugConfigurationService';
@@ -116,16 +115,13 @@ suite('Debugging - Attach Debugger', () => {
         const workspaceService = TypeMoq.Mock.ofType<IWorkspaceService>();
         const documentManager = TypeMoq.Mock.ofType<IDocumentManager>();
         const configurationService = TypeMoq.Mock.ofType<IConfigurationService>();
-        const experiments = TypeMoq.Mock.ofType<IExperimentsManager>();
-        experiments.setup((e) => e.inExperiment(DebugAdapterNewPtvsd.experiment)).returns(() => true);
 
         const launchResolver = TypeMoq.Mock.ofType<IDebugConfigurationResolver<LaunchRequestArguments>>();
         const attachResolver = new AttachConfigurationResolver(
             workspaceService.object,
             documentManager.object,
             platformService.object,
-            configurationService.object,
-            experiments.object
+            configurationService.object
         );
         const providerFactory = TypeMoq.Mock.ofType<IDebugConfigurationProviderFactory>().object;
         const multistepFactory = mock(MultiStepInputFactory);