chore: deprecate pylint-quotes so that we can upgrade pylint

Author: andylizf

.github/workflows/pylint.yml

@@ -30,7 +30,6 @@ jobs:
         python -m pip install --upgrade pip
         pip install ".[all]"
         pip install pylint==$(grep 'pylint==' requirements-dev.txt | cut -d'=' -f3)
-        pip install pylint-quotes==$(grep 'pylint-quotes==' requirements-dev.txt | cut -d'=' -f3)
     - name: Analysing the code with pylint
       run: |
-        pylint --load-plugins pylint_quotes sky
+        pylint sky

.pylintrc

@@ -429,13 +429,6 @@ valid-metaclass-classmethod-first-arg=mcs
 
 # Exceptions that will emit a warning when being caught. Defaults to
 # "Exception"
-overgeneral-exceptions=StandardError,
-                       Exception,
-                       BaseException
-
-#######
-
-# https://github.com/edaniszewski/pylint-quotes#configuration
-string-quote=single
-triple-quote=double
-docstring-quote=double
+overgeneral-exceptions=builtins.StandardError,
+                       builtins.Exception,
+                       builtins.BaseException

format.sh

@@ -23,7 +23,6 @@ builtin cd "$ROOT" || exit 1
 
 YAPF_VERSION=$(yapf --version | awk '{print $2}')
 PYLINT_VERSION=$(pylint --version | head -n 1 | awk '{print $2}')
-PYLINT_QUOTES_VERSION=$(pip list | grep pylint-quotes | awk '{print $2}')
 MYPY_VERSION=$(mypy --version | awk '{print $2}')
 BLACK_VERSION=$(black --version | head -n 1 | awk '{print $2}')
 
@@ -37,7 +36,6 @@ tool_version_check() {
 
 tool_version_check "yapf" $YAPF_VERSION "$(grep yapf requirements-dev.txt | cut -d'=' -f3)"
 tool_version_check "pylint" $PYLINT_VERSION "$(grep "pylint==" requirements-dev.txt | cut -d'=' -f3)"
-tool_version_check "pylint-quotes" $PYLINT_QUOTES_VERSION "$(grep "pylint-quotes==" requirements-dev.txt | cut -d'=' -f3)"
 tool_version_check "mypy" "$MYPY_VERSION" "$(grep mypy requirements-dev.txt | cut -d'=' -f3)"
 tool_version_check "black" "$BLACK_VERSION" "$(grep black requirements-dev.txt | cut -d'=' -f3)"
 
@@ -60,10 +58,6 @@ BLACK_INCLUDES=(
     'sky/skylet/providers/ibm'
 )
 
-PYLINT_FLAGS=(
-    '--load-plugins'  'pylint_quotes'
-)
-
 # Format specified files
 format() {
     yapf --in-place "${YAPF_FLAGS[@]}" "$@"

requirements-dev.txt

@@ -1,10 +1,8 @@
 # formatting
 yapf==0.32.0
-pylint==2.14.5
+pylint==3.2.7
 # formatting the node_providers code from upstream ray-project/ray project
 black==22.10.0
-# https://github.com/edaniszewski/pylint-quotes
-pylint-quotes==0.2.3
 toml==0.10.2
 isort==5.12.0
 

sky/backends/cloud_vm_ray_backend.py

@@ -2475,10 +2475,12 @@ def num_ips_per_node(self) -> int:
             num_ips = 1
         return num_ips
 
-    def __setstate__(self, state):
+    def __setstate__(self, state: Dict[str, Any]):
         self._version = self._VERSION
 
         version = state.pop('_version', None)
+        head_ip: Optional[str] = None
+
         if version is None:
             version = -1
             state.pop('cluster_region', None)
@@ -2502,7 +2504,6 @@ def __setstate__(self, state):
         if version < 9:
             # For backward compatibility, we should update the region of a
             # SkyPilot cluster on Kubernetes to the actual context it is using.
-            # pylint: disable=import-outside-toplevel
             launched_resources = state['launched_resources']
             if isinstance(launched_resources.cloud, clouds.Kubernetes):
                 yaml_config = common_utils.read_yaml(
@@ -3272,7 +3273,7 @@ def _exec_code_on_head(
         mkdir_code = (f'{cd} && mkdir -p {remote_log_dir} && '
                       f'touch {remote_log_path}')
         encoded_script = shlex.quote(codegen)
-        create_script_code = (f'{{ echo {encoded_script} > {script_path}; }}')
+        create_script_code = f'{{ echo {encoded_script} > {script_path}; }}'
         job_submit_cmd = (
             f'RAY_DASHBOARD_PORT=$({constants.SKY_PYTHON_CMD} -c "from sky.skylet import job_lib; print(job_lib.get_job_submission_port())" 2> /dev/null || echo 8265);'  # pylint: disable=line-too-long
             f'{cd} && {constants.SKY_RAY_CMD} job submit '
@@ -3829,6 +3830,7 @@ def teardown_no_lock(self,
             RuntimeError: If the cluster fails to be terminated/stopped.
         """
         cluster_status_fetched = False
+        prev_cluster_status = None
         if refresh_cluster_status:
             try:
                 prev_cluster_status, _ = (

sky/cli.py

@@ -210,7 +210,7 @@ def _merge_env_vars(env_dict: Optional[Dict[str, str]],
                  default=None,
                  type=int,
                  required=False,
-                 help=('OS disk size in GBs.')),
+                 help='OS disk size in GBs.'),
     click.option('--disk-tier',
                  default=None,
                  type=click.Choice(resources_utils.DiskTier.supported_tiers(),
@@ -392,7 +392,8 @@ def _install_shell_completion(ctx: click.Context, param: click.Parameter,
     else:
         click.secho(f'Unsupported shell: {value}', fg='red')
         ctx.exit()
-
+        # Though `ctx.exit()` already NoReturn, we need to make pylint happy.
+        assert False, 'Unreachable'
     try:
         subprocess.run(cmd,
                        shell=True,
@@ -447,6 +448,8 @@ def _uninstall_shell_completion(ctx: click.Context, param: click.Parameter,
     else:
         click.secho(f'Unsupported shell: {value}', fg='red')
         ctx.exit()
+        # Though `ctx.exit()` already NoReturn, we need to make pylint happy.
+        assert False, 'Unreachable'
 
     try:
         subprocess.run(cmd, shell=True, check=True)
@@ -3530,7 +3533,7 @@ def jobs():
               default=None,
               type=str,
               hidden=True,
-              help=('Alias for --name, the name of the managed job.'))
+              help='Alias for --name, the name of the managed job.')
 @click.option('--job-recovery',
               default=None,
               type=str,
@@ -4544,7 +4547,7 @@ def serve_logs(
         sky serve logs [SERVICE_NAME] 1
     """
     have_replica_id = replica_id is not None
-    num_flags = (controller + load_balancer + have_replica_id)
+    num_flags = controller + load_balancer + have_replica_id
     if num_flags > 1:
         raise click.UsageError('At most one of --controller, --load-balancer, '
                                '[REPLICA_ID] can be specified.')

sky/optimizer.py

@@ -435,6 +435,7 @@ def _optimize_by_dp(
             # FIXME: Account for egress costs for multi-node clusters
             for resources, execution_cost in node_to_cost_map[node].items():
                 min_pred_cost_plus_egress = np.inf
+                best_parent_hardware = resources_lib.Resources()
                 for parent_resources, parent_cost in \
                     dp_best_objective[parent].items():
                     egress_cost = Optimizer._egress_cost_or_time(

sky/serve/constants.py

@@ -100,3 +100,7 @@
 TERMINATE_REPLICA_VERSION_MISMATCH_ERROR = (
     'The version of service is outdated and does not support manually '
     'terminating replicas. Please terminate the service and spin up again.')
+
+# Default timeout in seconds for HTTP requests to avoid hanging indefinitely.
+# This is used for internal service communication requests.
+DEFAULT_HTTP_REQUEST_TIMEOUT_SECONDS = 30

sky/serve/replica_managers.py

@@ -528,7 +528,7 @@ def probe(
                 logger.info(f'Error when probing {replica_identity}: '
                             'Cannot get the endpoint.')
                 return self, False, probe_time
-            readiness_path = (f'{url}{readiness_path}')
+            readiness_path = f'{url}{readiness_path}'
             logger.info(f'Probing {replica_identity} with {readiness_path}.')
             if post_data is not None:
                 msg += 'POST'

sky/serve/serve_utils.py

@@ -300,7 +300,9 @@ def update_service_encoded(service_name: str, version: int, mode: str) -> str:
         json={
             'version': version,
             'mode': mode,
-        })
+        },
+        timeout=constants.DEFAULT_HTTP_REQUEST_TIMEOUT_SECONDS,
+    )
     if resp.status_code == 404:
         with ux_utils.print_exception_no_traceback():
             raise ValueError(
@@ -342,7 +344,9 @@ def terminate_replica(service_name: str, replica_id: int, purge: bool) -> str:
         json={
             'replica_id': replica_id,
             'purge': purge,
-        })
+        },
+        timeout=constants.DEFAULT_HTTP_REQUEST_TIMEOUT_SECONDS,
+    )
 
     message: str = resp.json()['message']
     if resp.status_code != 200: