From a2b8371d29553e3fbb1c3ac97c0628ff2fff71ab Mon Sep 17 00:00:00 2001 From: Romil Bhardwaj Date: Fri, 23 Aug 2024 11:13:21 -0700 Subject: [PATCH 1/4] Fix wheel creation --- sky/backends/wheel_utils.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/sky/backends/wheel_utils.py b/sky/backends/wheel_utils.py index 254094ad81a..ece21b5ce3e 100644 --- a/sky/backends/wheel_utils.py +++ b/sky/backends/wheel_utils.py @@ -161,12 +161,19 @@ def _get_latest_modification_time(path: pathlib.Path) -> float: last_modification_time = _get_latest_modification_time(SKY_PACKAGE_PATH) last_wheel_modification_time = _get_latest_modification_time(WHEEL_DIR) - # only build wheels if the wheel is outdated - if last_wheel_modification_time < last_modification_time: + # Only build wheels if the wheel is outdated or wheel does not exist + # for the requested version. + if (last_wheel_modification_time < last_modification_time) or not any( + WHEEL_DIR.glob(_WHEEL_PATTERN)): if not WHEEL_DIR.exists(): WHEEL_DIR.mkdir(parents=True, exist_ok=True) _build_sky_wheel() + # We remove all wheels except the latest one for garbage collection. + # Otherwise stale wheels will accumulate over time. + # TODO(romilb): If the user switches versions every alternate launch, + # the wheel will be rebuilt every time. At the risk of adding + # complexity, we can consider TTL caching wheels by version here. latest_wheel = _get_latest_wheel_and_remove_all_others() wheel_hash = latest_wheel.parent.name From 0867903bc2c78ec7c981dd6542a45c3ccdbd4fc0 Mon Sep 17 00:00:00 2001 From: Romil Bhardwaj Date: Fri, 23 Aug 2024 16:02:10 -0700 Subject: [PATCH 2/4] Fix wheel creation --- sky/backends/wheel_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sky/backends/wheel_utils.py b/sky/backends/wheel_utils.py index ece21b5ce3e..73bc71e2d02 100644 --- a/sky/backends/wheel_utils.py +++ b/sky/backends/wheel_utils.py @@ -164,7 +164,7 @@ def _get_latest_modification_time(path: pathlib.Path) -> float: # Only build wheels if the wheel is outdated or wheel does not exist # for the requested version. if (last_wheel_modification_time < last_modification_time) or not any( - WHEEL_DIR.glob(_WHEEL_PATTERN)): + WHEEL_DIR.glob(f'**/{_WHEEL_PATTERN}')): if not WHEEL_DIR.exists(): WHEEL_DIR.mkdir(parents=True, exist_ok=True) _build_sky_wheel() From 611d74a6c4e6ff22af09560cdd4812934bb80f7c Mon Sep 17 00:00:00 2001 From: Romil Bhardwaj Date: Fri, 23 Aug 2024 17:22:28 -0700 Subject: [PATCH 3/4] Refactor _get_latest_wheel_and_remove_all_others --- sky/backends/wheel_utils.py | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/sky/backends/wheel_utils.py b/sky/backends/wheel_utils.py index 73bc71e2d02..019c81259a6 100644 --- a/sky/backends/wheel_utils.py +++ b/sky/backends/wheel_utils.py @@ -39,26 +39,27 @@ f'{version.parse(sky.__version__)}-*.whl') -def _get_latest_wheel_and_remove_all_others() -> pathlib.Path: - wheel_name = (f'**/{_WHEEL_PATTERN}') +def _remove_stale_wheels(latest_wheel_dir: pathlib.Path) -> None: + """Remove all wheels except the latest one.""" + for f in WHEEL_DIR.iterdir(): + if f != latest_wheel_dir: + if f.is_dir() and not f.is_symlink(): + shutil.rmtree(f, ignore_errors=True) + + +def _get_latest_wheel() -> pathlib.Path: + wheel_name = f'**/{_WHEEL_PATTERN}' try: latest_wheel = max(WHEEL_DIR.glob(wheel_name), key=os.path.getctime) except ValueError: raise FileNotFoundError( 'Could not find built SkyPilot wheels with glob pattern ' f'{wheel_name} under {WHEEL_DIR!r}') from None - - latest_wheel_dir_name = latest_wheel.parent - # Cleanup older wheels. - for f in WHEEL_DIR.iterdir(): - if f != latest_wheel_dir_name: - if f.is_dir() and not f.is_symlink(): - shutil.rmtree(f, ignore_errors=True) return latest_wheel -def _build_sky_wheel(): - """Build a wheel for SkyPilot.""" +def _build_sky_wheel() -> pathlib.Path: + """Build a wheel for SkyPilot and return the path to the wheel.""" with tempfile.TemporaryDirectory() as tmp_dir: # prepare files tmp_dir = pathlib.Path(tmp_dir) @@ -129,6 +130,7 @@ def _build_sky_wheel(): wheel_dir = WHEEL_DIR / hash_of_latest_wheel wheel_dir.mkdir(parents=True, exist_ok=True) shutil.move(str(wheel_path), wheel_dir) + return wheel_dir / wheel_path.name def build_sky_wheel() -> Tuple[pathlib.Path, str]: @@ -167,14 +169,16 @@ def _get_latest_modification_time(path: pathlib.Path) -> float: WHEEL_DIR.glob(f'**/{_WHEEL_PATTERN}')): if not WHEEL_DIR.exists(): WHEEL_DIR.mkdir(parents=True, exist_ok=True) - _build_sky_wheel() + latest_wheel = _build_sky_wheel() + else: + latest_wheel = _get_latest_wheel() # We remove all wheels except the latest one for garbage collection. # Otherwise stale wheels will accumulate over time. # TODO(romilb): If the user switches versions every alternate launch, # the wheel will be rebuilt every time. At the risk of adding # complexity, we can consider TTL caching wheels by version here. - latest_wheel = _get_latest_wheel_and_remove_all_others() + _remove_stale_wheels(latest_wheel.parent) wheel_hash = latest_wheel.parent.name From e6be5e6b6bb223e4916e103ab0e49ce5fb178906 Mon Sep 17 00:00:00 2001 From: Romil Bhardwaj Date: Fri, 23 Aug 2024 17:38:28 -0700 Subject: [PATCH 4/4] lint --- sky/backends/wheel_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sky/backends/wheel_utils.py b/sky/backends/wheel_utils.py index 019c81259a6..44d47b52926 100644 --- a/sky/backends/wheel_utils.py +++ b/sky/backends/wheel_utils.py @@ -60,9 +60,9 @@ def _get_latest_wheel() -> pathlib.Path: def _build_sky_wheel() -> pathlib.Path: """Build a wheel for SkyPilot and return the path to the wheel.""" - with tempfile.TemporaryDirectory() as tmp_dir: + with tempfile.TemporaryDirectory() as tmp_dir_str: # prepare files - tmp_dir = pathlib.Path(tmp_dir) + tmp_dir = pathlib.Path(tmp_dir_str) sky_tmp_dir = tmp_dir / 'sky' sky_tmp_dir.mkdir() for item in SKY_PACKAGE_PATH.iterdir():