-
Notifications
You must be signed in to change notification settings - Fork 209
[UX] Make dstack project and dstack project set-default interactive for default project selection
#3488
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
[UX] Make dstack project and dstack project set-default interactive for default project selection
#3488
Changes from all commits
599ccdf
e60f9f3
0e8a728
95964e6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,19 +1,77 @@ | ||
| import argparse | ||
| from typing import Any, Union | ||
| import sys | ||
| from typing import Any, Optional, Union | ||
|
|
||
| from requests import HTTPError | ||
| from rich.table import Table | ||
|
|
||
| try: | ||
| import questionary | ||
|
|
||
| is_project_menu_supported = sys.stdin.isatty() | ||
| except (ImportError, NotImplementedError, AttributeError): | ||
| is_project_menu_supported = False | ||
|
|
||
| import dstack.api.server | ||
| from dstack._internal.cli.commands import BaseCommand | ||
| from dstack._internal.cli.utils.common import add_row_from_dict, confirm_ask, console | ||
| from dstack._internal.core.errors import ClientError, CLIError | ||
| from dstack._internal.core.models.config import ProjectConfig | ||
| from dstack._internal.core.services.configs import ConfigManager | ||
| from dstack._internal.utils.logging import get_logger | ||
|
|
||
| logger = get_logger(__name__) | ||
|
|
||
|
|
||
| def select_default_project( | ||
| project_configs: list[ProjectConfig], default_project: Optional[ProjectConfig] | ||
| ) -> Optional[ProjectConfig]: | ||
| """Show an interactive menu to select a default project. | ||
|
|
||
| This method only prompts for selection and does not update the configuration. | ||
| Use `ConfigManager.configure_project()` and `ConfigManager.save()` to persist | ||
| the selected project as default. | ||
|
|
||
| Args: | ||
| project_configs: Non-empty list of available project configurations. | ||
| default_project: Currently default project, if any. | ||
|
|
||
| Returns: | ||
| Selected project configuration, or None if cancelled. | ||
|
|
||
| Raises: | ||
| CLIError: If `is_project_menu_supported` is False or `project_configs` is empty. | ||
| """ | ||
| if not is_project_menu_supported: | ||
| raise CLIError("Interactive menu is not supported on this platform") | ||
|
|
||
| if len(project_configs) == 0: | ||
| raise CLIError("No projects configured") | ||
|
|
||
| menu_entries = [] | ||
| default_index = None | ||
| for i, project_config in enumerate(project_configs): | ||
| is_default = project_config.name == default_project.name if default_project else False | ||
| entry = f"{project_config.name} ({project_config.url})" | ||
| if is_default: | ||
| default_index = i | ||
| menu_entries.append((entry, i)) | ||
|
|
||
| choices = [questionary.Choice(title=entry, value=index) for entry, index in menu_entries] # pyright: ignore[reportPossiblyUnboundVariable] | ||
| default_value = default_index | ||
| selected_index = questionary.select( # pyright: ignore[reportPossiblyUnboundVariable] | ||
| message="Select the default project:", | ||
| choices=choices, | ||
| default=default_value, # pyright: ignore[reportArgumentType] | ||
| qmark="", | ||
| instruction="(↑↓ Enter)", | ||
| ).ask() | ||
|
|
||
| if selected_index is not None and isinstance(selected_index, int): | ||
| return project_configs[selected_index] | ||
| return None | ||
|
|
||
|
|
||
| class ProjectCommand(BaseCommand): | ||
| NAME = "project" | ||
| DESCRIPTION = "Manage projects configs" | ||
|
|
@@ -67,14 +125,17 @@ def _register(self): | |
| # Set default subcommand | ||
| set_default_parser = subparsers.add_parser("set-default", help="Set default project") | ||
| set_default_parser.add_argument( | ||
| "name", type=str, help="The name of the project to set as default" | ||
| "name", | ||
| type=str, | ||
| nargs="?" if is_project_menu_supported else None, | ||
| help="The name of the project to set as default", | ||
| ) | ||
| set_default_parser.set_defaults(subfunc=self._set_default) | ||
|
|
||
| def _command(self, args: argparse.Namespace): | ||
| super()._command(args) | ||
| if not hasattr(args, "subfunc"): | ||
| args.subfunc = self._list | ||
| args.subfunc = self._project | ||
| args.subfunc(args) | ||
|
|
||
| def _add(self, args: argparse.Namespace): | ||
|
|
@@ -156,14 +217,47 @@ def _list(self, args: argparse.Namespace): | |
|
|
||
| console.print(table) | ||
|
|
||
| def _project(self, args: argparse.Namespace): | ||
| if is_project_menu_supported and not getattr(args, "verbose", False): | ||
| config_manager = ConfigManager() | ||
| project_configs = config_manager.list_project_configs() | ||
| default_project = config_manager.get_project_config() | ||
| selected_project = select_default_project(project_configs, default_project) | ||
| if selected_project is not None: | ||
| config_manager.configure_project( | ||
| name=selected_project.name, | ||
| url=selected_project.url, | ||
| token=selected_project.token, | ||
| default=True, | ||
| ) | ||
| config_manager.save() | ||
| console.print("[grey58]OK[/]") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Duplicated code for interactive project selectionLow Severity The interactive project selection logic in Additional Locations (1) |
||
| else: | ||
| self._list(args) | ||
|
|
||
| def _set_default(self, args: argparse.Namespace): | ||
| config_manager = ConfigManager() | ||
| project_config = config_manager.get_project_config(args.name) | ||
| if project_config is None: | ||
| raise CLIError(f"Project '{args.name}' not found") | ||
| if args.name: | ||
| config_manager = ConfigManager() | ||
| project_config = config_manager.get_project_config(args.name) | ||
| if project_config is None: | ||
| raise CLIError(f"Project '{args.name}' not found") | ||
|
|
||
| config_manager.configure_project( | ||
| name=args.name, url=project_config.url, token=project_config.token, default=True | ||
| ) | ||
| config_manager.save() | ||
| console.print("[grey58]OK[/]") | ||
| config_manager.configure_project( | ||
| name=args.name, url=project_config.url, token=project_config.token, default=True | ||
| ) | ||
| config_manager.save() | ||
| console.print("[grey58]OK[/]") | ||
| else: | ||
| config_manager = ConfigManager() | ||
| project_configs = config_manager.list_project_configs() | ||
| default_project = config_manager.get_project_config() | ||
| selected_project = select_default_project(project_configs, default_project) | ||
| if selected_project is not None: | ||
| config_manager.configure_project( | ||
| name=selected_project.name, | ||
| url=selected_project.url, | ||
| token=selected_project.token, | ||
| default=True, | ||
| ) | ||
| config_manager.save() | ||
| console.print("[grey58]OK[/]") | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Invalid default value passed to questionary select
Medium Severity
When no project is marked as default,
default_indexremainsNoneand is passed directly toquestionary.select()as thedefaultparameter. This causes a type mismatch (hence the pyright ignore comment) and may result in unexpected behavior. This scenario can occur when the default project is deleted, leaving other projects without a default flag.