Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ref: create Remote Reference (config) #4264

Merged
merged 20 commits into from
Feb 23, 2023
Merged

Conversation

jorgeorpinel
Copy link
Contributor

@jorgeorpinel jorgeorpinel commented Jan 26, 2023

Addresses the main part of #2866:

Currently most of the information about remote storage is in the command reference, mainly under the remote add/modify commands (which have expandable details sections for each supported remote type. We can leave some basics in there (cmd refs should be self-contained), but the details need to be in the User Guide instead. Probably as part of #2856

Maybe also

There should be some basic info and linking from the config guide into the remotes guide though.


In review app: https://dvc-org-guide-data-mgmt-leaijc.herokuapp.com/doc/user-guide/data-management/remote-storage


AFTER:

@jorgeorpinel jorgeorpinel added A: docs Area: user documentation (gatsby-theme-iterative) p1-important Active priorities to deal within next sprints C: ref Content of /doc/*-reference labels Jan 26, 2023
@shcheklein shcheklein temporarily deployed to dvc-org-guide-data-mgmt-leaijc January 26, 2023 08:02 Inactive
@github-actions
Copy link
Contributor

github-actions bot commented Jan 26, 2023

Link Check Report

All 34 links passed!

@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Jan 26, 2023

Hi @dberenbaum, me again...

So far this only creates the first of a proposed page-per-remote Ref section (could be renamed or moved to a wider Configuration Ref). This was pretty-labor intensive so let's review this one first to decide whether to move fwd with this strategy. Thanks, PTAL when you can:

  • Changes to remote add ref.: Shortened and un-hid the S3 section.
  • Changes to remote modify ref.: simplified the S3 section a lot (but still long enough to stay hidden)
  • New Amazon S3 remote ref page with all the details in a more readable/guided presentation

Cc @daavoo @casperdcl

@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Jan 26, 2023

Discussed a bit with @shcheklein and we agreed this is a good direction (long overdue) but also to further simplify cmd refs and to bury these pages under the Data Management guide for now (we expect readers to find them via search results or links probably)... ⌛

@daavoo
Copy link
Contributor

daavoo commented Jan 26, 2023

So far this only creates the first of a proposed page-per-remote Ref section (could be renamed or moved to a wider Configuration Ref). This was pretty-labor intensive so let's review this one first to decide whether to move fwd with this strategy. Thanks, PTAL when you can:

I really like the changes and the separated page.
TBH I would even be more aggressive and remove any s3 specific info (i.e. Example: Customize an S3 remote section) from remote/add remote/modify pages in favor of just a link to the s3 specific page.

@dberenbaum
Copy link
Collaborator

Yup, direction looks good! Thank you @jorgeorpinel!

@dberenbaum
Copy link
Collaborator

dberenbaum commented Jan 26, 2023

  • Changes to remote modify ref.: simplified the S3 section a lot (but still long enough to stay hidden)

Agree with @daavoo that if we go this direction, I don't think we should keep this part of the cmd ref at all.

burry these pages under the Data Management guide for now

Not sure I follow. What pages do you want to bury? I really like the top-level remote ref section you added.

Edit: if it's getting to be too many top-level sections, we could have one top-level "Reference" section with subsections like "Command," "Python API," "Remote", "File."

@shcheklein shcheklein temporarily deployed to dvc-org-guide-data-mgmt-leaijc January 27, 2023 05:45 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-guide-data-mgmt-leaijc February 1, 2023 16:16 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-guide-data-mgmt-leaijc February 1, 2023 17:16 Inactive
and remove repeated content from cmd refs (link to guide)
@shcheklein shcheklein temporarily deployed to dvc-org-guide-data-mgmt-leaijc February 1, 2023 20:43 Inactive
@jorgeorpinel
Copy link
Contributor Author

I don't think we should keep this part of the cmd ref at all.

OK, I'm just listing the remote types in 3 places then: the Remote Storage guide index page, the remote add ref, and the remote modify ref (WIP).

What pages do you want to bury?

To avoid introducing a new top-level section on the docs nav, we are burying the specific storage type pages under User Guide -> Data Management -> Remote Storage -> each page (e.g. S3). Not expecting people to navigate there by clicking the nav (but rather links or search results), this should be fine 🐶🔥.

@jorgeorpinel jorgeorpinel added the C: guide Content of /doc/user-guide label Feb 1, 2023
@shcheklein shcheklein temporarily deployed to dvc-org-guide-data-mgmt-leaijc February 2, 2023 05:23 Inactive
Copy link
Contributor Author

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dberenbaum should we review this now? The PR is starting to grow too much. Maybe we can merge it in with just the first type in and I'll then make another PR for the remaining ones. Cc @shcheklein

Also, this PR already has a child to update related links (#4282)...

List of changes:

Comment on lines -127 to +130
"remote-storage",
{
"slug": "remote-storage",
"source": "remote-storage/index.md",
"children": ["amazon-s3"]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made this into a section (moved remote-storage.md to remote-storage/index.md)

Comment on lines 1 to 6
# Amazon S3

`dvc remote add` a (new) remote name and a valid [S3] URL:

```cli
$ dvc remote add -d myremote s3://<bucket>/<key>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created Amazon S3 page, we can use as template for the remaining types.

Comment on lines +100 to +104
## S3-compatible servers (non-Amazon)

Set the `endpointurl` parameter with the URL to connect to the S3-compatible
service (e.g. [MinIO], [IBM Cloud Object Storage], etc.). For example, let's set
up a [DigitalOcean Space] (equivalent to a bucket in S3) called `mystore` found
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Amazon S3 page includes the non-Amazon, S3-compatible section, since it only involves one config option.

Comment on lines -97 to -134
### Amazon S3

> 💡 Before adding an S3 remote, be sure to
> [Create a Bucket](https://docs.aws.amazon.com/AmazonS3/latest/gsg/CreatingABucket.html).

```cli
$ dvc remote add -d myremote s3://mybucket/path
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

S3 info is now totally removed (just linked instead) from remote add...

Comment on lines -122 to -157
### S3-compatible storage

For object storage that supports an S3-compatible API (e.g.
[Minio](https://min.io/),
[DigitalOcean Spaces](https://www.digitalocean.com/products/spaces/),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(including S3-compatible)

Comment on lines -112 to -146
### Amazon S3

- `url` - remote location, in the `s3://<bucket>/<key>` format:

```cli
$ dvc remote modify myremote url s3://mybucket/path
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...and from remote modify

Comment on lines -366 to -391
### S3-compatible storage
Each type of storage has different config options you can set. See all the
details in the pages linked below.

- `endpointurl` - URL to connect to the S3-compatible storage server or service
(e.g. [Minio](https://min.io/),
[DigitalOcean Spaces](https://www.digitalocean.com/products/spaces/),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(including S3-compatible as well)

Comment on lines +111 to +117
## File systems (local remotes)

<admon type="tip">

Not related to the `--local` option of `dvc remote` and `dvc config`!

</admon>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is the same as what's now in https://dvc.org/doc/user-guide/data-management/remote-storage (I guess I moved it in separate commits so the Git diff isn't clear),

except for this new section on local remotes, which (like S3-compatible) I'm not sure require their own page.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rest of the changes are mainly links and admonitions around this new section and other changes here.

Note there's also a nested PR specifically for link updates as well: #4282

@shcheklein shcheklein temporarily deployed to dvc-org-guide-data-mgmt-leaijc February 23, 2023 04:21 Inactive
@github-actions
Copy link
Contributor

github-actions bot commented Feb 23, 2023

Link Check Report

There were no links to check!

@jorgeorpinel
Copy link
Contributor Author

Is there any discussion you can point me to where you and @shcheklein explain why it would be too much to add it to the top-level or as another reference type? I liked the previous version at the top level since it's one of the first things almost all users need to configure.

Sorry @dberenbaum, it was during a call but the logic is pretty much what I wrote. Either way it's a simple change to make but it may be best to decide before merging to avoid having to make redirects soon after.

Can we finalize and merge this one?

I've solved conflicts for now so this is ready for merge, I think.

@shcheklein shcheklein temporarily deployed to dvc-org-guide-data-mgmt-leaijc February 23, 2023 04:34 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-guide-data-mgmt-leaijc February 23, 2023 04:35 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-guide-data-mgmt-leaijc February 23, 2023 04:41 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-guide-data-mgmt-leaijc February 23, 2023 04:51 Inactive
@jorgeorpinel jorgeorpinel added the ⌛ status: wait-core-merge Waiting for related product PR merge/release label Feb 23, 2023
Copy link
Collaborator

@dberenbaum dberenbaum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jorgeorpinel! Added a couple small edits to clarify, especially while we are in limbo trying to transfer all the ref info to guides. As long as it looks okay with those changes, I'll go ahead and merge.

@shcheklein shcheklein temporarily deployed to dvc-org-guide-data-mgmt-leaijc February 23, 2023 16:12 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-guide-data-mgmt-leaijc February 23, 2023 17:02 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-guide-data-mgmt-leaijc February 23, 2023 17:12 Inactive
@dberenbaum dberenbaum merged commit 9bf61df into main Feb 23, 2023
@dberenbaum dberenbaum deleted the guide/data-mgmt/remote-config branch February 23, 2023 18:25
dberenbaum pushed a commit that referenced this pull request Feb 23, 2023
* ref: start Remote Reference (config)

* Restyled by prettier (#4265)

Co-authored-by: Restyled.io <[email protected]>

* guide: move Remote Storage ref into Data Mgmt

* start: links to new Remotes guide and

and some typo fixes

* guide: finalize S3 storage page and

and remove repeated content from cmd refs (link to guide)

* guide: link to remote storage types in link

* guide: move "local remotes" to Remotes (index page) and

update admonitions and links

* ref: remove S3 examples

* guide: emphasize that remotes use regular cloud storage config

* Update content/docs/user-guide/data-management/remote-storage/amazon-s3.md

* guide: drop `worktree` cloud versioning from Remotes Config

per #4264 (comment)

* guide: move cloud versioning near the top of Remote Config

per #4264 (review)

---------

Co-authored-by: restyled-io[bot] <32688539+restyled-io[bot]@users.noreply.github.com>
Co-authored-by: Restyled.io <[email protected]>
Co-authored-by: Dave Berenbaum <[email protected]>
@jorgeorpinel
Copy link
Contributor Author

LGTM

dberenbaum pushed a commit that referenced this pull request Feb 27, 2023
* ref: start Remote Reference (config)

* Restyled by prettier (#4265)

Co-authored-by: Restyled.io <[email protected]>

* guide: move Remote Storage ref into Data Mgmt

* start: links to new Remotes guide and

and some typo fixes

* guide: finalize S3 storage page and

and remove repeated content from cmd refs (link to guide)

* guide: move "local remotes" to Remotes (index page) and

update admonitions and links

* ref: remove S3 examples

* guide: Azure remote page and start GCS

* guide: finish GCS page and

improvements to the other ones (S3, Azure)

* guide: small link fix in GDrive how-to

* guide: emphasize that remotes use regular cloud storage config

* Update content/docs/user-guide/data-management/remote-storage/amazon-s3.md

* guide: drop `worktree` cloud versioning from Remotes Config

per #4264 (comment)

* guide: move cloud versioning near the top of Remote Config

per #4264 (review)

* fix a link

* typo

* reformat all storage types (Data Mgmt/ Remote Storage)

* guide: move admon about pending Remote guides up

rel. #4284 (review)

* link all remote types (instead of admon)

per #4284 (review)

* Restyled by prettier (#4333)

Co-authored-by: Restyled.io <[email protected]>

* Update content/docs/user-guide/data-management/remote-storage/amazon-s3.md

Co-authored-by: Jorge Orpinel <[email protected]>

---------

Co-authored-by: restyled-io[bot] <32688539+restyled-io[bot]@users.noreply.github.com>
Co-authored-by: Restyled.io <[email protected]>
Co-authored-by: Dave Berenbaum <[email protected]>
@jorgeorpinel jorgeorpinel removed the ⌛ status: wait-core-merge Waiting for related product PR merge/release label Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: docs Area: user documentation (gatsby-theme-iterative) C: guide Content of /doc/user-guide C: ref Content of /doc/*-reference p1-important Active priorities to deal within next sprints
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants