Repository: skypilot-org/skypilot
Pull request: [Storage] Make s3 bucket creation log visible to the users
<!-- Describe the changes in this PR -->
This resolves #3729 with some refactoring.

Tested (run the relevant ones):

- [x] Code formatting: `bash format.sh`
- [x] Smoke tests `pytest tests/test_smoke.py::TestStorageWithCredentials --aws`
- [x] manual `sky launch` with the task yaml mentioned in #3729:
```
$ sky launch test.yaml --cloud gcp -y
Task from YAML spec: test.yaml
I 07-06 21:59:14 storage.py:1380] Created S3 bucket no-log-s3-creation in us-east-1
I 07-06 21:59:18 storage.py:1824] Created GCS bucket log-gcs-creation in US with storage class STANDARD
I 07-06 21:59:22 optimizer.py:695] == Optimizer ==
I 07-06 21:59:22 optimizer.py:706] Target: minimizing cost
I 07-06 21:59:22 optimizer.py:718] Estimated cost: $0.4 / hour
I 07-06 21:59:22 optimizer.py:718]
I 07-06 21:59:22 optimizer.py:843] Considered resources (1 node):
I 07-06 21:59:22 optimizer.py:913] ----------------------------------------------------------------------------------------------
I 07-06 21:59:22 optimizer.py:913]  CLOUD   INSTANCE        vCPUs   Mem(GB)   ACCELERATORS   REGION/ZONE     COST ($)   CHOSEN
I 07-06 21:59:22 optimizer.py:913] ----------------------------------------------------------------------------------------------
I 07-06 21:59:22 optimizer.py:913]  GCP     n2-standard-8   8       32        -              us-central1-a   0.39          ✔
I 07-06 21:59:22 optimizer.py:913] ----------------------------------------------------------------------------------------------
I 07-06 21:59:22 optimizer.py:913]
Running task on cluster sky-xxxx-gcpuser...
I 07-06 21:59:22 cloud_vm_ray_backend.py:4411] Creating a new cluster: 'sky-xxxx-gcpuser' [1x GCP(n2-standard-8)].
I 07-06 21:59:22 cloud_vm_ray_backend.py:4411] Tip: to reuse an existing cluster, specify --cluster (-c). Run `sky status` to see existing clusters.
I 07-06 21:59:25 cloud_vm_ray_backend.py:1406] To view detailed progress: tail -n100 -f /home/gcpuser/sky_logs/sky-2024-07-06-21-59-20-661208/provision.log
I 07-06 21:59:27 provisioner.py:76] Launching on GCP us-central1 (us-central1-a)
I 07-06 22:00:27 provisioner.py:458] Successfully provisioned or found existing instance.
I 07-06 22:01:14 provisioner.py:560] Successfully provisioned cluster: sky-xxxx-gcpuser
```


Problem statement:
[Storage] creating s3 bucket fails to display log to user
<!-- Describe the bug report / feature request here -->
When we create sky managed storages as part of `file_mounts` config from task yaml, we display the created bucket to the users but we currently fail to display for s3 buckets. To reproduce:

`test.yaml:`
```
file_mounts:
  /s3-test:
    name: no-log-s3-creation
    source: ~/yaml
    store: s3

  /gcs-test:
    name: log-gcs-creation
    source: ~/yaml
    store: gcs
```

Below, we see the log being displayed for GCS bucket, but not for S3.
```
$ sky launch test.yaml --cloud gcp -y
Task from YAML spec: test.yaml
I 07-06 21:35:42 storage.py:1826] Created GCS bucket log-gcs-creation in US with storage class STANDARD
I 07-06 21:35:45 optimizer.py:695] == Optimizer ==
I 07-06 21:35:45 optimizer.py:706] Target: minimizing cost
I 07-06 21:35:45 optimizer.py:718] Estimated cost: $0.4 / hour
I 07-06 21:35:45 optimizer.py:718]
I 07-06 21:35:45 optimizer.py:843] Considered resources (1 node):
I 07-06 21:35:45 optimizer.py:913] ----------------------------------------------------------------------------------------------
I 07-06 21:35:45 optimizer.py:913]  CLOUD   INSTANCE        vCPUs   Mem(GB)   ACCELERATORS   REGION/ZONE     COST ($)   CHOSEN
I 07-06 21:35:45 optimizer.py:913] ----------------------------------------------------------------------------------------------
I 07-06 21:35:45 optimizer.py:913]  GCP     n2-standard-8   8       32        -              us-central1-a   0.39          ✔
I 07-06 21:35:45 optimizer.py:913] ----------------------------------------------------------------------------------------------
I 07-06 21:35:45 optimizer.py:913]
Running task on cluster sky-xxxx-gcpuser...
I 07-06 21:35:45 cloud_vm_ray_backend.py:4411] Creating a new cluster: 'sky-xxxx-gcpuser' [1x GCP(n2-standard-8)].
I 07-06 21:35:45 cloud_vm_ray_backend.py:4411] Tip: to reuse an existing cluster, specify --cluster (-c). Run `sky status` to see existing clusters.
I 07-06 21:35:48 cloud_vm_ray_backend.py:1406] To view detailed progress: tail -n100 -f /home/gcpuser/sky_logs/sky-2024-07-06-21-35-44-212709/provision.log
I 07-06 21:35:51 provisioner.py:76] Launching on GCP us-central1 (us-central1-a)
I 07-06 21:36:56 provisioner.py:458] Successfully provisioned or found existing instance.
I 07-06 21:37:42 provisioner.py:560] Successfully provisioned cluster: sky-xxxx-gcpuser

```
Need this to be fixed so that all the creation logs are displayed.

Hints from issue discussion:

Commit message: nit

Patch to review:
diff --git a/sky/data/storage.py b/sky/data/storage.py
index e43406c3951..bb89ad9605e 100644
--- a/sky/data/storage.py
+++ b/sky/data/storage.py
@@ -1368,24 +1368,22 @@ def _create_s3_bucket(self,
         """
         s3_client = self.client
         try:
-            if region is None:
-                s3_client.create_bucket(Bucket=bucket_name)
-            else:
-                if region == 'us-east-1':
-                    # If default us-east-1 region is used, the
-                    # LocationConstraint must not be specified.
-                    # https://stackoverflow.com/a/51912090
-                    s3_client.create_bucket(Bucket=bucket_name)
-                else:
-                    location = {'LocationConstraint': region}
-                    s3_client.create_bucket(Bucket=bucket_name,
-                                            CreateBucketConfiguration=location)
-                logger.info(f'Created S3 bucket {bucket_name} in {region}')
+            create_bucket_config: Dict[str, Any] = {'Bucket': bucket_name}
+            # If default us-east-1 region of create_bucket API is used,
+            # the LocationConstraint must not be specified.
+            # Reference: https://stackoverflow.com/a/51912090
+            if region and region != 'us-east-1':
+                create_bucket_config['CreateBucketConfiguration'] = {
+                    'LocationConstraint': region
+                }
+            s3_client.create_bucket(**create_bucket_config)
+            logger.info(
+                f'Created S3 bucket {bucket_name!r} in {region or "us-east-1"}')
         except aws.botocore_exceptions().ClientError as e:
             with ux_utils.print_exception_no_traceback():
                 raise exceptions.StorageBucketCreateError(
-                    f'Attempted to create a bucket '
-                    f'{self.name} but failed.') from e
+                    f'Attempted to create a bucket {self.name} but failed.'
+                ) from e
         return aws.resource('s3').Bucket(bucket_name)
 
     def _delete_s3_bucket(self, bucket_name: str) -> bool:


Code review comments:
diff_hunk: @@ -1368,24 +1368,22 @@ def _create_s3_bucket(self,
         """
         s3_client = self.client
         try:
-            if region is None:
-                s3_client.create_bucket(Bucket=bucket_name)
-            else:
-                if region == 'us-east-1':
-                    # If default us-east-1 region is used, the
-                    # LocationConstraint must not be specified.
-                    # https://stackoverflow.com/a/51912090
-                    s3_client.create_bucket(Bucket=bucket_name)
-                else:
-                    location = {'LocationConstraint': region}
-                    s3_client.create_bucket(Bucket=bucket_name,
-                                            CreateBucketConfiguration=location)
-                logger.info(f'Created S3 bucket {bucket_name} in {region}')
+            create_bucket_config: Dict[str, Any] = {'Bucket': bucket_name}
+            # If default us-east-1 region of create_bucket API is used,
+            # the LocationConstraint must not be specified.
+            # Reference: https://stackoverflow.com/a/51912090
+            if region and region != 'us-east-1':
path: sky/data/storage.py
text: @user1:
```suggestion
            if region is not None and region != 'us-east-1':
```
Always use if foo is None: (or is not None) to check for a None value.

https://google.github.io/styleguide/pyguide.html#2144-decision