Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Setup "dvc is unavailable" section text content #4098

Merged
merged 13 commits into from
Jun 14, 2023
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