search

conda-incubator / conda-store-ui

conda-store-ui is a frontend for conda-store powered by react
https://conda-incubator.github.io/conda-store-ui/
BSD 3-Clause "New" or "Revised" License
13 stars 18 forks source link

Lockfile upload #401

Open gabalafou opened 2 months ago

gabalafou commented 2 months ago

Closes #399.

Companion backend PR: https://github.com/conda-incubator/conda-store/pull/772

Description

This PR takes over #395.

PR 395 implemented "create environment from lockfile" using a code editor. But because hand-editing lockfiles is error prone, we decided that the UI should be implemented as a file upload instead.

This pull request:

Screenshots

Pull request checklist

gabalafou commented 2 months ago

Creating this as a draft PR to solicit feedback. Couple questions:

dharhas commented 2 months ago

So if I upload a lockfile, what happens to the 'Specification' Tab. Does it get disabled?

gabalafou commented 2 months ago

So if I upload a lockfile, what happens to the 'Specification' Tab. Does it get disabled?

Not at the moment. I think that's worth implementing if we stick with this direction because otherwise I think it can create the misleading impression that the two tabs are synced with each other in the same way that the GUI-YAML toggle is.

gabalafou commented 1 month ago

https://github.com/conda-incubator/conda-store-ui/assets/317883/47817b5f-902d-4521-94bf-123cab8abe08

@smeragoel @kcpevey, what do we think about the implementation shown in this video?

kcpevey commented 1 month ago

@gabalafou I really like it!!

I'm wondering about what it looks like AFTER you upload the lockfile, or rather, the view of an existing environment built by lock file. Is that what test_env1269 is? In that case, what does it look like when you go Edit (via spec) -> YAML? Are the requested/installed packages mocked here or is that already functional once build via lockfile?

gabalafou commented 1 month ago

Great! Good question. I haven't coded this up yet, but I was thinking that when you load an environment that was last built from a lockfile, you will see that same box that says "Lockfile" but instead of "upload" it will say "download". Then, if you click the "edit" button that section will become upload again. Am I making sense or do we need a visual?

kcpevey commented 1 month ago

I followed you, and I think what you described are pieces that need to be included. I'd also like to see an uneditable view of the lockfile that is currently loaded.

For example, in a specification-based environment, when I view an existing environemnt I see requested and installed deps. When I view an existing lockfile environment (as far as I can tell), we won't have any access to requested or installed deps and the best "view" we can get is the lockfile itself.

gabalafou commented 4 weeks ago

Great feedback. Let me code up quickly what all I think I'm hearing, and we'll iterate from there.

dharhas commented 3 weeks ago

Do we need to add descriptive text to indicate that these are "conda env.yaml" files and "conda lockfiles". i.e. to avoid folks trying to upload poetry lock files etc?

gabalafou commented 1 week ago

Sorry, I messed up my last push. Fixing...

gabalafou commented 1 week ago

Visual tour of latest changes

View environment - lockfile

This screenshot shows the "view environment" page for an environment whose active build was created from a lockfile upload. Of note:

From this view, if you click the edit button, you get taken to...

Edit environment - lockfile mode

Not much new here to note except that the upload section of the form is now titled "Conda Lockfile Upload," again to reenforce the idea that it is for Conda lockfiles and not some other format.

From this view, if you click the "Conda Lockfile Upload" heading, it expands to show...

Note about Conda lockfile format

We currently only support the Conda lockfile format. Other lockfile formats (such as Poetry) are not supported.

From this view, if you click "Switch to Specification", you get taken to...

Switch to Specification warning

From this view, if you click "Continue", you get taken to...

Edit environment - GUI specification mode

Here I've implemented the request that when going into specification mode from a lockfile build, the user starts with a clean slate. None of the dependencies in the lockfile are shown in this view. The user is in fact starting over. If they click save here, the environment is rebuilt from the specification shown in the GUI. The lockfile is completely forgotten.

(Oh hmm, I just noticed while making this that "channels" is not reset. I think it's carrying the value of all the channels found in the lockfile. I should probably reset that.)

From this view, if you click the toggle from GUI to YAML, you get taken to...

Edit environment - YAML specification mode

I added a details/summary above the YAML with the summary line "Conda environment.yml"

If you click the summary line, it expands to show...

Note about Conda environment.yml format

We currently only support the Conda environment.yml format. Other environment specification file formats are not supported.

The end.

kcpevey commented 1 day ago

The end.

Standing Ovation!!

This design is wonderful! Also kudos for presenting all that info in an understandable fashion :)

The ONLY thing that I'll give minor pushback on is the details/summary under conda environment.yml and conda lockfile upload. It feels like a lot of UI for just that quick comment. Could we instead add a little info button circle and instead of an "i" in the circle, its a "!" (not sure if that's a thing, but I'm just thinking of drawing a little more attention to it). Honestly, this is a nitpick and I'm flexible on this point.

Other commentary:

User does not have to actually view the .conda-lock.yml file to see the dependencies that were installed in this build. They are listed in a table. The channels that were used are also listed.

Definitely the right choice.

Here I've implemented the request that when going into specification mode from a lockfile build, the user starts with a clean slate.

I agree.