[Core] Explicit encoding for opening files and update pylint (#3153)

* explicit encoding for files

* format

* Update pylint

* update pylint

.github/workflows/pylint.yml

@@ -28,7 +28,7 @@ jobs:
       run: |
         python -m pip install --upgrade pip
         pip install ".[all]"
-        pip install pylint==2.8.2
+        pip install pylint==2.14.5
         pip install pylint-quotes==0.2.3
     - name: Analysing the code with pylint
       run: |

.pylintrc

@@ -5,7 +5,7 @@
 # Its canonical open-source location is:
 #   https://google.github.io/styleguide/pylintrc
 
-[MASTER]
+[MAIN]
 
 # Files or directories to be skipped. They should be base names, not paths.
 ignore=third_party,ray_patches,providers
@@ -50,7 +50,8 @@ confidence=
 # --enable=similarities". If you want to run only the classes checker, but have
 # no Warning level messages displayed, use"--disable=all --enable=classes
 # --disable=W"
-disable=abstract-method,
+disable=R,
+        abstract-method,
         apply-builtin,
         arguments-differ,
         attribute-defined-outside-init,
@@ -60,6 +61,7 @@ disable=abstract-method,
         buffer-builtin,
         c-extension-no-member,
         consider-using-enumerate,
+        consider-using-f-string, # FIXME(sky): make pass
         cmp-builtin,
         cmp-method,
         coerce-builtin,
@@ -76,7 +78,7 @@ disable=abstract-method,
         global-statement,
         hex-method,
         idiv-method,
-        implicit-str-concat-in-sequence,
+        implicit-str-concat,
         import-error,
         import-self,
         import-star-module-level,
@@ -139,6 +141,7 @@ disable=abstract-method,
         trailing-newlines,
         unichr-builtin,
         unicode-builtin,
+        unnecessary-lambda-assignment, # FIXME(sky): make pass.
         unnecessary-pass,
         unpacking-in-except,
         useless-else-on-loop,
@@ -157,12 +160,6 @@ disable=abstract-method,
 # mypackage.mymodule.MyReporterClass.
 output-format=text
 
-# Put messages in a separate file for each module / package specified on the
-# command line instead of printing them on stdout. Reports (if any) will be
-# written in a file name "pylint_global.[txt|html]". This option is deprecated
-# and it will be removed in Pylint 2.0.
-files-output=no
-
 # Tells whether to display a full report or only the messages
 reports=no
 
@@ -269,7 +266,7 @@ generated-members=
 # Maximum number of characters on a single line.
 max-line-length=80
 
-# TODO(https://github.com/PyCQA/pylint/issues/3352): Direct pylint to exempt
+# TODO(https://github.com/pylint-dev/pylint/issues/3352): Direct pylint to exempt
 # lines made too long by directives to pytype.
 
 # Regexp for a line that is allowed to be longer than the limit.
@@ -281,12 +278,6 @@ ignore-long-lines=(?x)(
 # else.
 single-line-if-stmt=yes
 
-# List of optional constructs for which whitespace checking is disabled. `dict-
-# separator` is used to allow tabulation in dicts, etc.: {1  : 1,\n222: 2}.
-# `trailing-comma` allows a space between comma and closing bracket: (a, ).
-# `empty-line` allows space-only lines.
-no-space-check=
-
 # Maximum number of lines in a module
 max-module-lines=99999
 

requirements-dev.txt

@@ -1,6 +1,6 @@
 # formatting
 yapf==0.32.0
-pylint==2.8.2
+pylint==2.14.5
 # formatting the node_providers code from upstream ray-project/ray project
 black==22.10.0
 # https://github.com/edaniszewski/pylint-quotes

sky/adaptors/cloudflare.py

@@ -152,7 +152,7 @@ def create_endpoint():
     """Reads accountid necessary to interact with R2"""
 
     accountid_path = os.path.expanduser(ACCOUNT_ID_PATH)
-    with open(accountid_path, 'r') as f:
+    with open(accountid_path, 'r', encoding='utf-8') as f:
         lines = f.readlines()
         accountid = lines[0]
 
@@ -202,7 +202,7 @@ def r2_profile_in_aws_cred() -> bool:
     profile_path = os.path.expanduser(R2_CREDENTIALS_PATH)
     r2_profile_exists = False
     if os.path.isfile(profile_path):
-        with open(profile_path, 'r') as file:
+        with open(profile_path, 'r', encoding='utf-8') as file:
             for line in file:
                 if f'[{R2_PROFILE_NAME}]' in line:
                     r2_profile_exists = True

sky/authentication.py

@@ -88,12 +88,14 @@ def _save_key_pair(private_key_path: str, public_key_path: str,
     with open(
             private_key_path,
             'w',
+            encoding='utf-8',
             opener=functools.partial(os.open, mode=0o600),
     ) as f:
         f.write(private_key)
 
     with open(public_key_path,
               'w',
+              encoding='utf-8',
               opener=functools.partial(os.open, mode=0o644)) as f:
         f.write(public_key)
 
@@ -118,7 +120,7 @@ def get_or_generate_keys() -> Tuple[str, str]:
 
 def configure_ssh_info(config: Dict[str, Any]) -> Dict[str, Any]:
     _, public_key_path = get_or_generate_keys()
-    with open(public_key_path, 'r') as f:
+    with open(public_key_path, 'r', encoding='utf-8') as f:
         public_key = f.read().strip()
     config_str = common_utils.dump_yaml_str(config)
     config_str = config_str.replace('skypilot:ssh_user',
@@ -225,7 +227,7 @@ def setup_gcp_authentication(config: Dict[str, Any]) -> Dict[str, Any]:
                 'GCP credential backup file '
                 f'{sky_backup_config_path!r} does not exist.')
 
-            with open(sky_backup_config_path, 'r') as infile:
+            with open(sky_backup_config_path, 'r', encoding='utf-8') as infile:
                 for line in infile:
                     if line.startswith('account'):
                         account = line.split('=')[1].strip()
@@ -272,7 +274,7 @@ def setup_gcp_authentication(config: Dict[str, Any]) -> Dict[str, Any]:
 # and public key content, then encode it back.
 def setup_azure_authentication(config: Dict[str, Any]) -> Dict[str, Any]:
     _, public_key_path = get_or_generate_keys()
-    with open(public_key_path, 'r') as f:
+    with open(public_key_path, 'r', encoding='utf-8') as f:
         public_key = f.read().strip()
     for node_type in config['available_node_types']:
         node_config = config['available_node_types'][node_type]['node_config']
@@ -303,7 +305,7 @@ def setup_lambda_authentication(config: Dict[str, Any]) -> Dict[str, Any]:
     # Ensure ssh key is registered with Lambda Cloud
     lambda_client = lambda_utils.LambdaCloudClient()
     public_key_path = os.path.expanduser(PUBLIC_SSH_KEY_PATH)
-    with open(public_key_path, 'r') as f:
+    with open(public_key_path, 'r', encoding='utf-8') as f:
         public_key = f.read().strip()
     prefix = f'sky-key-{common_utils.get_user_hash()}'
     name, exists = lambda_client.get_unique_ssh_key_name(prefix, public_key)

sky/backends/backend.py

@@ -12,6 +12,7 @@
     from sky.data import storage as storage_lib
 
 Path = str
+# pylint: disable=invalid-name
 _ResourceHandleType = typing.TypeVar('_ResourceHandleType',
                                      bound='ResourceHandle')
 

sky/backends/backend_utils.py

@@ -485,10 +485,11 @@ def add_cluster(
             config = ['\n']
             with open(config_path,
                       'w',
+                      encoding='utf-8',
                       opener=functools.partial(os.open, mode=0o644)) as f:
                 f.writelines(config)
 
-        with open(config_path, 'r') as f:
+        with open(config_path, 'r', encoding='utf-8') as f:
             config = f.readlines()
 
         ssh_dir = cls.ssh_cluster_path.format('')
@@ -506,7 +507,7 @@ def add_cluster(
                 break
         if not found:
             # Did not find Include string. Insert `Include` lines.
-            with open(config_path, 'w') as f:
+            with open(config_path, 'w', encoding='utf-8') as f:
                 config.insert(
                     0,
                     f'# Added by SkyPilot for ssh config of all clusters\n{include_str}\n'
@@ -543,6 +544,7 @@ def add_cluster(
 
         with open(cluster_config_path,
                   'w',
+                  encoding='utf-8',
                   opener=functools.partial(os.open, mode=0o644)) as f:
             f.write(codegen)
 
@@ -571,7 +573,7 @@ def _remove_stale_cluster_config_for_backward_compatibility(
         if not os.path.exists(config_path):
             return
 
-        with open(config_path) as f:
+        with open(config_path, 'r', encoding='utf-8') as f:
             config = f.readlines()
 
         start_line_idx = None
@@ -623,20 +625,20 @@ def _remove_stale_cluster_config_for_backward_compatibility(
             config[prev_end_line_idx:end_line_idx] = [
                 '\n'
             ] if end_line_idx is not None else []
-            with open(config_path, 'w') as f:
+            with open(config_path, 'w', encoding='utf-8') as f:
                 f.write(''.join(config).strip())
                 f.write('\n' * 2)
 
         # Delete include statement if it exists in the config.
         sky_autogen_comment = ('# Added by sky (use `sky stop/down '
                                f'{cluster_name}` to remove)')
-        with open(config_path) as f:
+        with open(config_path, 'r', encoding='utf-8') as f:
             config = f.readlines()
 
         for i, line in enumerate(config):
             config_str = line.strip()
             if f'Include {cluster_config_path}' in config_str:
-                with open(config_path, 'w') as f:
+                with open(config_path, 'w', encoding='utf-8') as f:
                     if i < len(config) - 1 and config[i + 1] == '\n':
                         del config[i + 1]
                     # Delete Include string
@@ -854,7 +856,7 @@ def write_cluster_config(
     # Dump the Ray ports to a file for Ray job submission
     dump_port_command = (
         f'python -c \'import json, os; json.dump({constants.SKY_REMOTE_RAY_PORT_DICT_STR}, '
-        f'open(os.path.expanduser("{constants.SKY_REMOTE_RAY_PORT_FILE}"), "w"))\''
+        f'open(os.path.expanduser("{constants.SKY_REMOTE_RAY_PORT_FILE}"), "w", encoding="utf-8"))\''
     )
 
     # Use a tmp file path to avoid incomplete YAML file being re-used in the
@@ -938,15 +940,15 @@ def write_cluster_config(
 
     # Restore the old yaml content for backward compatibility.
     if os.path.exists(yaml_path) and keep_launch_fields_in_existing_config:
-        with open(yaml_path, 'r') as f:
+        with open(yaml_path, 'r', encoding='utf-8') as f:
             old_yaml_content = f.read()
-        with open(tmp_yaml_path, 'r') as f:
+        with open(tmp_yaml_path, 'r', encoding='utf-8') as f:
             new_yaml_content = f.read()
         restored_yaml_content = _replace_yaml_dicts(
             new_yaml_content, old_yaml_content,
             _RAY_YAML_KEYS_TO_RESTORE_FOR_BACK_COMPATIBILITY,
             _RAY_YAML_KEYS_TO_RESTORE_EXCEPTIONS)
-        with open(tmp_yaml_path, 'w') as f:
+        with open(tmp_yaml_path, 'w', encoding='utf-8') as f:
             f.write(restored_yaml_content)
 
     # Read the cluster name from the tmp yaml file, to take the backward

sky/backends/cloud_vm_ray_backend.py

@@ -164,7 +164,9 @@ def write_ray_up_script_with_patched_launch_hash_fn(
     does not include any `ssh_proxy_command` under `auth` as part of the hash
     calculation.
     """
-    with open(_RAY_UP_WITH_MONKEY_PATCHED_HASH_LAUNCH_CONF_PATH, 'r') as f:
+    with open(_RAY_UP_WITH_MONKEY_PATCHED_HASH_LAUNCH_CONF_PATH,
+              'r',
+              encoding='utf-8') as f:
         ray_up_no_restart_script = f.read().format(
             ray_yaml_path=repr(cluster_config_path),
             ray_up_kwargs=ray_up_kwargs)
@@ -1581,8 +1583,9 @@ def _retry_zones(
                         cluster_yaml=handle.cluster_yaml,
                         prev_cluster_ever_up=prev_cluster_ever_up,
                         log_dir=self.log_dir)
-                    # NOTE: We will handle the logic of '_ensure_cluster_ray_started'
-                    # in 'provision_utils.post_provision_runtime_setup()' in the caller.
+                    # NOTE: We will handle the logic of '_ensure_cluster_ray_started' #pylint: disable=line-too-long
+                    # in 'provision_utils.post_provision_runtime_setup()' in the
+                    # caller.
                     resources_vars = (
                         to_provision.cloud.make_deploy_resources_variables(
                             to_provision, handle.cluster_name_on_cloud, region,
@@ -1666,21 +1669,23 @@ def _retry_zones(
             definitely_no_nodes_launched = False
             if status == GangSchedulingStatus.HEAD_FAILED:
                 # ray up failed for the head node.
-                definitely_no_nodes_launched = FailoverCloudErrorHandlerV1.update_blocklist_on_error(
-                    self._blocked_resources, to_provision, region, zones,
-                    stdout, stderr)
+                definitely_no_nodes_launched = (
+                    FailoverCloudErrorHandlerV1.update_blocklist_on_error(
+                        self._blocked_resources, to_provision, region, zones,
+                        stdout, stderr))
             else:
                 # gang scheduling failed.
                 assert status == GangSchedulingStatus.GANG_FAILED, status
                 # The stdout/stderr of ray up is not useful here, since
                 # head node is successfully provisioned.
-                definitely_no_nodes_launched = FailoverCloudErrorHandlerV1.update_blocklist_on_error(
-                    self._blocked_resources,
-                    to_provision,
-                    region,
-                    zones=zones,
-                    stdout=None,
-                    stderr=None)
+                definitely_no_nodes_launched = (
+                    FailoverCloudErrorHandlerV1.update_blocklist_on_error(
+                        self._blocked_resources,
+                        to_provision,
+                        region,
+                        zones=zones,
+                        stdout=None,
+                        stderr=None))
                 # GANG_FAILED means head is up, workers failed.
                 assert definitely_no_nodes_launched is False, (
                     definitely_no_nodes_launched)
@@ -3976,7 +3981,7 @@ def teardown_no_lock(self,
                         'SKYPILOT_ERROR_NO_NODES_LAUNCHED: '
                         'Metadata file does not exist.')
 
-                with open(provider.metadata.path, 'r') as f:
+                with open(provider.metadata.path, 'r', encoding='utf-8') as f:
                     metadata = json.load(f)
                     node_id = next(iter(metadata.values())).get(
                         'creation', {}).get('virtualServerId', None)

sky/backends/docker_utils.py

@@ -112,7 +112,8 @@ def add_script_to_dockerfile(dockerfile_contents: str,
     dockerfile_contents += '\n' + DOCKERFILE_RUNCMD.format(run_command=cmd)
 
     # Write Dockerfile
-    with open(os.path.join(build_dir, 'Dockerfile'), 'w') as f:
+    with open(os.path.join(build_dir, 'Dockerfile'), 'w',
+              encoding='utf-8') as f:
         f.write(dockerfile_contents)
 
     img_metadata['workdir_name'] = workdir_name
@@ -227,6 +228,6 @@ def bash_codegen(workdir_name: str,
     multiline_cmds = f'cd /{workdir_name}\n{multiline_cmds}'
     script_contents = make_bash_from_multiline(multiline_cmds)
     if out_path:
-        with open(out_path, 'w') as fp:
+        with open(out_path, 'w', encoding='utf-8') as fp:
             fp.write(script_contents)
     return script_contents

sky/backends/monkey_patches/monkey_patch_ray_up.py

@@ -43,7 +43,8 @@ def monkey_patch_hash_launch_conf(node_conf, auth):
     full_auth.pop('ssh_proxy_command', None)  # NOTE: skypilot changes.
     for key_type in ['ssh_private_key', 'ssh_public_key']:
         if key_type in auth:
-            with open(os.path.expanduser(auth[key_type])) as key:
+            with open(os.path.expanduser(auth[key_type]),
+                      encoding='utf-8') as key:
                 full_auth[key_type] = key.read()
     hasher.update(
         json.dumps([node_conf, full_auth], sort_keys=True).encode('utf-8'))

sky/backends/onprem_utils.py

@@ -70,7 +70,7 @@ def check_and_get_local_clusters(suppress_error: bool = False) -> List[str]:
     name_to_path_dict: Dict[str, str] = {}
 
     for path in local_cluster_paths:
-        with open(path, 'r') as f:
+        with open(path, 'r', encoding='utf-8') as f:
             yaml_config = yaml.safe_load(f)
             if not suppress_error:
                 common_utils.validate_schema(yaml_config,

sky/benchmark/benchmark_utils.py

@@ -278,7 +278,7 @@ def _delete_remote_dir(remote_dir: str, bucket_type: data.StoreType) -> None:
 
 
 def _read_timestamp(path: str) -> float:
-    with open(path, 'r') as f:
+    with open(path, 'r', encoding='utf-8') as f:
         timestamp = f.readlines()
     assert len(timestamp) == 1
     return float(timestamp[0].strip())
@@ -396,7 +396,7 @@ def _update_benchmark_result(benchmark_result: Dict[str, Any]) -> Optional[str]:
     message = None
     if summary_path is not None and os.path.exists(summary_path):
         # (1) SkyCallback has saved the summary.
-        with open(summary_path, 'r') as f:
+        with open(summary_path, 'r', encoding='utf-8') as f:
             summary = json.load(f)
         if end_time is None:
             last_time = summary['last_step_time']

sky/callbacks/sky_callback/base.py

@@ -157,7 +157,7 @@ def _update_summary(self) -> None:
                 summary.estimated_total_time = total_time
 
     def _write_summary(self) -> None:
-        with open(self._log_path, 'w') as f:
+        with open(self._log_path, 'w', encoding='utf-8') as f:
             json.dump(self._summary.__dict__, f)
 
     def run(self) -> None: