Skip to content

Commit

Permalink
refactor due to reviewer's suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
andylizf committed Oct 6, 2024
1 parent 1512cba commit 15f14db
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 27 deletions.
14 changes: 9 additions & 5 deletions sky/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -4332,7 +4332,7 @@ def serve_status(all: bool, endpoint: bool, service_names: List[str]):
required=False,
help='Skip confirmation prompt.')
@click.option('--replica-id',
'-r',
'-rid',
default=None,
type=int,
help='Tear down a given replica')
Expand Down Expand Up @@ -4379,11 +4379,15 @@ def serve_down(service_names: List[str], all: bool, purge: bool, yes: bool,
raise click.UsageError(
'Can only specify one of SERVICE_NAMES or --all. '
f'Provided {argument_str!r}.')

replica_id_is_defined = replica_id is not None
if replica_id_is_defined and len(service_names) != 1:
raise click.UsageError(
'Must specify one and only one service when replica ID is '
'specified.')
if replica_id_is_defined:
if len(service_names) != 1:
raise click.UsageError('The --replica-id option can only be used '
'with a single service name.')
if all:
raise click.UsageError('The --replica-id option cannot be used '
'with the --all option.')

backend_utils.is_controller_accessible(
controller=controller_utils.Controllers.SKY_SERVE_CONTROLLER,
Expand Down
30 changes: 12 additions & 18 deletions sky/serve/controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,38 +180,32 @@ async def terminate_replica(request: fastapi.Request):
request_data = await request.json()
try:
replica_id = request_data.get('replica_id')
if replica_id is None:
return {
'code': 400,
'message': 'Error: replica ID is not specified.'
}
assert isinstance(replica_id,
int), 'Error: replica ID is not specified.'
purge = request_data.get('purge')
if purge is None:
return {
'code': 400,
'message': 'Error: purge is not specified.'
}
assert isinstance(purge, bool), 'Error: purge is not specified.'
replica_info = serve_state.get_replica_info_from_id(
self._service_name, replica_id)
if replica_info is None:
return {
'code': 400,
'message': f'Error: replica {replica_id} '
f'does not exist.'
}
assert replica_info is not None, f'Error: replica ' \
f'{replica_id} does not exist.'

if purge:
return self._purge_replica(replica_info)

logger.info(f'Terminating replica {replica_id}...')
self._replica_manager.scale_down(replica_id)
return {'message': f'Success terminating replica {replica_id}.'}
return fastapi.Response(
status_code=200,
content={
'message': f'Success terminating replica {replica_id}.'
})

except Exception as e: # pylint: disable=broad-except
error_message = (f'Error in terminate_replica: '
f'{common_utils.format_exception(e)}')
logger.error(error_message)
return {'code': 500, 'message': error_message}
return fastapi.Response(status_code=500,
content={'message': error_message})

@self._app.on_event('startup')
def configure_logger():
Expand Down
7 changes: 4 additions & 3 deletions sky/serve/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,8 @@ def update(
'Service controller is stopped. There is no service to update. '
f'To spin up a new service, use {backend_utils.BOLD}'
f'sky serve up{backend_utils.RESET_BOLD}',
non_existent_message='To spin up a new service, '
non_existent_message='Service does not exist. '
'To spin up a new service, '
f'use {backend_utils.BOLD}sky serve up{backend_utils.RESET_BOLD}',
)

Expand Down Expand Up @@ -521,8 +522,8 @@ def terminate_replica(service_name: str, replica_id: int, purge: bool) -> None:
controller=controller_utils.Controllers.SKY_SERVE_CONTROLLER,
stopped_message=
'No service is running now. Please spin up a service first.',
non_existent_message='To spin up a new service, '
f'use {backend_utils.BOLD}sky serve up{backend_utils.RESET_BOLD}',
non_existent_message='No service is running now. '
'Please spin up a service first.',
)

backend = backend_utils.get_backend_from_handle(handle)
Expand Down
6 changes: 5 additions & 1 deletion sky/serve/serve_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -313,8 +313,12 @@ def terminate_replica(service_name: str, replica_id: int, purge: bool) -> str:
service_status = _get_service_status(service_name)
if service_status is None:
raise ValueError(f'Service {service_name!r} does not exist.')
controller_port = service_status['controller_port']
replica_info = serve_state.get_replica_info_from_id(service_name,
replica_id)
if replica_info is None:
raise ValueError(f'Replica {replica_id} does not exist.')

controller_port = service_status['controller_port']
resp = requests.post(
_CONTROLLER_URL.format(CONTROLLER_PORT=controller_port) +
'/controller/terminate_replica',
Expand Down

0 comments on commit 15f14db

Please sign in to comment.