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 UI and connect to constructor manager API #63

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

goanpeca
Copy link
Collaborator

@goanpeca goanpeca commented Jan 26, 2023

Overview

Bundle Application and Constructor Manager

## Constructor based installers

These installers will contain 3 main environments when created:

  • A base environment which will contain:
    • conda (package and environment manager)
    • mamba (conda "replacement" for performing faster solves, transactions and downloads)
    • conda-lock (create locked and reproducible environments)
  • A constructor-manager environment which will contain:
  • An application environment, using the conventions <package-name>-<version>, for the example image above, napari-0.4.15

More information on packaging on https://napari.org/dev/developers/packaging.html#constructor-based-installers

constructor-manager-ui

This pull request is in charge of connecting the previously created UI (#40) the constructor-manager-ui package which in its present form is a pyqt/pyside based application. It provides a graphical interface to run the different actions to manage an application built with constructor

The ui is called from the command line interface with constructor-manager-ui <package-name>. Since the application will be called using standard application calls created by menuinst, arguments will be passed with configuration files handled by constructor-manager-client calls.

Screenshot 2023-02-07 at 1 48 41 PM

  • check-updates: Query for new updates available for the managed application by providing one or more anaconda.org channels. By default we query the conda-forge channel located at https://anaconda.org/conda-forge/
  • update: update a current application to a new version of it. This process will create a new conda environment following the convention <package-name>-<new-version>, create new menu shortcuts (using the menuinst branch https://github.com/conda/menuinst/tree/cep-devel/menuinst), create a new restore point (using conda-lock), remove the old environment and the corresponding shortcuts for the old versions of the managed application.
  • revert: this will revert the current application to the previously installed version on the computer if a restore point is found. This will follow a similar process of creating a new conda environment with the convention <package-name>-<old-version>.
  • restore: similar to revert but restore to a previously found state of the current version. This command could become a single one by providing the specific restore file (which is created by conda lock)
  • open: open the managed give application.

Work in progress

  • Handle settings
  • Release package to pypi
  • Release package to conda-forge
  • Tests (just some small ones to check basic functionality is in place)

Future work

  • Add better status handling
  • Add uninstall management

@goanpeca goanpeca changed the title Update CLI to take generic parameters and connect to API Update UI and connect to constructor manager API Jan 26, 2023
@goanpeca
Copy link
Collaborator Author

goanpeca commented Mar 15, 2023

Fixing the tests, but this one is ready for review @aganders3, @dalthviz, @potating-potato

(Also need to complete docstrings)

Thanks!

@goanpeca goanpeca marked this pull request as ready for review March 15, 2023 13:46
Copy link
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

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

Hi @goanpeca ! Left some comments/questions regarding commented out code, some qss styles, some print statements, the CI workflow reorganization and a traceback related with the log level setting that appeared when I was trying to run some commands


return install_information_group
log_level = getattr(logging, log_level.upper())
Copy link
Member

Choose a reason for hiding this comment

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

Tried running constructor-manager-ui napari --current-version 0.4.16 --build-string pyside --plugins-url https://api.napari-hub.org/plugins --channel conda-forge locally and got the following traceback related to this line:

Traceback (most recent call last):
  File "C:\Users\dalth\anaconda3\envs\constructor-manager\lib\runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Users\dalth\anaconda3\envs\constructor-manager\lib\runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "C:\Users\dalth\anaconda3\envs\constructor-manager\Scripts\constructor-manager-ui.exe\__main__.py", line 7, in <module>
  File "E:\Acer\Documentos\Quansight\Napari\packaging\constructor-manager-ui\src\constructor_manager_ui\main.py", line 58, in run
    _configure_logging(settings["log"])
  File "E:\Acer\Documentos\Quansight\Napari\packaging\constructor-manager-ui\src\constructor_manager_ui\main.py", line 33, in _configure_logging
    log_level = getattr(logging, log_level.upper())
AttributeError: 'NoneType' object has no attribute 'upper'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is part of the issue of calling on windows. Fixing over the CLI PR

Comment on lines +340 to +342
background-color: @background-color13;
background-color: @background-selection-color01;
background-color: #758193;
Copy link
Member

Choose a reason for hiding this comment

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

Should here be only the entry for #758193 while also making it part of the colors constants available?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, we can do that

Comment on lines +84 to +85
# f"Last modified {self.current_version['last_modified']}"
# )
Copy link
Member

Choose a reason for hiding this comment

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

I guess this is part of a TODO (as well as the commented text for labels code bellow), right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Correct

# Installation Actions
feedback_group = self._create_feedback_group()
main_layout.addWidget(feedback_group, stretch=1)
print(self.log)
Copy link
Member

Choose a reason for hiding this comment

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

Should this and other print statements in the file be transformed into logger.info/logger.debug calls?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes absolutely

Comment on lines +350 to +352
# while ConstructorManagerWorker._WORKERS:
# worker = ConstructorManagerWorker._WORKERS.pop()
# self._terminate_worker(worker)
Copy link
Member

Choose a reason for hiding this comment

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

Should this be removed or uncommented?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed, we do not want to force close in the middle of an update.

Comment on lines +102 to +114
- name: Test constructor-manager-ui (linux)
if: contains(matrix.platform, 'ubuntu')
run: |
# Run Tests
cd constructor-manager-ui/src
xvfb-run pytest constructor_manager_ui --cov=constructor_manager_ui

- name: Test constructor-manager-ui (other)
if: "!contains(matrix.platform, 'ubuntu')"
run: |
# Run Tests
cd constructor-manager-ui/src
pytest constructor_manager_ui --cov=constructor_manager_ui
Copy link
Member

Choose a reason for hiding this comment

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

I guess the UI part will not use tox for the tests on the CI? Should the tox.ini file be removed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, since all the workflow for these packages revolves around constructor using a base environment and a constructor-manager environment, tox does not help a lot. Will remove!

@DragaDoncila
Copy link

Hi @goanpeca I'm curious whether you have examples of built napari applications that work with this PR, #34 and #46? Essentially it would be great to have a bundle we can download and play with that makes use of this Constructor Manager.

Otherwise as I understand it each piece can be played with individually either from the command line or by following the instructions in the PR description?

@goanpeca
Copy link
Collaborator Author

Hi @DragaDoncila, I will update a small snippet to install all the needed deps (including installing from these PRs) so you can test the whole workflow. Thanks!

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.

3 participants