command-reference/update: add docs for new --rev option #890

ilgooz · 2020-01-02T13:19:36Z

--

Disregard the recommendations below if you use Edit on GitHub button to improve the docs 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 enables GitHub to link the PR to the corresponding bug and close it automatically when PR is merged.

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

shcheklein · 2020-01-02T18:12:05Z

static/docs/command-reference/update.md

@@ -39,6 +39,12 @@ to learn how to "update" fixed-revision imports.

 ## Options

+- `--rev` - specific


see the note in the description above ... we should probably update it as well? does --rev override dvc import --rev, for example?

#890 (comment)

done by 82c8b5c, can you please re-check?

shcheklein · 2020-01-02T18:13:21Z

static/docs/command-reference/update.md

+- `--rev` - specific
+  [Git revision](https://git-scm.com/book/en/v2/Git-Internals-Git-References)
+  (such as a branch name, a tag, or a commit hash) of the DVC repository to
+  update the data from. Repository's revision formed by the first import is used


do we lock a specific revision for import-url or for dvc import something (no --rev)? It's not 100% clear.

I created a discussion for choosing a behavior here: iterative/dvc#2849 (comment)

changed by 82c8b5c, can you please re-check?

shcheklein

@ilgooz good stuff! thanks for updating the docs. Please, take a look at the comments. Also, it would be great to have an example to see the clear difference in the behavior.

jorgeorpinel

Thanks for the PR! I have some comments, see below.

Also, see #890 (review)

I think we should revise the last paragraph in the description "... when the --rev (revision) option of dvc import has been used to create an import stage..." and/or add a new one explaining the relationship/interactions/cases when using --rev options in dvc import and then in dvc update again. If you can get that change started I can review the wording and phrasing if needed.

static/docs/command-reference/update.md

jorgeorpinel · 2020-01-02T18:18:23Z

Oops it seems Ivan and myself did similar reviews simultaneously. Sorry 😬 I'll try to combine them.

ilgooz · 2020-01-04T18:38:20Z

Big thanks @jorgeorpinel! I'll continue with applying the requested changes once we figure out this one: iterative/dvc#2849 (comment)

ilgooz · 2020-01-04T23:19:23Z

@ilgooz good stuff! thanks for updating the docs. Please, take a look at the comments. Also, it would be great to have an example to see the clear difference in the behavior.

added an example by 82c8b5c but we need to create a new branch named v2 with updated source on the iterative/example-get-started repo to create new results.

ilgooz · 2020-01-05T00:16:32Z

public/static/docs/command-reference/update.md

+-> 'model.pkl'
+```
+
+Since the source files has changed, we see the results of our "update".


Suggested change

Since the source files has changed, we see the results of our "update".

Since the source files are different at `v2` comparing to the currently locked revision, we see the side effects of our "update" as different results.

?

Maybe something like

Suggested change

Since the source files has changed, we see the results of our "update".

This actually has an effect because the `model.pkl.dvc` file at tag `v2` points to different data that what's currently in the `rev_lock` field of the import stage.

Notice I also avoided the term "lock" here as well.
(Please rewrap.)

I would also keep the entire

Note that dvc update updates the rev_lock field...

note here, at the end of the doc.

jorgeorpinel

Phew! Sorry, lots of comments in this review, but we're getting there.

jorgeorpinel · 2020-01-05T01:17:16Z

public/static/docs/command-reference/import.md

-> In the above example, the value for `rev` in the new import stage will be
-> `master`, which happens to be the default branch in this Git repository, so
-> the command is equivalent to not using `--rev` at all.
+moves (e.g. a branch), you may use `dvc update` preferably with `--rev` option


Suggested change

moves (e.g. a branch), you may use `dvc update` preferably with `--rev` option

moves (e.g. a branch), you may use `dvc update`

Why "preferably with --rev option"?

jorgeorpinel · 2020-01-05T01:19:56Z

public/static/docs/command-reference/import.md

-> `master`, which happens to be the default branch in this Git repository, so
-> the command is equivalent to not using `--rev` at all.
+moves (e.g. a branch), you may use `dvc update` preferably with `--rev` option
+to bring the data up to date. Using `--rev` also updates the fixed-revision.


Suggested change

to bring the data up to date. Using `--rev` also updates the fixed-revision.

to bring the data up to date.

Instead of that last sentence, I would keep some of the deleted text from above and change it a little bit. Somehing like:

For typically static references (e.g. tags), or for SHA commits, in order to update an import, it's necessary to use dvc update --rev with a different reference, as shown below. This will replace the rev field in the DVC-file (import stage).

$ dvc update --rev <new-ref> \ [email protected]:iterative/dataset-registry.git \ use-cases/cats-dogs

Yes, I agree that it should be consistent and title should be updated. It's not important to have "re-import" now. It def worth mentioning that it overwrites the file if you run it again, though. Not sure about the example section - let you decide :)

I vote for removing the "re-import" term everywhere then (including from the corresponding link hrefs).

jorgeorpinel · 2020-01-05T01:26:10Z

public/static/docs/command-reference/update.md

@@ -7,7 +7,7 @@ repositories</abbr>, and corresponding
 ## Synopsis

 ```usage
-usage: dvc update [-h] [-q | -v] targets [targets ...]
+usage: dvc update [-h] [-q | -v] [--rev [REV]] targets [targets ...]


Please break into 2 lines so there's no scroll bar on the rendered cmd ref:

Suggested change

usage: dvc update [-h] [-q | -v] [--rev [REV]] targets [targets ...]

usage: dvc update [-h] [-q | -v] [--rev [REV]]

targets [targets ...]

jorgeorpinel · 2020-01-05T01:28:53Z

public/static/docs/command-reference/update.md

-Another detail to note is that when the `--rev` (revision) option of
-`dvc import` has been used to create an import stage, DVC is not aware of what
-kind of
-[Git revision](https://git-scm.com/book/en/v2/Git-Internals-Git-References) this
-is, for example a branch or a tag. For typically static references (e.g. tags),
-or for SHA commits, `dvc update` will not have any effect on the import. Refer
-to the
-[re-importing example](/doc/command-reference/import#example-fixed-revisions-re-importing)
-to learn how to "update" fixed-revision imports.


The whole sentence

Another detail to note is that when the `--rev` (revision) option of `dvc import` has been used to create an import stage, DVC is not aware of what kind of [Git revision](https://git-scm.com/book/en/v2/Git-Internals-Git-References) this is, for example a branch or a tag.

is still valid and informative, I think.

And yes I know I said we should revise this paragraph in #890 (comment) but didn't necessarily mean delete this sentence 🙂

jorgeorpinel · 2020-01-05T01:31:01Z

public/static/docs/command-reference/update.md

-to the
-[re-importing example](/doc/command-reference/import#example-fixed-revisions-re-importing)
-to learn how to "update" fixed-revision imports.
+To "update" fixed-revision to static references (e.g. tags), or for SHA commits,


Suggested change

To "update" fixed-revision to static references (e.g. tags), or for SHA commits,

To update [fixed-revision](/doc/command-reference/import#example-fixed-revisions-re-importing) import stages (e.g. tags or SHA commits),

(Please rewrap to 80 chars.)

jorgeorpinel · 2020-01-05T01:34:53Z

public/static/docs/command-reference/update.md

+- `--rev` - specific
+  [Git revision](https://git-scm.com/book/en/v2/Git-Internals-Git-References)
+  (such as a branch name, a tag, or a commit hash) of the DVC repository to
+  update the data from. Using this option also updates the fixed-revision.


Using this option also updates the fixed-revision.

OK but maybe we should explain what a fixed revision is and what exactly gets updated in the DVC-file (import stage), instead of using that term. Please lmk if you'd like some suggestions, I can def. help with the writing part if needed.

For inspiration check out this not, found in the Examples section below:

Note that dvc update updates the rev_lock field of the corresponding DVC-file (when there are changes to bring in).

jorgeorpinel · 2020-01-05T01:40:49Z

public/static/docs/command-reference/update.md

+To update from and also lock to a specific revision of a <abbr>data
+artifact</abbr>, we may use the `--rev` option this time:


Nice addition to the examples! How about:

To force an update from a specific version of a data artifact, we may specify a Git revision with the --rev option:

for this sentence? And please wrap <abbr>data artifact</abbr> in the first paragraph of this whole Examples section instead (thanks for catching this.)

(p.s. I think let's avoid the term "lock" here which results confusing given how it's previously used, and given our dvc lock command, among other types of locking in DVC.)

jorgeorpinel · 2020-01-05T01:51:17Z

public/static/docs/command-reference/update.md

+-> 'model.pkl'
+```
+
+Since the source files has changed, we see the results of our "update".


Maybe something like

Suggested change

Since the source files has changed, we see the results of our "update".

This actually has an effect because the `model.pkl.dvc` file at tag `v2` points to different data that what's currently in the `rev_lock` field of the import stage.

Notice I also avoided the term "lock" here as well.
(Please rewrap.)

jorgeorpinel · 2020-01-05T01:52:02Z

public/static/docs/command-reference/update.md

+-> 'model.pkl'
+```
+
+Since the source files has changed, we see the results of our "update".


I would also keep the entire

Note that dvc update updates the rev_lock field...

note here, at the end of the doc.

jorgeorpinel · 2020-01-14T17:05:39Z

OK so this is going stale. Should we close until iterative/dvc/pull/2993 is merged? Or just wait for now? @shcheklein

shcheklein · 2020-01-14T17:13:20Z

@jorgeorpinel let's keep it open until we make some decision on the core project side, please. @ilgooz could you please address the comments in the dvc repo so that we can move this forward?

shcheklein · 2020-01-21T21:06:16Z

closing this since the PR in the core repo is closed as stale

command-reference/update: add docs for new --rev option

99bd5b9

ilgooz force-pushed the feature/command-update-rev branch from 38cebc2 to 99bd5b9 Compare January 2, 2020 13:26

ilgooz marked this pull request as ready for review January 2, 2020 13:27

ilgooz mentioned this pull request Jan 2, 2020

commands/update: add --rev option iterative/dvc#2993

Closed

3 tasks

shcheklein reviewed Jan 2, 2020

View reviewed changes

shcheklein requested changes Jan 2, 2020

View reviewed changes

jorgeorpinel suggested changes Jan 2, 2020

View reviewed changes

static/docs/command-reference/update.md Outdated Show resolved Hide resolved

static/docs/command-reference/update.md Outdated Show resolved Hide resolved

command-reference: update docs related 'update' command

82c8b5c

Merge branch 'master' into feature/command-update-rev

85892b2

ilgooz requested review from shcheklein and jorgeorpinel January 4, 2020 23:52

ilgooz commented Jan 5, 2020

View reviewed changes

jorgeorpinel suggested changes Jan 5, 2020

View reviewed changes

weekly-digest bot mentioned this pull request Jan 5, 2020

Weekly Digest (29 December, 2019 - 5 January, 2020) #892

Closed

weekly-digest bot mentioned this pull request Jan 12, 2020

Weekly Digest (5 January, 2020 - 12 January, 2020) #916

Closed

jorgeorpinel mentioned this pull request Jan 16, 2020

doc: regular updates (mid Jan) #915

Merged

weekly-digest bot mentioned this pull request Jan 19, 2020

Weekly Digest (12 January, 2020 - 19 January, 2020) #932

Closed

shcheklein closed this Jan 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

command-reference/update: add docs for new --rev option #890

command-reference/update: add docs for new --rev option #890

ilgooz commented Jan 2, 2020 •

edited

Loading

shcheklein Jan 2, 2020

This comment was marked as resolved.

ilgooz Jan 4, 2020

ilgooz Jan 4, 2020

shcheklein Jan 2, 2020

ilgooz Jan 4, 2020

ilgooz Jan 4, 2020

shcheklein left a comment

jorgeorpinel left a comment •

edited

Loading

jorgeorpinel commented Jan 2, 2020

ilgooz commented Jan 4, 2020

ilgooz commented Jan 4, 2020 •

edited

Loading

ilgooz Jan 5, 2020 •

edited

Loading

jorgeorpinel Jan 5, 2020

jorgeorpinel Jan 5, 2020

jorgeorpinel left a comment

jorgeorpinel Jan 5, 2020

jorgeorpinel Jan 5, 2020

This comment was marked as resolved.

shcheklein Jan 5, 2020

jorgeorpinel Jan 5, 2020

jorgeorpinel Jan 5, 2020

jorgeorpinel Jan 5, 2020

jorgeorpinel Jan 5, 2020

jorgeorpinel Jan 5, 2020

jorgeorpinel Jan 5, 2020

jorgeorpinel Jan 5, 2020

jorgeorpinel Jan 5, 2020

jorgeorpinel Jan 5, 2020

jorgeorpinel Jan 5, 2020

jorgeorpinel Jan 5, 2020

jorgeorpinel commented Jan 14, 2020

shcheklein commented Jan 14, 2020

shcheklein commented Jan 21, 2020

		@@ -39,6 +39,12 @@ to learn how to "update" fixed-revision imports.

		## Options

		- `--rev` - specific

	Since the source files has changed, we see the results of our "update".
	Since the source files are different at `v2` comparing to the currently locked revision, we see the side effects of our "update" as different results.

	Since the source files has changed, we see the results of our "update".
	This actually has an effect because the `model.pkl.dvc` file at tag `v2` points to different data that what's currently in the `rev_lock` field of the import stage.

	moves (e.g. a branch), you may use `dvc update` preferably with `--rev` option
	moves (e.g. a branch), you may use `dvc update`

	to bring the data up to date. Using `--rev` also updates the fixed-revision.
	to bring the data up to date.

	usage: dvc update [-h] [-q \| -v] [--rev [REV]] targets [targets ...]
	usage: dvc update [-h] [-q \| -v] [--rev [REV]]
	targets [targets ...]

	To "update" fixed-revision to static references (e.g. tags), or for SHA commits,
	To update [fixed-revision](/doc/command-reference/import#example-fixed-revisions-re-importing) import stages (e.g. tags or SHA commits),

		To update from and also lock to a specific revision of a <abbr>data
		artifact</abbr>, we may use the `--rev` option this time:

command-reference/update: add docs for new --rev option #890

command-reference/update: add docs for new --rev option #890

Conversation

ilgooz commented Jan 2, 2020 • edited Loading

Choose a reason for hiding this comment

This comment was marked as resolved.

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

shcheklein left a comment

Choose a reason for hiding this comment

jorgeorpinel left a comment • edited Loading

Choose a reason for hiding this comment

jorgeorpinel commented Jan 2, 2020

ilgooz commented Jan 4, 2020

ilgooz commented Jan 4, 2020 • edited Loading

ilgooz Jan 5, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorgeorpinel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

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

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

jorgeorpinel commented Jan 14, 2020

shcheklein commented Jan 14, 2020

shcheklein commented Jan 21, 2020

ilgooz commented Jan 2, 2020 •

edited

Loading

jorgeorpinel left a comment •

edited

Loading

ilgooz commented Jan 4, 2020 •

edited

Loading

ilgooz Jan 5, 2020 •

edited

Loading