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

feat: Allow installing custom packages inside Wave Studio #2193 #2229

Merged
merged 30 commits into from
Jan 23, 2024

Conversation

marek-mihok
Copy link
Contributor

@marek-mihok marek-mihok commented Jan 4, 2024

The PR fulfills these requirements: (check all the apply)

  • It's submitted to the main branch.
  • When resolving a specific issue, it's referenced in the PR's title (e.g. feat: Add a button #xxx, where "xxx" is the issue number).
  • When resolving a specific issue, the PR description includes Closes #xxx, where "xxx" is the issue number.
  • If changes were made to ui folder, unit tests (make test) still pass.
  • New/updated tests are included

Screen.Recording.2024-01-16.at.15.24.35.mov

Code explanation

The core concept

The core function doing the magic of package installation is our pip function which uses subprocess.Popen. In contrast to subprocess.run waiting for the result, Popen allows us to handle custom logic during its execution. This allows us to read its progress output and show display it to user. For reading the output from within the app, we forward stdout into subprocess.PIPE. We also use text=True attribute to get output as a string instead of bytes object.
Popen is used inside context manager to properly close the pipes after the execution and handle other cleanup logic. Inside the Popen context we run the event loop checking whether task is finished. If it is not, it updates the current progress displayed to user. It it is finished - if p.poll() has some value - it shows success/error message.
The whole logic is wrapped inside try and except block to handle UI updates when task is cancelled.

The pip function has following arguments: q: Q, command: str, args: [str], on_success: Callable = None, on_error: Callable = None, on_cancel: Callable = None. The q is used for UI updates, command stands for pip command, in our case it is install or uninstall, args are additional arguments that can be passed into pip command, e.g. -r for installing from requirements.txt file or -y to automatically bypass questions with yes answers during uninstall process.

Step by step

Clicking the button Manage packages in header items opens the side panel with the package management interface.

This interface consists of a table of currently installed packages loaded from project/requirements.txt file and two buttons - Add package and Add from requirements.txt. Please note that side panel items are obtained via get_side_panel_items() function. Having a separate function is useful when updating side panel with currently installed packages after install/uninstall.

Installing single package

By clicking on Add package the q.args.show_add_package_fields: handler part is executed showing package name and an optional package version fields with the Add button triggering the installation process.

When the Add button is clicked and the package name field is not empty, the new asyncio task is created inside q.args.add_package: handler:

q.client.task = asyncio.create_task(install(q, q.args.package_name, q.args.package_version))

Please note that return value is saved into q.client.task for having its reference in case one needs to cancel the execution.

The install function then calls our pip function, blocking the side panel, displaying current console progress to the user and running the pip install command.

Once the pip finishes successfuly, on_install_success is called,
which calls

update_requirements({f'{package_name}': version(package_name)})

The update_requirements function adds installed package into requirements.txt file if it exists otherwise new one is created. If the package was instaled before, but only the version has changed, the version is updated.

When the install process is finished, the appropriate (error or success) message is shown and one must press Go back to package manager button which calls the q.args.finish_message_dismiss: handler. The on_pip_finish() is called unblocking the side panel, making it closable again and updating it with currently installed packages.

Installing from requirements.txt

By clicking on Add from requirements.txt the q.args.show_add_requirements: handler is called showing the file upload component. Once the requirements file is uploaded q.args.upload_requirements: handler is called saving the file into project/requirements.txt and running the install function.

The rest of the process is the same as during single package installation with the 2 differences. The update_requirements is called without any parameters just to update the uploaded requirements file with the installed versions. The second difference is that if the installation process fails, the requirements file is removed.

Uninstalling the package(s)

Select the table rows containing packages you want to uninstall. The Remove selected button disabled state is based whether any item is selected using the table select event and the q.events.table and q.events.table.select is not None: handler. Once the buttons is clicked, asyncio.create_task(uninstall(q, q.client.selected_packages)) is called.

Closes #2193

@marek-mihok marek-mihok marked this pull request as ready for review January 4, 2024 14:02
Copy link
Collaborator

@mturoci mturoci left a comment

Choose a reason for hiding this comment

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

Thanks @marek-mihok, needs some work.

  • Think whether side_panel wouldn't be a better fit than a dialog.
  • The UX shall be closer to the terminal experience etc. - as a user, I would like to see the streamed output of my pip install rightaway. "Installing..." message has not much meaning for me.
  • The code is hard to follow. Could you add a description in the PR?

studio/studio.py Outdated Show resolved Hide resolved
studio/studio.py Outdated Show resolved Hide resolved
studio/studio.py Outdated Show resolved Hide resolved
studio/studio.py Outdated Show resolved Hide resolved
studio/studio.py Outdated Show resolved Hide resolved
studio/studio.py Outdated Show resolved Hide resolved
studio/studio.py Outdated Show resolved Hide resolved
studio/studio.py Outdated Show resolved Hide resolved
studio/studio.py Outdated Show resolved Hide resolved
studio/studio.py Outdated Show resolved Hide resolved
@marek-mihok marek-mihok force-pushed the feat/issue-2193 branch 2 times, most recently from 77fd183 to 77da445 Compare January 16, 2024 10:24
@marek-mihok
Copy link
Contributor Author

marek-mihok commented Jan 16, 2024

@mturoci thanks for the valuable feedback and for the patience with my first "real" python task 🙂

Think whether side_panel wouldn't be a better fit than a dialog.

Good idea, definitely more suitable when containing more content.
Done ✅

The UX shall be closer to the terminal experience etc. - as a user, I would like to see the streamed output of my pip install rightaway. "Installing..." message has not much meaning for me.

The idea was to keep it simple and if user needs more detail, it is available one a single click.
No problem with your idea though.
Done ✅

The code is hard to follow. Could you add a description in the PR?

Done ✅
In addition to enriched PR description I've refactored the code to be more easy to understand.

Copy link
Collaborator

@mturoci mturoci left a comment

Choose a reason for hiding this comment

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

Thanks @marek-mihok. Looking better. Still needs some UX improvements:

  • When I first click on Manage Packages, nothing happens. Need to click it twice to open the side panel.
  • When adding a new package, there is no way to go back if I change my mind and decide not to install it. Split the UI into tabs: installed (table), new package, import requirements.txt
  • New package form can be vertical instead of horizontal (we have enough space).
  • New package submit button should be disabled until a package name is provided.
  • Put "Remove selected" btn above the table, set its height to fill the rest of the side panel.

@marek-mihok
Copy link
Contributor Author

@mturoci

When I first click on Manage Packages, nothing happens. Need to click it twice to open the side panel.

Seems it was broken since dynamic value change for dropdown was introduced. Fixed. ✅

When adding a new package, there is no way to go back if I change my mind and decide not to install it. Split the UI into tabs: installed (table), new package, import requirements.txt

Done ✅

New package form can be vertical instead of horizontal (we have enough space).

Done ✅

New package submit button should be disabled until a package name is provided.

Done ✅

Put "Remove selected" btn above the table, set its height to fill the rest of the side panel.

Unfortunately, setting height=1 does not work when table is inside the side panel.

@mturoci
Copy link
Collaborator

mturoci commented Jan 18, 2024

Unfortunately, setting height=1 does not work when table is inside the side panel.

You can use CSS calc then. Same for terminal output. Would be also good to have the installation terminal always scrolled to the bottom (so that users do not need to scroll manually), the UX should be as close to the real terminal as possible as mentioned in the prev comment. There is `scrollLogsToBottom function already so implementation should be a matter of couple of minutes.

Once these are done, ready to merge. Tks!

@marek-mihok
Copy link
Contributor Author

marek-mihok commented Jan 18, 2024

All done ✅

You can use CSS calc then. Same for terminal output.

For table it works, but for terminal output I had to use custom JS, since text does no support height attribute.

Maybe it would be good to add support for it in the future; it's definitely useful for markdown text.

@mturoci mturoci merged commit 3a9f4b7 into main Jan 23, 2024
2 checks passed
@mturoci mturoci deleted the feat/issue-2193 branch January 23, 2024 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow for installing arbitrary packages in Wave-studio
2 participants