Merge pull request #9 from ni-kismet/users/fvisser/shared-code-refactor

Universal Pagination & Modernization

Author: fredvisser

.github/copilot-instructions.md

@@ -3,24 +3,41 @@
 ## General Best Practices
 
 - All code must be PEP8-compliant and pass linting (black, ni-python-styleguide).
-- All new and modified code must include appropriate docstrings and type hints where possible.
-- All public functions and classes must have docstrings.
+- All new and modified code must include comprehensive type hints using `typing` module (Any, Dict, List, Optional, Tuple).
+- All public functions and classes must have docstrings following Google docstring format.
 - Use environment variables and keyring for all credentials and sensitive data.
 - All CLI commands must provide clear help text and validation.
-- Use Click for all CLI interfaces (not Typer).
+- Use Click for all CLI interfaces (not Typer) with inline @click.option decorators.
 - All API interactions must handle errors gracefully and provide user-friendly messages.
 - All new features and bugfixes must include or update unit tests.
 - All code must be cross-platform (Windows, macOS, Linux) unless otherwise specified.
 - All generated, build, and cache files must be excluded via `.gitignore`.
 
+## CLI Architecture & Patterns
+
+### Command Structure
+- Use inline @click.option decorators directly on command functions (avoid decorator abstractions)
+- All command modules follow the pattern: `register_{module}_commands(cli: Any) -> None`
+- All command functions must have complete type annotations: parameters and return types
+- Command groups use `@cli.group()` pattern with typed function signatures
+
+### Universal Response Handling
+- Use `UniversalResponseHandler` class for consistent response processing across all commands
+- Use `handle_api_error(exc)` function for standardized API error handling with appropriate exit codes
+- Use `format_success(message, data)` function for consistent success message formatting
+- For mock responses in tests, use type annotation pattern: `resp: Any = MockResponse()` for Pylance compatibility
+
 ## CLI Best Practices (Based on https://clig.dev)
 
 ### Output & Formatting
 - All list commands must support `--format/-f` option with `table` (default) and `json` formats
-- Use `--output` for file path outputs (export, save operations)
+- All list commands must support `--take/-t` option with default of 25 items
+- Use `--output/-o` for file path outputs (export, save operations)
+- Use `table_utils.output_formatted_list()` for consistent table formatting with box-drawing characters
+- Use `cli_utils.paginate_list_output()` for interactive pagination of table results (25 items per page with Y/n prompts)
 - Use consistent visual indicators: `✓` for success messages, `✗` for error messages  
 - Send all error messages to stderr using `click.echo(..., err=True)`
-- For JSON output, display all data at once (no pagination); for table output, retain pagination
+- For JSON output, display all data at once (no pagination); for table output, use interactive pagination
 - Handle empty results gracefully: `[]` for JSON, descriptive message for table format
 
 ### Error Handling & Exit Codes
@@ -31,9 +48,26 @@
   - `NOT_FOUND = 3`: Resource not found
   - `PERMISSION_DENIED = 4`: Permission/authorization error
   - `NETWORK_ERROR = 5`: Network connectivity error
-- Use `handle_api_error(exc)` function for consistent API error handling
+- Use `handle_api_error(exc)` function for consistent API error handling with automatic exit code selection
 - Use `format_success(message, data)` function for consistent success messages
 - Always exit with appropriate codes using `sys.exit(ExitCodes.*)` rather than raising ClickException
+- Use `cli_utils.validate_output_format()` to standardize format validation
+
+### Code Organization & Utilities
+- Use `cli_utils.py` for common CLI utilities like validation and resource resolution
+- Use `table_utils.py` for professional table formatting with box-drawing characters
+- Use `universal_handlers.py` for standardized response processing across all commands
+- Use `workspace_utils.py` for workspace-specific operations and filtering
+- All utility modules must follow the established type annotation patterns
+
+### Mock Response Pattern for Testing
+- For test compatibility with type checkers, use: `resp: Any = FilteredResponse()` or `resp: Any = MockResponse()`
+- FilteredResponse and MockResponse classes must implement:
+  ```python
+  def json(self) -> Dict[str, Any]: ...
+  @property
+  def status_code(self) -> int: ...
+  ```
 
 ### Command Structure
 - All commands must validate required parameters and provide helpful error messages
@@ -62,7 +96,7 @@
 - All code must pass CI (lint, test, build) before merging.
 - All new features must be documented in `README.md`.
 - All code must be reviewed by at least one other developer.
-- All new CLI commands must include JSON output support via `--output/-o` option.
+- All new CLI commands must include JSON output support via `--format/-f` option.
 - All error handling must use standardized exit codes and consistent formatting.
 
 ## Copilot-Specific Instructions
@@ -73,14 +107,19 @@
   3. Report any failures or issues to the user.
 - If you add a new CLI command, ensure it:
   - Is covered by a unit test in `tests/unit/`
-  - Supports `--output/-o` option with `table` and `json` formats
+  - Supports `--format/-f` option with `table` and `json` formats
+  - Supports `--take/-t` option with default of 25 items for list commands
   - Uses consistent error handling with appropriate exit codes
   - Follows the success/error message formatting standards
+  - Uses `UniversalResponseHandler` for response processing with `enable_pagination=True`
+  - Uses `table_utils.output_formatted_list()` for table output
+  - Implements interactive pagination for table results (Y/n prompts for next 25 results)
 - If you update the CLI interface, update the `README.md` accordingly.
 - Never commit or suggest committing files listed in `.gitignore`.
 - When implementing list commands, ensure JSON output shows all results (no pagination).
 - Use `handle_api_error()` for all API error handling instead of generic Click exceptions.
 - Use `format_success()` for all success messages to maintain consistency.
+- For type compatibility with Pylance, use `: Any = MockResponse()` pattern for mock responses.
 
 ---
 

slcli/cli_formatters.py

@@ -0,0 +1,210 @@
+"""Unified table formatters for all CLI resource types."""
+
+from datetime import datetime
+from typing import Any, Dict, List
+
+
+def format_templates_table(template: Dict[str, Any]) -> List[str]:
+    """Format template data for table display."""
+    name = template.get("name", "Unknown")
+    version = template.get("version", "N/A")
+    created = _format_timestamp(template.get("createdTimestamp"))
+    status = template.get("status", "Unknown")
+
+    return [name, version, created, status]
+
+
+def format_users_table(user: Dict[str, Any]) -> List[str]:
+    """Format user data for table display."""
+    username = user.get("username", "Unknown")
+    email = user.get("email", "N/A")
+    role = user.get("role", "N/A")
+    status = "Active" if user.get("enabled", True) else "Disabled"
+
+    return [username, email, role, status]
+
+
+def format_workflows_table(workflow: Dict[str, Any]) -> List[str]:
+    """Format workflow data for table display."""
+    name = workflow.get("name", "Unknown")
+    status = workflow.get("status", "Unknown")
+    last_run = _format_timestamp(workflow.get("lastRunTimestamp"))
+    duration = _format_duration(workflow.get("lastRunDuration"))
+
+    return [name, status, last_run, duration]
+
+
+def format_notebooks_table(notebook: Dict[str, Any]) -> List[str]:
+    """Format notebook data for table display."""
+    name = notebook.get("name", "Unknown")
+    notebook_type = notebook.get("type", "Unknown")
+    modified = _format_timestamp(notebook.get("modifiedTimestamp"))
+    size = _format_file_size(notebook.get("size", 0))
+
+    return [name, notebook_type, modified, size]
+
+
+def format_workspaces_table(workspace: Dict[str, Any]) -> List[str]:
+    """Format workspace data for table display."""
+    name = workspace.get("name", "Unknown")
+    workspace_type = workspace.get("type", "Unknown")
+    file_count = str(workspace.get("fileCount", 0))
+    modified = _format_timestamp(workspace.get("modifiedTimestamp"))
+
+    return [name, workspace_type, file_count, modified]
+
+
+def format_dff_files_table(file_info: Dict[str, Any]) -> List[str]:
+    """Format DFF file data for table display."""
+    name = file_info.get("name", "Unknown")
+    size = _format_file_size(file_info.get("size", 0))
+    modified = _format_timestamp(file_info.get("modifiedTimestamp"))
+    file_type = file_info.get("type", "file").title()
+
+    return [name, size, modified, file_type]
+
+
+def format_dff_data_table(data_info: Dict[str, Any]) -> List[str]:
+    """Format DFF data entry for table display."""
+    id_val = data_info.get("id", "Unknown")
+    name = data_info.get("name", "N/A")
+    created = _format_timestamp(data_info.get("createdTimestamp"))
+    size = _format_file_size(data_info.get("size", 0))
+
+    return [id_val, name, created, size]
+
+
+def format_tags_table(tag: Dict[str, Any]) -> List[str]:
+    """Format tag data for table display."""
+    name = tag.get("name", "Unknown")
+    tag_type = tag.get("type", "Unknown")
+    count = str(tag.get("dataCount", 0))
+    created = _format_timestamp(tag.get("createdTimestamp"))
+
+    return [name, tag_type, count, created]
+
+
+def format_systems_table(system: Dict[str, Any]) -> List[str]:
+    """Format system data for table display."""
+    name = system.get("name", "Unknown")
+    status = system.get("status", "Unknown")
+    ip_address = system.get("ipAddress", "N/A")
+    last_seen = _format_timestamp(system.get("lastSeenTimestamp"))
+
+    return [name, status, ip_address, last_seen]
+
+
+def format_assets_table(asset: Dict[str, Any]) -> List[str]:
+    """Format asset data for table display."""
+    name = asset.get("name", "Unknown")
+    asset_type = asset.get("type", "Unknown")
+    location = asset.get("location", "N/A")
+    status = asset.get("status", "Unknown")
+
+    return [name, asset_type, location, status]
+
+
+def _format_timestamp(timestamp: Any) -> str:
+    """Format timestamp for table display."""
+    if not timestamp:
+        return "N/A"
+
+    try:
+        if isinstance(timestamp, str):
+            # Handle ISO format timestamps
+            dt = datetime.fromisoformat(timestamp.replace("Z", "+00:00"))
+        elif isinstance(timestamp, (int, float)):
+            # Handle Unix timestamps
+            dt = datetime.fromtimestamp(timestamp)
+        else:
+            return "N/A"
+
+        # Return relative time if recent, otherwise date
+        now = datetime.now()
+        diff = now - dt.replace(tzinfo=None)
+
+        if diff.days == 0:
+            return dt.strftime("%H:%M")
+        elif diff.days < 7:
+            return f"{diff.days}d ago"
+        else:
+            return dt.strftime("%Y-%m-%d")
+
+    except (ValueError, TypeError, AttributeError):
+        return "N/A"
+
+
+def _format_duration(duration: Any) -> str:
+    """Format duration for table display."""
+    if not duration:
+        return "N/A"
+
+    try:
+        if isinstance(duration, str):
+            return duration
+        elif isinstance(duration, (int, float)):
+            # Convert seconds to readable format
+            if duration < 60:
+                return f"{duration:.1f}s"
+            elif duration < 3600:
+                return f"{duration // 60:.0f}m {duration % 60:.0f}s"
+            else:
+                hours = duration // 3600
+                minutes = (duration % 3600) // 60
+                return f"{hours:.0f}h {minutes:.0f}m"
+        else:
+            return "N/A"
+    except (ValueError, TypeError):
+        return "N/A"
+
+
+def _format_file_size(size: Any) -> str:
+    """Format file size for table display."""
+    if not size or size == 0:
+        return "0 B"
+
+    try:
+        size = float(size)
+        units = ["B", "KB", "MB", "GB", "TB"]
+        unit_index = 0
+
+        while size >= 1024 and unit_index < len(units) - 1:
+            size /= 1024
+            unit_index += 1
+
+        if unit_index == 0:
+            return f"{size:.0f} {units[unit_index]}"
+        else:
+            return f"{size:.1f} {units[unit_index]}"
+
+    except (ValueError, TypeError):
+        return "N/A"
+
+
+# Formatter mapping for dynamic lookup
+FORMATTER_MAP = {
+    "template": format_templates_table,
+    "templates": format_templates_table,
+    "user": format_users_table,
+    "users": format_users_table,
+    "workflow": format_workflows_table,
+    "workflows": format_workflows_table,
+    "notebook": format_notebooks_table,
+    "notebooks": format_notebooks_table,
+    "workspace": format_workspaces_table,
+    "workspaces": format_workspaces_table,
+    "file": format_dff_files_table,
+    "files": format_dff_files_table,
+    "data": format_dff_data_table,
+    "tag": format_tags_table,
+    "tags": format_tags_table,
+    "system": format_systems_table,
+    "systems": format_systems_table,
+    "asset": format_assets_table,
+    "assets": format_assets_table,
+}
+
+
+def get_formatter(resource_type: str):
+    """Get the appropriate formatter function for a resource type."""
+    return FORMATTER_MAP.get(resource_type.lower())