Skip to content

Commit

Permalink
Fix DVC setup showing false info on incompatible global DVC (#3961)
Browse files Browse the repository at this point in the history
* Fix DVC setup showing false info on global DVC
* keeps section (and extension status) showing accurate information when extension auto selects a global version of DVC

* Apply solution that involves less confusion
  • Loading branch information
julieg18 authored May 23, 2023
1 parent 247f9e7 commit c82b555
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 5 deletions.
16 changes: 13 additions & 3 deletions extension/src/cli/dvc/discovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,16 @@ const tryGlobalFallbackVersion = async (
const tryGlobal = await getVersionDetails(setup, cwd, true)
const { cliCompatible, isAvailable, isCompatible, version } = tryGlobal

if (isCompatible) {
setup.unsetPythonBinPath()
if (!isCompatible) {
return {
isAvailable,
isCompatible,
version
}
}

setup.unsetPythonBinPath()

return processVersionDetails(
setup,
cliCompatible,
Expand All @@ -152,7 +158,11 @@ const extensionCanAutoRunCli = async (
} = await getVersionDetails(setup, cwd)

if (pythonCliCompatible === CliCompatible.NO_NOT_FOUND) {
return tryGlobalFallbackVersion(setup, cwd)
const globalResults = await tryGlobalFallbackVersion(setup, cwd)

if (globalResults.isCompatible) {
return globalResults
}
}

return processVersionDetails(
Expand Down
5 changes: 3 additions & 2 deletions extension/src/setup/runner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -485,8 +485,9 @@ describe('run', () => {
await flushPromises()
expect(mockedWarnWithOptions).toHaveBeenCalledTimes(1)
expect(mockedWarnWithOptions).toHaveBeenCalledWith(
'The extension cannot initialize because the DVC CLI version is incompatible.',
Response.SHOW_SETUP
'An error was thrown when trying to access the CLI.',
Response.SHOW_SETUP,
Response.NEVER
)
expect(mockedGetCliVersion).toHaveBeenCalledTimes(2)
expect(mockedResetMembers).toHaveBeenCalledTimes(1)
Expand Down

0 comments on commit c82b555

Please sign in to comment.