Simplify DVC CLI Location/Version Logic (#3784)

* Simplify incompatible version toasts
* Configure extension status button to open setup on click

Author: julieg18

extension/src/cli/dvc/discovery.ts

@@ -1,8 +1,4 @@
-import {
-  LATEST_TESTED_CLI_VERSION,
-  MAX_CLI_VERSION,
-  MIN_CLI_VERSION
-} from './contract'
+import { LATEST_TESTED_CLI_VERSION } from './contract'
 import { CliCompatible, isVersionCompatible } from './version'
 import { IExtensionSetup } from '../../interfaces'
 import { Toast } from '../../vscode/toast'
@@ -12,90 +8,77 @@ import {
   getConfigValue,
   setUserConfigValue
 } from '../../vscode/config'
-import { getPythonBinPath } from '../../extensions/python'
 import { getFirstWorkspaceFolder } from '../../vscode/workspaceFolders'
 import { delay } from '../../util/time'
 import { SetupSection } from '../../setup/webview/contract'
 
-export const warnUnableToVerifyVersion = () =>
-  Toast.warnWithOptions(
+const warnWithSetupAction = async (
+  setup: IExtensionSetup,
+  warningText: string
+): Promise<void> => {
+  const response = await Toast.warnWithOptions(warningText, Response.SHOW_SETUP)
+
+  if (response === Response.SHOW_SETUP) {
+    return setup.showSetup(SetupSection.DVC)
+  }
+}
+
+const warnUnableToVerifyVersion = (setup: IExtensionSetup) => {
+  void warnWithSetupAction(
+    setup,
     'The extension cannot initialize as we were unable to verify the DVC CLI version.'
   )
+}
 
-export const warnVersionIncompatible = (
-  version: string,
-  update: 'CLI' | 'extension'
-): void => {
-  void Toast.warnWithOptions(
-    `The extension cannot initialize because you are using version ${version} of the DVC CLI. The expected version is ${MIN_CLI_VERSION} <= DVC < ${MAX_CLI_VERSION}. Please upgrade to the most recent version of the ${update} and reload this window.`
+const warnVersionIncompatible = (setup: IExtensionSetup): void => {
+  void warnWithSetupAction(
+    setup,
+    'The extension cannot initialize because the DVC CLI version is incompatible.'
   )
 }
 
-export const warnAheadOfLatestTested = (): void => {
+const warnAheadOfLatestTested = (): void => {
   void Toast.warnWithOptions(
     `The located DVC CLI is at least a minor version ahead of the latest version the extension was tested with (${LATEST_TESTED_CLI_VERSION}). This could lead to unexpected behaviour. Please upgrade to the most recent version of the extension and reload this window.`
   )
 }
 
 const warnUserCLIInaccessible = async (
-  setup: IExtensionSetup,
-  warningText: string
+  setup: IExtensionSetup
 ): Promise<void> => {
   if (getConfigValue<boolean>(ConfigKey.DO_NOT_SHOW_CLI_UNAVAILABLE)) {
     return
   }
 
   const response = await Toast.warnWithOptions(
-    warningText,
+    'An error was thrown when trying to access the CLI.',
     Response.SHOW_SETUP,
     Response.NEVER
   )
 
   switch (response) {
     case Response.SHOW_SETUP:
-      return setup.showSetup(SetupSection.EXPERIMENTS)
+      return setup.showSetup(SetupSection.DVC)
     case Response.NEVER:
       return setUserConfigValue(ConfigKey.DO_NOT_SHOW_CLI_UNAVAILABLE, true)
   }
 }
 
-const warnUserCLIInaccessibleAnywhere = async (
-  setup: IExtensionSetup,
-  globalDvcVersion: string | undefined
-): Promise<void> => {
-  const binPath = await getPythonBinPath()
-
-  return warnUserCLIInaccessible(
-    setup,
-    `The extension is unable to initialize. The CLI was not located using the interpreter provided by the Python extension. ${
-      globalDvcVersion ? globalDvcVersion + ' is' : 'The CLI is also not'
-    } installed globally. For auto Python environment activation, ensure the correct interpreter is set. Active Python interpreter: ${
-      binPath || 'unset'
-    }.`
-  )
-}
-
 const warnUser = (
   setup: IExtensionSetup,
-  cliCompatible: CliCompatible,
-  version: string | undefined
+  cliCompatible: CliCompatible
 ): void => {
   if (!setup.shouldWarnUserIfCLIUnavailable()) {
     return
   }
   switch (cliCompatible) {
-    case CliCompatible.NO_BEHIND_MIN_VERSION:
-      return warnVersionIncompatible(version as string, 'CLI')
+    case CliCompatible.NO_INCOMPATIBLE:
+      return warnVersionIncompatible(setup)
     case CliCompatible.NO_CANNOT_VERIFY:
-      void warnUnableToVerifyVersion()
+      void warnUnableToVerifyVersion(setup)
       return
-    case CliCompatible.NO_MAJOR_VERSION_AHEAD:
-      return warnVersionIncompatible(version as string, 'extension')
     case CliCompatible.NO_NOT_FOUND:
-      void warnUserCLIInaccessible(
-        setup,
-        'An error was thrown when trying to access the CLI.'
-      )
+      void warnUserCLIInaccessible(setup)
       return
     case CliCompatible.YES_MINOR_VERSION_AHEAD_OF_TESTED:
       return warnAheadOfLatestTested()
@@ -128,7 +111,6 @@ const getVersionDetails = async (
 ): Promise<
   CanRunCli & {
     cliCompatible: CliCompatible
-    version: string | undefined
   }
 > => {
   const version = await setup.getCliVersion(cwd, tryGlobalCli)
@@ -140,11 +122,12 @@ const getVersionDetails = async (
 const processVersionDetails = (
   setup: IExtensionSetup,
   cliCompatible: CliCompatible,
-  version: string | undefined,
   isAvailable: boolean,
-  isCompatible: boolean | undefined
+  isCompatible: boolean | undefined,
+  version: string | undefined
 ): CanRunCli => {
-  warnUser(setup, cliCompatible, version)
+  warnUser(setup, cliCompatible)
+
   return {
     isAvailable,
     isCompatible,
@@ -159,21 +142,17 @@ const tryGlobalFallbackVersion = async (
   const tryGlobal = await getVersionDetails(setup, cwd, true)
   const { cliCompatible, isAvailable, isCompatible, version } = tryGlobal
 
-  if (setup.shouldWarnUserIfCLIUnavailable() && !isCompatible) {
-    void warnUserCLIInaccessibleAnywhere(setup, version)
-  }
-  if (
-    setup.shouldWarnUserIfCLIUnavailable() &&
-    cliCompatible === CliCompatible.YES_MINOR_VERSION_AHEAD_OF_TESTED
-  ) {
-    warnAheadOfLatestTested()
-  }
-
   if (isCompatible) {
     setup.unsetPythonBinPath()
   }
 
-  return { isAvailable, isCompatible, version }
+  return processVersionDetails(
+    setup,
+    cliCompatible,
+    isAvailable,
+    isCompatible,
+    version
+  )
 }
 
 const extensionCanAutoRunCli = async (
@@ -190,12 +169,13 @@ const extensionCanAutoRunCli = async (
   if (pythonCliCompatible === CliCompatible.NO_NOT_FOUND) {
     return tryGlobalFallbackVersion(setup, cwd)
   }
+
   return processVersionDetails(
     setup,
     pythonCliCompatible,
-    pythonVersion,
     pythonVersionIsAvailable,
-    pythonVersionIsCompatible
+    pythonVersionIsCompatible,
+    pythonVersion
   )
 }
 
@@ -213,9 +193,9 @@ export const extensionCanRunCli = async (
   return processVersionDetails(
     setup,
     cliCompatible,
-    version,
     isAvailable,
-    isCompatible
+    isCompatible,
+    version
   )
 }
 

extension/src/cli/dvc/version.test.ts

@@ -127,34 +127,34 @@ describe('isVersionCompatible', () => {
     )
   })
 
-  it('should return behind min version if the provided version is a patch version before the minimum expected version', () => {
+  it('should return behind incompatible if the provided version is a patch version before the minimum expected version', () => {
     const isCompatible = isVersionCompatible(
       [minMajor, minMinor, minPatch - 1].join('.')
     )
 
-    expect(isCompatible).toStrictEqual(CliCompatible.NO_BEHIND_MIN_VERSION)
+    expect(isCompatible).toStrictEqual(CliCompatible.NO_INCOMPATIBLE)
   })
 
-  it('should return behind min version if the provided minor version is before the minimum expected version', () => {
+  it('should return behind incompatible if the provided minor version is before the minimum expected version', () => {
     const isCompatible = isVersionCompatible(
       [minMajor, minMinor - 1, minPatch + 100].join('.')
     )
 
-    expect(isCompatible).toStrictEqual(CliCompatible.NO_BEHIND_MIN_VERSION)
+    expect(isCompatible).toStrictEqual(CliCompatible.NO_INCOMPATIBLE)
   })
 
-  it('should return behind min version if the provided major version is before the minimum expected version', () => {
+  it('should return behind incompatible if the provided major version is before the minimum expected version', () => {
     const isCompatible = isVersionCompatible(
       [minMajor - 1, minMinor + 1000, minPatch + 100].join('.')
     )
 
-    expect(isCompatible).toStrictEqual(CliCompatible.NO_BEHIND_MIN_VERSION)
+    expect(isCompatible).toStrictEqual(CliCompatible.NO_INCOMPATIBLE)
   })
 
-  it('should return major ahead if the provided major version is above the expected major version', () => {
+  it('should return incompatible if the provided major version is above the expected major version', () => {
     const isCompatible = isVersionCompatible('3.0.0')
 
-    expect(isCompatible).toStrictEqual(CliCompatible.NO_MAJOR_VERSION_AHEAD)
+    expect(isCompatible).toStrictEqual(CliCompatible.NO_INCOMPATIBLE)
   })
 
   it('should return cannot verify if the provided version is malformed', () => {

extension/src/cli/dvc/version.ts

@@ -5,9 +5,8 @@ import {
 } from './contract'
 
 export enum CliCompatible {
-  NO_BEHIND_MIN_VERSION = 'no-behind-min-version',
   NO_CANNOT_VERIFY = 'no-cannot-verify',
-  NO_MAJOR_VERSION_AHEAD = 'no-major-version-ahead',
+  NO_INCOMPATIBLE = 'no-incompatible',
   NO_NOT_FOUND = 'no-not-found',
   YES_MINOR_VERSION_AHEAD_OF_TESTED = 'yes-minor-version-ahead-of-tested',
   YES = 'yes'
@@ -49,23 +48,20 @@ const checkCLIVersion = (currentSemVer: {
     minor: currentMinor,
     patch: currentPatch
   } = currentSemVer
-
-  if (currentMajor >= Number(MAX_CLI_VERSION)) {
-    return CliCompatible.NO_MAJOR_VERSION_AHEAD
-  }
-
   const {
     major: minMajor,
     minor: minMinor,
     patch: minPatch
   } = extractSemver(MIN_CLI_VERSION) as ParsedSemver
 
-  if (
+  const isAheadMaxVersion = currentMajor >= Number(MAX_CLI_VERSION)
+  const isBehindMinVersion =
     currentMajor < minMajor ||
     currentMinor < minMinor ||
     (currentMinor === minMinor && currentPatch < Number(minPatch))
-  ) {
-    return CliCompatible.NO_BEHIND_MIN_VERSION
+
+  if (isAheadMaxVersion || isBehindMinVersion) {
+    return CliCompatible.NO_INCOMPATIBLE
   }
 
   return cliIsCompatible(currentMajor, currentMinor)

extension/src/setup/runner.test.ts

@@ -19,11 +19,7 @@ import { Toast } from '../vscode/toast'
 import { Response } from '../vscode/response'
 import { VscodePython } from '../extensions/python'
 import { executeProcess } from '../process/execution'
-import {
-  LATEST_TESTED_CLI_VERSION,
-  MAX_CLI_VERSION,
-  MIN_CLI_VERSION
-} from '../cli/dvc/contract'
+import { LATEST_TESTED_CLI_VERSION, MIN_CLI_VERSION } from '../cli/dvc/contract'
 import { extractSemver, ParsedSemver } from '../cli/dvc/version'
 import { delay } from '../util/time'
 import { Title } from '../vscode/title'
@@ -455,6 +451,24 @@ describe('run', () => {
     expect(mockedInitialize).toHaveBeenCalledTimes(1)
   })
 
+  it('should send a specific message to the user if the extension is unable to verify the version', async () => {
+    const unverifyableVersion = 'not a valid version'
+    mockedGetFirstWorkspaceFolder.mockReturnValueOnce(mockedCwd)
+    mockedShouldWarnUserIfCLIUnavailable.mockReturnValueOnce(true)
+    mockedGetCliVersion.mockResolvedValueOnce(unverifyableVersion)
+
+    await run(setup)
+    await flushPromises()
+    expect(mockedWarnWithOptions).toHaveBeenCalledTimes(1)
+    expect(mockedWarnWithOptions).toHaveBeenCalledWith(
+      'The extension cannot initialize as we were unable to verify the DVC CLI version.',
+      Response.SHOW_SETUP
+    )
+    expect(mockedGetCliVersion).toHaveBeenCalledTimes(1)
+    expect(mockedResetMembers).toHaveBeenCalledTimes(1)
+    expect(mockedInitialize).not.toHaveBeenCalled()
+  })
+
   it('should send a specific message to the user if the Python extension is being used, the CLI is not available in the virtual environment and the global CLI is not compatible', async () => {
     const belowMinVersion = '2.0.0'
     mockedGetFirstWorkspaceFolder.mockReturnValueOnce(mockedCwd)
@@ -472,9 +486,8 @@ describe('run', () => {
     await flushPromises()
     expect(mockedWarnWithOptions).toHaveBeenCalledTimes(1)
     expect(mockedWarnWithOptions).toHaveBeenCalledWith(
-      `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}.`,
-      Response.SHOW_SETUP,
-      Response.NEVER
+      'The extension cannot initialize because the DVC CLI version is incompatible.',
+      Response.SHOW_SETUP
     )
     expect(mockedGetCliVersion).toHaveBeenCalledTimes(2)
     expect(mockedResetMembers).toHaveBeenCalledTimes(1)
@@ -539,7 +552,8 @@ describe('run', () => {
     await flushPromises()
     expect(mockedWarnWithOptions).toHaveBeenCalledTimes(1)
     expect(mockedWarnWithOptions).toHaveBeenCalledWith(
-      `The extension cannot initialize because you are using version ${MajorAhead} of the DVC CLI. The expected version is ${MIN_CLI_VERSION} <= DVC < ${MAX_CLI_VERSION}. Please upgrade to the most recent version of the extension and reload this window.`
+      'The extension cannot initialize because the DVC CLI version is incompatible.',
+      'Setup'
     )
     expect(mockedGetCliVersion).toHaveBeenCalledTimes(1)
     expect(mockedResetMembers).toHaveBeenCalledTimes(1)
@@ -564,7 +578,7 @@ describe('run', () => {
     await flushPromises()
     expect(mockedWarnWithOptions).toHaveBeenCalledTimes(1)
     expect(mockedWarnWithOptions).toHaveBeenCalledWith(
-      `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}.`,
+      'An error was thrown when trying to access the CLI.',
       Response.SHOW_SETUP,
       Response.NEVER
     )

extension/src/status.ts

@@ -93,13 +93,13 @@ export class Status extends Disposable {
 
     if (this.available) {
       return {
-        command: RegisteredCommands.EXTENSION_SETUP_WORKSPACE,
-        title: Title.SETUP_WORKSPACE
+        command: RegisteredCommands.SETUP_SHOW,
+        title: Title.SHOW_SETUP
       }
     }
 
     return {
-      command: RegisteredCommands.SETUP_SHOW,
+      command: RegisteredCommands.SETUP_SHOW_DVC,
       title: Title.SHOW_SETUP
     }
   }