Skip to content

Commit d788d18

Browse files
DonJayamanneDonJayamanne
authored
Use a terminal to get environment variables of an activated interpreter (microsoft#9744)
* Use terminal to get activated env vars * News entry * Added telemetry * Fix tests * Rethrow errors from terminal to calling code
1 parent e6707d6 commit d788d18

37 files changed

+1232
-306
lines changed

‎experiments.json‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,18 @@
7878
"min": 20,
7979
"max": 24
8080
},
81+
{
82+
"name": "UseTerminalToGetActivatedEnvVars - enabled",
83+
"salt": "UseTerminalToGetActivatedEnvVars",
84+
"min": 0,
85+
"max": 20
86+
},
87+
{
88+
"name": "UseTerminalToGetActivatedEnvVars - control",
89+
"salt": "UseTerminalToGetActivatedEnvVars",
90+
"min": 20,
91+
"max": 100
92+
},
8193
{
8294
"name": "AA_testing - control",
8395
"salt": "AA_testing",

‎news/3 Code Health/8928.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Use a hidden terminal to retrieve environment variables of an actived Python Interpreter.

‎package.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1465,6 +1465,7 @@
14651465
"Reload - experiment",
14661466
"AA_testing - experiment",
14671467
"WebHostNotebook - experiment",
1468+
"UseTerminalToGetActivatedEnvVars - experiment",
14681469
"All"
14691470
]
14701471
},
@@ -1484,6 +1485,7 @@
14841485
"Reload - experiment",
14851486
"AA_testing - experiment",
14861487
"WebHostNotebook - experiment",
1488+
"UseTerminalToGetActivatedEnvVars - experiment",
14871489
"All"
14881490
]
14891491
},

‎pythonFiles/printEnvVariablesToFile.py‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
# Copyright (c) Microsoft Corporation. All rights reserved.
2+
# Licensed under the MIT License.
3+
4+
import os
5+
import json
6+
import sys
7+
8+
9+
# Last argument is the target file into which we'll write the env variables as json.
10+
json_file = sys.argv[-1]
11+
12+
with open(json_file, 'w') as outfile:
13+
json.dump(dict(os.environ), outfile)

‎pythonFiles/shell_exec.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,3 +34,9 @@
3434
# Signal end of execution with failure state.
3535
fp.write('FAIL\n')
3636
fp.flush()
37+
try:
38+
# ALso log the error for use from the other side.
39+
with open(lock_file + '.error', 'w') as fpError:
40+
fpError.write(traceback.format_exc())
41+
except Exception:
42+
pass

‎src/client/common/constants.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,3 +101,6 @@ export function isUnitTestExecution(): boolean {
101101
}
102102

103103
export * from '../constants';
104+
105+
// Terminals with this name prefix will not be automatically activated.
106+
export const terminalNamePrefixNotToAutoActivate = 'VSC_NO_ACTIVATE';

‎src/client/common/experimentGroups.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,17 @@ export enum WebHostNotebook {
4242
experiment = 'WebHostNotebook - experiment'
4343
}
4444

45+
/**
46+
* Experiment to check whether to to use a terminal to generate the environment variables of activated environments.
47+
*
48+
* @export
49+
* @enum {number}
50+
*/
51+
export enum UseTerminalToGetActivatedEnvVars {
52+
control = 'UseTerminalToGetActivatedEnvVars - control',
53+
experiment = 'UseTerminalToGetActivatedEnvVars - experiment'
54+
}
55+
4556
// Dummy experiment added to validate metrics of A/B testing
4657
export enum ValidateABTesting {
4758
control = 'AA_testing - control',

‎src/client/common/terminal/factory.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,12 @@ export class TerminalServiceFactory implements ITerminalServiceFactory {
3030
const title = isUri(arg1) ? undefined : arg1?.title || arg2;
3131
const terminalTitle = typeof title === 'string' && title.trim().length > 0 ? title.trim() : 'Python';
3232
const interpreter = isUri(arg1) ? undefined : arg1?.interpreter;
33+
const hideFromUser = isUri(arg1) ? false : arg1?.hideFromUser === true;
3334
const env = isUri(arg1) ? undefined : arg1?.env;
3435

3536
const options: TerminalCreationOptions = {
3637
env,
38+
hideFromUser,
3739
interpreter,
3840
resource,
3941
title

‎src/client/common/terminal/service.ts‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,16 @@ export class TerminalService implements ITerminalService, Disposable {
3939
public async sendCommand(command: string, args: string[], _?: CancellationToken): Promise<void> {
4040
await this.ensureTerminal();
4141
const text = this.terminalHelper.buildCommandForTerminal(this.terminalShellType, command, args);
42-
this.terminal!.show(true);
42+
if (!this.options?.hideFromUser) {
43+
this.terminal!.show(true);
44+
}
4345
this.terminal!.sendText(text, true);
4446
}
4547
public async sendText(text: string): Promise<void> {
4648
await this.ensureTerminal();
47-
this.terminal!.show(true);
49+
if (!this.options?.hideFromUser) {
50+
this.terminal!.show(true);
51+
}
4852
this.terminal!.sendText(text);
4953
}
5054
public async show(preserveFocus: boolean = true): Promise<void> {
@@ -56,18 +60,21 @@ export class TerminalService implements ITerminalService, Disposable {
5660
return;
5761
}
5862
this.terminalShellType = this.terminalHelper.identifyTerminalShell(this.terminal);
59-
this.terminal = this.terminalManager.createTerminal({ name: this.options?.title || 'Python', env: this.options?.env });
63+
this.terminal = this.terminalManager.createTerminal({ name: this.options?.title || 'Python', env: this.options?.env, hideFromUser: this.options?.hideFromUser });
6064

6165
// Sometimes the terminal takes some time to start up before it can start accepting input.
6266
await new Promise(resolve => setTimeout(resolve, 100));
6367

6468
await this.terminalActivator.activateEnvironmentInTerminal(this.terminal!, {
6569
resource: this.options?.resource,
6670
preserveFocus,
67-
interpreter: this.options?.interpreter
71+
interpreter: this.options?.interpreter,
72+
hideFromUser: this.options?.hideFromUser
6873
});
6974

70-
this.terminal!.show(preserveFocus);
75+
if (!this.options?.hideFromUser) {
76+
this.terminal!.show(preserveFocus);
77+
}
7178

7279
this.sendTelemetry().ignoreErrors();
7380
}

‎src/client/common/terminal/syncTerminalService.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ class ExecutionState implements Disposable {
4949
}
5050
this.state = state;
5151
if (state & State.errored) {
52-
this._completed.reject(new Error(`Command failed with errors, check the terminal for details. Command: ${this.command.join(' ')}`));
52+
const errorContents = await this.fs.readFile(`${this.lockFile}.error`).catch(() => '');
53+
this._completed.reject(new Error(`Command failed with errors, check the terminal for details. Command: ${this.command.join(' ')}\n${errorContents}`));
5354
} else if (state & State.completed) {
5455
this._completed.resolve();
5556
}
@@ -115,7 +116,7 @@ export class SynchronousTerminalService implements ITerminalService, Disposable
115116
}
116117
}
117118
}
118-
public async sendCommand(command: string, args: string[], cancel?: CancellationToken): Promise<void> {
119+
public async sendCommand(command: string, args: string[], cancel?: CancellationToken, swallowExceptions: boolean = true): Promise<void> {
119120
if (!cancel) {
120121
return this.terminalService.sendCommand(command, args);
121122
}
@@ -129,7 +130,8 @@ export class SynchronousTerminalService implements ITerminalService, Disposable
129130
...args,
130131
lockFile.filePath.fileToCommandArgument()
131132
]);
132-
await Cancellation.race(() => state.completed.catch(noop), cancel);
133+
const promise = swallowExceptions ? state.completed : state.completed.catch(noop);
134+
await Cancellation.race(() => promise, cancel);
133135
} finally {
134136
state.dispose();
135137
lockFile.dispose();

0 commit comments

Comments
 (0)