introduce `dvc update` for ext repos by efiop · Pull Request #2218 · treeverse/dvc

efiop · 2019-07-02T13:51:33Z

Have you followed the guidelines in our
Contributing document?
Does your PR affect documented changes or does it add new functionality
that should be documented? If yes, have you created a PR for
dvc.org documenting it or at
least opened an issue for it? If so, please add a link to it.

treeverse/dvc.org#474

efiop · 2019-07-05T12:25:08Z

@Suor Updated my patch to get rid of enter/exit. Please take a look 🙂

shcheklein · 2019-07-05T16:15:28Z

can we reuse force or something, to avoid this locked flag manipulation ... does not look really nice

force doesn't affect locked stages. But I'll take a look.

shcheklein · 2019-07-05T16:33:54Z

please, update zsh/bash autocompletes if it's not done yet

shcheklein · 2019-07-05T16:36:19Z

are we running status for the locked stage?

We do, but not for the dependencies. So update available will only be visible if you dvc unlock import.dvc

is there a way to check the status before updating it then?

Yes, if you dvc unlock import.dvc first and then run dvc status

Hmm ... not an easy interface ... don't see an easy way to make it better ... may be make dvc status target.dvc ignore the lock?

@shcheklein Maybe dvc update something.dvc --dry then? Changing dvc status itself doesn't feel right.

@shcheklein In any case, that additional command can be introduced on top, as it doesn't affect the rest of the logic.

--dry sounds like a good start

Why status ignores locked stages? This doesn't make sense from consistency POV only from optimization one. Is locked about optimization?

Yes, locked is also about not actively going and checking dependencies of the stage(e.g. not re-downloading external repo, not going to s3 to check for external s3 dependency etc). Also, locked is kinda making a data source from any stage, so showing status for its dependencies doesn't seem like a desired behaviour.

Fixes treeverse#1774 Fixes treeverse#2139 Fixes treeverse#2201 Signed-off-by: Ruslan Kuprieiev <ruslan@iterative.ai>

Signed-off-by: Ruslan Kuprieiev <ruslan@iterative.ai>

shcheklein · 2019-07-05T22:15:26Z

add a few tickets to update lock flag description, lock/unlock command references, import-url command reference with the new semantics (in case we change it)

Signed-off-by: Ruslan Kuprieiev <ruslan@iterative.ai>

efiop · 2019-07-05T23:15:37Z

@shcheklein Added #2238 for import-url. I don't think locked and dvc lock/unlock commands need any updates caused by this PR.

efiop · 2019-07-05T23:16:59Z

@shcheklein Ah, you probably mean that those now should mention that it not only dvc lock that is setting up that flag now. Got it.

shcheklein · 2019-07-05T23:17:21Z

@efiop I meant docs, btw :) I think it worth mentioning on their command references that there is an update command. We'll see if it makes sense.

efiop · 2019-07-05T23:19:28Z

@shcheklein Got it. Change for import-url will come separately, so we'll handle the docs for it there. Added a ticket for lock treeverse/dvc.org#478

shcheklein

🎉

Suor · 2019-07-06T03:15:31Z

+
+            out = repo.find_out_by_relpath(self.def_path)
+            repo.fetch(out.stage.path)
+            to.info = copy.copy(out.info)


Is it safe to change passed argument here?

@Suor yes, we are effectively doing to.info.save(), but it will run later anyway to verify that everything is in check.

efiop changed the title ~~{WIP] introduce dvc update for ext repos~~ [WIP] introduce dvc update for ext repos Jul 2, 2019

efiop force-pushed the update branch 3 times, most recently from bc45a8e to d03ed49 Compare July 3, 2019 15:21

efiop changed the title ~~[WIP] introduce dvc update for ext repos~~ introduce dvc update for ext repos Jul 3, 2019

efiop force-pushed the update branch 4 times, most recently from 8834cee to e16aafc Compare July 3, 2019 19:40

efiop changed the title ~~introduce dvc update for ext repos~~ [WIP] introduce dvc update for ext repos Jul 3, 2019

efiop force-pushed the update branch 4 times, most recently from 556ef47 to e21320c Compare July 4, 2019 12:17

efiop changed the title ~~[WIP] introduce dvc update for ext repos~~ introduce dvc update for ext repos Jul 4, 2019

efiop force-pushed the update branch from db11466 to 0603e0b Compare July 4, 2019 12:50

efiop mentioned this pull request Jul 4, 2019

cmd ref: add dvc update treeverse/dvc.org#474

Closed

efiop requested review from Suor, pared and shcheklein July 4, 2019 13:58

efiop force-pushed the update branch from 0603e0b to e831ddb Compare July 4, 2019 22:15

shcheklein reviewed Jul 5, 2019

View reviewed changes

Comment thread dvc/dependency/repo.py Outdated

shcheklein reviewed Jul 5, 2019

View reviewed changes

Comment thread dvc/repo/imp.py Outdated

shcheklein reviewed Jul 5, 2019

View reviewed changes

Comment thread dvc/repo/get.py Outdated

efiop force-pushed the update branch 5 times, most recently from bfa2f99 to 70f5177 Compare July 5, 2019 12:11

efiop force-pushed the update branch from 4f38d69 to 8b50486 Compare July 5, 2019 16:21

shcheklein reviewed Jul 5, 2019

View reviewed changes

Comment thread dvc/external_repo.py Outdated

shcheklein reviewed Jul 5, 2019

View reviewed changes

Comment thread dvc/external_repo.py Outdated

shcheklein reviewed Jul 5, 2019

View reviewed changes

Comment thread dvc/external_repo.py Outdated

shcheklein reviewed Jul 5, 2019

View reviewed changes

Comment thread dvc/command/update.py Outdated

shcheklein reviewed Jul 5, 2019

View reviewed changes

Comment thread dvc/dependency/repo.py Outdated

efiop force-pushed the update branch from 8b50486 to 46f59c9 Compare July 5, 2019 16:43

efiop changed the title ~~introduce dvc update for ext repos~~ [WIP] introduce dvc update for ext repos Jul 5, 2019

erepo: dvc: implement update for imports from ext repos

67ccfe4

Fixes treeverse#1774 Fixes treeverse#2139 Fixes treeverse#2201 Signed-off-by: Ruslan Kuprieiev <ruslan@iterative.ai>

efiop force-pushed the update branch from 46f59c9 to 67ccfe4 Compare July 5, 2019 18:44

efiop changed the title ~~[WIP] introduce dvc update for ext repos~~ introduce dvc update for ext repos Jul 5, 2019

efiop mentioned this pull request Jul 5, 2019

investigate error handlling 'dvc metrics show -a' fetch remote branch #2213

Closed

efiop requested a review from shcheklein July 5, 2019 20:55

shcheklein reviewed Jul 5, 2019

View reviewed changes

Comment thread dvc/dependency/repo.py Outdated

deps: repo: make private

70b5596

Signed-off-by: Ruslan Kuprieiev <ruslan@iterative.ai>

update: only support repo imports

aa7315f

Signed-off-by: Ruslan Kuprieiev <ruslan@iterative.ai>

efiop mentioned this pull request Jul 5, 2019

import-url: lock by default #2238

Closed

efiop requested a review from shcheklein July 5, 2019 23:26

shcheklein approved these changes Jul 5, 2019

View reviewed changes

efiop merged commit 932b0d7 into treeverse:master Jul 5, 2019

efiop deleted the update branch July 5, 2019 23:28

Suor reviewed Jul 6, 2019

View reviewed changes

Conversation

efiop commented Jul 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

efiop commented Jul 5, 2019

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shcheklein commented Jul 5, 2019

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

shcheklein commented Jul 5, 2019

Uh oh!

efiop commented Jul 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

efiop commented Jul 5, 2019

Uh oh!

shcheklein commented Jul 5, 2019

Uh oh!

efiop commented Jul 5, 2019

Uh oh!

shcheklein left a comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

efiop commented Jul 2, 2019 •

edited

Loading

efiop commented Jul 5, 2019 •

edited

Loading