Repository: skypilot-org/skypilot
Pull request: [DO] Refactor DigitalOcean credential path handling
Introduced `get_credentials_path` to encapsulate credential path logic, improving code readability and maintainability. Replaced direct access to `CREDENTIALS_PATH` with the new method in relevant parts of the code, ensuring consistency in handling credentials.

Fix https://github.com/skypilot-org/skypilot/issues/5060
Now:
Don't change constat
Don't need do's requirements to get credential path

Tested (run the relevant ones):

- [x] Code formatting: install pre-commit (auto-check on commit) or `bash format.sh`



Problem statement:
sky jobs launched failed
One of our users report jobs launch failure after upgrade to latest nightly:

```
E 03-28 10:45:10 sdk.py:1468] === Traceback on SkyPilot API Server ===
E 03-28 10:45:10 sdk.py:1468] Traceback (most recent call last):
E 03-28 10:45:10 sdk.py:1468]   File "/opt/conda/lib/python3.10/site-packages/sky/server/requests/executor.py", line 259, in _request_execution_wrapper
E 03-28 10:45:10 sdk.py:1468]     return_value = func(**request_body.to_kwargs())
E 03-28 10:45:10 sdk.py:1468]   File "/opt/conda/lib/python3.10/site-packages/sky/utils/common_utils.py", line 465, in _record
E 03-28 10:45:10 sdk.py:1468]     return f(*args, **kwargs)
E 03-28 10:45:10 sdk.py:1468]   File "/opt/conda/lib/python3.10/site-packages/sky/utils/common_utils.py", line 465, in _record
E 03-28 10:45:10 sdk.py:1468]     return f(*args, **kwargs)
E 03-28 10:45:10 sdk.py:1468]   File "/opt/conda/lib/python3.10/site-packages/sky/jobs/server/core.py", line 188, in launch
E 03-28 10:45:10 sdk.py:1468]     return execution.launch(task=controller_task,
E 03-28 10:45:10 sdk.py:1468]   File "/opt/conda/lib/python3.10/site-packages/sky/utils/common_utils.py", line 465, in _record
E 03-28 10:45:10 sdk.py:1468]     return f(*args, **kwargs)
E 03-28 10:45:10 sdk.py:1468]   File "/opt/conda/lib/python3.10/site-packages/sky/utils/common_utils.py", line 465, in _record
E 03-28 10:45:10 sdk.py:1468]     return f(*args, **kwargs)
E 03-28 10:45:10 sdk.py:1468]   File "/opt/conda/lib/python3.10/site-packages/sky/execution.py", line 537, in launch
E 03-28 10:45:10 sdk.py:1468]     return _execute(
E 03-28 10:45:10 sdk.py:1468]   File "/opt/conda/lib/python3.10/site-packages/sky/execution.py", line 311, in _execute
E 03-28 10:45:10 sdk.py:1468]     handle = backend.provision(
E 03-28 10:45:10 sdk.py:1468]   File "/opt/conda/lib/python3.10/site-packages/sky/utils/common_utils.py", line 465, in _record
E 03-28 10:45:10 sdk.py:1468]     return f(*args, **kwargs)
E 03-28 10:45:10 sdk.py:1468]   File "/opt/conda/lib/python3.10/site-packages/sky/utils/common_utils.py", line 445, in _record
E 03-28 10:45:10 sdk.py:1468]     return f(*args, **kwargs)
E 03-28 10:45:10 sdk.py:1468]   File "/opt/conda/lib/python3.10/site-packages/sky/backends/backend.py", line 84, in provision
E 03-28 10:45:10 sdk.py:1468]     return self._provision(task, to_provision, dryrun, stream_logs,
E 03-28 10:45:10 sdk.py:1468]   File "/opt/conda/lib/python3.10/site-packages/sky/backends/cloud_vm_ray_backend.py", line 2895, in _provision
E 03-28 10:45:10 sdk.py:1468]     config_dict = retry_provisioner.provision_with_retries(
E 03-28 10:45:10 sdk.py:1468]   File "/opt/conda/lib/python3.10/site-packages/sky/utils/common_utils.py", line 465, in _record
E 03-28 10:45:10 sdk.py:1468]     return f(*args, **kwargs)
E 03-28 10:45:10 sdk.py:1468]   File "/opt/conda/lib/python3.10/site-packages/sky/backends/cloud_vm_ray_backend.py", line 2050, in provision_with_retries
E 03-28 10:45:10 sdk.py:1468]     config_dict = self._retry_zones(
E 03-28 10:45:10 sdk.py:1468]   File "/opt/conda/lib/python3.10/site-packages/sky/backends/cloud_vm_ray_backend.py", line 1420, in _retry_zones
E 03-28 10:45:10 sdk.py:1468]     config_dict = backend_utils.write_cluster_config(
E 03-28 10:45:10 sdk.py:1468]   File "/opt/conda/lib/python3.10/site-packages/sky/utils/common_utils.py", line 465, in _record
E 03-28 10:45:10 sdk.py:1468]     return f(*args, **kwargs)
E 03-28 10:45:10 sdk.py:1468]   File "/opt/conda/lib/python3.10/site-packages/sky/backends/backend_utils.py", line 650, in write_cluster_config
E 03-28 10:45:10 sdk.py:1468]     credentials = sky_check.get_cloud_credential_file_mounts(excluded_clouds)
E 03-28 10:45:10 sdk.py:1468]   File "/opt/conda/lib/python3.10/site-packages/sky/check.py", line 261, in get_cloud_credential_file_mounts
E 03-28 10:45:10 sdk.py:1468]     cloud_file_mounts = cloud.get_credential_file_mounts()
E 03-28 10:45:10 sdk.py:1468]   File "/opt/conda/lib/python3.10/site-packages/sky/clouds/do.py", line 283, in get_credential_file_mounts
E 03-28 10:45:10 sdk.py:1468]     if not os.path.exists(os.path.expanduser(do_utils.CREDENTIALS_PATH)):
E 03-28 10:45:10 sdk.py:1468]   File "/opt/conda/lib/python3.10/posixpath.py", line 232, in expanduser
E 03-28 10:45:10 sdk.py:1468]     path = os.fspath(path)
E 03-28 10:45:10 sdk.py:1468] TypeError: expected str, bytes or os.PathLike object, not NoneType
E 03-28 10:45:10 sdk.py:1468] 
D 03-28 10:45:10 sdk.py:82] To stream request logs: sky api logs dadbbc64-25a2-45eb-b1be-26c73ac346fd
TypeError: expected str, bytes or os.PathLike object, not NoneType
```

Hints from issue discussion:
I think we should merge #5061 immediately to resolve the issue on master, and then we can follow up with #5066.

Commit message: Merge branch 'master' into fixdocreds

Patch to review:
diff --git a/sky/clouds/do.py b/sky/clouds/do.py
index 41b7bc49bc5..c4d74def67f 100644
--- a/sky/clouds/do.py
+++ b/sky/clouds/do.py
@@ -280,12 +280,14 @@ def _check_compute_credentials(cls) -> Tuple[bool, Optional[str]]:
         return True, None
 
     def get_credential_file_mounts(self) -> Dict[str, str]:
-        if do_utils.CREDENTIALS_PATH is None:
+        if do_utils.get_credentials_path() is None:
             return {}
-        if not os.path.exists(os.path.expanduser(do_utils.CREDENTIALS_PATH)):
+        if not os.path.exists(
+                os.path.expanduser(do_utils.get_credentials_path())):
             return {}
         return {
-            f'~/.config/doctl/{_CREDENTIAL_FILE}': do_utils.CREDENTIALS_PATH
+            f'~/.config/doctl/{_CREDENTIAL_FILE}':
+                do_utils.get_credentials_path()
         }
 
     @classmethod
diff --git a/sky/provision/do/utils.py b/sky/provision/do/utils.py
index f706007134e..5fc68d737f3 100644
--- a/sky/provision/do/utils.py
+++ b/sky/provision/do/utils.py
@@ -31,40 +31,43 @@
 MAX_ATTEMPTS = 6
 SSH_KEY_NAME_ON_DO = f'sky-key-{common_utils.get_user_hash()}'
 
-CREDENTIALS_PATH = '~/.config/doctl/config.yaml'
 _client = None
 _ssh_key_id = None
+_credentials_path = None
 
 
 class DigitalOceanError(Exception):
     pass
 
 
+def get_credentials_path():
+    global _credentials_path
+    if _credentials_path is None:
+        credentials_found = 0
+        for path in POSSIBLE_CREDENTIALS_PATHS:
+            if os.path.exists(path):
+                logger.debug(f'Digital Ocean credential path found at {path}')
+                _credentials_path = path
+                credentials_found += 1
+        if credentials_found > 1:
+            logger.debug('More than 1 credential file found')
+    return _credentials_path
+
+
 def _init_client():
-    global _client, CREDENTIALS_PATH
+    global _client
     assert _client is None
-    CREDENTIALS_PATH = None
-    credentials_found = 0
-    for path in POSSIBLE_CREDENTIALS_PATHS:
-        if os.path.exists(path):
-            CREDENTIALS_PATH = path
-            credentials_found += 1
-            logger.debug(f'Digital Ocean credential path found at {path}')
-    if not credentials_found > 1:
-        logger.debug('more than 1 credential file found')
-    if CREDENTIALS_PATH is None:
-        raise DigitalOceanError(
-            'no credentials file found from '
-            f'the following paths {POSSIBLE_CREDENTIALS_PATHS}')
-
     # attempt default context
-    credentials = common_utils.read_yaml(CREDENTIALS_PATH)
+    if get_credentials_path() is None:
+        raise DigitalOceanError(
+            'No credentials found, please run `doctl auth init`')
+    credentials = common_utils.read_yaml(get_credentials_path())
     default_token = credentials.get('access-token', None)
     if default_token is not None:
         try:
             test_client = do.pydo.Client(token=default_token)
             test_client.droplets.list()
-            logger.debug('trying `default` context')
+            logger.debug('Trying `default` context')
             _client = test_client
             return _client
         except do.exceptions().HttpResponseError:
@@ -76,7 +79,7 @@ def _init_client():
             try:
                 test_client = do.pydo.Client(token=api_token)
                 test_client.droplets.list()
-                logger.debug(f'using {context} context')
+                logger.debug(f'Using "{context}" context')
                 _client = test_client
                 break
             except do.exceptions().HttpResponseError:


Code review comments:
diff_hunk: @@ -280,12 +280,14 @@ def _check_compute_credentials(cls) -> Tuple[bool, Optional[str]]:
         return True, None
 
     def get_credential_file_mounts(self) -> Dict[str, str]:
-        if do_utils.CREDENTIALS_PATH is None:
+        if do_utils.get_credentials_path() is None:
path: sky/clouds/do.py
text: @user1:
```suggestion
        credential_path = do_utils.get_credentials_path()
        if credential_path is None:
```

diff_hunk: @@ -31,40 +31,43 @@
 MAX_ATTEMPTS = 6
 SSH_KEY_NAME_ON_DO = f'sky-key-{common_utils.get_user_hash()}'
 
-CREDENTIALS_PATH = '~/.config/doctl/config.yaml'
 _client = None
 _ssh_key_id = None
+_credentials_path = None
 
 
 class DigitalOceanError(Exception):
     pass
 
 
+def get_credentials_path():
+    global _credentials_path
+    if _credentials_path is None:
+        credentials_found = 0
+        for path in POSSIBLE_CREDENTIALS_PATHS:
+            if os.path.exists(path):
+                logger.debug(f'Digital Ocean credential path found at {path}')
+                _credentials_path = path
+                credentials_found += 1
+        if credentials_found > 1:
+            logger.debug('More than 1 credential file found')
+    return _credentials_path
path: sky/provision/do/utils.py
text: @user2:
```suggestion
from sky.utils import annotations

@user1.lru_cache(scope='request')
def get_credentials_path() -> Optional[str]:
    credentials_found = 0
    credentials_path = None
    for path in POSSIBLE_CREDENTIALS_PATHS:
        if os.path.exists(path):
            logger.debug(f'Digital Ocean credential path found at {path}')
            credentials_path = path
            credentials_found += 1
    if credentials_found > 1:
        logger.debug('More than 1 credential file found')
    return credentials_path
```

diff_hunk: @@ -280,12 +280,14 @@ def _check_compute_credentials(cls) -> Tuple[bool, Optional[str]]:
         return True, None
 
     def get_credential_file_mounts(self) -> Dict[str, str]:
-        if do_utils.CREDENTIALS_PATH is None:
+        if do_utils.get_credentials_path() is None:
             return {}
-        if not os.path.exists(os.path.expanduser(do_utils.CREDENTIALS_PATH)):
+        if not os.path.exists(
+                os.path.expanduser(do_utils.get_credentials_path())):
path: sky/clouds/do.py
text: @user1:
```suggestion
        if not os.path.exists(
                os.path.expanduser(credential_path)):
```

diff_hunk: @@ -280,12 +280,14 @@ def _check_compute_credentials(cls) -> Tuple[bool, Optional[str]]:
         return True, None
 
     def get_credential_file_mounts(self) -> Dict[str, str]:
-        if do_utils.CREDENTIALS_PATH is None:
+        if do_utils.get_credentials_path() is None:
             return {}
-        if not os.path.exists(os.path.expanduser(do_utils.CREDENTIALS_PATH)):
+        if not os.path.exists(
+                os.path.expanduser(do_utils.get_credentials_path())):
             return {}
         return {
-            f'~/.config/doctl/{_CREDENTIAL_FILE}': do_utils.CREDENTIALS_PATH
+            f'~/.config/doctl/{_CREDENTIAL_FILE}':
+                do_utils.get_credentials_path()
path: sky/clouds/do.py
text: @user1:
```suggestion
                credential_path
```