Skip to content

Commit

Permalink
remove sky jobs launch --fast (skypilot-org#4467)
Browse files Browse the repository at this point in the history
* remove sky jobs launch --fast

The --fast behavior is now always enabled. This was unsafe before but since
\skypilot-org#4289 it should be safe.

We will remove the flag before 0.8.0 so that it never touches a stable version.

sky launch still has the --fast flag. This flag is unsafe because it could cause
setup to be skipped even though it should be re-run. In the managed jobs case,
this is not an issue because we fully control the setup and know it will not
change.

* fix lint
  • Loading branch information
cg505 authored Dec 16, 2024
1 parent f76db0d commit f0ebf13
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 19 deletions.
24 changes: 15 additions & 9 deletions sky/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -3601,15 +3601,12 @@ def jobs():
default=False,
required=False,
help='Skip confirmation prompt.')
# TODO(cooperc): remove this flag once --fast can robustly detect cluster
# yaml config changes
# TODO(cooperc): remove this flag before releasing 0.8.0
@click.option('--fast',
default=False,
is_flag=True,
help='[Experimental] Launch the job faster by skipping '
'controller initialization steps. If you update SkyPilot or '
'your local cloud credentials, they will not be reflected until '
'you run `sky jobs launch` at least once without this flag.')
help=('[Deprecated] Does nothing. Previous flag behavior is now '
'enabled by default.'))
@timeline.event
@usage_lib.entrypoint
def jobs_launch(
Expand All @@ -3634,7 +3631,7 @@ def jobs_launch(
disk_tier: Optional[str],
ports: Tuple[str],
detach_run: bool,
retry_until_up: bool,
retry_until_up: Optional[bool],
yes: bool,
fast: bool,
):
Expand Down Expand Up @@ -3692,6 +3689,16 @@ def jobs_launch(
else:
retry_until_up = True

# Deprecation. The default behavior is fast, and the flag will be removed.
# The flag was not present in 0.7.x (only nightly), so we will remove before
# 0.8.0 so that it never enters a stable release.
if fast:
click.secho(
'Flag --fast is deprecated, as the behavior is now default. The '
'flag will be removed soon. Please do not use it, so that you '
'avoid "No such option" errors.',
fg='yellow')

if not isinstance(task_or_dag, sky.Dag):
assert isinstance(task_or_dag, sky.Task), task_or_dag
with sky.Dag() as dag:
Expand Down Expand Up @@ -3733,8 +3740,7 @@ def jobs_launch(
managed_jobs.launch(dag,
name,
detach_run=detach_run,
retry_until_up=retry_until_up,
fast=fast)
retry_until_up=retry_until_up)


@jobs.command('queue', cls=_DocumentedCodeCommand)
Expand Down
20 changes: 10 additions & 10 deletions sky/jobs/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,13 @@
@timeline.event
@usage_lib.entrypoint
def launch(
task: Union['sky.Task', 'sky.Dag'],
name: Optional[str] = None,
stream_logs: bool = True,
detach_run: bool = False,
retry_until_up: bool = False,
fast: bool = False,
task: Union['sky.Task', 'sky.Dag'],
name: Optional[str] = None,
stream_logs: bool = True,
detach_run: bool = False,
retry_until_up: bool = False,
# TODO(cooperc): remove fast arg before 0.8.0
fast: bool = True, # pylint: disable=unused-argument for compatibility
) -> None:
# NOTE(dev): Keep the docstring consistent between the Python API and CLI.
"""Launch a managed job.
Expand All @@ -54,9 +55,8 @@ def launch(
managed job.
name: Name of the managed job.
detach_run: Whether to detach the run.
fast: Whether to use sky.launch(fast=True) for the jobs controller. If
True, the SkyPilot wheel and the cloud credentials may not be updated
on the jobs controller.
fast: [Deprecated] Does nothing, and will be removed soon. We will
always use fast mode as it's fully safe now.
Raises:
ValueError: cluster does not exist. Or, the entrypoint is not a valid
Expand Down Expand Up @@ -149,7 +149,7 @@ def launch(
idle_minutes_to_autostop=skylet_constants.
CONTROLLER_IDLE_MINUTES_TO_AUTOSTOP,
retry_until_up=True,
fast=fast,
fast=True,
_disable_controller_check=True)


Expand Down

0 comments on commit f0ebf13

Please sign in to comment.