Skip to content

Commit 4d1e879

Browse files
mattseddonmattseddon
authored
Simplify Setup the Workspace by removing manual option when Python extension is installed (#2967)
* simplify setup by removing options when Python extension is installed * refactor for readability * remove select interpreter as option from discovery warnings
1 parent cd16a9d commit 4d1e879

File tree

6 files changed

+137
-230
lines changed

6 files changed

+137
-230
lines changed

‎README.md‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -146,15 +146,15 @@ These are the VS Code [settings] available for the Extension:
146146

147147
[settings]: https://code.visualstudio.com/docs/getstarted/settings
148148

149-
| **Option** | **Description** |
150-
| -------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------- |
151-
| `dvc.dvcPath` | Path or shell command to the DVC binary. Required unless Microsoft's [Python extension] is installed and the `dvc` package found in its environment. |
152-
| `dvc.pythonPath` | Path to the desired Python interpreter to use with DVC. Required when using a virtual environment. |
153-
| `dvc.experimentsTableHeadMaxHeight` | Maximum height of experiment table head rows. |
154-
| `dvc.doNotShowWalkthroughAfterInstall` | Do not prompt to show the Get Started page after installing. Useful for pre-configured development environments |
155-
| `dvc.doNotRecommendRedHatExtension` | Do not prompt to install the Red Hat YAML extension, which helps with DVC YAML schema validation (`dvc.yaml` and `.dvc` files). |
156-
| `dvc.doNotShowCliUnavailable` | Do not warn when the workspace contains a DVC project but the DVC binary is unavailable. |
157-
| `dvc.doNotShowUnableToFilter` | Do not warn before disabling auto-apply filters when these would result in too many experiments being selected. |
149+
| **Option** | **Description** |
150+
| -------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------- |
151+
| `dvc.dvcPath` | Path or shell command to the DVC binary. Required unless Microsoft's [Python extension] is installed and the `dvc` package found in its environment. |
152+
| `dvc.pythonPath` | Path to the desired Python interpreter to use with DVC. Should only be utilized when using a virtual environment without Microsoft's [Python extension]. |
153+
| `dvc.experimentsTableHeadMaxHeight` | Maximum height of experiment table head rows. |
154+
| `dvc.doNotShowWalkthroughAfterInstall` | Do not prompt to show the Get Started page after installing. Useful for pre-configured development environments |
155+
| `dvc.doNotRecommendRedHatExtension` | Do not prompt to install the Red Hat YAML extension, which helps with DVC YAML schema validation (`dvc.yaml` and `.dvc` files). |
156+
| `dvc.doNotShowCliUnavailable` | Do not warn when the workspace contains a DVC project but the DVC binary is unavailable. |
157+
| `dvc.doNotShowUnableToFilter` | Do not warn before disabling auto-apply filters when these would result in too many experiments being selected. |
158158

159159
> **Note** that the `Setup The Workspace` command helps you set up the basic
160160
> ones at the [Workspace level] (saved to `.vscode/setting.json`).

‎extension/src/cli/dvc/discovery.ts‎

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,20 +12,10 @@ import {
1212
getConfigValue,
1313
setUserConfigValue
1414
} from '../../vscode/config'
15-
import {
16-
getPythonBinPath,
17-
isPythonExtensionInstalled,
18-
selectPythonInterpreter
19-
} from '../../extensions/python'
15+
import { getPythonBinPath } from '../../extensions/python'
2016
import { getFirstWorkspaceFolder } from '../../vscode/workspaceFolders'
2117
import { delay } from '../../util/time'
2218

23-
const getToastOptions = (isPythonExtensionInstalled: boolean): Response[] => {
24-
return isPythonExtensionInstalled
25-
? [Response.SETUP_WORKSPACE, Response.SELECT_INTERPRETER, Response.NEVER]
26-
: [Response.SETUP_WORKSPACE, Response.NEVER]
27-
}
28-
2919
export const warnUnableToVerifyVersion = () =>
3020
Toast.warnWithOptions(
3121
'The extension cannot initialize as we were unable to verify the DVC CLI version.'
@@ -48,7 +38,6 @@ export const warnAheadOfLatestTested = (): void => {
4838

4939
const warnUserCLIInaccessible = async (
5040
extension: IExtension,
51-
isMsPythonInstalled: boolean,
5241
warningText: string
5342
): Promise<void> => {
5443
if (getConfigValue<boolean>(ConfigKey.DO_NOT_SHOW_CLI_UNAVAILABLE)) {
@@ -57,12 +46,11 @@ const warnUserCLIInaccessible = async (
5746

5847
const response = await Toast.warnWithOptions(
5948
warningText,
60-
...getToastOptions(isMsPythonInstalled)
49+
Response.SETUP_WORKSPACE,
50+
Response.NEVER
6151
)
6252

6353
switch (response) {
64-
case Response.SELECT_INTERPRETER:
65-
return selectPythonInterpreter()
6654
case Response.SETUP_WORKSPACE:
6755
return extension.setupWorkspace()
6856
case Response.NEVER:
@@ -78,7 +66,6 @@ const warnUserCLIInaccessibleAnywhere = async (
7866

7967
return warnUserCLIInaccessible(
8068
extension,
81-
true,
8269
`The extension is unable to initialize. The CLI was not located using the interpreter provided by the Python extension. ${
8370
globalDvcVersion ? globalDvcVersion + ' is' : 'The CLI is also not'
8471
} installed globally. For auto Python environment activation, ensure the correct interpreter is set. Active Python interpreter: ${binPath}.`
@@ -104,7 +91,6 @@ const warnUser = (
10491
case CliCompatible.NO_NOT_FOUND:
10592
warnUserCLIInaccessible(
10693
extension,
107-
isPythonExtensionInstalled(),
10894
'An error was thrown when trying to access the CLI.'
10995
)
11096
return

‎extension/src/setup.test.ts‎

Lines changed: 65 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { extensions, Extension, commands } from 'vscode'
33
import { setup, setupWithGlobalRecheck, setupWorkspace } from './setup'
44
import { flushPromises } from './test/util/jest'
55
import {
6+
ConfigKey,
67
getConfigValue,
78
setConfigValue,
89
setUserConfigValue
@@ -25,6 +26,7 @@ import {
2526
} from './cli/dvc/constants'
2627
import { extractSemver, ParsedSemver } from './cli/dvc/version'
2728
import { delay } from './util/time'
29+
import { Title } from './vscode/title'
2830

2931
jest.mock('vscode')
3032
jest.mock('./vscode/config')
@@ -100,119 +102,125 @@ beforeEach(() => {
100102
})
101103

102104
describe('setupWorkspace', () => {
103-
it('should return without setting any options if the dialog is cancelled at the virtual environment step', async () => {
105+
it('should present two options if the python extension is installed (Auto & Global)', async () => {
104106
mockedQuickPickValue.mockResolvedValueOnce(undefined)
105107

106108
await setupWorkspace()
107109

108110
expect(mockedQuickPickValue).toHaveBeenCalledTimes(1)
109-
expect(mockedSetConfigValue).not.toHaveBeenCalled()
111+
expect(mockedQuickPickValue).toHaveBeenCalledWith(
112+
[
113+
expect.objectContaining({ label: 'Auto' }),
114+
expect.objectContaining({ label: 'Global' })
115+
],
116+
{
117+
placeHolder: 'Select an environment where DVC is installed',
118+
title: Title.SETUP_WORKSPACE
119+
}
120+
)
110121
})
111122

112-
it('should return without setting any options if the dialog is cancelled at the DVC in virtual environment step', async () => {
113-
mockedQuickPickValue.mockResolvedValueOnce(2)
114-
mockedQuickPickYesOrNo.mockResolvedValueOnce(undefined)
123+
it('should present two options if the python extension is NOT installed (Manual & Global)', async () => {
124+
mockedExtensions.all = []
115125

116126
await setupWorkspace()
117127

118128
expect(mockedQuickPickValue).toHaveBeenCalledTimes(1)
119-
expect(mockedQuickPickYesOrNo).toHaveBeenCalledTimes(1)
120-
expect(mockedSetConfigValue).toHaveBeenCalledTimes(1)
121-
expect(mockedSetConfigValue).toHaveBeenCalledWith(
122-
'dvc.pythonPath',
123-
undefined
129+
expect(mockedQuickPickValue).toHaveBeenCalledWith(
130+
[
131+
expect.objectContaining({ label: 'Manual' }),
132+
expect.objectContaining({ label: 'Global' })
133+
],
134+
{
135+
placeHolder: 'Select an environment where DVC is installed',
136+
title: Title.SETUP_WORKSPACE
137+
}
124138
)
125139
})
126140

127-
it('should set the dvc path option to undefined if the CLI is installed in a virtual environment', async () => {
141+
it('should set the dvc path and python path options to undefined if the CLI is being auto detected inside a virtual environment', async () => {
128142
mockedQuickPickValue.mockResolvedValueOnce(2)
129-
mockedQuickPickYesOrNo.mockResolvedValueOnce(true)
130143

131144
await setupWorkspace()
132145

133146
expect(mockedQuickPickValue).toHaveBeenCalledTimes(1)
134-
expect(mockedQuickPickYesOrNo).toHaveBeenCalledTimes(1)
135-
expect(mockedSetConfigValue).toHaveBeenCalledWith('dvc.dvcPath', undefined)
147+
expect(mockedSetConfigValue).toHaveBeenCalledWith(
148+
ConfigKey.DVC_PATH,
149+
undefined
150+
)
151+
expect(mockedSetConfigValue).toHaveBeenCalledWith(
152+
ConfigKey.PYTHON_PATH,
153+
undefined
154+
)
136155
})
137156

138157
it('should return without setting any options if the dialog is cancelled at the virtual environment without DVC step', async () => {
139-
mockedQuickPickValue.mockResolvedValueOnce(2)
140-
mockedQuickPickYesOrNo.mockResolvedValueOnce(false)
141-
mockedQuickPickYesOrNo.mockResolvedValueOnce(undefined)
158+
mockedQuickPickValue.mockResolvedValueOnce(1)
159+
mockedQuickPickOneOrInput.mockResolvedValueOnce(undefined)
142160

143161
await setupWorkspace()
144162

145163
expect(mockedQuickPickValue).toHaveBeenCalledTimes(1)
146-
expect(mockedQuickPickYesOrNo).toHaveBeenCalledTimes(2)
147-
expect(mockedSetConfigValue).toHaveBeenCalledTimes(1)
148-
expect(mockedSetConfigValue).toHaveBeenCalledWith(
149-
'dvc.pythonPath',
150-
undefined
151-
)
164+
expect(mockedQuickPickOneOrInput).toHaveBeenCalledTimes(1)
165+
expect(mockedSetConfigValue).not.toHaveBeenCalled()
152166
})
153167

154168
it("should set the dvc path option to dvc if there is a virtual environment which doesn't include the CLI but it is available globally", async () => {
155-
mockedQuickPickValue.mockResolvedValueOnce(2)
156-
mockedQuickPickYesOrNo.mockResolvedValueOnce(false)
157-
mockedQuickPickYesOrNo.mockResolvedValueOnce(true)
169+
mockedQuickPickValue.mockResolvedValueOnce(1)
170+
mockedQuickPickYesOrNo
171+
.mockResolvedValueOnce(false)
172+
.mockResolvedValueOnce(true)
173+
mockedQuickPickOneOrInput.mockResolvedValueOnce('pick')
174+
mockedPickFile.mockResolvedValueOnce(mockedPythonPath)
158175

159176
await setupWorkspace()
160177

161178
expect(mockedQuickPickValue).toHaveBeenCalledTimes(1)
162179
expect(mockedQuickPickYesOrNo).toHaveBeenCalledTimes(2)
180+
expect(mockedQuickPickOneOrInput).toHaveBeenCalledTimes(1)
181+
expect(mockedPickFile).toHaveBeenCalledTimes(1)
163182
expect(mockedSetConfigValue).toHaveBeenCalledTimes(2)
164183
expect(mockedSetConfigValue).toHaveBeenCalledWith(
165-
'dvc.pythonPath',
166-
undefined
184+
ConfigKey.PYTHON_PATH,
185+
mockedPythonPath
167186
)
168-
expect(mockedSetConfigValue).toHaveBeenCalledWith('dvc.dvcPath', 'dvc')
187+
expect(mockedSetConfigValue).toHaveBeenCalledWith(ConfigKey.DVC_PATH, 'dvc')
169188
})
170189

171-
it("should set the dvc path option to the entered value if there is a virtual environment that doesn't include a CLI and there is no global option", async () => {
172-
const mockedDvcPath = resolve('some', 'path', 'to', 'dvc')
173-
mockedQuickPickValue.mockResolvedValueOnce(2)
174-
mockedQuickPickYesOrNo.mockResolvedValueOnce(false)
175-
mockedQuickPickYesOrNo.mockResolvedValueOnce(false)
176-
mockedQuickPickOneOrInput.mockResolvedValueOnce(mockedDvcPath)
190+
it('should return without setting any options if the dialog is cancelled at the virtual environment step', async () => {
191+
mockedQuickPickValue.mockResolvedValueOnce(undefined)
177192

178193
await setupWorkspace()
179194

180195
expect(mockedQuickPickValue).toHaveBeenCalledTimes(1)
181-
expect(mockedQuickPickYesOrNo).toHaveBeenCalledTimes(2)
182-
expect(mockedQuickPickOneOrInput).toHaveBeenCalledTimes(1)
183-
expect(mockedPickFile).not.toHaveBeenCalled()
184-
expect(mockedSetConfigValue).toHaveBeenCalledTimes(2)
185-
expect(mockedSetConfigValue).toHaveBeenCalledWith(
186-
'dvc.pythonPath',
187-
undefined
188-
)
189-
expect(mockedSetConfigValue).toHaveBeenCalledWith(
190-
'dvc.dvcPath',
191-
mockedDvcPath
192-
)
196+
expect(mockedSetConfigValue).not.toHaveBeenCalled()
193197
})
194198

195199
it("should set the dvc path option to the picked file's path if there is a virtual environment that doesn't include a CLI and there is no global option", async () => {
196200
const mockedDvcPath = resolve('some', 'path', 'to', 'dvc')
197-
mockedQuickPickValue.mockResolvedValueOnce(2)
201+
mockedQuickPickValue.mockResolvedValueOnce(1)
198202
mockedQuickPickYesOrNo.mockResolvedValueOnce(false)
199203
mockedQuickPickYesOrNo.mockResolvedValueOnce(false)
200-
mockedQuickPickOneOrInput.mockResolvedValueOnce('pick')
201-
mockedPickFile.mockResolvedValueOnce(mockedDvcPath)
204+
mockedQuickPickOneOrInput
205+
.mockResolvedValueOnce('pick')
206+
.mockResolvedValueOnce('pick')
207+
mockedPickFile
208+
.mockResolvedValueOnce(mockedPythonPath)
209+
.mockResolvedValueOnce(mockedDvcPath)
202210

203211
await setupWorkspace()
204212

205213
expect(mockedQuickPickValue).toHaveBeenCalledTimes(1)
206214
expect(mockedQuickPickYesOrNo).toHaveBeenCalledTimes(2)
207-
expect(mockedQuickPickOneOrInput).toHaveBeenCalledTimes(1)
208-
expect(mockedPickFile).toHaveBeenCalledTimes(1)
215+
expect(mockedQuickPickOneOrInput).toHaveBeenCalledTimes(2)
216+
expect(mockedPickFile).toHaveBeenCalledTimes(2)
209217
expect(mockedSetConfigValue).toHaveBeenCalledTimes(2)
210218
expect(mockedSetConfigValue).toHaveBeenCalledWith(
211-
'dvc.pythonPath',
212-
undefined
219+
ConfigKey.PYTHON_PATH,
220+
mockedPythonPath
213221
)
214222
expect(mockedSetConfigValue).toHaveBeenCalledWith(
215-
'dvc.dvcPath',
223+
ConfigKey.DVC_PATH,
216224
mockedDvcPath
217225
)
218226
})
@@ -228,7 +236,6 @@ describe('setupWorkspace', () => {
228236
expect(mockedQuickPickOneOrInput).toHaveBeenCalledTimes(1)
229237
expect(mockedSetConfigValue).not.toHaveBeenCalled()
230238
})
231-
232239
it("should set the python and dvc path options to the picked file's path if there is a virtual environment that doesn't include a CLI and there is no global option", async () => {
233240
const mockedPythonPath = resolve('some', 'path', 'to', 'python')
234241
const mockedDvcPath = resolve('some', 'path', 'to', 'dvc')
@@ -248,11 +255,11 @@ describe('setupWorkspace', () => {
248255
expect(mockedPickFile).toHaveBeenCalledTimes(2)
249256
expect(mockedSetConfigValue).toHaveBeenCalledTimes(2)
250257
expect(mockedSetConfigValue).toHaveBeenCalledWith(
251-
'dvc.pythonPath',
258+
ConfigKey.PYTHON_PATH,
252259
mockedPythonPath
253260
)
254261
expect(mockedSetConfigValue).toHaveBeenCalledWith(
255-
'dvc.dvcPath',
262+
ConfigKey.DVC_PATH,
256263
mockedDvcPath
257264
)
258265
})
@@ -377,30 +384,6 @@ describe('setup', () => {
377384
expect(mockedInitialize).not.toHaveBeenCalled()
378385
})
379386

380-
it('should try to select the python interpreter if the workspace contains a DVC project, the cli cannot be found and the user decides to select the python interpreter', async () => {
381-
mockedGetFirstWorkspaceFolder.mockReturnValueOnce(mockedCwd)
382-
mockedHasRoots.mockReturnValueOnce(true)
383-
mockedIsPythonExtensionUsed.mockResolvedValueOnce(false)
384-
mockedGetCliVersion.mockResolvedValueOnce(undefined)
385-
mockedWarnWithOptions.mockResolvedValueOnce(Response.SELECT_INTERPRETER)
386-
mockedExecuteProcess.mockImplementation(({ executable }) =>
387-
Promise.resolve(executable)
388-
)
389-
mockedReady.mockResolvedValue(true)
390-
mockedGetExtension.mockReturnValue(mockedVscodePython)
391-
392-
await setup(extension)
393-
await flushPromises()
394-
expect(mockedSetRoots).toHaveBeenCalledTimes(1)
395-
expect(mockedGetConfigValue).toHaveBeenCalledTimes(1)
396-
expect(mockedWarnWithOptions).toHaveBeenCalledTimes(1)
397-
expect(mockedSetupWorkspace).toHaveBeenCalledTimes(0)
398-
expect(mockedExecuteCommand).toHaveBeenCalledTimes(1)
399-
expect(mockedSetUserConfigValue).not.toHaveBeenCalled()
400-
expect(mockedResetMembers).toHaveBeenCalledTimes(1)
401-
expect(mockedInitialize).not.toHaveBeenCalled()
402-
})
403-
404387
it('should set a user config option if the workspace contains a DVC project, the cli cannot be found and the user selects never', async () => {
405388
mockedGetFirstWorkspaceFolder.mockReturnValueOnce(mockedCwd)
406389
mockedHasRoots.mockReturnValueOnce(true)
@@ -488,7 +471,6 @@ describe('setup', () => {
488471
expect(mockedWarnWithOptions).toHaveBeenCalledWith(
489472
`The extension is unable to initialize. The CLI was not located using the interpreter provided by the Python extension. ${belowMinVersion} is installed globally. For auto Python environment activation, ensure the correct interpreter is set. Active Python interpreter: ${mockedPythonPath}.`,
490473
Response.SETUP_WORKSPACE,
491-
Response.SELECT_INTERPRETER,
492474
Response.NEVER
493475
)
494476
expect(mockedGetCliVersion).toHaveBeenCalledTimes(2)
@@ -579,7 +561,6 @@ describe('setup', () => {
579561
expect(mockedWarnWithOptions).toHaveBeenCalledWith(
580562
`The extension is unable to initialize. The CLI was not located using the interpreter provided by the Python extension. The CLI is also not installed globally. For auto Python environment activation, ensure the correct interpreter is set. Active Python interpreter: ${mockedPythonPath}.`,
581563
Response.SETUP_WORKSPACE,
582-
Response.SELECT_INTERPRETER,
583564
Response.NEVER
584565
)
585566
expect(mockedGetCliVersion).toHaveBeenCalledTimes(2)

0 commit comments

Comments
 (0)