Repository: skypilot-org/skypilot

Issue:
[Storage] Fail to use R2 as default storage when no other storage clouds are enabled
<!-- Describe the bug report / feature request here -->

To reproduce:
1. Disable other storage clouds but cloudflare
2. `sky spot launch -n test task.yaml`
```
resources:
  cpus: 2+

file_mounts:
  test.yaml: test.yaml

run: |
  ls test.yaml
```

The following error will occur due to the cloudflare cannot be recognized by the `CloudRegistry`:
```
  File "/home/azureuser/skypilot/sky/spot/core.py", line 77, in launch
    controller_utils.maybe_translate_local_file_mounts_and_sync_up(
  File "/home/azureuser/skypilot/sky/utils/controller_utils.py", line 604, in maybe_translate_local_file_mounts_and_sync_up
    task.sync_storage_mounts()
  File "/home/azureuser/skypilot/sky/task.py", line 926, in sync_storage_mounts
    store_type, store_region = self._get_preferred_store()
  File "/home/azureuser/skypilot/sky/task.py", line 909, in _get_preferred_store
    storage_cloud = clouds.CLOUD_REGISTRY.from_str(
  File "/home/azureuser/skypilot/sky/clouds/cloud_registry.py", line 20, in from_str
    raise ValueError(f'Cloud {name!r} is not a valid cloud among '
ValueError: Cloud 'Cloudflare' is not a valid cloud among ['gcp', 'aws', 'azure', 'cudo', 'fluidstack', 'ibm', 'kubernetes', 'lambda', 'oci', 'paperspace', 'runpod', 'scp', 'vsphere']
```



<!-- If relevant, fill in versioning info to help us troubleshoot -->
_Version & Commit info:_
* `sky -v`: PLEASE_FILL_IN
* `sky -c`: PLEASE_FILL_IN


Discussion:

Patch:
diff --git a/docs/source/_static/custom.js b/docs/source/_static/custom.js
index 92339b6c..26b55b6e 100644
--- a/docs/source/_static/custom.js
+++ b/docs/source/_static/custom.js
@@ -5,7 +5,7 @@ document.addEventListener('DOMContentLoaded', function () {
        script.setAttribute('data-project-name', 'SkyPilot');
        script.setAttribute('data-project-color', '#4C4C4D');
        script.setAttribute('data-project-logo', 'https://avatars.githubusercontent.com/u/109387420?s=100&v=4');
-       script.setAttribute('data-modal-disclaimer', 'Results are automatically generated and may be inaccurate or contain inappropriate information. Do not include any sensitive information in your query.');
+       script.setAttribute('data-modal-disclaimer', 'Results are automatically generated and may be inaccurate or contain inappropriate information. Do not include any sensitive information in your query.\n**To get further assistance, you can chat directly with the development team** by joining the [SkyPilot Slack](https://slack.skypilot.co/).');
        script.setAttribute('data-modal-title', 'SkyPilot Docs AI - Ask a Question.');
        script.setAttribute('data-button-position-bottom', '85px');
        script.async = true;
diff --git a/sky/data/storage.py b/sky/data/storage.py
index 06cdcbca..e43406c3 100644
--- a/sky/data/storage.py
+++ b/sky/data/storage.py
@@ -110,15 +110,24 @@ class StoreType(enum.Enum):
     IBM = 'IBM'
 
     @classmethod
-    def from_cloud(cls, cloud: clouds.Cloud) -> 'StoreType':
-        if isinstance(cloud, clouds.AWS):
+    def from_cloud(cls, cloud: str) -> 'StoreType':
+        if cloud.lower() == str(clouds.AWS()).lower():
             return StoreType.S3
-        elif isinstance(cloud, clouds.GCP):
+        elif cloud.lower() == str(clouds.GCP()).lower():
             return StoreType.GCS
-        elif isinstance(cloud, clouds.Azure):
-            return StoreType.AZURE
-        elif isinstance(cloud, clouds.IBM):
+        elif cloud.lower() == str(clouds.IBM()).lower():
             return StoreType.IBM
+        elif cloud.lower() == cloudflare.NAME.lower():
+            return StoreType.R2
+        elif cloud.lower() == str(clouds.Azure()).lower():
+            with ux_utils.print_exception_no_traceback():
+                raise ValueError('Azure Blob Storage is not supported yet.')
+        elif cloud.lower() == str(clouds.Lambda()).lower():
+            with ux_utils.print_exception_no_traceback():
+                raise ValueError('Lambda Cloud does not provide cloud storage.')
+        elif cloud.lower() == str(clouds.SCP()).lower():
+            with ux_utils.print_exception_no_traceback():
+                raise ValueError('SCP does not provide cloud storage.')
 
         raise ValueError(f'Unsupported cloud for StoreType: {cloud}')
 
@@ -136,51 +145,28 @@ class StoreType(enum.Enum):
             with ux_utils.print_exception_no_traceback():
                 raise ValueError(f'Unknown store type: {store}')
 
+    def store_prefix(self) -> str:
+        if self == StoreType.S3:
+            return 's3://'
+        elif self == StoreType.GCS:
+            return 'gs://'
+        elif self == StoreType.R2:
+            return 'r2://'
+        elif self == StoreType.IBM:
+            return 'cos://'
+        elif self == StoreType.AZURE:
+            with ux_utils.print_exception_no_traceback():
+                raise ValueError('Azure Blob Storage is not supported yet.')
+        else:
+            with ux_utils.print_exception_no_traceback():
+                raise ValueError(f'Unknown store type: {self}')
+
 
 class StorageMode(enum.Enum):
     MOUNT = 'MOUNT'
     COPY = 'COPY'
 
 
-def get_storetype_from_cloud(cloud: clouds.Cloud) -> StoreType:
-    if isinstance(cloud, clouds.AWS):
-        return StoreType.S3
-    elif isinstance(cloud, clouds.GCP):
-        return StoreType.GCS
-    elif isinstance(cloud, clouds.IBM):
-        return StoreType.IBM
-    elif isinstance(cloud, clouds.Azure):
-        with ux_utils.print_exception_no_traceback():
-            raise ValueError('Azure Blob Storage is not supported yet.')
-    elif isinstance(cloud, clouds.Lambda):
-        with ux_utils.print_exception_no_traceback():
-            raise ValueError('Lambda Cloud does not provide cloud storage.')
-    elif isinstance(cloud, clouds.SCP):
-        with ux_utils.print_exception_no_traceback():
-            raise ValueError('SCP does not provide cloud storage.')
-    else:
-        with ux_utils.print_exception_no_traceback():
-            raise ValueError(f'Unknown cloud type: {cloud}')
-
-
-def get_store_prefix(storetype: StoreType) -> str:
-    if storetype == StoreType.S3:
-        return 's3://'
-    elif storetype == StoreType.GCS:
-        return 'gs://'
-    # R2 storages use 's3://' as a prefix for various aws cli commands
-    elif storetype == StoreType.R2:
-        return 's3://'
-    elif storetype == StoreType.IBM:
-        return 'cos://'
-    elif storetype == StoreType.AZURE:
-        with ux_utils.print_exception_no_traceback():
-            raise ValueError('Azure Blob Storage is not supported yet.')
-    else:
-        with ux_utils.print_exception_no_traceback():
-            raise ValueError(f'Unknown store type: {storetype}')
-
-
 class AbstractStore:
     """AbstractStore abstracts away the different storage types exposed by
     different clouds.
@@ -353,7 +339,7 @@ class AbstractStore:
             # bucket's URL as 'source'.
             if handle is None:
                 with ux_utils.print_exception_no_traceback():
-                    store_prefix = get_store_prefix(StoreType.from_store(self))
+                    store_prefix = StoreType.from_store(self).store_prefix()
                     raise exceptions.StorageSpecError(
                         'Attempted to mount a non-sky managed bucket '
                         f'{self.name!r} without specifying the storage source.'
diff --git a/sky/task.py b/sky/task.py
index 9350e261..9919bce4 100644
--- a/sky/task.py
+++ b/sky/task.py
@@ -348,6 +348,15 @@ class Task:
         config: Dict[str, Any],
         env_overrides: Optional[List[Tuple[str, str]]] = None,
     ) -> 'Task':
+        # More robust handling for 'envs': explicitly convert keys and values to
+        # str, since users may pass '123' as keys/values which will get parsed
+        # as int causing validate_schema() to fail.
+        envs = config.get('envs')
+        if envs is not None and isinstance(envs, dict):
+            config['envs'] = {str(k): str(v) for k, v in envs.items()}
+
+        common_utils.validate_schema(config, schemas.get_task_schema(),
+                                     'Invalid task YAML: ')
         if env_overrides is not None:
             # We must override env vars before constructing the Task, because
             # the Storage object creation is eager and it (its name/source
@@ -359,16 +368,6 @@ class Task:
             new_envs.update(env_overrides)
             config['envs'] = new_envs
 
-        # More robust handling for 'envs': explicitly convert keys and values to
-        # str, since users may pass '123' as keys/values which will get parsed
-        # as int causing validate_schema() to fail.
-        envs = config.get('envs')
-        if envs is not None and isinstance(envs, dict):
-            config['envs'] = {str(k): str(v) for k, v in envs.items()}
-
-        common_utils.validate_schema(config, schemas.get_task_schema(),
-                                     'Invalid task YAML: ')
-
         # Fill in any Task.envs into file_mounts (src/dst paths, storage
         # name/source).
         if config.get('file_mounts') is not None:
@@ -901,17 +900,20 @@ class Task:
             resources = list(self.resources)[0]
             storage_cloud = resources.cloud
             storage_region = resources.region
+
         if storage_cloud is not None:
             if str(storage_cloud) not in enabled_storage_clouds:
                 storage_cloud = None
 
+        storage_cloud_str = None
         if storage_cloud is None:
-            storage_cloud = clouds.CLOUD_REGISTRY.from_str(
-                enabled_storage_clouds[0])
-            assert storage_cloud is not None, enabled_storage_clouds[0]
+            storage_cloud_str = enabled_storage_clouds[0]
+            assert storage_cloud_str is not None, enabled_storage_clouds[0]
             storage_region = None  # Use default region in the Store class
+        else:
+            storage_cloud_str = str(storage_cloud)
 
-        store_type = storage_lib.get_storetype_from_cloud(storage_cloud)
+        store_type = storage_lib.StoreType.from_cloud(storage_cloud_str)
         return store_type, storage_region
 
     def sync_storage_mounts(self) -> None:
diff --git a/sky/utils/common_utils.py b/sky/utils/common_utils.py
index 14a79a4d..0cef1a8d 100644
--- a/sky/utils/common_utils.py
+++ b/sky/utils/common_utils.py
@@ -567,20 +567,24 @@ def validate_schema(obj, schema, err_msg_prefix='', skip_none=True):
                            'The `envs` field contains invalid keys:\n' +
                            e.message)
             else:
-                err_msg = err_msg_prefix + 'The following fields are invalid:'
+                err_msg = err_msg_prefix
                 known_fields = set(e.schema.get('properties', {}).keys())
                 for field in e.instance:
                     if field not in known_fields:
                         most_similar_field = difflib.get_close_matches(
                             field, known_fields, 1)
                         if most_similar_field:
-                            err_msg += (f'\nInstead of {field!r}, did you mean '
+                            err_msg += (f'Instead of {field!r}, did you mean '
                                         f'{most_similar_field[0]!r}?')
                         else:
-                            err_msg += f'\nFound unsupported field {field!r}.'
+                            err_msg += f'Found unsupported field {field!r}.'
         else:
+            message = e.message
+            # Object in jsonschema is represented as dict in Python. Replace
+            # 'object' with 'dict' for better readability.
+            message = message.replace('type \'object\'', 'type \'dict\'')
             # Example e.json_path value: '$.resources'
-            err_msg = (err_msg_prefix + e.message +
+            err_msg = (err_msg_prefix + message +
                        f'. Check problematic field(s): {e.json_path}')
 
     if err_msg:
diff --git a/sky/utils/controller_utils.py b/sky/utils/controller_utils.py
index a35b61a2..71096044 100644
--- a/sky/utils/controller_utils.py
+++ b/sky/utils/controller_utils.py
@@ -606,11 +606,11 @@ def maybe_translate_local_file_mounts_and_sync_up(task: 'task_lib.Task',
     # Step 5: Add the file download into the file mounts, such as
     #  /original-dst: s3://spot-fm-file-only-bucket-name/file-0
     new_file_mounts = {}
+    storage = task.storage_mounts[file_mount_remote_tmp_dir]
+    store_type = list(storage.stores.keys())[0]
+    store_prefix = store_type.store_prefix()
+    bucket_url = store_prefix + file_bucket_name
     for dst, src in copy_mounts_with_file_in_src.items():
-        storage = task.storage_mounts[file_mount_remote_tmp_dir]
-        store_type = list(storage.stores.keys())[0]
-        store_prefix = storage_lib.get_store_prefix(store_type)
-        bucket_url = store_prefix + file_bucket_name
         file_id = src_to_file_id[src]
         new_file_mounts[dst] = bucket_url + f'/file-{file_id}'
     task.update_file_mounts(new_file_mounts)
@@ -627,14 +627,6 @@ def maybe_translate_local_file_mounts_and_sync_up(task: 'task_lib.Task',
             assert len(store_types) == 1, (
                 'We only support one store type for now.', storage_obj.stores)
             store_type = store_types[0]
-            if store_type == storage_lib.StoreType.S3:
-                storage_obj.source = f's3://{storage_obj.name}'
-            elif store_type == storage_lib.StoreType.GCS:
-                storage_obj.source = f'gs://{storage_obj.name}'
-            elif store_type == storage_lib.StoreType.R2:
-                storage_obj.source = f'r2://{storage_obj.name}'
-            else:
-                with ux_utils.print_exception_no_traceback():
-                    raise exceptions.NotSupportedError(
-                        f'Unsupported store type: {store_type}')
+            store_prefix = store_type.store_prefix()
+            storage_obj.source = f'{store_prefix}{storage_obj.name}'
             storage_obj.force_delete = True


Test patch:
diff --git a/tests/test_yaml_parser.py b/tests/test_yaml_parser.py
index c02d77b8..03380849 100644
--- a/tests/test_yaml_parser.py
+++ b/tests/test_yaml_parser.py
@@ -111,3 +111,26 @@ def test_invalid_fields_storage(tmp_path):
     with pytest.raises(AssertionError) as e:
         Task.from_yaml(config_path)
     assert 'Invalid storage args' in e.value.args[0]
+
+
+def test_invalid_envs_key(tmp_path):
+    config_path = _create_config_file(
+        textwrap.dedent(f"""\
+            envs:
+                invalid_env_$_key: value
+            """), tmp_path)
+    with pytest.raises(ValueError) as e:
+        Task.from_yaml(config_path)
+    assert 'does not match any of the regexes:' in e.value.args[0]
+
+
+def test_invalid_envs_type(tmp_path):
+    config_path = _create_config_file(
+        textwrap.dedent(f"""\
+            envs:
+                - env_key1: abc
+                - env_key2: abc
+            """), tmp_path)
+    with pytest.raises(ValueError) as e:
+        Task.from_yaml(config_path)
+    assert 'is not of type \'dict\'' in e.value.args[0]