Skip to content

Add description for Azure url format#1337

Merged
shcheklein merged 3 commits into
treeverse:masterfrom
feedbackfruits:azure-external-dependencies
May 23, 2020
Merged

Add description for Azure url format#1337
shcheklein merged 3 commits into
treeverse:masterfrom
feedbackfruits:azure-external-dependencies

Conversation

@steffansluis
Copy link
Copy Markdown
Contributor

@steffansluis steffansluis commented May 22, 2020

  • Add a table entry for the URL format to make use of dvc import-url with Microsoft Azure Blob Storage.
  • Add example for usage of Microsoft Azure Blob Storage with external dependencies.

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. 🙏

@steffansluis steffansluis force-pushed the azure-external-dependencies branch from 004617e to dae4297 Compare May 22, 2020 20:53
@efiop
Copy link
Copy Markdown
Contributor

efiop commented May 22, 2020

Thank you for the docs @steffansluis ! 🙏 Could you also add azure to https://dvc.org/doc/user-guide/external-dependencies , please?

@shcheklein
Copy link
Copy Markdown
Contributor

Thanks @steffansluis 🙏

@steffansluis steffansluis force-pushed the azure-external-dependencies branch from acbc7ed to a1af810 Compare May 22, 2020 23:06
@efiop efiop requested a review from jorgeorpinel May 22, 2020 23:14
Comment thread content/docs/command-reference/import-url.md Outdated
Comment on lines +73 to +77
az storage copy \
-d data.json \
--source-account-name my-account \
--source-container my-container-name \
--source-blob data.txt
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The indentation here seems excessive? Code block lines shouldn't go over 72 characters long, please. How about just align the flags under copy? Or go further back if still they go over 72 🙂

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The indentation is based on the indentation used in the HDFS example. I completely agree that it is excessive, but I figured there was probably a reason for it. I'll move it back a bit so they line up better with copy, I did check though and the longest line was 67 chars so it should be fine regardless

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for bringing this to out attention @steffansluis I'll review this in a separate PR 🙂

Copy link
Copy Markdown
Contributor

@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.

Just a couple minor comments so far. Thanks for the contribution!

steffansluis and others added 2 commits May 23, 2020 14:30
Apply unrelated suggested change: Rename `Google Storage` to `Google Cloud Storage`

Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
@steffansluis steffansluis force-pushed the azure-external-dependencies branch from d95546c to 2ea62ad Compare May 23, 2020 12:31
@steffansluis steffansluis requested a review from jorgeorpinel May 23, 2020 12:32
@shcheklein shcheklein temporarily deployed to dvc-landing-azure-exter-evfenr May 23, 2020 15:31 Inactive
@shcheklein shcheklein merged commit 47f8751 into treeverse:master May 23, 2020
@shcheklein
Copy link
Copy Markdown
Contributor

Thanks @steffansluis 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants