Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Mypy] Add more mypy #1564

Merged
merged 32 commits into from
Feb 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
5568587
backend_utils mypy
Michaelvll Dec 29, 2022
e112cd3
More mypy
Michaelvll Dec 30, 2022
a98ed69
format
Michaelvll Dec 30, 2022
11f2c75
order
Michaelvll Dec 30, 2022
8289dbc
change to exclude
Michaelvll Dec 30, 2022
75c8ddd
remove unused registry.py
Michaelvll Dec 30, 2022
98cf7a4
add back task.py
Michaelvll Dec 30, 2022
67b3fda
dependency
Michaelvll Dec 30, 2022
31a99c3
quote
Michaelvll Dec 30, 2022
a2e0075
fix circular import
Michaelvll Dec 30, 2022
bf75ee5
fix
Michaelvll Dec 30, 2022
fdb65fd
remove unused
Michaelvll Dec 30, 2022
96da35e
Merge branch 'master' of github.com:concretevitamin/sky-experiments i…
Michaelvll Jan 3, 2023
9483dab
Merge branch 'master' of github.com:concretevitamin/sky-experiments i…
Michaelvll Jan 11, 2023
0715076
fix issue with master
Michaelvll Jan 11, 2023
9f3e2e7
Merge branch 'master' of github.com:concretevitamin/sky-experiments i…
Michaelvll Jan 12, 2023
2ee2081
Merge branch 'master' of github.com:concretevitamin/sky-experiments i…
Michaelvll Jan 15, 2023
5672f7c
refactor ResourceHandle
Michaelvll Jan 15, 2023
8f8f07b
fix
Michaelvll Jan 15, 2023
9685d09
dryrun for local docker backend
Michaelvll Jan 15, 2023
78af26a
lint
Michaelvll Jan 15, 2023
ddac3dc
Merge branch 'master' of github.com:concretevitamin/sky-experiments i…
Michaelvll Jan 22, 2023
e751f49
fix merge issue
Michaelvll Jan 22, 2023
093889c
Merge branch 'master' of github.com:concretevitamin/sky-experiments i…
Michaelvll Feb 14, 2023
7242b64
Merge branch 'master' of github.com:concretevitamin/sky-experiments i…
Michaelvll Feb 21, 2023
17bc41f
Address comments
Michaelvll Feb 21, 2023
23b84b2
lint
Michaelvll Feb 21, 2023
59a14c1
Merge branch 'master' of github.com:concretevitamin/sky-experiments i…
Michaelvll Feb 22, 2023
374ba72
format
Michaelvll Feb 22, 2023
f682eaf
remote type ignore
Michaelvll Feb 24, 2023
e085f4a
Merge branch 'master' of github.com:concretevitamin/sky-experiments i…
Michaelvll Feb 24, 2023
278960d
add typing extensions to the dependency
Michaelvll Feb 24, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions format.sh
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ format_changed() {
# exist on both branches.
MERGEBASE="$(git merge-base origin/master HEAD)"

if ! git diff --diff-filter=ACM --quiet --exit-code "$MERGEBASE" -- '*.py' &>/dev/null; then
git diff --name-only --diff-filter=ACM "$MERGEBASE" -- '*.py' | xargs -P 5 \
if ! git diff --diff-filter=ACM --quiet --exit-code "$MERGEBASE" -- '*.py' '*.pyi' &>/dev/null; then
git diff --name-only --diff-filter=ACM "$MERGEBASE" -- '*.py' '*.pyi' | xargs -P 5 \
yapf --in-place "${YAPF_EXCLUDES[@]}" "${YAPF_FLAGS[@]}"
fi

Expand Down
1 change: 1 addition & 0 deletions requirements-dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ toml==0.10.2
mypy==0.991
types-PyYAML
types-requests
types-setuptools

# testing
pytest
Expand Down
18 changes: 9 additions & 9 deletions sky/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ def _wait_for_compute_global_operation(project_name: str, operation_name: str,
# avoid duplicated codes.
# Retry for the GCP as sometimes there will be connection reset by peer error.
@common_utils.retry
@gcp.import_package
def setup_gcp_authentication(config: Dict[str, Any]) -> Dict[str, Any]:
private_key_path, public_key_path = get_or_generate_keys()
config = copy.deepcopy(config)
Expand All @@ -156,7 +155,7 @@ def setup_gcp_authentication(config: Dict[str, Any]) -> Dict[str, Any]:

try:
project = compute.projects().get(project=project_id).execute()
except gcp.googleapiclient.errors.HttpError as e:
except gcp.http_error_exception() as e:
# Can happen for a new project where Compute Engine API is disabled.
#
# Example message:
Expand Down Expand Up @@ -191,9 +190,10 @@ def setup_gcp_authentication(config: Dict[str, Any]) -> Dict[str, Any]:
'Please check your network connection.')
raise

project_oslogin = next(
project_oslogin: str = next(
(item for item in project['commonInstanceMetadata'].get('items', [])
if item['key'] == 'enable-oslogin'), {}).get('value', 'False')
if item['key'] == 'enable-oslogin'), {}).get('value',
'False') # type: ignore

if project_oslogin.lower() == 'true':
# project.
Expand Down Expand Up @@ -245,17 +245,17 @@ def setup_gcp_authentication(config: Dict[str, Any]) -> Dict[str, Any]:
'utf-8'):
subprocess_utils.handle_returncode(proc.returncode, enable_ssh_cmd,
'Failed to enable ssh port.',
proc.stderr)
proc.stderr.decode('utf-8'))
return config

# OS Login is not enabled for the project. Add the ssh key directly to the
# metadata.
# TODO(zhwu): Use cloud init to add ssh public key, to avoid the permission
# issue. A blocker is that the cloud init is not installed in the debian
# image by default.
project_keys = next(
project_keys: str = next(
(item for item in project['commonInstanceMetadata'].get('items', [])
if item['key'] == 'ssh-keys'), {}).get('value', '')
if item['key'] == 'ssh-keys'), {}).get('value', '') # type: ignore
ssh_keys = project_keys.split('\n') if project_keys else []

# Get public key from file.
Expand Down Expand Up @@ -287,8 +287,8 @@ def setup_gcp_authentication(config: Dict[str, Any]) -> Dict[str, Any]:
if len(ssh_key_index) == 0:
metadata.append({'key': 'ssh-keys', 'value': new_ssh_key})
else:
ssh_key_index = ssh_key_index[0]
metadata[ssh_key_index]['value'] += '\n' + new_ssh_key
first_ssh_key_index = ssh_key_index[0]
metadata[first_ssh_key_index]['value'] += '\n' + new_ssh_key

project['commonInstanceMetadata']['items'] = metadata

Expand Down
12 changes: 8 additions & 4 deletions sky/backends/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
"""Sky Backends."""
from sky.backends.backend import Backend
from sky.backends.cloud_vm_ray_backend import CloudVmRayBackend
from sky.backends.local_docker_backend import LocalDockerBackend
from sky.backends.backend import Backend, ResourceHandle
from sky.backends.cloud_vm_ray_backend import CloudVmRayBackend, CloudVmRayResourceHandle
from sky.backends.local_docker_backend import LocalDockerBackend, LocalDockerResourceHandle

__all__ = ['Backend', 'CloudVmRayBackend', 'LocalDockerBackend']
__all__ = [
'Backend', 'ResourceHandle', 'CloudVmRayBackend',
'CloudVmRayResourceHandle', 'LocalDockerBackend',
'LocalDockerResourceHandle'
]
78 changes: 43 additions & 35 deletions sky/backends/backend.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"""Sky backend interface."""
import typing
from typing import Dict, Optional
from typing import Dict, Generic, Optional

import sky
from sky.utils import timeline
Expand All @@ -12,36 +12,43 @@
from sky.data import storage as storage_lib

Path = str
_ResourceHandleType = typing.TypeVar('_ResourceHandleType',
bound='ResourceHandle')


class Backend:
# Backend-specific handle to the launched resources (e.g., a cluster).
# Examples: 'cluster.yaml'; 'ray://...', 'k8s://...'.
class ResourceHandle:

def get_cluster_name(self) -> str:
raise NotImplementedError


class Backend(Generic[_ResourceHandleType]):
"""Backend interface: handles provisioning, setup, and scheduling."""

# NAME is used to identify the backend class from cli/yaml.
NAME = 'backend'

# Backend-specific handle to the launched resources (e.g., a cluster).
# Examples: 'cluster.yaml'; 'ray://...', 'k8s://...'.
class ResourceHandle:

def get_cluster_name(self) -> str:
raise NotImplementedError
# Backward compatibility, with the old name of the handle.
ResourceHandle = ResourceHandle # pylint: disable=invalid-name

# --- APIs ---
def check_resources_fit_cluster(self, handle: ResourceHandle,
def check_resources_fit_cluster(self, handle: _ResourceHandleType,
task: 'task_lib.Task') -> None:
"""Check whether resources of the task are satisfied by cluster."""
raise NotImplementedError

@timeline.event
@usage_lib.messages.usage.update_runtime('provision')
def provision(self,
task: 'task_lib.Task',
to_provision: Optional['resources.Resources'],
dryrun: bool,
stream_logs: bool,
cluster_name: Optional[str] = None,
retry_until_up: bool = False) -> ResourceHandle:
def provision(
self,
task: 'task_lib.Task',
to_provision: Optional['resources.Resources'],
dryrun: bool,
stream_logs: bool,
cluster_name: Optional[str] = None,
retry_until_up: bool = False) -> Optional[_ResourceHandleType]:
if cluster_name is None:
cluster_name = sky.backends.backend_utils.generate_cluster_name()
usage_lib.record_cluster_name_for_current_operation(cluster_name)
Expand All @@ -51,22 +58,22 @@ def provision(self,

@timeline.event
@usage_lib.messages.usage.update_runtime('sync_workdir')
def sync_workdir(self, handle: ResourceHandle, workdir: Path) -> None:
def sync_workdir(self, handle: _ResourceHandleType, workdir: Path) -> None:
return self._sync_workdir(handle, workdir)

@timeline.event
@usage_lib.messages.usage.update_runtime('sync_file_mounts')
def sync_file_mounts(
self,
handle: ResourceHandle,
handle: _ResourceHandleType,
all_file_mounts: Dict[Path, Path],
storage_mounts: Dict[Path, 'storage_lib.Storage'],
) -> None:
return self._sync_file_mounts(handle, all_file_mounts, storage_mounts)

@timeline.event
@usage_lib.messages.usage.update_runtime('setup')
def setup(self, handle: ResourceHandle, task: 'task_lib.Task',
def setup(self, handle: _ResourceHandleType, task: 'task_lib.Task',
detach_setup: bool) -> None:
return self._setup(handle, task, detach_setup)

Expand All @@ -75,15 +82,15 @@ def add_storage_objects(self, task: 'task_lib.Task') -> None:

@timeline.event
@usage_lib.messages.usage.update_runtime('execute')
def execute(self, handle: ResourceHandle, task: 'task_lib.Task',
def execute(self, handle: _ResourceHandleType, task: 'task_lib.Task',
detach_run: bool) -> None:
usage_lib.record_cluster_name_for_current_operation(
handle.get_cluster_name())
usage_lib.messages.usage.update_actual_task(task)
return self._execute(handle, task, detach_run)

@timeline.event
def post_execute(self, handle: ResourceHandle, down: bool) -> None:
def post_execute(self, handle: _ResourceHandleType, down: bool) -> None:
"""Post execute(): e.g., print helpful inspection messages."""
return self._post_execute(handle, down)

Expand All @@ -94,7 +101,7 @@ def teardown_ephemeral_storage(self, task: 'task_lib.Task') -> None:
@timeline.event
@usage_lib.messages.usage.update_runtime('teardown')
def teardown(self,
handle: ResourceHandle,
handle: _ResourceHandleType,
terminate: bool,
purge: bool = False) -> None:
self._teardown(handle, terminate, purge)
Expand All @@ -104,42 +111,43 @@ def register_info(self, **kwargs) -> None:
pass

# --- Implementations of the APIs ---
def _provision(self,
task: 'task_lib.Task',
to_provision: Optional['resources.Resources'],
dryrun: bool,
stream_logs: bool,
cluster_name: str,
retry_until_up: bool = False) -> ResourceHandle:
def _provision(
self,
task: 'task_lib.Task',
to_provision: Optional['resources.Resources'],
dryrun: bool,
stream_logs: bool,
cluster_name: str,
retry_until_up: bool = False) -> Optional[_ResourceHandleType]:
raise NotImplementedError

def _sync_workdir(self, handle: ResourceHandle, workdir: Path) -> None:
def _sync_workdir(self, handle: _ResourceHandleType, workdir: Path) -> None:
raise NotImplementedError

def _sync_file_mounts(
self,
handle: ResourceHandle,
handle: _ResourceHandleType,
all_file_mounts: Dict[Path, Path],
storage_mounts: Dict[Path, 'storage_lib.Storage'],
) -> None:
raise NotImplementedError

def _setup(self, handle: ResourceHandle, task: 'task_lib.Task',
def _setup(self, handle: _ResourceHandleType, task: 'task_lib.Task',
detach_setup: bool) -> None:
raise NotImplementedError

def _execute(self, handle: ResourceHandle, task: 'task_lib.Task',
def _execute(self, handle: _ResourceHandleType, task: 'task_lib.Task',
detach_run: bool) -> None:
raise NotImplementedError

def _post_execute(self, handle: ResourceHandle, down: bool) -> None:
def _post_execute(self, handle: _ResourceHandleType, down: bool) -> None:
raise NotImplementedError

def _teardown_ephemeral_storage(self, task: 'task_lib.Task') -> None:
raise NotImplementedError

def _teardown(self,
handle: ResourceHandle,
handle: _ResourceHandleType,
terminate: bool,
purge: bool = False):
raise NotImplementedError
Loading