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

Misc. updates #1945

Merged
merged 26 commits into from
Nov 27, 2020
Merged

Misc. updates #1945

merged 26 commits into from
Nov 27, 2020

Conversation

jorgeorpinel
Copy link
Contributor

@jorgeorpinel jorgeorpinel commented Nov 14, 2020

Copy edits stashed from recent PRs

@shcheklein shcheklein temporarily deployed to dvc-landing-jorge-bp9exyntbiiv November 14, 2020 05:46 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-bp9exyntbiiv November 14, 2020 06:03 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-bp9exyntbiiv November 14, 2020 06:04 Inactive
@jorgeorpinel jorgeorpinel changed the title cmd: link add --glob to the glob py mod Misc. updates Nov 14, 2020
@jorgeorpinel jorgeorpinel marked this pull request as ready for review November 14, 2020 06:05
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-bp9exyntbiiv November 14, 2020 06:06 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-bp9exyntbiiv November 16, 2020 19:01 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-bp9exyntbiiv November 16, 2020 20:26 Inactive
jorgeorpinel added a commit that referenced this pull request Nov 16, 2020
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-bp9exyntbiiv November 16, 2020 20:26 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-bp9exyntbiiv November 16, 2020 20:28 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-bp9exyntbiiv November 16, 2020 22:57 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.

Note changes per #1941 (comment):

content/docs/command-reference/gc.md Show resolved Hide resolved
content/docs/command-reference/status.md Show resolved Hide resolved
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-bp9exyntbiiv November 18, 2020 17:54 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-bp9exyntbiiv November 19, 2020 22:59 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-bp9exyntbiiv November 21, 2020 01:30 Inactive
stages. DVC will track changes in them and reflect this in the output of
`dvc status`.
[stages](/doc/command-reference/run). DVC will track changes in them and reflect
this in the output of `dvc status`.
Copy link
Member

Choose a reason for hiding this comment

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

to be honest mentioning dvc status is strange here. The most important thing that it will for example detect this change on dvc repro.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I changed these texts in dd1501b for both external docs since describing what deps/outs are isn't a goal here.

In order to specify an external <abbr>dependency</abbr> for your stage, use the
usual `-d` option in `dvc run` with the external path or URL to your desired
file or directory.
> Note [remote storage](/doc/command-reference/remote) is a separate feature.
Copy link
Member

Choose a reason for hiding this comment

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

why is it needed?

Copy link
Contributor Author

@jorgeorpinel jorgeorpinel Nov 27, 2020

Choose a reason for hiding this comment

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

Because we mention remotes before so it may be confusing.

Is a short note by the end of the section too much?

@@ -13,18 +13,16 @@ track data outside of the <abbr>project</abbr>.
## How external outputs work

DVC can track existing files or directories on an external location with
`dvc add` (`out` field). It can also create external files or directories as
outputs for `dvc.yaml` files (only `outs` field, not metrics or plots).
`dvc add`. It can also define external outputs for `dvc.yaml` stages to create.
Copy link
Member

Choose a reason for hiding this comment

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

DVC pipelines stages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not seeing need to mention pipelines in these docs, we're already going over deps/outs and stages. Too many concepts otherwise, probably? You can click on "stage" to get to pipelines.

[external cache](/doc/use-cases/shared-development-server#configure-the-external-shared-cache)
in the same external/remote file.

> Note that [remote storage](/doc/command-reference/remote) is a separate
Copy link
Member

Choose a reason for hiding this comment

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

same here, why is it needed?


1. Adding a `dvc remote` to use as cache for data in the external location, and
1. Adding a `dvc remote` in the same location as the desired outputs, and
configure it as external <abbr>cache</abbr> with `dvc config`.
Copy link
Member

Choose a reason for hiding this comment

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

as an external cache?

Copy link
Contributor Author

@jorgeorpinel jorgeorpinel Nov 27, 2020

Choose a reason for hiding this comment

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

No "an" needed. Maybe "the" but it's OK like this too (generic to any hypothetical project), I think.

from S3 to process it.

External <abbr>dependencies</abbr> and
External dependencies and
[external outputs](/doc/user-guide/managing-external-data) provide ways to track
Copy link
Member

Choose a reason for hiding this comment

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

track and version?

Copy link
Contributor Author

@jorgeorpinel jorgeorpinel Nov 27, 2020

Choose a reason for hiding this comment

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

Do we version dependencies? Only when they're outputs of a previous stage, I think.

I guess we do keep track of their versions in any case, but don't really control those versions if they are external.

Copy link
Member

Choose a reason for hiding this comment

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

This sentence is about External dependencies and [external outputs]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True. Updating in 60e8055.


Currently, the following types (protocols) of external dependencies are
supported:
DVC can track files or directories on an external location as
Copy link
Member

Choose a reason for hiding this comment

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

sounds like it repeats a lot of the previous paragraph?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True 🤦. Updated in cfe6e07.

Copy link
Member

@shcheklein shcheklein left a comment

Choose a reason for hiding this comment

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

looks good to me, a few minor questions

@jorgeorpinel
Copy link
Contributor Author

Merging this! (I have lots of staged regular updates 😅) but please follow up on any discussions above @shcheklein, and I'll address them in a coming up PR.

@jorgeorpinel jorgeorpinel merged commit aa4b2b8 into master Nov 27, 2020
jorgeorpinel added a commit that referenced this pull request Nov 28, 2020
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.

3 participants