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

Add Python Extension's "Create Env" command into DVC Setup #4058

Merged
merged 6 commits into from
Jun 12, 2023

Conversation

julieg18
Copy link
Contributor

@julieg18 julieg18 commented Jun 7, 2023

  • adds an "Update Env" button that gives the user a choice between creating a new environment or selecting from already created ones

Demo

Screen.Recording.2023-06-10.at.5.31.09.PM.mov
image image

Part of #3935

@julieg18 julieg18 added the product PR that affects product label Jun 7, 2023
@julieg18 julieg18 self-assigned this Jun 7, 2023
@@ -29,6 +33,9 @@ export const CliUnavailable: React.FC<PropsWithChildren> = ({ children }) => {
</p>
<div className={styles.sideBySideButtons}>
<Button onClick={installDvc} text="Install (pip)" />
{isPythonExtensionUsed && (
<Button onClick={updatePythonEnvironment} text="Update Env" />
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pr's focus is adding the logic and quick pick for updating the python environment or creating a new one. I'm going to do separate prs for:

  • adding a --user to the command if the environment is global
  • improving the step's contents (for example, updating the text for "Configure")

We can merge this as is or I'll add a do not merge label until the other two prs are ready :)

@julieg18 julieg18 marked this pull request as ready for review June 10, 2023 22:45
@julieg18 julieg18 requested a review from shcheklein June 10, 2023 22:46
extension/src/setup/index.ts Outdated Show resolved Hide resolved
@mattseddon
Copy link
Member

mattseddon commented Jun 12, 2023

Fine to merge after comment is addressed.

@julieg18 julieg18 enabled auto-merge (squash) June 12, 2023 15:31
@codeclimate
Copy link

codeclimate bot commented Jun 12, 2023

Code Climate has analyzed commit 2d1742e and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (85% is the threshold).

This pull request will bring the total coverage in the repository to 95.2% (0.0% change).

View more on Code Climate.

@julieg18 julieg18 merged commit 31373bc into main Jun 12, 2023
@julieg18 julieg18 deleted the add-create-env-command branch June 12, 2023 15:50
@shcheklein
Copy link
Member

QQ - does it means that we don't need Configure anymore though?

@julieg18
Copy link
Contributor Author

QQ - does it means that we don't need Configure anymore though?

Configure (aka "Setup the Workspace" command) would be needed if the user has DVC globally installed, like with brew.

@shcheklein
Copy link
Member

Setup the Workspace

what does it give on top of Update Env (why update btw?) ? Global vs env choice? Can we combine those somehow into one flow?

@julieg18
Copy link
Contributor Author

what does it give on top of Update Env

"Setup the Workspace" or "Configure" is focused on locating an already installed version of DVC, starting by asking if you have it installed globally or automatically.
"Update Env" on the other hand is focused on updating or creating the environment selected by the Python Extension. It's there to help users who don't have DVC installed but want to install it with our auto-installation feature within their chosen environment.

(why update btw?) ?

I chose the term "update" since the action updates the selected python environment, either by creating a new one or selecting from already created ones. No strong preference on the name though!

Global vs env choice? Can we combine those somehow into one flow?

Apologies, I'm a little confused by your question. Could you give me some more details?

By the way, I've posted some ideas for the next iteration of the "DVC is unavailable" step in #3935. Might be easier to continue the discussion there for visibility :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product PR that affects product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants