Skip to content

Commit

Permalink
Remove backward compatibility code in the 0.8.0 release (#4650)
Browse files Browse the repository at this point in the history
* remove 0.8.0

* bug fix

* cached_cluster_info fix

* smoke test bug fix

* restore change

* restore comment

* Update sky/backends/cloud_vm_ray_backend.py

Co-authored-by: Zhanghao Wu <[email protected]>

* verify ret

* restore change

---------

Co-authored-by: Zhanghao Wu <[email protected]>
  • Loading branch information
zpoint and Michaelvll authored Feb 8, 2025
1 parent 1eaba80 commit e774fc3
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 47 deletions.
5 changes: 0 additions & 5 deletions sky/backends/backend_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -844,11 +844,6 @@ def write_cluster_config(

# User-supplied global instance tags from ~/.sky/config.yaml.
labels = skypilot_config.get_nested((str(cloud).lower(), 'labels'), {})
# Deprecated: instance_tags have been replaced by labels. For backward
# compatibility, we support them and the schema allows them only if
# `labels` are not specified. This should be removed after 0.8.0.
labels = skypilot_config.get_nested((str(cloud).lower(), 'instance_tags'),
labels)
# labels is a dict, which is guaranteed by the type check in
# schemas.py
assert isinstance(labels, dict), labels
Expand Down
2 changes: 1 addition & 1 deletion sky/backends/cloud_vm_ray_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -2446,7 +2446,7 @@ def get_command_runners(self,
# can be None. We need to update it here, even when force_cached is
# set to True.
# TODO: We can remove `self.cached_external_ips is None` after
# version 0.8.0.
# all clouds moved to new provisioner.
if force_cached and self.cached_external_ips is None:
raise RuntimeError(
'Tried to use cached cluster info, but it\'s missing for '
Expand Down
31 changes: 0 additions & 31 deletions sky/provision/azure/instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
from sky.adaptors import azure
from sky.provision import common
from sky.provision import constants
from sky.provision.azure import config as config_lib
from sky.utils import common_utils
from sky.utils import subprocess_utils
from sky.utils import ux_utils
Expand Down Expand Up @@ -52,10 +51,6 @@

_RESOURCE_GROUP_NOT_FOUND_ERROR_MESSAGE = 'ResourceGroupNotFound'
_POLL_INTERVAL = 1
# TODO(Doyoung): _LEGACY_NSG_NAME can be remove this after 0.8.0 to ignore
# legacy nsg names.
_LEGACY_NSG_NAME = 'ray-{cluster_name_on_cloud}-nsg'
_SECOND_LEGACY_NSG_NAME = 'sky-{cluster_name_on_cloud}-nsg'


class AzureInstanceStatus(enum.Enum):
Expand Down Expand Up @@ -987,32 +982,6 @@ def _fetch_and_map_status(node, resource_group: str) -> None:
return statuses


# TODO(Doyoung): _get_cluster_nsg can be remove this after 0.8.0 to ignore
# legacy nsg names.
def _get_cluster_nsg(network_client: Client, resource_group: str,
cluster_name_on_cloud: str) -> NetworkSecurityGroup:
"""Retrieve the NSG associated with the given name of the cluster."""
list_network_security_groups = _get_azure_sdk_function(
client=network_client.network_security_groups, function_name='list')
legacy_nsg_name = _LEGACY_NSG_NAME.format(
cluster_name_on_cloud=cluster_name_on_cloud)
second_legacy_nsg_name = _SECOND_LEGACY_NSG_NAME.format(
cluster_name_on_cloud=cluster_name_on_cloud)
_, nsg_name = config_lib.get_cluster_id_and_nsg_name(
resource_group=resource_group,
cluster_name_on_cloud=cluster_name_on_cloud)
possible_nsg_names = [nsg_name, legacy_nsg_name, second_legacy_nsg_name]
for nsg in list_network_security_groups(resource_group):
if nsg.name in possible_nsg_names:
return nsg

# Raise an error if no matching NSG is found
raise ValueError('Failed to find a matching NSG for cluster '
f'{cluster_name_on_cloud!r} in resource group '
f'{resource_group!r}. Expected NSG names were: '
f'{possible_nsg_names}.')


def open_ports(
cluster_name_on_cloud: str,
ports: List[str],
Expand Down
9 changes: 0 additions & 9 deletions sky/utils/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -624,15 +624,6 @@ def get_cluster_schema():
}

_LABELS_SCHEMA = {
# Deprecated: 'instance_tags' is replaced by 'labels'. Keeping for backward
# compatibility. Will be removed after 0.8.0.
'instance_tags': {
'type': 'object',
'required': [],
'additionalProperties': {
'type': 'string',
},
},
'labels': {
'type': 'object',
'required': [],
Expand Down
5 changes: 4 additions & 1 deletion tests/smoke_tests/test_managed_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -820,7 +820,10 @@ def test_managed_jobs_intermediate_storage(generic_cloud: str):
[
*smoke_tests_utils.STORAGE_SETUP_COMMANDS,
# Verify command fails with correct error - run only once
f'err=$(sky jobs launch -n {name} --cloud {generic_cloud} {file_path} -y 2>&1); ret=$?; echo "$err" ; [ $ret -eq 0 ] || ! echo "$err" | grep "StorageBucketCreateError: Jobs bucket \'{intermediate_storage_name}\' does not exist. Please check jobs.bucket configuration in your SkyPilot config." > /dev/null && exit 1 || exit 0',
f'err=$(sky jobs launch -n {name} --cloud {generic_cloud} {file_path} -y 2>&1); '
f'ret=$?; if [ $ret -ne 0 ] && echo "$err" | grep -q "StorageBucketCreateError: '
f'Jobs bucket \'{intermediate_storage_name}\' does not exist."; then exit 0; '
f'else exit 1; fi',
f'aws s3api create-bucket --bucket {intermediate_storage_name}',
f'sky jobs launch -n {name} --cloud {generic_cloud} {file_path} -y',
# fail because the bucket does not exist
Expand Down

0 comments on commit e774fc3

Please sign in to comment.