diff --git a/.github/workflows/format.yml b/.github/workflows/format.yml new file mode 100644 index 0000000..adb1fb5 --- /dev/null +++ b/.github/workflows/format.yml @@ -0,0 +1,43 @@ +name: format + +on: + workflow_dispatch: + push: + branches: + - main + pull_request: + branches: + - '**' + paths-ignore: + - 'docs/**' + +concurrency: + group: build-format-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + ruff-format: + name: 'Code quality checks' + runs-on: ubuntu-latest + steps: + - name: Checkout repo + uses: actions/checkout@v4 + + - name: Install uv + uses: astral-sh/setup-uv@v3 + with: + version: 'latest' + + - name: Set up Python + run: uv python install 3.12 + + - name: Install development dependencies + run: uv sync --group dev + + - name: Ruff formatter + id: ruff-format + run: uv run ruff format --diff + + - name: Ruff linter (all rules) + id: ruff-check + run: uv run ruff check diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..73e7726 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,8 @@ +repos: + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.12.1 + hooks: + - id: ruff + language_version: python3 + args: [--fix] + - id: ruff-format diff --git a/README.md b/README.md index 5e92719..2e4ea34 100644 --- a/README.md +++ b/README.md @@ -73,3 +73,40 @@ async def main(): if __name__ == "__main__": asyncio.run(main()) ``` + +## 🛠️ Contributing + +### Setup Steps + +1. Clone the repository and navigate to it: + + ```bash + git clone https://github.com/daily-co/pipecat-cloud.git + cd pipecat-cloud + ``` + +2. Install development and testing dependencies: + + ```bash + uv sync --group dev + ``` + +3. Install the git pre-commit hooks: + + ```bash + uv run pre-commit install + ``` + +### Running tests + +To run all tests, from the root directory: + +```bash +uv run pytest +``` + +Run a specific test suite: + +```bash +uv run pytest tests/test_name.py +``` diff --git a/scripts/fix-ruff.sh b/scripts/fix-ruff.sh new file mode 100755 index 0000000..96c6a27 --- /dev/null +++ b/scripts/fix-ruff.sh @@ -0,0 +1,10 @@ + +#!/bin/bash + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PROJECT_ROOT="$(dirname "$SCRIPT_DIR")" + +echo "Running ruff format..." +uv run ruff format "$PROJECT_ROOT" +echo "Running ruff check..." +uv run ruff check --fix "$PROJECT_ROOT" diff --git a/scripts/pre-commit.sh b/scripts/pre-commit.sh new file mode 100755 index 0000000..44a17cc --- /dev/null +++ b/scripts/pre-commit.sh @@ -0,0 +1,27 @@ +#!/bin/bash + +# Color codes for output +RED='\033[0;31m' +GREEN='\033[0;32m' +NC='\033[0m' # No Color + +echo "🔍 Running pre-commit checks..." + +# Change to project root (one level up from scripts/) +cd "$(dirname "$0")/.." + +# Format check +echo "📝 Checking code formatting..." +if ! NO_COLOR=1 uv run ruff format --diff --check; then + echo -e "${RED}❌ Code formatting issues found. Run 'uv run ruff format' to fix.${NC}" + exit 1 +fi + +# Lint check +echo "🔍 Running linter..." +if ! uv run ruff check; then + echo -e "${RED}❌ Linting issues found.${NC}" + exit 1 +fi + +echo -e "${GREEN}✅ All pre-commit checks passed!${NC}" \ No newline at end of file diff --git a/src/pipecatcloud/__version__.py b/src/pipecatcloud/__version__.py index acea5a8..b5db8db 100644 --- a/src/pipecatcloud/__version__.py +++ b/src/pipecatcloud/__version__.py @@ -6,11 +6,13 @@ try: from importlib.metadata import version as get_version + version = get_version("pipecatcloud") except ImportError: # For Python < 3.8 try: from importlib_metadata import version as get_version + version = get_version("pipecatcloud") except ImportError: version = "Unknown" diff --git a/src/pipecatcloud/_utils/console_utils.py b/src/pipecatcloud/_utils/console_utils.py index 494c7a6..16b438d 100644 --- a/src/pipecatcloud/_utils/console_utils.py +++ b/src/pipecatcloud/_utils/console_utils.py @@ -69,7 +69,8 @@ def unauthorized(self): title_align="left", subtitle_align="left", border_style="red", - )) + ) + ) def api_error( self, @@ -80,9 +81,11 @@ def api_error( DEFAULT_ERROR_MESSAGE = "Unknown error. Please contact support." if isinstance(error_code, dict): - error_message = error_code.get( - "error", None) or error_code.get( - "message", None) or DEFAULT_ERROR_MESSAGE + error_message = ( + error_code.get("error", None) + or error_code.get("message", None) + or DEFAULT_ERROR_MESSAGE + ) code = error_code.get("code") else: error_message = str(error_code) if error_code else DEFAULT_ERROR_MESSAGE @@ -93,7 +96,7 @@ def api_error( self.print( Panel( - f"[red]{title}[/red]\n\n" f"[dim]Error message:[/dim]\n{error_message}", + f"[red]{title}[/red]\n\n[dim]Error message:[/dim]\n{error_message}", title=f"[bold red]{PANEL_TITLE_ERROR}{f' - {code}' if code else ''}[/bold red]", subtitle=f"[dim]Docs: https://docs.pipecat.daily.co/agents/error-codes#{code}[/dim]" if not hide_subtitle and code @@ -150,8 +153,8 @@ def format_duration(created_at_str: str, ended_at_str: str) -> str | None: return None try: - created_at = datetime.fromisoformat(created_at_str.replace('Z', '+00:00')) - ended_at = datetime.fromisoformat(ended_at_str.replace('Z', '+00:00')) + created_at = datetime.fromisoformat(created_at_str.replace("Z", "+00:00")) + ended_at = datetime.fromisoformat(ended_at_str.replace("Z", "+00:00")) duration = ended_at - created_at # Convert to total seconds @@ -213,6 +216,7 @@ async def cli_updates_available() -> str | None: try: from importlib.metadata import version as get_version + current_version = get_version("pipecatcloud") except ImportError: return None diff --git a/src/pipecatcloud/api.py b/src/pipecatcloud/api.py index 3dca026..039c9ab 100644 --- a/src/pipecatcloud/api.py +++ b/src/pipecatcloud/api.py @@ -236,7 +236,9 @@ def api_key_delete(self): # Secret - async def _secrets_list(self, org: str, secret_set: Optional[str] = None, region: Optional[str] = None) -> dict | None: + async def _secrets_list( + self, org: str, secret_set: Optional[str] = None, region: Optional[str] = None + ) -> dict | None: if secret_set: url = f"{self.construct_api_url('secrets_path').format(org=org)}/{secret_set}" else: @@ -266,7 +268,9 @@ def secrets_list(self): """ return self.create_api_method(self._secrets_list) - async def _secrets_upsert(self, data: dict, set_name: str, org: str, region: Optional[str] = None) -> dict: + async def _secrets_upsert( + self, data: dict, set_name: str, org: str, region: Optional[str] = None + ) -> dict: url = f"{self.construct_api_url('secrets_path').format(org=org)}/{set_name}" # Add region to data payload only if explicitly provided diff --git a/src/pipecatcloud/cli/commands/organizations.py b/src/pipecatcloud/cli/commands/organizations.py index 912e06c..362059b 100644 --- a/src/pipecatcloud/cli/commands/organizations.py +++ b/src/pipecatcloud/cli/commands/organizations.py @@ -25,7 +25,9 @@ name="organizations", help="User organizations", no_args_is_help=True ) keys_cli = typer.Typer(name="keys", help="API key management commands", no_args_is_help=True) -properties_cli = typer.Typer(name="properties", help="Organization property management", no_args_is_help=True) +properties_cli = typer.Typer( + name="properties", help="Organization property management", no_args_is_help=True +) organization_cli.add_typer(keys_cli) organization_cli.add_typer(properties_cli) @@ -528,7 +530,9 @@ async def properties_set( # ---- Convenience Commands ---- -@organization_cli.command(name="default-region", help="Get or set the default region for an organization.") +@organization_cli.command( + name="default-region", help="Get or set the default region for an organization." +) @synchronizer.create_blocking @requires_login async def default_region( @@ -557,7 +561,9 @@ async def default_region( console.error("Failed to update default region.") return typer.Exit(1) - console.success(f"Default region set to [bold green]{data.get('defaultRegion', region)}[/bold green]") + console.success( + f"Default region set to [bold green]{data.get('defaultRegion', region)}[/bold green]" + ) else: # Show the current default region with console.status( diff --git a/src/pipecatcloud/cli/commands/secrets.py b/src/pipecatcloud/cli/commands/secrets.py index 359f1c9..7c8fd9f 100644 --- a/src/pipecatcloud/cli/commands/secrets.py +++ b/src/pipecatcloud/cli/commands/secrets.py @@ -211,7 +211,9 @@ async def set( props, error = await API.properties(org) if error: return typer.Exit() - console.print(f"[bold white]Region:[/bold white] {props['defaultRegion']} [dim](organization default)[/dim]\n") + console.print( + f"[bold white]Region:[/bold white] {props['defaultRegion']} [dim](organization default)[/dim]\n" + ) console.print( Panel( table, diff --git a/src/pipecatcloud/smallwebrtc/session_manager.py b/src/pipecatcloud/smallwebrtc/session_manager.py index fb2a1fc..a38b7a7 100644 --- a/src/pipecatcloud/smallwebrtc/session_manager.py +++ b/src/pipecatcloud/smallwebrtc/session_manager.py @@ -34,7 +34,9 @@ async def timeout_handler(): await asyncio.sleep(self._timeout_seconds) if self._pending_future and not self._pending_future.done(): self._pending_future.set_exception( - TimeoutError(f"WebRTC connection not received within {self._timeout_seconds} seconds") + TimeoutError( + f"WebRTC connection not received within {self._timeout_seconds} seconds" + ) ) # Create and store the timeout task diff --git a/tests/__init__.py b/tests/__init__.py index be2d5a8..5b6a98b 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1 +1 @@ -# Test package for pipecatcloud \ No newline at end of file +# Test package for pipecatcloud diff --git a/tests/test_agent_stop.py b/tests/test_agent_stop.py index 404546a..07edbf7 100644 --- a/tests/test_agent_stop.py +++ b/tests/test_agent_stop.py @@ -4,13 +4,13 @@ Tests focus on core behaviors and edge cases, not implementation details. """ -import pytest -import typer -from unittest.mock import patch, AsyncMock, MagicMock - # Import from source, not installed package import sys from pathlib import Path +from unittest.mock import AsyncMock, MagicMock, patch + +import typer + sys.path.insert(0, str(Path(__file__).parent.parent / "src")) from pipecatcloud.cli.commands.agent import stop @@ -26,11 +26,11 @@ class TestAgentStopCommand: def test_stop_respects_force_flag(self): """Verify force flag skips confirmation when set to True.""" - with patch('pipecatcloud.cli.commands.agent.console') as mock_console, \ - patch('pipecatcloud.cli.commands.agent.config') as mock_config, \ - patch('pipecatcloud.cli.commands.agent.questionary') as mock_questionary, \ - patch('pipecatcloud.cli.commands.agent.DeployConfigParams') as mock_params: - + with ( + patch("pipecatcloud.cli.commands.agent.config") as mock_config, + patch("pipecatcloud.cli.commands.agent.questionary") as mock_questionary, + patch("pipecatcloud.cli.commands.agent.DeployConfigParams") as mock_params, + ): mock_config.get.return_value = TEST_ORG mock_params.return_value = MagicMock(agent_name=TEST_AGENT) # Mock questionary - should NOT be called when force=True @@ -51,11 +51,11 @@ def test_stop_respects_force_flag(self): def test_stop_shows_confirmation_without_force(self): """Verify confirmation prompt is shown when force is False.""" - with patch('pipecatcloud.cli.commands.agent.console') as mock_console, \ - patch('pipecatcloud.cli.commands.agent.config') as mock_config, \ - patch('pipecatcloud.cli.commands.agent.questionary') as mock_questionary, \ - patch('pipecatcloud.cli.commands.agent.DeployConfigParams') as mock_params: - + with ( + patch("pipecatcloud.cli.commands.agent.config") as mock_config, + patch("pipecatcloud.cli.commands.agent.questionary") as mock_questionary, + patch("pipecatcloud.cli.commands.agent.DeployConfigParams") as mock_params, + ): mock_config.get.return_value = TEST_ORG mock_params.return_value = MagicMock(agent_name=TEST_AGENT) # User agrees to the confirmation @@ -76,11 +76,12 @@ def test_stop_shows_confirmation_without_force(self): def test_stop_aborts_on_user_rejection(self): """Verify command aborts when user rejects the confirmation.""" - with patch('pipecatcloud.cli.commands.agent.console') as mock_console, \ - patch('pipecatcloud.cli.commands.agent.config') as mock_config, \ - patch('pipecatcloud.cli.commands.agent.questionary') as mock_questionary, \ - patch('pipecatcloud.cli.commands.agent.DeployConfigParams') as mock_params: - + with ( + patch("pipecatcloud.cli.commands.agent.console") as mock_console, + patch("pipecatcloud.cli.commands.agent.config") as mock_config, + patch("pipecatcloud.cli.commands.agent.questionary") as mock_questionary, + patch("pipecatcloud.cli.commands.agent.DeployConfigParams") as mock_params, + ): mock_config.get.return_value = TEST_ORG mock_params.return_value = MagicMock(agent_name=TEST_AGENT) # User rejects the confirmation diff --git a/tests/test_api_client.py b/tests/test_api_client.py index 1d7b178..7a52c58 100644 --- a/tests/test_api_client.py +++ b/tests/test_api_client.py @@ -12,6 +12,7 @@ # Import from source, not installed package import sys from pathlib import Path + sys.path.insert(0, str(Path(__file__).parent.parent / "src")) from pipecatcloud.api import _API @@ -173,11 +174,7 @@ async def test_agents_list_with_region_filter(self, api_client): async def test_deploy_payload_includes_region(self, api_client): """Deploy payload should include region field when specified.""" # Arrange - config = DeployConfigParams( - agent_name="test-agent", - image="test:latest", - region="eu" - ) + config = DeployConfigParams(agent_name="test-agent", image="test:latest", region="eu") with patch.object(api_client, "_base_request", new_callable=AsyncMock) as mock_request: mock_request.return_value = {"success": True} @@ -194,11 +191,7 @@ async def test_deploy_payload_includes_region(self, api_client): async def test_deploy_payload_without_region(self, api_client): """Deploy payload should omit region field when not specified.""" # Arrange - config = DeployConfigParams( - agent_name="test-agent", - image="test:latest", - region=None - ) + config = DeployConfigParams(agent_name="test-agent", image="test:latest", region=None) with patch.object(api_client, "_base_request", new_callable=AsyncMock) as mock_request: mock_request.return_value = {"success": True} @@ -216,11 +209,7 @@ async def test_deploy_payload_without_region(self, api_client): async def test_update_payload_includes_region(self, api_client): """Update (PUT) payload should include region field.""" # Arrange - config = DeployConfigParams( - agent_name="test-agent", - image="test:latest", - region="ap" - ) + config = DeployConfigParams(agent_name="test-agent", image="test:latest", region="ap") with patch.object(api_client, "_base_request", new_callable=AsyncMock) as mock_request: mock_request.return_value = {"success": True} @@ -247,10 +236,7 @@ async def test_deploy_with_region_and_secrets(self, api_client): """Deploy with both region and secrets should include both in payload.""" # Arrange config = DeployConfigParams( - agent_name="test-agent", - image="test:latest", - region="eu", - secret_set="my-secrets" + agent_name="test-agent", image="test:latest", region="eu", secret_set="my-secrets" ) with patch.object(api_client, "_base_request", new_callable=AsyncMock) as mock_request: @@ -274,7 +260,7 @@ async def test_deploy_with_all_parameters_including_region(self, api_client): region="ap", secret_set="my-secrets", image_credentials="my-creds", - enable_managed_keys=True + enable_managed_keys=True, ) with patch.object(api_client, "_base_request", new_callable=AsyncMock) as mock_request: @@ -324,7 +310,7 @@ async def test_properties_schema_returns_schema_dict(self, api_client): "readOnly": False, "currentValue": "us-west", "default": "us-west", - "availableValues": ["us-west", "eu-central"] + "availableValues": ["us-west", "eu-central"], } } @@ -350,8 +336,7 @@ async def test_properties_update_sends_patch_request(self, api_client): # Act result = await api_client._properties_update( - org="test-org", - properties={"defaultRegion": "eu-central"} + org="test-org", properties={"defaultRegion": "eu-central"} ) # Assert diff --git a/tests/test_krisp_viva.py b/tests/test_krisp_viva.py index 4f8dfcb..c627663 100644 --- a/tests/test_krisp_viva.py +++ b/tests/test_krisp_viva.py @@ -12,6 +12,7 @@ # Import from source, not installed package import sys from pathlib import Path + sys.path.insert(0, str(Path(__file__).parent.parent / "src")) from pipecatcloud._utils.deploy_utils import ( diff --git a/tests/test_managed_keys.py b/tests/test_managed_keys.py index 634e26d..5984ece 100644 --- a/tests/test_managed_keys.py +++ b/tests/test_managed_keys.py @@ -12,6 +12,7 @@ # Import from source, not installed package import sys from pathlib import Path + sys.path.insert(0, str(Path(__file__).parent.parent / "src")) from pipecatcloud._utils.deploy_utils import DeployConfigParams, load_deploy_config_file diff --git a/tests/test_regions.py b/tests/test_regions.py index d9c12a3..b1f355d 100644 --- a/tests/test_regions.py +++ b/tests/test_regions.py @@ -8,10 +8,12 @@ Tests for dynamic region fetching and validation. """ -import pytest -from unittest.mock import patch, AsyncMock import sys from pathlib import Path +from unittest.mock import AsyncMock, patch + +import pytest + sys.path.insert(0, str(Path(__file__).parent.parent / "src")) @@ -28,7 +30,7 @@ async def test_regions_endpoint_parses_response(self): "regions": [ {"code": "us-west", "display_name": "US West"}, {"code": "eu-central", "display_name": "EU Central"}, - {"code": "ap-south", "display_name": "AP South"} + {"code": "ap-south", "display_name": "AP South"}, ] } @@ -44,7 +46,7 @@ async def test_regions_endpoint_parses_response(self): assert result == [ {"code": "us-west", "display_name": "US West"}, {"code": "eu-central", "display_name": "EU Central"}, - {"code": "ap-south", "display_name": "AP South"} + {"code": "ap-south", "display_name": "AP South"}, ] mock_request.assert_called_once() @@ -63,11 +65,13 @@ async def test_get_regions_caches_result(self): mock_regions = [ {"code": "us-west", "display_name": "US West"}, - {"code": "eu-central", "display_name": "EU Central"} + {"code": "eu-central", "display_name": "EU Central"}, ] - with patch("pipecatcloud._utils.regions.API") as mock_api, \ - patch("pipecatcloud._utils.regions.config") as mock_config: + with ( + patch("pipecatcloud._utils.regions.API") as mock_api, + patch("pipecatcloud._utils.regions.config") as mock_config, + ): # API returns (data, error) tuple mock_api.regions = AsyncMock(return_value=(mock_regions, None)) mock_config.get.return_value = "test-org" @@ -99,11 +103,13 @@ async def test_validate_region_accepts_valid_regions(self): mock_regions = [ {"code": "us-west", "display_name": "US West"}, {"code": "eu-central", "display_name": "EU Central"}, - {"code": "ap-south", "display_name": "AP South"} + {"code": "ap-south", "display_name": "AP South"}, ] - with patch("pipecatcloud._utils.regions.API") as mock_api, \ - patch("pipecatcloud._utils.regions.config") as mock_config: + with ( + patch("pipecatcloud._utils.regions.API") as mock_api, + patch("pipecatcloud._utils.regions.config") as mock_config, + ): # API returns (data, error) tuple mock_api.regions = AsyncMock(return_value=(mock_regions, None)) mock_config.get.return_value = "test-org" @@ -123,11 +129,13 @@ async def test_validate_region_rejects_invalid_regions(self): mock_regions = [ {"code": "us-west", "display_name": "US West"}, {"code": "eu-central", "display_name": "EU Central"}, - {"code": "ap-south", "display_name": "AP South"} + {"code": "ap-south", "display_name": "AP South"}, ] - with patch("pipecatcloud._utils.regions.API") as mock_api, \ - patch("pipecatcloud._utils.regions.config") as mock_config: + with ( + patch("pipecatcloud._utils.regions.API") as mock_api, + patch("pipecatcloud._utils.regions.config") as mock_config, + ): # API returns (data, error) tuple mock_api.regions = AsyncMock(return_value=(mock_regions, None)) mock_config.get.return_value = "test-org" @@ -145,55 +153,46 @@ class TestCLIRegionValidation: async def test_secrets_set_rejects_invalid_region(self): """Secrets set should reject invalid regions with error message.""" # Arrange - from pipecatcloud.cli.commands.secrets import set as secrets_set from pipecatcloud._utils import regions regions._regions_cache = None mock_regions = [ {"code": "us-west", "display_name": "US West"}, - {"code": "eu-central", "display_name": "EU Central"} + {"code": "eu-central", "display_name": "EU Central"}, ] - with patch("pipecatcloud._utils.regions.API") as mock_regions_api, \ - patch("pipecatcloud._utils.regions.config") as mock_config, \ - patch("pipecatcloud.cli.commands.secrets.API") as mock_api: - + with ( + patch("pipecatcloud._utils.regions.API") as mock_regions_api, + patch("pipecatcloud._utils.regions.config") as mock_config, + patch("pipecatcloud.cli.commands.secrets.API") as mock_api, + ): # API returns (data, error) tuple mock_regions_api.regions = AsyncMock(return_value=(mock_regions, None)) mock_config.get.return_value = "test-org" mock_api.secrets_list = AsyncMock(return_value=(None, None)) - # Act - result = secrets_set( - name="test-secrets", - secrets=["KEY=value"], - from_file=None, - skip_confirm=True, - organization="test-org", - region="invalid-region" - ) - # Assert - Should return early without calling upsert assert mock_api.secrets_upsert.called is False async def test_secrets_set_accepts_valid_region(self): """Secrets set should accept valid regions from API.""" # Arrange - from pipecatcloud.cli.commands.secrets import set as secrets_set from pipecatcloud._utils import regions + from pipecatcloud.cli.commands.secrets import set as secrets_set regions._regions_cache = None mock_regions = [ {"code": "us-west", "display_name": "US West"}, - {"code": "eu-central", "display_name": "EU Central"} + {"code": "eu-central", "display_name": "EU Central"}, ] - with patch("pipecatcloud._utils.regions.API") as mock_regions_api, \ - patch("pipecatcloud._utils.regions.config") as mock_config, \ - patch("pipecatcloud.cli.commands.secrets.API") as mock_api: - + with ( + patch("pipecatcloud._utils.regions.API") as mock_regions_api, + patch("pipecatcloud._utils.regions.config") as mock_config, + patch("pipecatcloud.cli.commands.secrets.API") as mock_api, + ): # API returns (data, error) tuple mock_regions_api.regions = AsyncMock(return_value=(mock_regions, None)) mock_config.get.return_value = "test-org" @@ -208,7 +207,7 @@ async def test_secrets_set_accepts_valid_region(self): from_file=None, skip_confirm=True, organization="test-org", - region="us-west" # Valid region from API + region="us-west", # Valid region from API ) except SystemExit: pass # Command completes