docs: Std. SSH URLs and rel. notes about SFTP #1649

jorgeorpinel · 2020-08-01T16:47:53Z

You may disregard these recommendations if you used the Edit on GitHub button from dvc.org to improve a doc in place.

❗ Please read the guidelines in the Contributing to the Documentation list if you make any substantial changes to the documentation or JS engine.

🐛 Please make sure to mention Fix #issue (if applicable) in the description of the PR. This causes GitHub to close it automatically when the PR is merged.

Please choose to allow us to edit your branch when creating the PR.

Thank you for the contribution - we'll try to review it as soon as possible. 🙏

per iterative/dvc#4167 (comment)

content/docs/command-reference/remote/modify.md

content/docs/user-guide/external-dependencies.md

content/docs/user-guide/managing-external-data.md

content/docs/user-guide/external-dependencies.md

content/docs/user-guide/managing-external-data.md

per #1649 (review) and #1649 (review)

content/docs/user-guide/external-dependencies.md

per #1649 (review) and #1649 (review)

per #1649 (review)

jorgeorpinel · 2020-08-11T23:50:51Z

Just one discussion pending here @shcheklein ~~but we should probably try to merge #1649 first?~~ Please check it when you have a change — I think it's ready.

content/docs/command-reference/remote/add.md

content/docs/user-guide/external-dependencies.md

content/docs/user-guide/managing-external-data.md

content/docs/command-reference/get-url.md

content/docs/user-guide/external-dependencies.md

per #1649 (review)

content/docs/command-reference/import-url.md

content/docs/command-reference/remote/add.md

shcheklein · 2020-08-16T02:40:59Z

content/docs/user-guide/external-dependencies.md

+$ dvc run -n download_file
+          -d ssh://[email protected]/path/from/sftp/root/to/data \
+          -o data \
+          scp [email protected]:/path/from/sftp/root/to/data data


not sure that scp is using sftp and it's the same convention ... how often sftp root is not /?

Good point about scp not being SFTP! Missed that 🤦 I'm going to remove these stfp mentions from the paths... ⌛

how often sftp root is not /?

Not sure but it's a note we already have in remote add so it has been deemed important before. Plus per this iterative/dvc#4167 (comment) I thought we definitely wanted to mention this clearly in all the docs where it may cause issues.

OK so... I'm removing all the /from/sftp/... paths again. Having the note underneath should be enough I think.

In fact, should we just remove these SFTP notes completely? And or consider renaming ssh:// to sftp:// if that's the actual protocol we're using... Then users could be expected to know what kinds of URLs and paths are supported. Cc @efiop

In fact, should we just remove these SFTP notes completely?

Could just create 1 note in remote modify/add help. The rest could be removed, yes.

nd or consider renaming ssh:// to sftp:// if that's the actual protocol we're using...

Not really worth renaming. Plus, we do use ssh too, but only for external data management, so it is not a direct swap. Would just keep as is for now.

I left the notes everywhere just in case. It's a shorter note version now, anyway.

Agreed about not renaming. Thanks @efiop

per #1649 (review)

per #1649 (comment)

jorgeorpinel · 2020-08-18T07:59:57Z

content/docs/command-reference/remote/add.md

-> (On Linux, see the `ChrootDirectory` setting in `/etc/ssh/sshd_config`.) In
-> these cases, the path component in the SSH URL (e.g. `/path/to/dir` above)
-> should be specified relative to the SFTP root instead. For example, on some
-> Sinology NAS drives, the SFTP root might be in directory `/volume1`, in which
-> case you should use path `/path/to/dir` instead of `/volume1/path/to/dir`.


I decided to remove these details. They seem more like tips unrelated to DVC. Users can research SFTP details on other sites easily, I think. Left a quick note: "Note that the server's SFTP root might differ from its physical root (/)." that should be enough of a tip for the few users running into that issue.

ounds good! and we can elaborate when we have a separate page per remote I guess

We're planning to do separate pages per remote? I don't recall that decision. It sounds like a good plan except that the remote types are already burried in docs->cmd ref->remote->add/modify->expandable sections so not sure adding yet another level to that tree is a good idea.

Maybe the remote types should all be in the remote ref. index? And the remote concept explanation in Basic Concepts.

content/docs/user-guide/external-dependencies.md

shcheklein · 2020-08-18T21:28:18Z

content/docs/command-reference/get-url.md

@@ -39,7 +39,7 @@ DVC supports several types of (local or) remote locations (protocols):
 | `local` | Local path     | `/path/to/local/data`                      |
 | `s3`    | Amazon S3      | `s3://mybucket/data`                       |
 | `gs`    | Google Storage | `gs://mybucket/data`                       |
-| `ssh`   | SSH server     | `ssh://[email protected]:/path/to/data`     |
+| `ssh`   | SSH server     | `ssh://[email protected]/path/to/data`      |


do we need the trailing space?

Yes. It's all formatted by Prettier this way.

shcheklein · 2020-08-18T21:28:26Z

content/docs/command-reference/import-url.md

@@ -62,7 +62,7 @@ DVC supports several types of (local or) remote locations (protocols):
 | `s3`     | Amazon S3                                           | `s3://mybucket/data`                       |
 | `azure`  | Microsoft Azure Blob Storage                        | `azure://my-container-name/path/to/data`   |
 | `gs`     | Google Cloud Storage                                | `gs://mybucket/data`                       |
-| `ssh`    | SSH server                                          | `ssh://[email protected]:/path/to/data`     |
+| `ssh`    | SSH server                                          | `ssh://[email protected]/path/to/data`      |


Auto-formatted. To align columns I guess. Looks "prettier" tehehe

shcheklein · 2020-08-18T21:31:50Z

approved! just a few minor comments ...

docs: recommend absolute paths for SSH and HDFS remote connections

715b8b9

per iterative/dvc#4167 (comment)

shcheklein temporarily deployed to dvc-landing-remote-absp-0hep2s August 1, 2020 16:48 Inactive

jorgeorpinel mentioned this pull request Aug 1, 2020

ssh: support scp-like relpaths in urls iterative/dvc#4167

Closed

jorgeorpinel requested a review from efiop August 1, 2020 16:48

jorgeorpinel commented Aug 1, 2020

View reviewed changes

content/docs/command-reference/remote/modify.md Outdated Show resolved Hide resolved

efiop reviewed Aug 1, 2020

View reviewed changes

content/docs/user-guide/external-dependencies.md Outdated Show resolved Hide resolved

efiop reviewed Aug 1, 2020

View reviewed changes

content/docs/user-guide/managing-external-data.md Outdated Show resolved Hide resolved

shcheklein reviewed Aug 1, 2020

View reviewed changes

content/docs/user-guide/external-dependencies.md Outdated Show resolved Hide resolved

efiop reviewed Aug 1, 2020

View reviewed changes

content/docs/user-guide/managing-external-data.md Outdated Show resolved Hide resolved

cmd: forgot to save changes to get-url.md (see prev commit)

d27e07d

jorgeorpinel temporarily deployed to dvc-landing-remote-absp-0hep2s August 1, 2020 18:20 Inactive

Merge branch 'master' into remote/abspath

c27303b

jorgeorpinel changed the title ~~docs: recommend absolute paths for SSH and HDFS remote connections~~ docs: absolute paths for SSH and HDFS remote connections Aug 11, 2020

jorgeorpinel added a commit that referenced this pull request Aug 11, 2020

guide: improve absolute paths (and note) for SSH x deps

8eee7e9

per #1649 (review) and #1649 (review)

jorgeorpinel commented Aug 11, 2020

View reviewed changes

content/docs/user-guide/external-dependencies.md Outdated Show resolved Hide resolved

jorgeorpinel added a commit that referenced this pull request Aug 11, 2020

guide: improve absolute paths (and note) for SSH x deps

ce0048a

per #1649 (review) and #1649 (review)

jorgeorpinel force-pushed the remote/abspath branch from 8eee7e9 to ce0048a Compare August 11, 2020 05:59

guide: improve absolute paths (and note) for SSH x deps

94926ee

per #1649 (review) and #1649 (review)

jorgeorpinel force-pushed the remote/abspath branch from ce0048a to 94926ee Compare August 11, 2020 06:00

jorgeorpinel added 2 commits August 11, 2020 01:01

guide: update SSH/HDFS abs paths (and notes) in x outs page

8c60723

per #1649 (review)

guide: revert SCP URLs from external data guides

8703362

per #1649 (review)

This comment has been minimized.

Sign in to view

jorgeorpinel requested a review from shcheklein August 11, 2020 06:05

guide: update note about rel. SSH paths from SFTP root

ce32ddf

per #1649 (review)

jorgeorpinel changed the title ~~docs: absolute paths for SSH and HDFS remote connections~~ docs: absolute paths for SSH and HDFS remote URLs Aug 11, 2020

shcheklein reviewed Aug 12, 2020

View reviewed changes

content/docs/command-reference/remote/add.md Outdated Show resolved Hide resolved

shcheklein reviewed Aug 12, 2020

View reviewed changes

content/docs/user-guide/external-dependencies.md Outdated Show resolved Hide resolved

shcheklein reviewed Aug 12, 2020

View reviewed changes

content/docs/user-guide/managing-external-data.md Outdated Show resolved Hide resolved

shcheklein reviewed Aug 12, 2020

View reviewed changes

content/docs/command-reference/get-url.md Outdated Show resolved Hide resolved

shcheklein reviewed Aug 14, 2020

View reviewed changes

content/docs/user-guide/external-dependencies.md Outdated Show resolved Hide resolved

jorgeorpinel added 2 commits August 14, 2020 18:47

docs: roll back absolute HDFS URLs

7414ce6

per #1649 (review)

cmd: change SSH example URL in import-url

710720d

jorgeorpinel requested a review from shcheklein August 15, 2020 00:24

jorgeorpinel changed the title ~~docs: absolute paths for SSH and HDFS remote URLs~~ docs: absolute paths for SSH URLs Aug 15, 2020

jorgeorpinel changed the title ~~docs: absolute paths for SSH URLs~~ docs: SSH URLs relative to SFTP root Aug 15, 2020

shcheklein reviewed Aug 16, 2020

View reviewed changes

content/docs/command-reference/import-url.md Outdated Show resolved Hide resolved

shcheklein reviewed Aug 16, 2020

View reviewed changes

content/docs/command-reference/remote/add.md Outdated Show resolved Hide resolved

shcheklein reviewed Aug 16, 2020

View reviewed changes

jorgeorpinel added 6 commits August 18, 2020 01:02

Merge branch 'master' into remote/abspath

3be66c1

cmd: remove "sftp" mention in SSH URLs of get/import-url

6ec6717

per #1649 (review)

cmd: use std. SFTP note from remote add in other SSH URL examples

f68eb40

cmd: remove "absolute" HDFS URL path from remote modify

dfbc33e

guide: fix SSH paths in external outputs doc

2879de2

ssh: remove 'sftp' from example URLs...

990de59

per #1649 (comment)

jorgeorpinel changed the title ~~docs: SSH URLs relative to SFTP root~~ docs: Std. SSH URLs and rel. notes about SFTP Aug 18, 2020

jorgeorpinel commented Aug 18, 2020

View reviewed changes

jorgeorpinel force-pushed the remote/abspath branch from 6d16af5 to 2010c71 Compare August 18, 2020 08:05

jorgeorpinel commented Aug 18, 2020

View reviewed changes

content/docs/user-guide/external-dependencies.md Show resolved Hide resolved

guide: roll-back unrelated changes to URLs in external data docs

0fb4b0d

jorgeorpinel force-pushed the remote/abspath branch from 2010c71 to 0fb4b0d Compare August 18, 2020 08:08

jorgeorpinel commented Aug 18, 2020

View reviewed changes

content/docs/user-guide/external-dependencies.md Show resolved Hide resolved

guide: roll-back unnecessary new line in x outs page

34bae5c

jorgeorpinel requested a review from shcheklein August 18, 2020 08:11

shcheklein reviewed Aug 18, 2020

View reviewed changes

shcheklein approved these changes Aug 18, 2020

View reviewed changes

jorgeorpinel merged commit 229826e into master Aug 20, 2020

jorgeorpinel deleted the remote/abspath branch August 20, 2020 22:48

docs: Std. SSH URLs and rel. notes about SFTP #1649

docs: Std. SSH URLs and rel. notes about SFTP #1649

Uh oh!

Conversation

jorgeorpinel commented Aug 1, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

jorgeorpinel commented Aug 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shcheklein commented Aug 18, 2020

Uh oh!

Uh oh!

jorgeorpinel commented Aug 11, 2020 •

edited

Loading