From a4ddaa165dcba6a6e0c62acde163f082169d2c68 Mon Sep 17 00:00:00 2001 From: Jean Luciano Date: Thu, 13 Feb 2025 15:22:54 -0600 Subject: [PATCH 1/2] delete command and remove flag added --- src/prefect_cloud/cli/root.py | 39 ++++++------ src/prefect_cloud/client.py | 25 +++----- src/prefect_cloud/deployments.py | 21 +++---- tests/test_client.py | 26 -------- tests/test_schedules.py | 104 ++++++++++++++++++------------- 5 files changed, 94 insertions(+), 121 deletions(-) diff --git a/src/prefect_cloud/cli/root.py b/src/prefect_cloud/cli/root.py index 234af1e..8cc57fd 100644 --- a/src/prefect_cloud/cli/root.py +++ b/src/prefect_cloud/cli/root.py @@ -242,7 +242,7 @@ async def schedule( help="Name or ID of the deployment to schedule", autocompletion=completions.complete_deployment, ), - schedule: str = typer.Argument( + schedule: str = typer.Option( ..., help="Cron schedule string or 'none' to unschedule", ), @@ -253,6 +253,12 @@ async def schedule( help="Flow Run parameters in NAME=VALUE format", default_factory=list, ), + remove: bool = typer.Option( + False, + "--remove", + "-r", + help="Remove the schedule", + ), ): """ Set a deployment to run on a schedule @@ -267,8 +273,13 @@ async def schedule( Remove schedule: $ prefect-cloud schedule flow_name/deployment_name none """ - parameters_dict = process_key_value_pairs(parameters) if parameters else {} - await deployments.schedule(deployment, schedule, parameters_dict) + if remove: + await deployments.schedule(deployment, "none") + else: + if not schedule: + exit_with_error("Schedule cannot be empty") + parameters_dict = process_key_value_pairs(parameters) if parameters else {} + await deployments.schedule(deployment, schedule, parameters_dict) @app.command(rich_help_panel="Manage Deployments") @@ -329,31 +340,17 @@ def describe_schedule(schedule: DeploymentSchedule) -> Text: @app.command(rich_help_panel="Manage Deployments") -async def pause( - deployment: str = typer.Argument( - ..., - help="Name or ID of the deployment to pause", - autocompletion=completions.complete_deployment, - ), -): - """ - Pause a scheduled deployment - """ - await deployments.pause(deployment) - - -@app.command(rich_help_panel="Manage Deployments") -async def resume( +async def delete( deployment: str = typer.Argument( ..., - help="Name or ID of the deployment to resume", + help="Name or ID of the deployment to delete", autocompletion=completions.complete_deployment, ), ): """ - Resume a paused deployment + Delete a deployment """ - await deployments.resume(deployment) + await deployments.delete(deployment) @app.command(rich_help_panel="Auth") diff --git a/src/prefect_cloud/client.py b/src/prefect_cloud/client.py index 6435327..9285add 100644 --- a/src/prefect_cloud/client.py +++ b/src/prefect_cloud/client.py @@ -183,6 +183,15 @@ async def create_deployment( return UUID(deployment_id) + async def delete_deployment(self, deployment_id: "UUID"): + try: + await self.request("DELETE", f"/deployments/{deployment_id}") + except HTTPStatusError as e: + if e.response.status_code == 404: + raise ObjectNotFound(http_exc=e) from e + else: + raise + async def create_block_document( self, block_document: "BlockDocument | BlockDocumentCreate", @@ -800,22 +809,6 @@ async def create_credentials_secret(self, name: str, credentials: str): ) ) - async def pause_deployment(self, deployment_id: UUID): - deployment = await self.read_deployment(deployment_id) - - for schedule in deployment.schedules: - await self.update_deployment_schedule_active( - deployment.id, schedule.id, active=False - ) - - async def resume_deployment(self, deployment_id: UUID): - deployment = await self.read_deployment(deployment_id) - - for schedule in deployment.schedules: - await self.update_deployment_schedule_active( - deployment.id, schedule.id, active=True - ) - async def get_default_base_job_template_for_managed_work_pool( self, ) -> Optional[Dict[str, Any]]: diff --git a/src/prefect_cloud/deployments.py b/src/prefect_cloud/deployments.py index 664e201..ea40453 100644 --- a/src/prefect_cloud/deployments.py +++ b/src/prefect_cloud/deployments.py @@ -47,6 +47,13 @@ async def _get_deployment(deployment_: str) -> DeploymentResponse: return await client.read_deployment(deployment_id) +async def delete(deployment_: str): + deployment = await _get_deployment(deployment_) + + async with await get_prefect_cloud_client() as client: + await client.delete_deployment(deployment.id) + + async def run(deployment_: str) -> DeploymentFlowRun: deployment = await _get_deployment(deployment_) @@ -74,17 +81,3 @@ async def schedule( await client.create_deployment_schedule( deployment.id, new_schedule, True, parameters ) - - -async def pause(deployment_: str): - deployment = await _get_deployment(deployment_) - - async with await get_prefect_cloud_client() as client: - await client.pause_deployment(deployment.id) - - -async def resume(deployment_: str): - deployment = await _get_deployment(deployment_) - - async with await get_prefect_cloud_client() as client: - await client.resume_deployment(deployment.id) diff --git a/tests/test_client.py b/tests/test_client.py index 8437479..31f1fb0 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -197,32 +197,6 @@ async def test_create_deployment_schedule( assert result.schedule == schedule -async def test_pause_deployment( - client: PrefectCloudClient, - mock_deployment: DeploymentResponse, - respx_mock: respx.Router, -): - schedule = DeploymentSchedule( - id=uuid4(), - deployment_id=mock_deployment.id, - schedule=CronSchedule(cron="0 0 * * *", timezone="UTC"), - active=True, - ) - mock_deployment.schedules = [schedule] - - respx_mock.get(f"{PREFECT_API_URL}/deployments/{mock_deployment.id}").mock( - return_value=Response(200, json=mock_deployment.model_dump(mode="json")) - ) - patch_route = respx_mock.patch( - f"{PREFECT_API_URL}/deployments/{mock_deployment.id}/schedules/{schedule.id}" - ).mock(return_value=Response(204)) - - await client.pause_deployment(mock_deployment.id) - - assert patch_route.called - assert patch_route.calls.last.request.content == b'{"active":false}' - - async def test_get_default_base_job_template_for_managed_work_pool( client: PrefectCloudClient, respx_mock: respx.Router, diff --git a/tests/test_schedules.py b/tests/test_schedules.py index f422741..7e2875b 100644 --- a/tests/test_schedules.py +++ b/tests/test_schedules.py @@ -5,6 +5,7 @@ from httpx import Response from prefect_cloud import deployments +from prefect_cloud.client import ObjectNotFound from prefect_cloud.schemas.objects import ( CronSchedule, DeploymentFlowRun, @@ -215,50 +216,6 @@ async def test_schedule_none_removes_all_schedules( assert len(cloud_api.calls) == 2 # Only get and delete, no create -async def test_pause_deployment( - cloud_api: respx.Router, - mock_deployment_with_schedule: DeploymentResponse, - api_url: str, -): - cloud_api.get(f"{api_url}/deployments/{mock_deployment_with_schedule.id}").mock( - return_value=Response( - 200, json=mock_deployment_with_schedule.model_dump(mode="json") - ) - ) - patch_route = cloud_api.patch( - f"{api_url}" - f"/deployments/{mock_deployment_with_schedule.id}" - f"/schedules/{mock_deployment_with_schedule.schedules[0].id}" - ).mock(return_value=Response(204)) - - await deployments.pause(str(mock_deployment_with_schedule.id)) - - assert patch_route.called - assert patch_route.calls.last.request.content == b'{"active":false}' - - -async def test_resume_deployment( - cloud_api: respx.Router, - mock_deployment_with_schedule: DeploymentResponse, - api_url: str, -): - cloud_api.get(f"{api_url}/deployments/{mock_deployment_with_schedule.id}").mock( - return_value=Response( - 200, json=mock_deployment_with_schedule.model_dump(mode="json") - ) - ) - patch_route = cloud_api.patch( - f"{api_url}" - f"/deployments/{mock_deployment_with_schedule.id}" - f"/schedules/{mock_deployment_with_schedule.schedules[0].id}" - ).mock(return_value=Response(204)) - - await deployments.resume(str(mock_deployment_with_schedule.id)) - - assert patch_route.called - assert patch_route.calls.last.request.content == b'{"active":true}' - - async def test_list_returns_empty_context_when_no_deployments( cloud_api: respx.Router, api_url: str ): @@ -350,3 +307,62 @@ async def test_schedule_accepts_parameters( assert schedule_route.called request_body = schedule_route.calls.last.request.read().decode() assert '"parameters":{"key":"value"}' in request_body + + +async def test_delete_deployment( + cloud_api: respx.Router, mock_deployment: DeploymentResponse, api_url: str +): + """Test that a deployment can be deleted""" + # Mock the GET request to verify deployment exists + cloud_api.get(f"{api_url}/deployments/{mock_deployment.id}").mock( + return_value=Response(200, json=mock_deployment.model_dump(mode="json")) + ) + + # Mock the DELETE request + delete_route = cloud_api.delete(f"{api_url}/deployments/{mock_deployment.id}").mock( + return_value=Response(204) + ) + + await deployments.delete(str(mock_deployment.id)) + + assert delete_route.called + assert ( + delete_route.calls.last.request.url + == f"{api_url}/deployments/{mock_deployment.id}" + ) + + +async def test_delete_deployment_by_name( + cloud_api: respx.Router, mock_deployment: DeploymentResponse, api_url: str +): + """Test that a deployment can be deleted using flow_name/deployment_name format""" + # Mock the GET request for name lookup + cloud_api.get(f"{api_url}/deployments/name/my-flow/my-deployment").mock( + return_value=Response(200, json=mock_deployment.model_dump(mode="json")) + ) + + # Mock the DELETE request + delete_route = cloud_api.delete(f"{api_url}/deployments/{mock_deployment.id}").mock( + return_value=Response(204) + ) + + await deployments.delete("my-flow/my-deployment") + + assert delete_route.called + assert ( + delete_route.calls.last.request.url + == f"{api_url}/deployments/{mock_deployment.id}" + ) + + +async def test_delete_nonexistent_deployment(cloud_api: respx.Router, api_url: str): + """Test that deleting a nonexistent deployment raises ObjectNotFound""" + deployment_id = "11111111-1111-1111-1111-111111111111" + + # Mock 404 response for nonexistent deployment + cloud_api.get(f"{api_url}/deployments/{deployment_id}").mock( + return_value=Response(404, json={"detail": "Deployment not found"}) + ) + + with pytest.raises(ObjectNotFound): + await deployments.delete(deployment_id) From a502ddd81a320c21abec281140b293418c365294 Mon Sep 17 00:00:00 2001 From: Jean Luciano Date: Fri, 14 Feb 2025 13:12:41 -0600 Subject: [PATCH 2/2] moved remove to its own command --- src/prefect_cloud/cli/root.py | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/prefect_cloud/cli/root.py b/src/prefect_cloud/cli/root.py index 8cc57fd..7996345 100644 --- a/src/prefect_cloud/cli/root.py +++ b/src/prefect_cloud/cli/root.py @@ -242,7 +242,7 @@ async def schedule( help="Name or ID of the deployment to schedule", autocompletion=completions.complete_deployment, ), - schedule: str = typer.Option( + schedule: str = typer.Argument( ..., help="Cron schedule string or 'none' to unschedule", ), @@ -253,12 +253,6 @@ async def schedule( help="Flow Run parameters in NAME=VALUE format", default_factory=list, ), - remove: bool = typer.Option( - False, - "--remove", - "-r", - help="Remove the schedule", - ), ): """ Set a deployment to run on a schedule @@ -273,13 +267,22 @@ async def schedule( Remove schedule: $ prefect-cloud schedule flow_name/deployment_name none """ - if remove: - await deployments.schedule(deployment, "none") - else: - if not schedule: - exit_with_error("Schedule cannot be empty") - parameters_dict = process_key_value_pairs(parameters) if parameters else {} - await deployments.schedule(deployment, schedule, parameters_dict) + + parameters_dict = process_key_value_pairs(parameters) if parameters else {} + await deployments.schedule(deployment, schedule, parameters_dict) + + +@app.command(rich_help_panel="Manage Deployments") +async def deschedule( + deployment: str = typer.Argument( + ..., + help="Name or ID of the deployment to remove schedules from", + ), +): + """ + Remove deployment schedules + """ + await deployments.schedule(deployment, "none") @app.command(rich_help_panel="Manage Deployments")