-
Notifications
You must be signed in to change notification settings - Fork 394
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
cmd-ref: document --rev option on update #1007
Conversation
da40ffd
to
4cc57eb
Compare
|
||
```dvc | ||
$ dvc import --rev 72e0f1 [email protected]:iterative/example-get-started model.pkl | ||
Importing 'model.pkl ([email protected]:iterative/example-get-started)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to include obvious outputs
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
branch. | ||
|
||
```dvc | ||
$ dvc update --rev master model.pkl.dvc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's try to not change semantics - from fixed commit to not following the branch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have any good examples in mind? I'm using this as a kind of a hack for being short of examples. I tried using the recent aita_dataset
but, the dataset on master
was too large (~117 MB) just for the demonstration (but, it's quite good in that I can change the --rev from master
to lightweight
, quite good example really).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can use tags at least? there are two tags in the example-get-started - baseline-experiment and bigrams-experiment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tags are more or less static, and you have to use re-import or use dvc update --rev
to move the latest version
what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shcheklein, i moved to tags. Showing example of moving between experiments is good, but I feel, importing from a fixed revision and trying to move to a recent version of artifacts is also a good example worth showing off.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@skshetry agreed, but we need a bit different approach then (we can't rely on a specific hash since it might change). Git clone repo use git to take the latest hash? (basically show how to take it?) .... or even use dvc list
- may be a good idea for the list to show commit hashes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like a similar question as #1007 (comment):
It's baiscaly the same as Example: Fixed revisions and updating to different revision in dvc import. Maybe we should just copy/paste it...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good stuff @skshetry ! Please take a look at the comments - if you know how to address them - please do, otherwise we'll take of the PR eventually.
4cc57eb
to
eb07a3c
Compare
@jorgeorpinel I think this like 10 days already? I think it's perfect to merge. Could you just take a brief look, address minor changes if you see them and let's merge? |
@jorgeorpinel merging this, let's address what's left (if anything) as a regular update. |
Sorry for the delay. I'm just getting to this now. Will note any comments here but we'll have to address in a separate PR of course. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review post-merge. @skshetry are you willing/able to give it another pass? Thanks
The import stage is overwritten, and will now be able update normally with | ||
`dvc update`. | ||
|
||
> In the above example, the value for `rev` in the new import stage will be | ||
> `master` (default branch), so the command is equivalent to not using `--rev` | ||
> at all. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This info is still useful if using --rev master above, just a little different:
> In the above example, the value for `rev` in the new import stage will be
> `master` (a branch) so it will be able update normally going forward.
- `--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 repository to update | ||
the file or directory from (also starts tracking the given revision). | ||
|
||
> Note that this adds or updates a `rev` field in the DVC-file that fixes it | ||
> to this revision (and updates `rev_lock` in the DVC-file). This can have an | ||
> impact on the behavior of `dvc update` later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets just copy this from import:
- `--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 repository to update | |
the file or directory from (also starts tracking the given revision). | |
> Note that this adds or updates a `rev` field in the DVC-file that fixes it | |
> to this revision (and updates `rev_lock` in the DVC-file). This can have an | |
> impact on the behavior of `dvc update` later. | |
- `--rev` - commit hash, branch or tag name, etc. (any | |
[Git revision](https://git-scm.com/docs/revisions)) of the repository to | |
update the file or directory from. The latest commit in `master` (tip of the | |
default branch) is used by default when this option is not specified. | |
> Note that this adds a `rev` field in the import stage that fixes it to this | |
> revision. This can impact the behavior of `dvc update`. (See | |
> **re-importing** example below.) |
``` | ||
|
||
The import stage is overwritten, and will get updated from the latest changes in | ||
the given revision (i.e. `bigrams-experiment` tag). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
given commit (tag `bigrams-experiment)
@jorgeorpinel, I'll work on it later today. Thanks for the review. |
Thank you! |
Fixes #1001
❗ 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 chose 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. 🙏