Skip to content

Commit

Permalink
Update Setup "dvc is unavailable" section text content (#4098)
Browse files Browse the repository at this point in the history
* update button names
* simplify text content
* add warning with global environments
  • Loading branch information
julieg18 authored Jun 14, 2023
1 parent 4f5be9f commit 1b39901
Show file tree
Hide file tree
Showing 10 changed files with 84 additions and 25 deletions.
10 changes: 9 additions & 1 deletion extension/src/setup/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,11 @@ import { Title } from '../vscode/title'
import { getDVCAppDir } from '../util/appdirs'
import { getOptions } from '../cli/dvc/options'
import { isAboveLatestTestedVersion } from '../cli/dvc/version'
import { createPythonEnv, selectPythonInterpreter } from '../extensions/python'
import {
createPythonEnv,
isActivePythonEnvGlobal,
selectPythonInterpreter
} from '../extensions/python'

export class Setup
extends BaseRepository<TSetupData>
Expand Down Expand Up @@ -396,12 +400,16 @@ export class Setup

const pythonBinPath = await findPythonBinForInstall()

const isPythonEnvironmentGlobal =
isPythonExtensionUsed && (await isActivePythonEnvGlobal())

this.webviewMessages.sendWebviewMessage({
canGitInitialize,
cliCompatible: this.getCliCompatible(),
dvcCliDetails,
hasData,
isAboveLatestTestedVersion: isAboveLatestTestedVersion(this.cliVersion),
isPythonEnvironmentGlobal,
isPythonExtensionUsed,
isStudioConnected: this.studioIsConnected,
needsGitCommit,
Expand Down
1 change: 1 addition & 0 deletions extension/src/setup/webview/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export type SetupData = {
cliCompatible: boolean | undefined
dvcCliDetails: DvcCliDetails | undefined
hasData: boolean | undefined
isPythonEnvironmentGlobal: boolean | undefined
isPythonExtensionUsed: boolean
isStudioConnected: boolean
needsGitCommit: boolean
Expand Down
4 changes: 4 additions & 0 deletions extension/src/test/suite/setup/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ suite('Setup Test Suite', () => {
dvcCliDetails: { command: 'dvc', version: undefined },
hasData: false,
isAboveLatestTestedVersion: undefined,
isPythonEnvironmentGlobal: false,
isPythonExtensionUsed: false,
isStudioConnected: false,
needsGitCommit: true,
Expand Down Expand Up @@ -400,6 +401,7 @@ suite('Setup Test Suite', () => {
dvcCliDetails: { command: 'dvc', version: MIN_CLI_VERSION },
hasData: false,
isAboveLatestTestedVersion: false,
isPythonEnvironmentGlobal: false,
isPythonExtensionUsed: false,
isStudioConnected: false,
needsGitCommit: true,
Expand Down Expand Up @@ -452,6 +454,7 @@ suite('Setup Test Suite', () => {
},
hasData: false,
isAboveLatestTestedVersion: false,
isPythonEnvironmentGlobal: false,
isPythonExtensionUsed: false,
isStudioConnected: false,
needsGitCommit: false,
Expand Down Expand Up @@ -504,6 +507,7 @@ suite('Setup Test Suite', () => {
},
hasData: false,
isAboveLatestTestedVersion: false,
isPythonEnvironmentGlobal: false,
isPythonExtensionUsed: false,
isStudioConnected: false,
needsGitCommit: true,
Expand Down
36 changes: 28 additions & 8 deletions webview/src/setup/components/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const DEFAULT_DATA = {
},
hasData: false,
isAboveLatestTestedVersion: false,
isPythonEnvironmentGlobal: false,
isPythonExtensionUsed: false,
isStudioConnected: false,
needsGitCommit: false,
Expand Down Expand Up @@ -161,7 +162,7 @@ describe('App', () => {
})

const sentenceReg = new RegExp(
`DVC & DVCLive can be auto-installed with ${defaultInterpreter}.`
`Auto-install \\(pip\\) DVC & DVCLive with ${defaultInterpreter}`
)

expect(screen.getByText(sentenceReg)).toBeInTheDocument()
Expand All @@ -178,7 +179,7 @@ describe('App', () => {
pythonBinPath: 'python'
})

const button = screen.getByText('Configure')
const button = screen.getByText('Locate DVC')
fireEvent.click(button)

expect(mockPostMessage).toHaveBeenCalledWith({
Expand All @@ -197,7 +198,7 @@ describe('App', () => {
pythonBinPath: 'python'
})

const button = screen.getByText('Update Env')
const button = screen.getByText('Set Env')
fireEvent.click(button)

expect(mockPostMessage).toHaveBeenCalledWith({
Expand All @@ -223,6 +224,25 @@ describe('App', () => {
})
})

it('should let the user auto-install DVC but warn the user that their selected env is global when the Python extension is installed', () => {
const defaultInterpreter = 'python'
renderApp({
cliCompatible: undefined,
dvcCliDetails: {
command: 'python -m dvc',
version: undefined
},
isPythonEnvironmentGlobal: true,
isPythonExtensionUsed: true,
pythonBinPath: defaultInterpreter
})

const button = screen.getByText('Set Env')

expect(button).toBeInTheDocument()
expect(screen.getByText('Not a virtual environment)')).toBeInTheDocument()
})

it('should not show a screen saying that DVC is not installed if the cli is available', () => {
renderApp()

Expand Down Expand Up @@ -354,14 +374,14 @@ describe('App', () => {
expect(within(envDetails).getByText(command)).toBeInTheDocument()
})

it('should show the user the command used to run DVC with a "Configure" button if dvc is installed without the python extension', () => {
it('should show the user the command used to run DVC with a "Locate DVC" button if dvc is installed without the python extension', () => {
renderApp()

const envDetails = screen.getByTestId('dvc-env-details')

expect(within(envDetails).getByText('Command:')).toBeInTheDocument()

const configureButton = within(envDetails).getByText('Configure')
const configureButton = within(envDetails).getByText('Locate DVC')
const selectButton = within(envDetails).queryByText(
'Select Python Interpreter'
)
Expand All @@ -376,7 +396,7 @@ describe('App', () => {
})
})

it('should show the user the command used to run DVC with "Configure" and "Update Env" buttons if dvc is installed with the python extension', () => {
it('should show the user the command used to run DVC with "Locate DVC" and "Set Env" buttons if dvc is installed with the python extension', () => {
renderApp({
isPythonExtensionUsed: true
})
Expand All @@ -385,8 +405,8 @@ describe('App', () => {

expect(within(envDetails).getByText('Command:')).toBeInTheDocument()

const configureButton = within(envDetails).getByText('Configure')
const selectButton = within(envDetails).getByText('Update Env')
const configureButton = within(envDetails).getByText('Locate DVC')
const selectButton = within(envDetails).getByText('Set Env')

expect(configureButton).toBeInTheDocument()
expect(selectButton).toBeInTheDocument()
Expand Down
6 changes: 6 additions & 0 deletions webview/src/setup/components/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
updateCliCompatible,
updateDvcCliDetails,
updateIsAboveLatestTestedVersion,
updateIsPythonEnvironmentGlobal,
updateIsPythonExtensionUsed,
updateNeedsGitInitialized,
updateProjectInitialized,
Expand Down Expand Up @@ -82,6 +83,11 @@ export const feedStore = (
case 'hasData':
dispatch(updateExperimentsHasData(data.data.hasData))
continue
case 'isPythonEnvironmentGlobal':
dispatch(
updateIsPythonEnvironmentGlobal(data.data.isPythonEnvironmentGlobal)
)
continue
case 'isPythonExtensionUsed':
dispatch(updateIsPythonExtensionUsed(data.data.isPythonExtensionUsed))
continue
Expand Down
32 changes: 18 additions & 14 deletions webview/src/setup/components/dvc/CliUnavailable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,43 +9,47 @@ import {
setupWorkspace,
updatePythonEnvironment
} from '../../util/messages'
import { Warning } from '../../../shared/components/icons'

export const CliUnavailable: React.FC<PropsWithChildren> = ({ children }) => {
const { pythonBinPath, isPythonExtensionUsed } = useSelector(
(state: SetupState) => state.dvc
)
const { pythonBinPath, isPythonExtensionUsed, isPythonEnvironmentGlobal } =
useSelector((state: SetupState) => state.dvc)
const canInstall = !!pythonBinPath
const installationSentence = (
<>
The extension supports all{' '}
<a href="https://dvc.org/doc/install">installation types</a>. It can also
help to install needed packages via{' '}
<a href="https://packaging.python.org/en/latest/key_projects/#pip">pip</a>
.
<a href="https://dvc.org/doc/install">installation types</a>.
</>
)

const conditionalContents = canInstall ? (
<>
<p>
{installationSentence} DVC & DVCLive can be auto-installed with{' '}
{pythonBinPath}.
{installationSentence} Auto-install (pip) DVC & DVCLive with{' '}
{pythonBinPath}{' '}
{isPythonEnvironmentGlobal && (
<>
(<Warning className={styles.inlineWarningSvg} />{' '}
<span>Not a virtual environment)</span>
</>
)}
.
</p>
<div className={styles.sideBySideButtons}>
<Button onClick={installDvc} text="Install (pip)" />
{isPythonExtensionUsed && (
<Button onClick={updatePythonEnvironment} text="Update Env" />
<Button onClick={updatePythonEnvironment} text="Set Env" />
)}
<Button onClick={setupWorkspace} text="Configure" />
<Button onClick={setupWorkspace} text="Locate DVC" />
</div>
</>
) : (
<>
<p>
{installationSentence} Unfortunately, DVC & DVCLive cannot be
auto-installed as Python was not located.
{installationSentence} DVC & DVCLive cannot be auto-installed as Python
was not located.
</p>
<Button onClick={setupWorkspace} text="Configure" />
<Button onClick={setupWorkspace} text="Locate DVC" />
</>
)

Expand Down
4 changes: 2 additions & 2 deletions webview/src/setup/components/dvc/DvcEnvCommandRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export const DvcEnvCommandRow: React.FC<DvcEnvCommandRowProps> = ({
<span className={styles.command}>{commandText}</span>
<span className={styles.actions}>
<button className={styles.buttonAsLink} onClick={setupWorkspace}>
Configure
Locate DVC
</button>
{isPythonExtensionUsed && (
<>
Expand All @@ -30,7 +30,7 @@ export const DvcEnvCommandRow: React.FC<DvcEnvCommandRowProps> = ({
className={styles.buttonAsLink}
onClick={updatePythonEnvironment}
>
Update Env
Set Env
</button>
</>
)}
Expand Down
7 changes: 7 additions & 0 deletions webview/src/setup/components/dvc/styles.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@
}
}

.inlineWarningSvg {
vertical-align: middle;
fill: $warn-color;
width: 16px;
height: 16px;
}

.buttonAsLink {
@extend %buttonAsLink;

Expand Down
8 changes: 8 additions & 0 deletions webview/src/setup/state/dvcSlice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export const dvcInitialState: DvcState = {
cliCompatible: undefined,
dvcCliDetails: undefined,
isAboveLatestTestedVersion: undefined,
isPythonEnvironmentGlobal: undefined,
isPythonExtensionUsed: false,
needsGitInitialized: false,
projectInitialized: false,
Expand Down Expand Up @@ -47,6 +48,12 @@ export const dvcSlice = createSlice({
) => {
state.isAboveLatestTestedVersion = action.payload
},
updateIsPythonEnvironmentGlobal: (
state,
action: PayloadAction<boolean | undefined>
) => {
state.isPythonEnvironmentGlobal = action.payload
},
updateIsPythonExtensionUsed: (state, action: PayloadAction<boolean>) => {
state.isPythonExtensionUsed = action.payload
},
Expand All @@ -70,6 +77,7 @@ export const {
updateCliCompatible,
updateDvcCliDetails,
updateIsAboveLatestTestedVersion,
updateIsPythonEnvironmentGlobal,
updateIsPythonExtensionUsed,
updateNeedsGitInitialized,
updateProjectInitialized,
Expand Down
1 change: 1 addition & 0 deletions webview/src/stories/Setup.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const DEFAULT_DATA: SetupData = {
},
hasData: false,
isAboveLatestTestedVersion: false,
isPythonEnvironmentGlobal: false,
isPythonExtensionUsed: true,
isStudioConnected: true,
needsGitCommit: false,
Expand Down

0 comments on commit 1b39901

Please sign in to comment.