Skip to content

Single experiment group for nb editors #12301

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.

Sign up for GitHub

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

Merged
DonJayamanne merged 3 commits into from
Jun 12, 2020
Merged

Single experiment group for nb editors #12301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
835016d
Single experiment group for nb editors
DonJayamanne Jun 11, 2020
e4c6036
Fixed tests
DonJayamanne Jun 11, 2020
59067a0
Oops
DonJayamanne Jun 12, 2020
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
19 changes: 6 additions & 13 deletions experiments.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,18 +113,6 @@
"min": 0,
"max": 100
},
{
"name": "NativeNotebook - experiment",
"salt": "NativeNotebook",
"max": 0,
"min": 0
},
{
"name": "NativeNotebook - control",
"salt": "NativeNotebook",
"min": 0,
"max": 100
},
{
"name": "CollectLSRequestTiming - experiment",
"salt": "CollectLSRequestTiming",
Expand Down Expand Up @@ -196,6 +184,11 @@
"salt": "CustomEditorSupport",
"max": 0,
"min": 0
},
{
"name": "NativeNotebook - experiment",
"salt": "CustomEditorSupport",
"max": 0,
"min": 0
}

]
2 changes: 2 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1674,6 +1674,7 @@
"AA_testing - experiment",
"LocalZMQKernel - experiment",
"NativeNotebook - experiment",
"CustomEditorSupport - experiment",
"UseTerminalToGetActivatedEnvVars - experiment",
"CollectLSRequestTiming - experiment",
"CollectNodeLSRequestTiming - experiment",
Expand All @@ -1699,6 +1700,7 @@
"AA_testing - experiment",
"LocalZMQKernel - experiment",
"NativeNotebook - experiment",
"CustomEditorSupport - experiment",
"UseTerminalToGetActivatedEnvVars - experiment",
"CollectLSRequestTiming - experiment",
"CollectNodeLSRequestTiming - experiment",
Expand Down
4 changes: 2 additions & 2 deletions src/client/common/application/notebook.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import type {
NotebookOutputSelector
} from 'vscode-proposed';
import { UseProposedApi } from '../constants';
import { NativeNotebook } from '../experiments/groups';
import { NotebookEditorSupport } from '../experiments/groups';
import { IDisposableRegistry, IExperimentsManager } from '../types';
import {
IVSCodeNotebook,
Expand Down Expand Up @@ -66,7 +66,7 @@ export class VSCodeNotebook implements IVSCodeNotebook {
@inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry,
@inject(IExperimentsManager) readonly experimentManager: IExperimentsManager
) {
if (this.useProposedApi && experimentManager.inExperiment(NativeNotebook.experiment)) {
if (this.useProposedApi && experimentManager.inExperiment(NotebookEditorSupport.nativeNotebookExperiment)) {
this.addEventHandlers();
}
}
Expand Down
13 changes: 4 additions & 9 deletions src/client/common/experiments/groups.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,6 @@ export enum LocalZMQKernel {
experiment = 'LocalZMQKernel - experiment'
}

// Experiment to use VSC Notebook Implementation
export enum NativeNotebook {
control = 'NativeNotebook - control',
experiment = 'NativeNotebook - experiment'
}

// Experiment for supporting run by line in data science notebooks
export enum RunByLine {
control = 'RunByLine - control',
Expand Down Expand Up @@ -93,9 +87,10 @@ export enum DeprecatePythonPath {
}

/*
* Experiment to turn on custom editor API support.
* Experiment to turn on custom editor or VS Code Native Notebook API support.
*/
export enum CustomEditorSupport {
export enum NotebookEditorSupport {
control = 'CustomEditorSupport - control',
experiment = 'CustomEditorSupport - experiment'
customEditorExperiment = 'CustomEditorSupport - experiment',
nativeNotebookExperiment = 'NativeNotebook - experiment'
}
4 changes: 2 additions & 2 deletions src/client/common/experiments/manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
import { sleep } from '../utils/async';
import { swallowExceptions } from '../utils/decorators';
import { Experiments } from '../utils/localize';
import { NativeNotebook } from './groups';
import { NotebookEditorSupport } from './groups';

const EXPIRY_DURATION_MS = 30 * 60 * 1000;
export const isDownloadedStorageValidKey = 'IS_EXPERIMENTS_STORAGE_VALID_KEY';
Expand Down Expand Up @@ -158,7 +158,7 @@ export class ExperimentsManager implements IExperimentsManager {
for (const experiment of this.experimentStorage.value) {
// User cannot belong to NotebookExperiment if they are not using Insiders.
if (
(experiment.name === NativeNotebook.experiment || experiment.name === NativeNotebook.control) &&
experiment.name === NotebookEditorSupport.nativeNotebookExperiment &&
this.appEnvironment.channel === 'stable'
) {
continue;
Expand Down
6 changes: 3 additions & 3 deletions src/client/datascience/context/activeEditorContext.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { IExtensionSingleActivationService } from '../../activation/types';
import { ICommandManager, IDocumentManager, IVSCodeNotebook } from '../../common/application/types';
import { PYTHON_LANGUAGE } from '../../common/constants';
import { ContextKey } from '../../common/contextKey';
import { NativeNotebook } from '../../common/experiments/groups';
import { NotebookEditorSupport } from '../../common/experiments/groups';
import { IDisposable, IDisposableRegistry, IExperimentsManager } from '../../common/types';
import { EditorContexts } from '../constants';
import { IInteractiveWindow, IInteractiveWindowProvider, INotebookEditor, INotebookEditorProvider } from '../types';
Expand Down Expand Up @@ -72,7 +72,7 @@ export class ActiveEditorContextService implements IExtensionSingleActivationSer
if (this.docManager.activeTextEditor?.document.languageId === PYTHON_LANGUAGE) {
this.onDidChangeActiveTextEditor(this.docManager.activeTextEditor);
}
if (this.experiments.inExperiment(NativeNotebook.experiment)) {
if (this.experiments.inExperiment(NotebookEditorSupport.nativeNotebookExperiment)) {
this.vscodeNotebook.onDidChangeNotebookDocument(this.onDidChangeVSCodeNotebook, this, this.disposables);
this.vscodeNotebook.onDidCloseNotebookDocument(this.onDidChangeVSCodeNotebook, this, this.disposables);
this.vscodeNotebook.onDidOpenNotebookDocument(this.onDidChangeVSCodeNotebook, this, this.disposables);
Expand All @@ -81,7 +81,7 @@ export class ActiveEditorContextService implements IExtensionSingleActivationSer
}

private udpateNativeNotebookCellContext() {
if (!this.experiments.inExperiment(NativeNotebook.experiment)) {
if (!this.experiments.inExperiment(NotebookEditorSupport.nativeNotebookExperiment)) {
return;
}
this.hasNativeNotebookCells
Expand Down
4 changes: 2 additions & 2 deletions src/client/datascience/notebook/cellEditSyncService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type { NotebookCell, NotebookDocument } from '../../../../typings/vscode-
import { splitMultilineString } from '../../../datascience-ui/common';
import { IExtensionSingleActivationService } from '../../activation/types';
import { IDocumentManager, IVSCodeNotebook } from '../../common/application/types';
import { NativeNotebook } from '../../common/experiments/groups';
import { NotebookEditorSupport } from '../../common/experiments/groups';
import { IDisposable, IDisposableRegistry, IExperimentsManager } from '../../common/types';
import { isNotebookCell } from '../../common/utils/misc';
import { traceError } from '../../logging';
Expand All @@ -33,7 +33,7 @@ export class CellEditSyncService implements IExtensionSingleActivationService, I
}
}
public async activate(): Promise<void> {
if (!this.experiment.inExperiment(NativeNotebook.experiment)) {
if (!this.experiment.inExperiment(NotebookEditorSupport.nativeNotebookExperiment)) {
return;
}
this.documentManager.onDidChangeTextDocument(this.onDidChangeTextDocument, this, this.disposables);
Expand Down
4 changes: 2 additions & 2 deletions src/client/datascience/notebook/integration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { inject, injectable } from 'inversify';
import * as path from 'path';
import { IExtensionSingleActivationService } from '../../activation/types';
import { ICommandManager, IVSCodeNotebook } from '../../common/application/types';
import { NativeNotebook } from '../../common/experiments/groups';
import { NotebookEditorSupport } from '../../common/experiments/groups';
import { IFileSystem } from '../../common/platform/types';
import { IDisposableRegistry, IExperimentsManager, IExtensionContext } from '../../common/types';
import { noop } from '../../common/utils/misc';
Expand Down Expand Up @@ -35,7 +35,7 @@ export class NotebookIntegration implements IExtensionSingleActivationService {
// This condition is temporary.
// If user belongs to the experiment, then make the necessary changes to package.json.
// Once the API is final, we won't need to modify the package.json.
if (!this.experiment.inExperiment(NativeNotebook.experiment)) {
if (!this.experiment.inExperiment(NotebookEditorSupport.nativeNotebookExperiment)) {
return;
}

Expand Down
6 changes: 3 additions & 3 deletions src/client/datascience/serviceRegistry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
'use strict';
import { IExtensionSingleActivationService } from '../activation/types';
import { UseCustomEditorApi } from '../common/constants';
import { CustomEditorSupport, NativeNotebook } from '../common/experiments/groups';
import { NotebookEditorSupport } from '../common/experiments/groups';
import { StartPage } from '../common/startPage/startPage';
import { IStartPage } from '../common/startPage/types';
import { IExperimentsManager } from '../common/types';
Expand Down Expand Up @@ -160,8 +160,8 @@ import {
// tslint:disable-next-line: max-func-body-length
export function registerTypes(serviceManager: IServiceManager) {
const experiments = serviceManager.get<IExperimentsManager>(IExperimentsManager);
const useVSCodeNotebookAPI = experiments.inExperiment(NativeNotebook.experiment);
const inCustomEditorApiExperiment = experiments.inExperiment(CustomEditorSupport.experiment);
const useVSCodeNotebookAPI = experiments.inExperiment(NotebookEditorSupport.nativeNotebookExperiment);
const inCustomEditorApiExperiment = experiments.inExperiment(NotebookEditorSupport.customEditorExperiment);
const usingCustomEditor = inCustomEditorApiExperiment;
serviceManager.addSingletonInstance<boolean>(UseCustomEditorApi, usingCustomEditor);

Expand Down
20 changes: 11 additions & 9 deletions src/test/common/experiments/manager.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { IApplicationEnvironment } from '../../../client/common/application/type
import { PythonSettings } from '../../../client/common/configSettings';
import { ConfigurationService } from '../../../client/common/configuration/service';
import { CryptoUtils } from '../../../client/common/crypto';
import { NativeNotebook } from '../../../client/common/experiments/groups';
import { NotebookEditorSupport } from '../../../client/common/experiments/groups';
import {
configUri,
downloadedExperimentStorageKey,
Expand Down Expand Up @@ -895,8 +895,9 @@ suite('A/B experiments', () => {
});
test('NativeNotebook Experiment are not loaded in VSC Insiders', async () => {
const storageValue = [
{ name: NativeNotebook.control, salt: 'salt', min: 79, max: 94 },
{ name: NativeNotebook.experiment, salt: 'salt', min: 19, max: 30 }
{ name: NotebookEditorSupport.control, salt: 'salt', min: 0, max: 0 },
{ name: NotebookEditorSupport.customEditorExperiment, salt: 'salt', min: 0, max: 0 },
{ name: NotebookEditorSupport.nativeNotebookExperiment, salt: 'salt', min: 0, max: 100 }
];
experimentStorage.setup((n) => n.value).returns(() => storageValue);
when(appEnvironment.machineId).thenReturn('101');
Expand All @@ -905,10 +906,11 @@ suite('A/B experiments', () => {
expManager.populateUserExperiments();
assert.deepEqual(expManager.userExperiments, []);
});
test('NativeNotebook Experiment are loaded in VSC Insiders', async () => {
test('NativeNotebook Experiment is loaded in VSC Insiders', async () => {
const storageValue = [
{ name: NativeNotebook.control, salt: 'salt', min: 79, max: 94 },
{ name: NativeNotebook.experiment, salt: 'salt', min: 19, max: 30 }
{ name: NotebookEditorSupport.control, salt: 'salt', min: 0, max: 0 },
{ name: NotebookEditorSupport.customEditorExperiment, salt: 'salt', min: 0, max: 0 },
{ name: NotebookEditorSupport.nativeNotebookExperiment, salt: 'salt', min: 0, max: 100 }
];
experimentStorage.setup((n) => n.value).returns(() => storageValue);
when(appEnvironment.machineId).thenReturn('101');
Expand All @@ -917,9 +919,9 @@ suite('A/B experiments', () => {
expManager.populateUserExperiments();
assert.deepEqual(expManager.userExperiments, [
{
max: 94,
min: 79,
name: 'NativeNotebook - control',
min: 0,
max: 100,
name: 'NativeNotebook - experiment',
salt: 'salt'
}
]);
Expand Down