[Docs] Storage docs rewrite #3162

romilbhardwaj · 2024-02-14T23:59:14Z

Closes #3123.

TODO:

Mounting mode considerations section - a nice table to help users pick b/w COPY and MOUNT

Tested:

Rendered locally

…o storage-docs-rewrite

Michaelvll · 2024-02-16T18:22:42Z

Just activate the hidden doc in our doc page for easier review: https://skypilot.readthedocs.io/en/storage-docs-rewrite/reference/storage.html
Will get to this soon : )

concretevitamin

Very clean! Quick look first.

docs/source/reference/storage.rst

Michaelvll · 2024-02-16T18:40:34Z

docs/source/reference/storage.rst


-2.  **You want to mount an existing S3/GCS/R2 bucket to your remote VM -** specify
-    just the source field (e.g., s3://my-bucket/, gs://my-bucket/ or r2://my-bucket/).
+    .. tab-item:: Upload files to new bucket


Does this mean we are not able to upload files to existing bucket? This might be unexpected, as in some of our users use case, they woud like to have all the data in a single bucket.

I've reworded the titles and added a visually distinctnote - does that help?

Ahh, I was wondering what will happen if the bucket already exists and was not created by SkyPilot. For example, there is a team writing files to the same bucket, and the bucket will only be managed by SkyPilot for one of the teammates.

Just a kind bump for this @romilbhardwaj : )

Ahh thanks for the ping @Michaelvll, this had slipped through the cracks.

If the bucket was created by a team member, other members will have to use the source: s3://... pattern.

We disallow name: external-bucket pattern from mounting external buckets because e.g., if I want to create a bucket and use the name of a pre-existing public (or private) bucket, these semantics will silently mount the bucket and my task will fail when it tries to write the desired path (or worse yet, overwrite unintended files).

This behavior manifested in our smoke tests too (and is now fixed in #2804). Our test_using_file_mounts_with_env_vars was silently re-mounting skypilot-temp-gcs-test bucket which was shared in our GCS account, instead of creating a new bucket at each test instance.

Thanks that makes sense! I found it a bit non-intuitive, for the following case, but I guess it is fine to leave it to the future.

If I am a member of a org, and created a bucket for with mount mode for storing some jupyter notebooks:

file_mounts: /dst: name: my-bucket store: s3 mode: MOUNT run: jupyter notebook /dst

The person then shares the YAML with another member in the org. Does this mean the other member running this yaml will fail and have to change it to:

file_mounts: /dst: source: s3://my-bucket mode: MOUNT run: jupyter notebook /dst

Michaelvll

Thanks for revamping the doc for storage @romilbhardwaj! This page looks super nice. Left several comments about the ML specific use case clarification (similar as @concretevitamin 's comments).

docs/source/reference/storage.rst

romilbhardwaj · 2024-02-16T22:45:35Z

Thanks for the feedback @Michaelvll and @concretevitamin! Ready for another look.

concretevitamin

Thanks @romilbhardwaj, did a pass.

docs/source/reference/storage.rst

concretevitamin · 2024-02-18T18:44:06Z

docs/source/reference/storage.rst

@@ -1,303 +1,291 @@
 .. _sky-storage:

-SkyPilot Storage
-=================
+Using Cloud Buckets


Suggested change

Using Cloud Buckets

Cloud Object Storage

This seems most commonly used. Cloud Bucket doesn't show too many search results.

"Using": Repetitive w/ section title "Using Data"

Cloud buckets appears to be the more commonly used term:

Changed to just Cloud Buckets.

docs/source/reference/storage.rst

concretevitamin · 2024-02-18T18:47:30Z

docs/source/reference/storage.rst

-
-    # Remember to run `sky storage ls` and `sky storage delete` to delete the
-    # created storage objects!
+    .. tab-item:: Create (or re-use) a bucket


Suggested change

.. tab-item:: Create (or re-use) a bucket

.. tab-item:: Create or reuse a bucket

Can we make this the 2nd (or even 1st) tab? It's more common than "Upload files ..".

Moved to 2nd tab.

Also I've removed the mention of "reuse", since it conflicts with "Use an existing bucket". The re-use bit is clearly stated in a note box for people who are interested in using this:

Several options

"Use an existing bucket" / "Create and reuse a bucket" ("and" should avoid the conflict?)

"Use an external bucket" / "Create and reuse a bucket"

"Use an existing bucket" / "Create a bucket"

Slight preference for 1 & 2, since I think the "reuse" part may be common to warrant being in the tabs; that said, 3 is ok with me too if @romilbhardwaj and @Michaelvll agree :)

I'm fine with 2 or 3

I would prefer 1 & 3, as Use an extneral bucket sounds like it cannot be used with a SkyPilot managed storage. : )

docs/source/reference/storage.rst

docs/source/reference/yaml-spec.rst

docs/source/reference/storage.rst

concretevitamin · 2024-02-18T19:09:20Z

Also, should grep for SkyPilot Storage and rephrase. E.g., https://skypilot.readthedocs.io/en/storage-docs-rewrite/examples/syncing-code-artifacts.html

romilbhardwaj · 2024-02-20T03:19:43Z

Thanks @concretevitamin, ready for another look

docs/source/reference/storage.rst

docs/source/examples/syncing-code-artifacts.rst

concretevitamin · 2024-02-20T05:50:57Z

docs/source/reference/storage.rst

-
-    # Remember to run `sky storage ls` and `sky storage delete` to delete the
-    # created storage objects!
+    .. tab-item:: Create (or re-use) a bucket


Several options

"Use an existing bucket" / "Create and reuse a bucket" ("and" should avoid the conflict?)

"Use an external bucket" / "Create and reuse a bucket"

"Use an existing bucket" / "Create a bucket"

Slight preference for 1 & 2, since I think the "reuse" part may be common to warrant being in the tabs; that said, 3 is ok with me too if @romilbhardwaj and @Michaelvll agree :)

concretevitamin

LGTM, some minor nits. Thanks @romilbhardwaj.

docs/source/examples/syncing-code-artifacts.rst

romilbhardwaj · 2024-02-20T21:29:09Z

Thanks for the reviews! Merging this soon - lmk if you have any other feedback @Michaelvll @concretevitamin

romilbhardwaj added 8 commits November 20, 2023 14:45

wip

b317267

Merge branch 'master' of https://github.com/skypilot-org/skypilot int…

40e3a1a

…o storage-docs-rewrite

WIP

ceb9437

wip

e5f70f2

Add storage modes docs and infographic

80beefe

wip

40732d7

Add mounting mode table and common patterns

ac648f2

Add src

e0bb380

romilbhardwaj marked this pull request as ready for review February 15, 2024 20:41

romilbhardwaj requested review from Michaelvll and concretevitamin February 15, 2024 20:41

romilbhardwaj added 2 commits February 15, 2024 12:43

nits

8e868fa

nits

ce307b2

romilbhardwaj mentioned this pull request Feb 15, 2024

[Storage] Error Handle Attempts to Mount Non-sky Managed Bucket without Source #2804

Merged

6 tasks

concretevitamin reviewed Feb 16, 2024

View reviewed changes

Michaelvll reviewed Feb 16, 2024

View reviewed changes

comments

1340fb0

concretevitamin reviewed Feb 18, 2024

View reviewed changes

comments

a288552

concretevitamin reviewed Feb 20, 2024

View reviewed changes

concretevitamin approved these changes Feb 20, 2024

View reviewed changes

comments

c2ee722

Michaelvll reviewed Feb 20, 2024

View reviewed changes

docs/source/examples/syncing-code-artifacts.rst Outdated Show resolved Hide resolved

romilbhardwaj added 2 commits February 20, 2024 10:31

nits

0f54452

nits

6db55e9

romilbhardwaj merged commit 2a96ee4 into master Feb 20, 2024
19 checks passed

romilbhardwaj deleted the storage-docs-rewrite branch February 20, 2024 22:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Docs] Storage docs rewrite #3162

[Docs] Storage docs rewrite #3162

romilbhardwaj commented Feb 14, 2024 •

edited

Michaelvll commented Feb 16, 2024

concretevitamin left a comment

Michaelvll Feb 16, 2024

romilbhardwaj Feb 16, 2024

Michaelvll Feb 16, 2024

Michaelvll Feb 20, 2024

romilbhardwaj Feb 20, 2024

Michaelvll Feb 20, 2024

Michaelvll left a comment

romilbhardwaj commented Feb 16, 2024

concretevitamin left a comment

concretevitamin Feb 18, 2024

romilbhardwaj Feb 20, 2024

concretevitamin Feb 18, 2024

romilbhardwaj Feb 20, 2024

concretevitamin Feb 20, 2024

romilbhardwaj Feb 20, 2024

Michaelvll Feb 20, 2024 •

edited

concretevitamin commented Feb 18, 2024

romilbhardwaj commented Feb 20, 2024

concretevitamin Feb 20, 2024

concretevitamin left a comment

romilbhardwaj commented Feb 20, 2024

	.. tab-item:: Create (or re-use) a bucket
	.. tab-item:: Create or reuse a bucket

[Docs] Storage docs rewrite #3162

[Docs] Storage docs rewrite #3162

Conversation

romilbhardwaj commented Feb 14, 2024 • edited

Michaelvll commented Feb 16, 2024

concretevitamin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Michaelvll left a comment

Choose a reason for hiding this comment

romilbhardwaj commented Feb 16, 2024

concretevitamin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Michaelvll Feb 20, 2024 • edited

Choose a reason for hiding this comment

concretevitamin commented Feb 18, 2024

romilbhardwaj commented Feb 20, 2024

Choose a reason for hiding this comment

concretevitamin left a comment

Choose a reason for hiding this comment

romilbhardwaj commented Feb 20, 2024

romilbhardwaj commented Feb 14, 2024 •

edited

Michaelvll Feb 20, 2024 •

edited