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

Changes to storage format #13752

Merged
merged 6 commits into from
Jun 22, 2023
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
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions src/kernels/jupyter/connection/serverUriStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ export class JupyterServerUriStorage extends Disposables implements IJupyterServ
}
public async get(id: string): Promise<IJupyterServerUriEntry | undefined> {
this.hookupStorageEvents();
await this.newStorage.migrateMRU();
const savedList = await this.getAll();
return savedList.find((item) => item.serverId === id);
}
Expand All @@ -119,6 +120,7 @@ export class JupyterServerUriStorage extends Disposables implements IJupyterServ
options?: { time: number; displayName: string }
): Promise<void> {
this.hookupStorageEvents();
await this.newStorage.migrateMRU();
traceInfoIfCI(`setUri: ${jupyterHandle.id}.${jupyterHandle.handle}`);
const uri = generateUriFromRemoteProvider(jupyterHandle.id, jupyterHandle.handle);
const serverId = await computeServerId(uri);
Expand All @@ -142,10 +144,12 @@ export class JupyterServerUriStorage extends Disposables implements IJupyterServ
}
public async update(serverId: string) {
this.hookupStorageEvents();
await this.newStorage.migrateMRU();
await Promise.all([this.newStorage.update(serverId), this.oldStorage.update(serverId)]);
}
public async remove(serverId: string) {
this.hookupStorageEvents();
await this.newStorage.migrateMRU();
await Promise.all([this.newStorage.remove(serverId), this.oldStorage.remove(serverId)]);
}
}
Expand Down Expand Up @@ -280,7 +284,7 @@ class OldStorage {
}
public async getAll(): Promise<IJupyterServerUriEntry[]> {
if (this.lastSavedList) {
return this.lastSavedList;
return this.lastSavedList.then((items) => items.sort((a, b) => b.time - a.time));
}
const promise = async () => {
// List is in the global memento, URIs are in encrypted storage
Expand All @@ -305,7 +309,7 @@ class OldStorage {
return result.filter((item) => !!item) as IJupyterServerUriEntry[];
};
this.lastSavedList = promise();
return this.lastSavedList;
return this.lastSavedList.then((items) => items.sort((a, b) => b.time - a.time));
}
public async getAllRaw(): Promise<IJupyterServerUriEntry[]> {
// List is in the global memento, URIs are in encrypted storage
Expand Down Expand Up @@ -534,7 +538,7 @@ class NewStorage {
.catch(noop));
}
public async getAll(): Promise<IJupyterServerUriEntry[]> {
return this.getAllImpl(true);
return this.getAllImpl(true).then((items) => items.sort((a, b) => b.time - a.time));
}
public async clear(): Promise<void> {
const all = await this.getAllImpl(false);
Expand Down Expand Up @@ -577,6 +581,9 @@ class NewStorage {
return entries;
}
private async getAllRaw(): Promise<StorageMRUItem[]> {
if (!(await this.fs.exists(this.storageFile))) {
return [];
}
const json = await this.fs.readFile(this.storageFile);
return JSON.parse(json) as StorageMRUItem[];
}
Expand Down
2 changes: 2 additions & 0 deletions src/kernels/jupyter/connection/serverUriStorage.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ suite('Server Uri Storage', async () => {
const itemsInNewStorage: StorageMRUItem[] = [];
when(fs.writeFile(anything(), anything())).thenCall((_, data) => {
itemsInNewStorage.push(...JSON.parse(data.toString()));
when(fs.exists(anything())).thenResolve(true);
return Promise.resolve();
});
when(fs.readFile(anything())).thenCall(() => JSON.stringify(itemsInNewStorage));
Expand Down Expand Up @@ -122,6 +123,7 @@ suite('Server Uri Storage', async () => {
const itemsInNewStorage: StorageMRUItem[] = [];
when(fs.writeFile(anything(), anything())).thenCall((_, data) => {
itemsInNewStorage.push(...JSON.parse(data.toString()));
when(fs.exists(anything())).thenResolve(true);
return Promise.resolve();
});
when(fs.readFile(anything())).thenCall(() => JSON.stringify(itemsInNewStorage));
Expand Down
3 changes: 2 additions & 1 deletion src/platform/common/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ export class OldCacheCleaner implements IExtensionSyncActivationService {
'JUPYTER_REMOTE_KERNELSPECS_V3',
'JUPYTER_LOCAL_KERNELSPECS_V4',
'LOCAL_KERNEL_SPECS_CACHE_KEY_V_2022_10',
'LOCAL_KERNEL_PYTHON_AND_RELATED_SPECS_CACHE_KEY_V_2022_10'
'LOCAL_KERNEL_PYTHON_AND_RELATED_SPECS_CACHE_KEY_V_2022_10',
'user-jupyter-server-uri-list-v2'
]
.filter((key) => this.globalState.get(key, undefined) !== undefined)
.map((key) => this.globalState.update(key, undefined).then(noop, noop))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ import { IJupyterPasswordConnectInfo, JupyterPasswordConnect } from './jupyterPa
suite('User Uri Provider', () => {
['Old Password Manager', 'New Password Manager'].forEach((passwordManager) => {
['Old Storage Format', 'New Storage Format'].forEach((storageFormat) => {
suite(storageFormat, () => {
suite(`${passwordManager} - ${storageFormat}`, () => {
const isNewPasswordManager = passwordManager === 'New Password Manager';
const isNewStorageFormat = storageFormat === 'New Storage Format';
let provider: UserJupyterServerUrlProvider;
Expand Down Expand Up @@ -159,6 +159,13 @@ suite('User Uri Provider', () => {
return Promise.resolve();
});

when(
encryptedStorage.store(
Settings.JupyterServerRemoteLaunchService,
'user-jupyter-server-uri-list-v2',
anything()
)
).thenResolve();
when(
encryptedStorage.retrieve(
Settings.JupyterServerRemoteLaunchService,
Expand Down
49 changes: 44 additions & 5 deletions src/standalone/userJupyterServer/userServerUrlProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ import { validateSelectJupyterURI } from '../../kernels/jupyter/connection/serve
import { Deferred, createDeferred } from '../../platform/common/utils/async';

export const UserJupyterServerUriListKey = 'user-jupyter-server-uri-list';
export const UserJupyterServerUriListKeyV2 = 'user-jupyter-server-uri-list-v2';
export const UserJupyterServerUriListKeyV2 = 'user-jupyter-server-uri-list-version2';
export const UserJupyterServerUriListMementoKey = '_builtin.jupyterServerUrlProvider.uriList';
const GlobalStateUserAllowsInsecureConnections = 'DataScienceAllowInsecureConnections';

Expand Down Expand Up @@ -696,6 +696,39 @@ export class OldStorage {
}
}

type StorageItem = {
handle: string;
uri: string;
};
function serverToStorageFormat(
servers: {
handle: string;
uri: string;
serverInfo: IJupyterServerUri;
}[]
): StorageItem[] {
return servers.map((s) => ({ handle: s.handle, uri: s.uri }));
}
function storageFormatToServers(items: StorageItem[]) {
const servers: {
handle: string;
uri: string;
serverInfo: IJupyterServerUri;
}[] = [];
items.forEach((s) => {
const server = parseUri(s.uri);
if (!server) {
return;
}
servers.push({
handle: s.handle,
uri: s.uri,
serverInfo: server
});
});
return servers;
}

export class NewStorage {
private readonly _migrationDone: Deferred<void>;
private updatePromise = Promise.resolve();
Expand All @@ -721,7 +754,13 @@ export class NewStorage {
// Already migrated once before.
return this._migrationDone.resolve();
}

this.encryptedStorage
.store(
Settings.JupyterServerRemoteLaunchService,
'user-jupyter-server-uri-list-v2', // Removed as this storage data is not in the best format.
undefined
)
.catch(noop);
await this.encryptedStorage.store(
Settings.JupyterServerRemoteLaunchService,
UserJupyterServerUriListKeyV2,
Expand All @@ -738,7 +777,7 @@ export class NewStorage {
return [];
}
try {
return JSON.parse(data);
return storageFormatToServers(JSON.parse(data));
} catch {
return [];
}
Expand All @@ -751,7 +790,7 @@ export class NewStorage {
await this.encryptedStorage.store(
Settings.JupyterServerRemoteLaunchService,
UserJupyterServerUriListKeyV2,
JSON.stringify(servers)
JSON.stringify(serverToStorageFormat(servers))
);
})
.catch(noop));
Expand All @@ -763,7 +802,7 @@ export class NewStorage {
return this.encryptedStorage.store(
Settings.JupyterServerRemoteLaunchService,
UserJupyterServerUriListKeyV2,
JSON.stringify(servers)
JSON.stringify(serverToStorageFormat(servers))
);
})
.catch(noop));
Expand Down