Repository: skypilot-org/skypilot
Pull request #5878 (2025-06-05 02:04:51)

refactor update_config_no_lock
<!-- Describe the changes in this PR -->
1. rename the function u`pdate_api_server_config_no_lock` to communicate this function is meant for modifying server configs, not client configs
2. Attempt to use `_resolve_server_config_path` first to find the config path. `_resolve_server_config_path` accounts for `SKYPILOT_GLOBAL_CONFIG` environment variable being set which can change the location of the server side config file.


<!-- Describe the tests ran -->
<!-- Unit tests (tests/test_*.py) are part of GitHub CI; below are tests that launch on the cloud. -->

Tested (run the relevant ones):

- [ ] Code formatting: install pre-commit (auto-check on commit) or `bash format.sh`
- [ ] Any manual or new tests for this PR (please specify below)
- [ ] All smoke tests: `/smoke-test` (CI) or `pytest tests/test_smoke.py` (local)
- [ ] Relevant individual tests: `/smoke-test -k test_name` (CI) or `pytest tests/test_smoke.py::test_name` (local)
- [ ] Backward compatibility: `/quicktest-core` (CI) or `pytest tests/smoke_tests/test_backward_compat.py` (local)

<!-- CI commands (/-prefixed) can only be triggered by repo members -->



Patch:
diff --git a/sky/skypilot_config.py b/sky/skypilot_config.py
index fd7bba96f..4275d07e9 100644
--- a/sky/skypilot_config.py
+++ b/sky/skypilot_config.py
@@ -756,13 +756,15 @@ def apply_cli_config(cli_config: Optional[List[str]]) -> Dict[str, Any]:
     return parsed_config
 
 
-def update_config_no_lock(config: config_utils.Config) -> None:
+def update_api_server_config_no_lock(config: config_utils.Config) -> None:
     """Dumps the new config to a file and syncs to ConfigMap if in Kubernetes.
 
     Args:
         config: The config to save and sync.
     """
-    global_config_path = os.path.expanduser(get_user_config_path())
+    global_config_path = _resolve_server_config_path()
+    if global_config_path is None:
+        global_config_path = get_user_config_path()
 
     # Always save to the local file (PVC in Kubernetes, local file otherwise)
     common_utils.dump_yaml(global_config_path, dict(config))
diff --git a/sky/workspaces/core.py b/sky/workspaces/core.py
index da831154d..35ba912ea 100644
--- a/sky/workspaces/core.py
+++ b/sky/workspaces/core.py
@@ -66,7 +66,7 @@ def _update_workspaces_config(
             current_config['workspaces'] = current_workspaces
 
             # Write the configuration back to the file
-            skypilot_config.update_config_no_lock(current_config)
+            skypilot_config.update_api_server_config_no_lock(current_config)
 
             return current_workspaces
     except filelock.Timeout as e:
@@ -411,7 +411,7 @@ def update_config(config: Dict[str, Any]) -> Dict[str, Any]:
                                _WORKSPACE_CONFIG_LOCK_TIMEOUT_SECONDS):
             # Convert to config_utils.Config and save
             config_obj = config_utils.Config.from_dict(config)
-            skypilot_config.update_config_no_lock(config_obj)
+            skypilot_config.update_api_server_config_no_lock(config_obj)
     except filelock.Timeout as e:
         raise RuntimeError(
             f'Failed to update configuration due to a timeout '


Test patch:
diff --git a/tests/unit_tests/test_sky/utils/kubernetes/test_skypilot_config_configmap_sync.py b/tests/unit_tests/test_sky/utils/kubernetes/test_skypilot_config_configmap_sync.py
index 04670d126..1c2021588 100644
--- a/tests/unit_tests/test_sky/utils/kubernetes/test_skypilot_config_configmap_sync.py
+++ b/tests/unit_tests/test_sky/utils/kubernetes/test_skypilot_config_configmap_sync.py
@@ -147,9 +147,9 @@ class TestConfigMapSync(unittest.TestCase):
                 'patch_configmap_with_config')
     @mock.patch('sky.utils.kubernetes.config_map_utils.'
                 'is_running_in_kubernetes')
-    def test_update_config_no_lock_calls_patch_in_kubernetes(
+    def test_update_api_server_config_no_lock_calls_patch_in_kubernetes(
             self, mock_is_k8s, mock_patch):
-        """Test that update_config_no_lock calls ConfigMap patching in K8s."""
+        """Test that update_api_server_config_no_lock calls ConfigMap patching in K8s."""
         mock_is_k8s.return_value = True
 
         with tempfile.NamedTemporaryFile(mode='w', delete=False) as f:
@@ -162,7 +162,7 @@ class TestConfigMapSync(unittest.TestCase):
              mock.patch('sky.skypilot_config._reload_config'):
 
             config = config_utils.Config({'test': 'value'})
-            skypilot_config.update_config_no_lock(config)
+            skypilot_config.update_api_server_config_no_lock(config)
 
             # In Kubernetes, should call both dump_yaml and ConfigMap patching
             mock_patch.assert_called_once_with(config, config_path)
@@ -174,9 +174,9 @@ class TestConfigMapSync(unittest.TestCase):
                 'patch_configmap_with_config')
     @mock.patch('sky.utils.kubernetes.config_map_utils.'
                 'is_running_in_kubernetes')
-    def test_update_config_no_lock_calls_dump_yaml_non_kubernetes(
+    def test_update_api_server_config_no_lock_calls_dump_yaml_non_kubernetes(
             self, mock_is_k8s, mock_patch):
-        """Test that update_config_no_lock calls dump_yaml in non-K8s envs."""
+        """Test that update_api_server_config_no_lock calls dump_yaml in non-K8s envs."""
         mock_is_k8s.return_value = False
 
         with tempfile.NamedTemporaryFile(mode='w', delete=False) as f:
@@ -189,7 +189,7 @@ class TestConfigMapSync(unittest.TestCase):
              mock.patch('sky.skypilot_config._reload_config'):
 
             config = config_utils.Config({'test': 'value'})
-            skypilot_config.update_config_no_lock(config)
+            skypilot_config.update_api_server_config_no_lock(config)
 
             # In non-Kubernetes, should call dump_yaml but not ConfigMap patch
             mock_dump_yaml.assert_called_once_with(config_path, dict(config))